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/.env
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,4 @@ MISSION_CONTROL_USER=user
MISSION_CONTROL_PASSWORD=password

STRUCTURED_LOGGING_ENABLED=false
NSC_DISABLED=false

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.

This is good for a local default and testing, but we're going to need to set this in AWS Parameter Store and load the setting in each environment that way. Take a look at infra/app/app-config/env-config/environment-variables.tf to see how we do that with other variables like SITE_ALERT_ENABLED. Take a gander at the PR that added that for a guide on what's needed.

(And, in addition, we will need to go through and create this variable in Parameter Store in each environment, and set it to false.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I got it done. How do I go about getting credentials to create the variable in parameter store?

3 changes: 3 additions & 0 deletions app/.env.local.example
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ SLACK_TEST_EMAIL=
# Select the environment you are integrating with and fill in the values below.
# NOTE: Revisit this for multi-tenant app with agency-specific setup.
#
# (Optional) Disable the data-backed NSC flow and only expose the self-attestation flow:
# NSC_DISABLED=true
#
# Sandbox Environment
NSC_API_URL_SANDBOX=
NSC_TOKEN_URL_SANDBOX=
Expand Down
4 changes: 4 additions & 0 deletions app/app/controllers/activities/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ def current_identity!

private

def redirect_if_nsc_disabled
redirect_to new_activities_flow_education_path if nsc_disabled?
end

def after_activity_path
progress_result = progress_calculator.overall_result
progress_result.meets_routing_requirements ? activities_flow_summary_path : activities_flow_root_path
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class Activities::Education::TermCreditHoursController < Activities::BaseController
before_action :redirect_if_nsc_disabled
before_action :set_education_activity
before_action :set_term_credit_hours_vars, only: %i[edit update]
before_action :set_back_url, only: %i[edit update]
Expand Down
3 changes: 3 additions & 0 deletions app/app/controllers/activities/education_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class Activities::EducationController < Activities::BaseController
ARTIFICIAL_DELAY = 7.seconds
INDICATOR_COUNT = 3

before_action :redirect_if_nsc_disabled, only: %i[verify show sync error]
before_action :set_education_activity, only: %i[show edit update destroy review save_review]
before_action :set_back_url, only: %i[edit review]
after_action :track_info_viewed_event, only: %i[new edit]
Expand All @@ -24,6 +25,8 @@ def verify
def create
if params[:education_activity]
create_fully_self_attested_activity
elsif nsc_disabled?
redirect_to new_activities_flow_education_path
else
create_validated_activity
end
Expand Down
6 changes: 5 additions & 1 deletion app/app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class ApplicationController < ActionController::Base
ALPHANUMERIC_PREFIX_REGEXP = /^([a-zA-Z0-9]+)[^a-zA-Z0-9]*$/

helper :view
helper_method :current_agency, :show_menu?, :pilot_ended?, :get_site_alert_title, :get_site_alert_body, :activity_flow?, :session_timeout_enabled?, :session_timeout_duration, :internal_environment?
helper_method :current_agency, :show_menu?, :pilot_ended?, :get_site_alert_title, :get_site_alert_body, :activity_flow?, :session_timeout_enabled?, :session_timeout_duration, :internal_environment?, :nsc_disabled?
around_action :switch_locale
before_action :add_newrelic_metadata, :redirect_if_maintenance_mode, :set_device_id_cookie
after_action :apply_iframe_embedding
Expand Down Expand Up @@ -131,6 +131,10 @@ def activity_hub_enabled?
Rails.env.development? || ENV["ACTIVITY_HUB_ENABLED"] == "true"
end

def nsc_disabled?
Nsc.disabled?
end
Comment thread
krista-skylight marked this conversation as resolved.

def redirect_unless_activity_hub_enabled
return if activity_hub_enabled?

Expand Down
8 changes: 8 additions & 0 deletions app/app/helpers/activities_helper.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
module ActivitiesHelper
def education_add_path
if nsc_disabled?
new_activities_flow_education_path
else
verify_activities_flow_education_index_path
end
end

def show_activity?(type)
if @flow&.pre_populated_session?
@flow.pre_populated_activity_types.include?(type.to_sym)
Expand Down
4 changes: 4 additions & 0 deletions app/app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ def activity_type_enabled?(type)
current_agency&.activity_types&.[](type.to_sym)
end

def nsc_disabled?
Nsc.disabled?
end

def uswds_sprite_icon_href(icon_name)
"#{asset_path("@uswds/uswds/dist/img/sprite.svg")}##{icon_name}"
end
Expand Down
2 changes: 1 addition & 1 deletion app/app/views/activities/activities/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@
<%= render "activities/activities/activity_section",
activity_type: "education",
section_title: t("activities.education.title"),
add_path: verify_activities_flow_education_index_path,
add_path: education_add_path,
empty_message: t("activities.hub.empty.education"),
cards: education_card_data do %>
<% education_card_data.each do |card_data| %>
Expand Down
8 changes: 5 additions & 3 deletions app/app/views/activities/education/error.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@

<nav class="margin-top-5">
<%= link_to t(".enter_manually_button"), new_activities_flow_education_path, class: "usa-button" %>
<div class="margin-top-2 mobile-text-center">
<%= link_to t(".retry_button"), verify_activities_flow_education_index_path, class: "usa-button usa-button--unstyled" %>
</div>
<% unless nsc_disabled? %>
<div class="margin-top-2 mobile-text-center">
<%= link_to t(".retry_button"), verify_activities_flow_education_index_path, class: "usa-button usa-button--unstyled" %>
</div>
<% end %>
</nav>
9 changes: 9 additions & 0 deletions app/lib/nsc.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Nsc
def self.disabled?
ENV["NSC_DISABLED"] == "true"
end

def self.enabled?
!disabled?
end
end
15 changes: 15 additions & 0 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.

Used the precomputed AI_REVIEW_DIFF_RANGE (origin/main HEAD), which correctly captured PR #2047's NSC_DISABLED feature-flag change (23 files); no base re-resolution was needed. Reproduced CI's rspec job (Ruby 3.4.10, Postgres 16 container, npm ci + rake assets:precompile, RAILS_ENV=test) and ran bundle exec rspec spec/ --exclude-pattern '**/e2e/**': 2584 examples, 0 failures. An initial run showed ~70 red controller specs all failing on Sprockets AssetNotFound for the USWDS sprite; that was a missing assets:precompile step in my own bootstrap (which CI's setup-project action performs), and every one of those specs passed once assets were compiled, so nothing is classified. The new copy-drift assertions in employment_controller_spec.rb and add_your_work_controller_spec.rb were deleted alongside the intended en.yml/view changes, so no stale change-detector remains. spec/e2e/** was excluded, matching the rspec.yml CI job.

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.

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.

Bootstrapped per the repo's own CI (postgres:16 service container, rake db:schema:load, npm install + rake assets:precompile) and ran the CI test command bundle exec rspec spec/ --exclude-pattern '{e2e/**/*_spec.rb}' on Ruby 3.4.10 (repo pins 3.4.9; pin temporarily relaxed for the run and reverted). Result: 2584 examples, 0 failures — nothing to classify. The AI_REVIEW_DIFF_RANGE base (origin/main) was valid and was used as-is. A first pass without asset precompilation produced ~80 Sprockets AssetNotFound errors; those were a local bootstrap gap (ENVIRONMENT_ISSUE), disappeared after running CI's assets:precompile step, and are therefore not reported as classifications. spec/e2e/ is excluded by the CI rspec job and was not run.

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.

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.

Used the dispatcher-provided range origin/main..HEAD as-is; it correctly captured the PR #2047 change (NSC_DISABLED kill switch for education flows, 26 files), so no base re-resolution was needed. Reproduced CI's rspec.yml path: started Postgres, installed gems, npm install, rake assets:precompile, db:schema:load, then parallel_test spec/ -n 4 --exclude-pattern '^spec/e2e/' (same e2e exclusion CI uses; e2e runs in a separate workflow requiring browsers/external aggregators). Result: 2584 examples, 0 failures across all 4 processes; the 9 changed spec files alone give 339 examples, 0 failures. rubocop (664 files) and erb_lint (124 files) are also clean. Two bootstrap caveats: the Gemfile pins Ruby 3.4.9 but only 3.4.10 is available in the runner toolchain, so tests ran under 3.4.10 via a throwaway sidecar Gemfile.ci (deleted afterward, no tracked file modified); and an initial run performed before assets:precompile produced ~120 spurious Sprockets AssetNotFound errors in every view-rendering spec, which was an artifact of my own incomplete bootstrap rather than the change under test and which disappeared entirely once assets were compiled as CI does, so those are not classified. Nothing failed, so classifications is empty.

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 @@ -38,6 +38,21 @@
)
end

it "points the education add button at the verify page by default" do
expect(Capybara.string(response.body)).to have_css(
"[data-activity-type='education'] form[action='#{verify_activities_flow_education_index_path}']"
)
end

it "points the education add button at the self-attestation new page when NSC is disabled" do
stub_environment_variable("NSC_DISABLED", "true") do
get :index
expect(Capybara.string(response.body)).to have_css(
"[data-activity-type='education'] form[action='#{new_activities_flow_education_path}']"
)
end
end

it "shows current flow community service activities" do
expect(
assigns(:community_service_activities)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@
let(:tracked_flow) { activity_flow }
let(:perform_tracked_action) { get :edit, params: { education_id: education_activity.id, id: 0 } }

context "when NSC is disabled" do
it "redirects to the self-attestation new path" do
stub_environment_variable("NSC_DISABLED", "true") do
get :edit, params: { education_id: education_activity.id, id: 0 }

expect(response).to redirect_to(new_activities_flow_education_path)
end
end
end

it_behaves_like "tracks an event", TrackEvent::EducationTermCreditHoursViewed,
extra_attributes: -> { { education_activity_id: kind_of(Integer), term_index: 0, term_date_range: kind_of(String) } }

Expand Down Expand Up @@ -221,6 +231,16 @@
patch :update, params: { education_id: education_activity.id, id: 0, nsc_enrollment_term: { credit_hours: 4 } }
end

context "when NSC is disabled" do
it "redirects to the self-attestation new path" do
stub_environment_variable("NSC_DISABLED", "true") do
patch :update, params: { education_id: education_activity.id, id: 0, nsc_enrollment_term: { credit_hours: 4 } }

expect(response).to redirect_to(new_activities_flow_education_path)
end
end
end

it_behaves_like "tracks an event", TrackEvent::EducationTermCreditHoursSubmitted,
extra_attributes: -> { { education_activity_id: kind_of(Integer), term_index: 0, term_date_range: kind_of(String) } }

Expand Down
61 changes: 61 additions & 0 deletions app/spec/controllers/activities/education_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@
expect(response.body).to have_content(activity_flow.identity.last_name)
expect(response.body).to have_content(activity_flow.identity.date_of_birth.strftime("%B %-d, %Y"))
end

context "when NSC is disabled" do
it "redirects to the self-attestation new path" do
stub_environment_variable("NSC_DISABLED", "true") do
get :verify

expect(response).to redirect_to(new_activities_flow_education_path)
end
end
end
end

describe "POST #create" do
Expand Down Expand Up @@ -63,6 +73,27 @@
expect(response).to redirect_to(activities_flow_education_path(id: EducationActivity.last.id))
end

context "when NSC is disabled" do
it "redirects to the self-attestation new path and does not create an activity" do
stub_environment_variable("NSC_DISABLED", "true") do
expect { post :create }.not_to change(EducationActivity, :count)
expect(response).to redirect_to(new_activities_flow_education_path)
end
end

it "allows creating a self-attested EducationActivity" do
stub_environment_variable("NSC_DISABLED", "true") do
expect {
post :create, params: { education_activity: { school_name: "Test University", city: "Springfield", state: "IL", zip_code: "62701", street_address: "123 Main St" } }
}.to change(EducationActivity, :count).by(1)

activity = EducationActivity.last
expect(activity.data_source).to eq("fully_self_attested")
expect(response).to redirect_to(edit_activities_flow_education_month_path(education_id: activity.id, id: 0))
end
end
end

it "creates the validated activity as a draft" do
post :create

Expand Down Expand Up @@ -98,6 +129,16 @@
describe "GET #show" do
let(:education_activity) { create(:education_activity, activity_flow: activity_flow) }

context "when NSC is disabled" do
it "redirects to the self-attestation new path" do
stub_environment_variable("NSC_DISABLED", "true") do
get :show, params: { id: education_activity.id }

expect(response).to redirect_to(new_activities_flow_education_path)
end
end
end

it "renders the synchronization page" do
get :show, params: { id: education_activity.id }

Expand Down Expand Up @@ -211,6 +252,16 @@
describe "PATCH #sync" do
let(:education_activity) { create(:education_activity, activity_flow: activity_flow) }

context "when NSC is disabled" do
it "redirects to the self-attestation new path" do
stub_environment_variable("NSC_DISABLED", "true") do
patch :sync, params: { education_id: education_activity.id }

expect(response).to redirect_to(new_activities_flow_education_path)
end
end
end

context "when the EducationActivity is validated and succeeded" do
before do
education_activity.update(status: :succeeded, draft: true)
Expand Down Expand Up @@ -249,6 +300,16 @@
expect(response.body).to have_content(I18n.t("activities.education.error.retry_button"))
expect(response.body).to have_link(I18n.t("activities.education.error.enter_manually_button"), href: new_activities_flow_education_path)
end

context "when NSC is disabled" do
it "redirects to the self-attestation new path" do
stub_environment_variable("NSC_DISABLED", "true") do
get :error

expect(response).to redirect_to(new_activities_flow_education_path)
end
end
end
end

describe "GET #new" do
Expand Down
10 changes: 10 additions & 0 deletions app/spec/controllers/application_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,14 @@ def show
end
end
end

describe "#nsc_disabled?" do
it "delegates to Nsc.disabled?" do
allow(Nsc).to receive(:disabled?).and_return(true)
expect(controller.send(:nsc_disabled?)).to be true

allow(Nsc).to receive(:disabled?).and_return(false)
expect(controller.send(:nsc_disabled?)).to be false
end
end
end
16 changes: 16 additions & 0 deletions app/spec/helpers/activities_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,22 @@
RSpec.describe ActivitiesHelper do
include ActiveSupport::Testing::TimeHelpers

describe "#education_add_path" do
context "when NSC is not disabled" do
it "returns verify_activities_flow_education_index_path" do
expect(helper.education_add_path).to eq(verify_activities_flow_education_index_path)
end
end

context "when NSC is disabled" do
it "returns new_activities_flow_education_path" do
stub_environment_variable("NSC_DISABLED", "true") do
expect(helper.education_add_path).to eq(new_activities_flow_education_path)
end
end
end
end

describe "#show_activity?" do
context "in a pre-populated session" do
before do
Expand Down
10 changes: 10 additions & 0 deletions app/spec/helpers/application_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,16 @@
end
end

describe "#nsc_disabled?" do
it "delegates to Nsc.disabled?" do
allow(Nsc).to receive(:disabled?).and_return(true)
expect(helper.nsc_disabled?).to be true

allow(Nsc).to receive(:disabled?).and_return(false)
expect(helper.nsc_disabled?).to be false
end
end

describe ".format_decimal_amount" do
it "returns 0 for nil" do
expect(described_class.format_decimal_amount(nil)).to eq(0)
Expand Down
55 changes: 55 additions & 0 deletions app/spec/lib/nsc_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
require "rails_helper"

RSpec.describe Nsc do
describe ".disabled?" do
it "returns false when NSC_DISABLED is unset" do
stub_environment_variable("NSC_DISABLED", nil) do
expect(described_class.disabled?).to be false
end
end

it "returns false when NSC_DISABLED is 'false'" do
stub_environment_variable("NSC_DISABLED", "false") do
expect(described_class.disabled?).to be false
end
end

it "returns false when NSC_DISABLED is '0'" do
stub_environment_variable("NSC_DISABLED", "0") do
expect(described_class.disabled?).to be false
end
end

it "returns true when NSC_DISABLED is 'true'" do
stub_environment_variable("NSC_DISABLED", "true") do
expect(described_class.disabled?).to be true
end
end

it "returns false when NSC_DISABLED is '1'" do
stub_environment_variable("NSC_DISABLED", "1") do
expect(described_class.disabled?).to be false
end
end

it "returns false when NSC_DISABLED is 't'" do
stub_environment_variable("NSC_DISABLED", "t") do
expect(described_class.disabled?).to be false
end
end
end

describe ".enabled?" do
it "returns true when NSC_DISABLED is unset" do
stub_environment_variable("NSC_DISABLED", nil) do
expect(described_class.enabled?).to be true
end
end

it "returns false when NSC_DISABLED is 'true'" do
stub_environment_variable("NSC_DISABLED", "true") do
expect(described_class.enabled?).to be false
end
end
end
end
1 change: 1 addition & 0 deletions docs/local-replication.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ essentials for a working local app are the payroll-provider sandbox keys:
| `ARGYLE_API_TOKEN_SANDBOX_ID` | Argyle Console |
| `ARGYLE_API_TOKEN_SANDBOX_SECRET` | Argyle Console |
| `ARGYLE_SANDBOX_WEBHOOK_SECRET` | Generate one: `openssl rand -hex 64` |
| `NSC_DISABLED` | (Optional) Set to `true` to disable data-backed NSC education flow and only expose self-attestation |

Some values (`NEWRELIC_KEY`, `MIXPANEL_TOKEN`, `SLACK_TEST_EMAIL`, and others)
should be obtained from a teammate — they're kept in Nava's 1Password under
Expand Down
Loading
Loading