-
Notifications
You must be signed in to change notification settings - Fork 13
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.
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 224 check claimed as dependent for lines 10 and 11 #5312
Merged
Merged
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
cbc2649
work in progress mark on lines that will need changes
mluedke2 4be328e
Merge branch 'main' into nj-224-qual-dep
mluedke2 4caedd0
require to be claimed as dependent to count for either line 10 or 11
mluedke2 be44157
work in progress -- comments to fix tests
mluedke2 4e8c2fc
Merge branch 'main' into nj-224-qual-dep
mluedke2 2c66903
update xml and pdf tests to be independent from calculator
mluedke2 48e97f4
no need to change the zeus_two_deps, just use sinatra
mluedke2 59b2304
remove comment
mluedke2 0e52aed
Merge branch 'main' into nj-224-qual-dep
mluedke2 2cf53f6
simplify syntax
mluedke2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -553,40 +553,26 @@ | |
|
||
describe "Line 10 exemptions" do | ||
context "0 qualified dependent children" do | ||
let(:submission) { | ||
create :efile_submission, tax_return: nil, data_source: create( | ||
:state_file_nj_intake | ||
) | ||
} | ||
it "does not fill in" do | ||
allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_10_count).and_return 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an improvement! |
||
expect(pdf_fields["Text47"]).to eq "" | ||
expect(pdf_fields["undefined_12"]).to eq "" | ||
expect(pdf_fields["x 1500"]).to eq "" | ||
end | ||
end | ||
|
||
context "1 qualified dependent child" do | ||
let(:submission) { | ||
create :efile_submission, tax_return: nil, data_source: create( | ||
:state_file_nj_intake, | ||
:df_data_two_deps, | ||
) | ||
} | ||
it "fills in 1 for count and $1500 for exception" do | ||
allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_10_count).and_return 1 | ||
expect(pdf_fields["Text47"]).to eq "" | ||
expect(pdf_fields["undefined_12"]).to eq "1" | ||
expect(pdf_fields["x 1500"]).to eq "1500" | ||
end | ||
end | ||
|
||
context "10 qualified dependent children" do | ||
let(:submission) { | ||
create :efile_submission, tax_return: nil, data_source: create( | ||
:state_file_nj_intake, | ||
:df_data_many_deps, | ||
) | ||
} | ||
it "fills in 10 for count and $15000 for exception" do | ||
allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_10_count).and_return 10 | ||
expect(pdf_fields["Text47"]).to eq "1" | ||
expect(pdf_fields["undefined_12"]).to eq "0" | ||
expect(pdf_fields["x 1500"]).to eq "15000" | ||
|
@@ -596,27 +582,17 @@ | |
|
||
describe "Line 11 exemptions" do | ||
context "0 dependents not qualifying children" do | ||
let(:submission) { | ||
create :efile_submission, tax_return: nil, data_source: create( | ||
:state_file_nj_intake, | ||
:df_data_minimal, | ||
) | ||
} | ||
it "does not fill in" do | ||
allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_11_count).and_return 0 | ||
expect(pdf_fields["Text48"]).to eq "" | ||
expect(pdf_fields["undefined_13"]).to eq "" | ||
expect(pdf_fields["x 1500_2"]).to eq "" | ||
end | ||
end | ||
|
||
context "1 dependent not qualifying child" do | ||
let(:submission) { | ||
create :efile_submission, tax_return: nil, data_source: create( | ||
:state_file_nj_intake, | ||
:df_data_two_deps, | ||
) | ||
} | ||
it "fills in 1 for count and $1500 for exception" do | ||
allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_11_count).and_return 1 | ||
expect(pdf_fields["Text48"]).to eq "" | ||
expect(pdf_fields["undefined_13"]).to eq "1" | ||
expect(pdf_fields["x 1500_2"]).to eq "1500" | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -249,28 +249,20 @@ | |
end | ||
|
||
describe "qualified dependent children and other dependents" do | ||
context 'when 1 qualified child and 1 other dependent' do | ||
let(:intake) { create(:state_file_nj_intake, :df_data_two_deps) } | ||
it "sets NumOfQualiDependChild and NumOfOtherDepend to 1" do | ||
expect(xml.document.at('NumOfQualiDependChild').text).to eq "1" | ||
expect(xml.document.at('NumOfOtherDepend').text).to eq "1" | ||
end | ||
end | ||
|
||
context 'when 10 qualified children and 1 other dependent' do | ||
let(:intake) { create(:state_file_nj_intake, :df_data_many_deps) } | ||
it "sets NumOfQualiDependChild to 10 and NumOfOtherDepend to 1" do | ||
expect(xml.document.at('NumOfQualiDependChild').text).to eq "10" | ||
expect(xml.document.at('NumOfOtherDepend').text).to eq "1" | ||
end | ||
it "sets NumOfQualiDependChild to line 10 and NumOfOtherDepend to line 11" do | ||
allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_10_count).and_return 1 | ||
allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_11_count).and_return 2 | ||
|
||
expect(xml.document.at('NumOfQualiDependChild').text).to eq "1" | ||
expect(xml.document.at('NumOfOtherDepend').text).to eq "2" | ||
end | ||
context 'when 0 qualified child and 0 other dependent' do | ||
let(:intake) { create(:state_file_nj_intake, :df_data_minimal) } | ||
it "leaves NumOfQualiDependChild and NumOfOtherDepend blank" do | ||
expect(xml.document.at('NumOfQualiDependChild')).to eq nil | ||
expect(xml.document.at('NumOfOtherDepend')).to eq nil | ||
end | ||
|
||
it "leaves NumOfQualiDependChild and NumOfOtherDepend blank when lines 10 and 11 are empty" do | ||
allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_10_count).and_return 0 | ||
allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_11_count).and_return 0 | ||
|
||
expect(xml.document.at('NumOfQualiDependChild')).to eq nil | ||
expect(xml.document.at('NumOfOtherDepend')).to eq nil | ||
end | ||
end | ||
|
||
|
@@ -383,25 +375,10 @@ | |
end | ||
|
||
describe "total exemption - lines 13 and 30" do | ||
let(:intake) { create(:state_file_nj_intake, :primary_over_65, :primary_blind, :primary_veteran, :two_dependents_in_college) } | ||
it "totals lines 6-12 and stores the result in both TotalExemptionAmountA and TotalExemptionAmountB" do | ||
line_6_single_filer = 1_000 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense to remove this, as it is covered in the calculator tests |
||
line_7_over_65 = 1_000 | ||
line_8_blind = 1_000 | ||
line_9_veteran = 6_000 | ||
line_10_qualified_children = 1_500 | ||
line_11_other_dependents = 1_500 | ||
line_12_dependents_attending_college = 2_000 | ||
expected_sum = | ||
line_6_single_filer + | ||
line_7_over_65 + | ||
line_8_blind + | ||
line_9_veteran + | ||
line_10_qualified_children + | ||
line_11_other_dependents + | ||
line_12_dependents_attending_college | ||
expect(xml.at("Exemptions TotalExemptionAmountA").text).to eq(expected_sum.to_s) | ||
expect(xml.at("Body TotalExemptionAmountB").text).to eq(expected_sum.to_s) | ||
it "stores line 13 and 30 (equivalent) in both TotalExemptionAmountA and TotalExemptionAmountB" do | ||
allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_13).and_return 10_000 | ||
expect(xml.at("Exemptions TotalExemptionAmountA").text).to eq(10_000.to_s) | ||
expect(xml.at("Body TotalExemptionAmountB").text).to eq(10_000.to_s) | ||
end | ||
end | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[dust] I'm ok with how this is, but so you know, the
qualifying_child
column inStateFileDependent
is populated fromdirect_file_json_data
during import, and all of the dependents inintake.dependents
have already been filtered to be only claimed dependents.It would be a better practice to avoid extracting data from the json when it is available in the database, especially, for example, if you needed to access other dependent fields from the database that aren't in the json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mpidcock I didn't know that! When I search the codebase for either
is_claimed_dependent
orisClaimedDependent
, I didn't come across any references that pointed me towards logic like that? Can you help me find it?But regardless, I tried simplifying the logic as you said and sure enough the tests still pass. So I'll push a change to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mluedke2 Well, we don't actually have a method to filter claimed dependents, because the list of dependents sourced from the XML is specifically a list of only claimed dependents. It was new this year to have json data for other members of the household, but since our database gets instantiated with the xml data and augmented by the json data, the non-dependent household members are simply never added to the list of dependents. I hope that helps and let me know if I can clarify anything else!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok that makes sense. Thanks!