NJ 271 - fix when review screen displays health insurance section #5457

jachan · 2025-01-27T21:47:14Z

Link to pivotal/JIRA issue

https://github.com/newjersey/affordability-pm/issues/271

Is PM acceptance required? (delete one)

Yes - don't merge until JIRA issue is accepted!

Reminder: merge main into this branch and get green tests before merging to main

What was done?

Update NJ Review screen to have the same logic as the controller, only displays if there are dependents

vita-min/app/controllers/state_file/questions/nj_dependents_health_insurance_controller.rb

Line 7 in da60492

return false unless intake.dependents.any?

How to test?

Use Springsteen MFJ, navigate to review screen, ensure that "Dependents DO NOT have Health Insurance" section does not appear

Screenshots (for visual changes)

Before

After

github-actions · 2025-01-27T21:52:08Z

Heroku app: https://gyr-review-app-5457-2e40f7315ce3.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5457 (optionally add --tail)

tristanvanech · 2025-01-27T22:41:26Z

app/views/state_file/questions/nj_review/edit.html.erb

@@ -23,7 +23,7 @@
    </section>
  <% end %>

-  <% if current_intake.has_health_insurance_requirement_exception? || current_intake.eligibility_all_members_health_insurance_no? %>
+  <% if current_intake.dependents.any? && (current_intake.has_health_insurance_requirement_exception? || current_intake.eligibility_all_members_health_insurance_no?) %>


is the second conditional necessary? If they answered no to the health insurance eligibility question, they either will be offboarded (and so will never get to the review screen) or they meet an exception

That's a great catch -- it was in the previous code so not introduced here, but I suspect that if we took it out then tests would probably still pass and would be cleaner.

After some digging, I think you're right! This second condition is redundant because anyone at this point would have been screened out if they answered yes and didn't have an exception. Truth table below to validate that the logic behaves identically:

current_intake.has_health_insurance_requirement_exception? || current_intake.eligibility_all_members_health_insurance_no?
Exception True, Insurance_no? True = conditional True
Exception False, Insurance_no? True - screened out
Exception True, Insurance_no? False = conditional True
Exception False, Insurance_no? False = conditional False

current_intake.has_health_insurance_requirement_exception?
Exception True, Insurance_no? True = conditional True
Exception False, Insurance_no? True - screened out
Exception True, Insurance_no? False = conditional True
Exception False, Insurance_no? False = conditional False

tristanvanech · 2025-01-27T22:43:34Z

spec/controllers/state_file/questions/nj_review_controller_spec.rb

+
+    context 'when no dependents and gross income at threshold' do
+      it 'does not show the dependents without health insurance block' do
+        allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_29).and_return 20_000


I'm not sure if this is a test or app functionality but just highlighting that the income threshold is just one of the exceptions to health insurance eligibility (the other being claimed as dependents on another return) and it's 20k for some filing statuses, 10k for others

That is a good point - instead of getting into this or that reason for eligibility, which is all tested elsewhere, the mock here can have something like:

allow_any_instance_of(StateFileNjIntake).to receive(:eligibility_all_members_health_insurance_no?).and_return(false)

mluedke2 · 2025-01-27T23:56:43Z

spec/controllers/state_file/questions/nj_review_controller_spec.rb

+      it 'does not show the dependents without health insurance block' do
+        allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_29).and_return 20_000
+        get :edit       
+        expect(response.body).not_to have_text "Dependents who DO NOT have health insurance"


[pebble] - I think if we have this test, we should pair it with a test case for the opposite when we do expect the text

mluedke2 · 2025-01-27T23:57:56Z

spec/controllers/state_file/questions/nj_review_controller_spec.rb

+require "rails_helper"
+
+RSpec.describe StateFile::Questions::NjReviewController do
+  let(:intake) { create :state_file_nj_intake, :df_data_minimal }


[dust] - should we use one of the more realistic profiles here? and then use mocks below as needed?

Yeah, it's a good point - happy to make a new import for springsteen that represents no dependents.

jachan · 2025-01-28T16:48:38Z

spec/controllers/state_file/questions/nj_dependents_health_insurance_controller_spec.rb

@@ -29,14 +29,6 @@
          expect(described_class.show?(intake)).to eq true
        end
      end
-
-      context "and did not have a health insurance requirement exception, but all members did not have health insurance" do  


Test case would never happen as someone with no exceptions and who answered no to the eligibility question would be screened out.

vita-min/app/models/state_file_nj_intake.rb

Line 188 in d02c901

if self.eligibility_all_members_health_insurance_no? && !self.has_health_insurance_requirement_exception?

jachan · 2025-01-28T16:49:10Z

spec/factories/state_file_nj_intakes.rb

+    trait :df_data_no_deps do
+      raw_direct_file_data { StateFile::DirectFileApiResponseSampleService.new.read_xml('nj_springsteen_mfj') }
+      raw_direct_file_intake_data { StateFile::DirectFileApiResponseSampleService.new.read_json('nj_springsteen_mfj') }
+      primary_birth_date { Date.new(1990, 1, 1) }


Have to add birth dates or EITC check fails when trying to check age of filer/spouse.

jachan · 2025-01-28T22:57:37Z

spec/factories/state_file_nj_intakes.rb

@@ -181,8 +173,6 @@
    trait :df_data_no_deps do
      raw_direct_file_data { StateFile::DirectFileApiResponseSampleService.new.read_xml('nj_springsteen_mfj') }
      raw_direct_file_intake_data { StateFile::DirectFileApiResponseSampleService.new.read_json('nj_springsteen_mfj') }
-      primary_birth_date { Date.new(1990, 1, 1) }
-      spouse_birth_date { Date.new(1990, 1, 1) }


Spouse birth date was not set because filing status was incorrectly set to single.

jachan · 2025-01-28T22:57:49Z

spec/factories/state_file_nj_intakes.rb

-        head_of_household: 4,
-        qualifying_widow: 5,
-        }[evaluator.filing_status.to_sym] || evaluator.filing_status
-      intake.direct_file_data.filing_status = numeric_status


This code would always override filing status to be single

Moving this issue to a separate ticket: https://github.com/newjersey/affordability-pm/issues/279. Going to revert the logic for now, and use a persona with filing_status single (nj_lucky_single) and no dependents for the test.

thanks for making that follow-up ticket!

jnf

lgtm

mluedke2

looks good!

NJ 271 - fix when review screen displays health insurance section

8d0d43e

tristanvanech reviewed Jan 27, 2025

View reviewed changes

mluedke2 reviewed Jan 27, 2025

View reviewed changes

Simplify display logic

0103754

jachan commented Jan 28, 2025

View reviewed changes

Fix bug in intake that causes filing status to be incorrect

8d7e304

jachan commented Jan 28, 2025

View reviewed changes

jnf approved these changes Jan 28, 2025

View reviewed changes

jachan added 2 commits January 29, 2025 09:22

change no dependents persona to nj_lucky_single

5fd9080

forgot to add updated factory in last commit

c560923

jachan requested a review from mluedke2 January 29, 2025 15:04

mluedke2 approved these changes Jan 29, 2025

View reviewed changes

jachan merged commit a4dacde into main Jan 29, 2025
7 checks passed

jachan deleted the nj-271-health-insurance-review branch January 29, 2025 18:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NJ 271 - fix when review screen displays health insurance section #5457

NJ 271 - fix when review screen displays health insurance section #5457

jachan commented Jan 27, 2025 •

edited

Loading

github-actions bot commented Jan 27, 2025

tristanvanech Jan 27, 2025

mluedke2 Jan 28, 2025

jachan Jan 28, 2025

tristanvanech Jan 27, 2025

mluedke2 Jan 27, 2025 •

edited

Loading

mluedke2 Jan 27, 2025

mluedke2 Jan 27, 2025

jachan Jan 28, 2025

jachan Jan 28, 2025

jachan Jan 28, 2025

jachan Jan 28, 2025

jachan Jan 28, 2025

jachan Jan 29, 2025

mluedke2 Jan 29, 2025

jnf left a comment

mluedke2 left a comment

NJ 271 - fix when review screen displays health insurance section #5457

NJ 271 - fix when review screen displays health insurance section #5457

Conversation

jachan commented Jan 27, 2025 • edited Loading

Link to pivotal/JIRA issue

Is PM acceptance required? (delete one)

What was done?

How to test?

Screenshots (for visual changes)

github-actions bot commented Jan 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mluedke2 Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnf left a comment

Choose a reason for hiding this comment

mluedke2 left a comment

Choose a reason for hiding this comment

jachan commented Jan 27, 2025 •

edited

Loading

mluedke2 Jan 27, 2025 •

edited

Loading