Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/app/controllers/launcher_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ def render_launcher_error(message)
"lynette" => { first_name: "Lynette", last_name: "Oyola", date_of_birth: "1988-10-24" },
"rick" => { first_name: "Rick", last_name: "Banas", date_of_birth: "1979-08-18" },
"dominique" => { first_name: "Dominique", last_name: "Ricardo", date_of_birth: "1978-01-12" },
"scott" => { first_name: "Scott", last_name: "Tobin", date_of_birth: "1998-02-03" },
"linda" => { first_name: "Linda", last_name: "Cooper", date_of_birth: "1999-01-01" }
}.freeze

Expand Down
8 changes: 8 additions & 0 deletions app/app/jobs/application_job.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
class ApplicationJob < ActiveJob::Base
class SilencedError < StandardError; end

NON_RETRYABLE_ERRORS = [
NameError,
TypeError,
ArgumentError,
ActiveRecord::RecordInvalid
].freeze

around_perform :with_error_reporting

retry_on Exception, wait: :polynomially_longer, attempts: 5
retry_on(*NON_RETRYABLE_ERRORS, attempts: 1)
Comment thread
bencalegari marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should just retry_on StandardError instead of Exception. All of the NON_RETRYABLE_ERRORS you list are not StandardErrors, and there are other ones that are not StandardErrors too that we probably also don't want to retry on, like SyntaxError, NotImplementedError, and LoadError


class_attribute :max_attempts, default: 5

Expand Down
4 changes: 1 addition & 3 deletions app/app/models/education_activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,7 @@ def document_upload_header_title_i18n_key
def document_upload_terms_to_verify
return [] unless partially_self_attested?

nsc_enrollment_terms
.select(&:enrollment_less_than_half_time?)
.sort_by { |term| [ term.term_begin || Date.new(1900, 1, 1), term.term_end || Date.new(1900, 1, 1), term.school_name.to_s ] }
less_than_half_time_terms_in_reporting_window
end

def document_upload_term_credit_hours(term)
Expand Down
5 changes: 4 additions & 1 deletion app/app/services/education_activity_card_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,12 @@ def validated_month_data(month_start:, effective_term:)
end

def partial_self_attested_month_data(term:, month_start:)
credit_hours = @activity.review_term_credit_hours(term)
{
month: month_start,
enrollment_status: term.enrollment_status_display
enrollment_status: term.enrollment_status_display,
credit_hours: credit_hours,
community_engagement_hours: @activity.community_engagement_hours(credit_hours)
}
end

Expand Down
26 changes: 18 additions & 8 deletions app/app/services/launcher/nsc_forward_dating_service.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
class Launcher::NscForwardDatingService
LAUNCHER_SCENARIO_KEYS = %w[lynette rick dominique linda].freeze
LAUNCHER_SCENARIO_KEYS = %w[lynette rick dominique scott linda].freeze
# These dates match each persona's latest term end so NSC reports them as currently enrolled before forward-dating.
LAUNCHER_AS_OF_DATES = {
"lynette" => Date.new(2024, 11, 19),
"rick" => Date.new(2024, 11, 29)
"rick" => Date.new(2024, 11, 29),
"dominique" => Date.new(2024, 5, 9)
}.freeze

def self.applicable?(education_activity)
Expand Down Expand Up @@ -37,13 +38,10 @@ def launcher_as_of_date
end

def forward_dated_response(response)
latest_term_end = Array(response["enrollmentDetails"])
.flat_map { |detail| Array(detail["enrollmentData"]) }
.filter_map { |term| term["termEndDate"].presence && Date.parse(term["termEndDate"]) }
.max
return response unless latest_term_end
anchor_term_end = anchor_term_end_for(response)
return response unless anchor_term_end

delta_days = (@education_activity.activity_flow.reporting_window_range.max - latest_term_end).to_i
delta_days = (@education_activity.activity_flow.reporting_window_range.max - anchor_term_end).to_i
transformed_response = response.deep_dup

Array(transformed_response["enrollmentDetails"]).each do |detail|
Expand All @@ -56,6 +54,18 @@ def forward_dated_response(response)
transformed_response
end

def anchor_term_end_for(response)
term_ends = Array(response["enrollmentDetails"])
.flat_map { |detail| Array(detail["enrollmentData"]) }
.filter_map { |term| term["termEndDate"].presence && Date.parse(term["termEndDate"]) }
return if term_ends.empty?

as_of_date = launcher_as_of_date
return term_ends.max if as_of_date.blank?

term_ends.select { |term_end| term_end <= as_of_date }.max || term_ends.max
end

def shift_date_string(date_str, delta_days)
return date_str if date_str.blank?

Expand Down
10 changes: 7 additions & 3 deletions app/app/services/nsc_data_fetcher_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,23 @@ def update_education_activity(education_activity, response_data)
def save_enrollment_terms(enrollment_details)
activity_flow = @education_activity.activity_flow

identity = activity_flow.identity

enrollment_details.each do |enrollment_detail|
next unless enrollment_detail["currentEnrollmentStatus"] == CURRENTLY_ENROLLED

name_on_school_record = enrollment_detail["nameOnSchoolRecord"] || {}

enrollment_detail["enrollmentData"].each do |enrollment_data|
term_begin = Date.parse(enrollment_data["termBeginDate"])
term_end = Date.parse(enrollment_data["termEndDate"])
next unless activity_flow.within_reporting_window?(term_begin, term_end)

@education_activity.nsc_enrollment_terms.create!(
school_name: enrollment_detail["officialSchoolName"],
first_name: enrollment_detail["nameOnSchoolRecord"]["firstName"],
middle_name: enrollment_detail["nameOnSchoolRecord"]["middleName"],
last_name: enrollment_detail["nameOnSchoolRecord"]["lastName"],
first_name: name_on_school_record["firstName"] || identity&.first_name,
middle_name: name_on_school_record["middleName"],
last_name: name_on_school_record["lastName"] || identity&.last_name,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I'm not sure we should fall back to saving the identity data again in the nsc_enrollment_terms - since nsc_enrollment_terms is just capturing the NSC result, supplementing it with other data might confuse us as to whether we got the data from NSC or somewhere else.

Can we just leave these fields blank if NSC returns nothing, and then handle it in whatever is reading the NSC enrollment term?

Also, did you say NSC no longer returns this field, or only sometimes? Or do we think it will be returned in production but not in sandbox?

enrollment_status: enrollment_status(enrollment_data),
term_begin: term_begin,
term_end: term_end,
Expand Down
7 changes: 4 additions & 3 deletions app/app/views/activities/activities/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,10 @@
<div class="activity-month-details__data">
<% if month_data[:enrollment_status].present? %>
<p class="margin-y-0"><%= t("activities.hub.cards.enrollment_status", status: month_data[:enrollment_status]) %></p>
<% else %>
<p class="margin-y-0"><%= t("activities.hub.cards.credit_hours", amount: month_data[:credit_hours]) %></p>
<p class="margin-y-0"><%= t("activities.hub.cards.hours", count: month_data[:community_engagement_hours]) %></p>
<% end %>
<% if month_data[:credit_hours].present? %>
<p class="margin-y-0"><%= t("activities.hub.cards.credit_hours", amount: format_decimal_amount(month_data[:credit_hours])) %></p>
<p class="margin-y-0"><%= t("activities.hub.cards.hours", count: format_decimal_amount(month_data[:community_engagement_hours])) %></p>
<% end %>
</div>
</div>
Expand Down
15 changes: 13 additions & 2 deletions app/app/views/launcher/advanced.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,24 @@
data-reporting-window-months="2">
<label class="usa-radio__label" for="test_scenario_dominique">
<strong>Dominique Ricardo</strong>
<span class="advanced-launcher__scenario-desc">Not currently enrolled (enrollments outside window)</span>
<span class="advanced-launcher__scenario-desc">Enrolled full time (1 school)</span>
</label>
</div>
<div class="usa-radio">
<input class="usa-radio__input usa-radio__input--tile" type="radio"
name="test_scenario" id="test_scenario_scott" value="scott"
data-action="advanced-launcher#selectScenario"
data-reporting-window-months="2">
<label class="usa-radio__label" for="test_scenario_scott">
<strong>Scott Tobin</strong>
<span class="advanced-launcher__scenario-desc">Not currently enrolled (enrollment older than 18 months)</span>
</label>
</div>
<div class="usa-radio">
<input class="usa-radio__input usa-radio__input--tile" type="radio"
name="test_scenario" id="test_scenario_linda" value="linda"
data-action="advanced-launcher#selectScenario">
data-action="advanced-launcher#selectScenario"
data-reporting-window-months="2">
<label class="usa-radio__label" for="test_scenario_linda">
<strong>Linda Cooper</strong>
<span class="advanced-launcher__scenario-desc">No NSC record found</span>
Expand Down
17 changes: 9 additions & 8 deletions app/app/views/launcher/launcher.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
data-launcher-flow-value="activity"
data-launcher-window-value="application"
data-launcher-months-value="2"
data-launcher-status-value="lynette">
data-launcher-status-value="dominique">
<%= form_with(url: "/launcher", method: :post, html: { class: "launcher__main", autocomplete: "off" }, data: { turbo: false, launcher_target: "form" }) do |f| %>
<div class="launcher__title">
<h1>Emmy launcher</h1>
Expand All @@ -17,7 +17,7 @@
<%= hidden_field_tag :flow_type, "activity", data: { launcher_target: "flowInput" } %>
<%= hidden_field_tag :reporting_window, "application", data: { launcher_target: "windowInput" } %>
<%= hidden_field_tag :reporting_window_months, "2", data: { launcher_target: "monthsInput" } %>
<%= hidden_field_tag :test_scenario, "lynette", data: { launcher_target: "statusInput" } %>
<%= hidden_field_tag :test_scenario, "dominique", data: { launcher_target: "statusInput" } %>

<div class="launcher__step">
<div class="launcher__step-header">
Expand Down Expand Up @@ -162,15 +162,16 @@
</div>
<div class="launcher__radio-col">
<% [
[ "lynette", "Enrolled full time", "Full-time enrollment verified automatically via the National Student Clearinghouse." ],
[ "renewal_half_time_last_4_of_6_avery", "Enrolled half-time", "Half-time enrollment covering 4 of 6 months (good for renewal required-month testing)." ],
[ "partial_enrollment_maya", "Enrolled less-than-half-time", "Less-than-half-time enrollment (sends user through partially self-attested flow)." ],
[ "linda", "No NSC enrollment found", "Enrollment unable to be verified via NSC (sends user through self-attestation flow)." ]
[ "dominique", "Enrolled full time", "Full-time enrollment verified automatically via the National Student Clearinghouse." ],
[ "renewal_half_time_last_4_of_6_avery", "Enrolled half-time", "Half-time enrollment covering 4 of 6 months (good for renewal required-month testing)." ],
[ "partial_enrollment_maya", "Enrolled less-than-half-time", "Less-than-half-time enrollment (sends user through partially self-attested flow)." ],
[ "lynette", "Enrolled, intensity not reported", "NSC confirms enrollment but reports no intensity (asks the user for credit hours)." ],
[ "linda", "No NSC enrollment found", "Enrollment unable to be verified via NSC (sends user through self-attestation flow)." ]
].each do |value, title, desc| %>
<div class="launcher__rcard <%= "launcher__rcard--selected" if value == "lynette" %>"
<div class="launcher__rcard <%= "launcher__rcard--selected" if value == "dominique" %>"
role="radio"
tabindex="0"
aria-checked="<%= value == "lynette" %>"
aria-checked="<%= value == "dominique" %>"
data-launcher-target="statusCard"
data-value="<%= value %>"
data-action="click->launcher#selectStatus keydown.enter->launcher#selectStatus keydown.space->launcher#selectStatus">
Expand Down
10 changes: 5 additions & 5 deletions app/spec/controllers/activities/activities_controller_spec.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test-classifier: AI triage of failing tests

AI Test Classifier — no action required

Observed — these verdicts are grounded in the actual test run output.

Ran the repo's own CI test path (rspec.yml + setup-project action): started Postgres 16, installed gems with bundler, loaded the test schema, ran npm install and rake assets:precompile, then executed rspec. The precomputed AI_REVIEW_DIFF_RANGE (origin/main HEAD) was verified to genuinely reflect PR #2012's change, so no base re-resolution was needed. Targeted run of the 10 changed spec files: 368 examples, 0 failures. Full CI-scope run (spec/ excluding e2e, as rspec.yml does): 2564 examples, 0 failures. An earlier run showed 101 view-rendering controller failures, but all were 'The asset @uswds/uswds/dist/img/sprite.svg is not present in the asset pipeline' caused by my not yet having run the CI bootstrap's npm install + assets:precompile; after completing that bootstrap every one of them passed, so they are a bootstrap artifact and not classified as failures. Nothing to triage.

React 👍 if this is right (nothing needed triage) / 👎 if a real failure was missed, and on a 👎 please reply with a one-line reason. Advisory, non-blocking.

Original file line number Diff line number Diff line change
Expand Up @@ -648,16 +648,16 @@
get :index
end

it "shows only enrollment status on the education card" do
it "shows enrollment status alongside credit hours on the education card" do
expect(response.body).to include("Test University")
expect(response.body).to include(
I18n.t(
"activities.hub.cards.enrollment_status",
status: I18n.t("components.enrollment_term_table_component.status.less_than_half_time")
)
)
expect(response.body).not_to include(I18n.t("activities.hub.cards.credit_hours", amount: 4))
expect(response.body).not_to include(I18n.t("activities.hub.cards.hours", count: 52))
expect(response.body).to include(I18n.t("activities.hub.cards.credit_hours", amount: 4))
expect(response.body).to include(I18n.t("activities.hub.cards.hours", count: 52))
expect(response.body).not_to include(I18n.t("activities.hub.empty.education"))
end

Expand Down Expand Up @@ -719,8 +719,8 @@
status: I18n.t("components.enrollment_term_table_component.status.less_than_half_time")
)
).length).to eq(2)
expect(response.body).not_to include(I18n.t("activities.hub.cards.credit_hours", amount: 3))
expect(response.body).not_to include(I18n.t("activities.hub.cards.credit_hours", amount: 5))
expect(response.body).to include(I18n.t("activities.hub.cards.credit_hours", amount: 3))
expect(response.body).to include(I18n.t("activities.hub.cards.credit_hours", amount: 5))
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
reporting_window_months: 1
)
end
let(:reporting_window) { activity_flow.reporting_window_range }

before do
Rails.application.config.active_storage.service = :local
Expand Down Expand Up @@ -159,8 +160,8 @@
term = create_partial_term(
activity: partial_education_activity,
school_name: "University of Illinois",
term_begin: Date.new(2026, 1, 5),
term_end: Date.new(2026, 5, 15)
term_begin: reporting_window.begin,
term_end: reporting_window.end
)

get :new, params: { education_id: partial_education_activity.id }
Expand All @@ -174,14 +175,14 @@
create_partial_term(
activity: partial_education_activity,
school_name: "University A",
term_begin: Date.new(2026, 1, 5),
term_end: Date.new(2026, 5, 15)
term_begin: reporting_window.begin,
term_end: reporting_window.end
)
create_partial_term(
activity: partial_education_activity,
school_name: "College B",
term_begin: Date.new(2026, 1, 10),
term_end: Date.new(2026, 5, 20)
term_begin: reporting_window.begin,
term_end: reporting_window.end
)

get :new, params: { education_id: partial_education_activity.id }
Expand Down
14 changes: 14 additions & 0 deletions app/spec/controllers/launcher_controller_advanced_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,26 @@
expect(rendered).to include('Rick Banas')
expect(rendered).to match(/Enrolled half-time.*2 schools/)
expect(rendered).to include('Dominique Ricardo')
expect(rendered).to match(/Enrolled full time.*1 school/)
expect(rendered).to include('Scott Tobin')
expect(rendered).to include('Not currently enrolled')
expect(rendered).to include('Linda Cooper')
expect(rendered).to include('No NSC record found')
expect(rendered).to include('Sam Testuser')
expect(rendered).to include('Ziggy Testuser')
end

it "gives every NSC test scenario a reporting window so selecting one does not leave a stale months pill", :aggregate_failures do
get :advanced
rendered = response.body

%w[lynette rick dominique scott linda].each do |scenario_key|
radio = rendered[/<input[^>]*id="test_scenario_#{scenario_key}"[^>]*>/]
expect(radio).to be_present, "expected a radio for #{scenario_key}"
expect(radio).to include('data-reporting-window-months'), "expected #{scenario_key} to set reporting window months"
end
end

it "displays fake test scenario options with single and multi-term" do
get :advanced
rendered = response.body
Expand Down Expand Up @@ -649,6 +662,7 @@
it_behaves_like "creates CbvApplicant with correct data", "lynette", "Lynette", "Oyola", "1988-10-24"
it_behaves_like "creates CbvApplicant with correct data", "rick", "Rick", "Banas", "1979-08-18"
it_behaves_like "creates CbvApplicant with correct data", "dominique", "Dominique", "Ricardo", "1978-01-12"
it_behaves_like "creates CbvApplicant with correct data", "scott", "Scott", "Tobin", "1998-02-03"
it_behaves_like "creates CbvApplicant with correct data", "linda", "Linda", "Cooper", "1999-01-01"

it "creates an ActivityFlowInvitation and redirects to its URL" do
Expand Down
12 changes: 11 additions & 1 deletion app/spec/controllers/launcher_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,23 @@
expect(body).to include("Enrolled full time")
expect(body).to include("Enrolled half-time")
expect(body).to include("Enrolled less-than-half-time")
expect(body).to include("Enrolled, intensity not reported")
expect(body).to include("No NSC enrollment found")
expect(body).to include('value="lynette"')
expect(body).to include('value="dominique"')
expect(body).to include('value="renewal_half_time_last_4_of_6_avery"')
expect(body).to include('value="partial_enrollment_maya"')
expect(body).to include('value="lynette"')
expect(body).to include('value="linda"')
end

it "preselects the full-time scenario as the default student status" do
get :launcher
body = response.body
expect(body).to include('data-launcher-status-value="dominique"')
expect(body).to match(/launcher__rcard--selected[^>]*data-value="dominique"/)
expect(body).to match(/name="test_scenario"[^>]*value="dominique"/)
end

it "renders the launch buttons posting to /launcher" do
get :launcher
body = response.body
Expand Down
6 changes: 6 additions & 0 deletions app/spec/factories/identity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,11 @@
last_name { "Ricardo" }
date_of_birth { "1978-01-12" }
end

trait :nsc_scott do
first_name { "Scott" }
last_name { "Tobin" }
date_of_birth { "1998-02-03" }
end
end
end
Loading
Loading