Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jan 29, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8d0d43e
NJ 271 - fix when review screen displays health insurance section
jachan Jan 27, 2025
0103754
Simplify display logic
jachan Jan 28, 2025
8d7e304
Fix bug in intake that causes filing status to be incorrect
jachan Jan 28, 2025
5fd9080
change no dependents persona to nj_lucky_single
jachan Jan 29, 2025
c560923
forgot to add updated factory in last commit
jachan Jan 29, 2025
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
2 changes: 1 addition & 1 deletion app/views/state_file/questions/nj_review/edit.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?) %>
Copy link

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

<section id="missing-health-insurance" class="white-group">
<div class="spacing-below-5">
<h2 class="text--body text--bold spacing-below-5"><%=t(".missing_health_insurance") %></h2>
Expand Down
24 changes: 24 additions & 0 deletions spec/controllers/state_file/questions/nj_review_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
require "rails_helper"

RSpec.describe StateFile::Questions::NjReviewController do
let(:intake) { create :state_file_nj_intake, :df_data_minimal }
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

before do
sign_in intake
end

describe "#edit" do
render_views
it 'succeeds' do
get :edit
expect(response).to be_successful
end

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
Copy link

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@mluedke2 mluedke2 Jan 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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)

get :edit
expect(response.body).not_to have_text "Dependents who DO NOT have health insurance"
Copy link
Contributor

Choose a reason for hiding this comment

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

[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

end
end
end
end
Loading