Nj 224 check claimed as dependent for lines 10 and 11 #5312
Conversation
|
Heroku app: https://gyr-review-app-5312-f2565830cad7.herokuapp.com/ |
mluedke2
changed the title
| @intake.direct_file_json_data.dependents.count do |dependent| | ||
| dependent.qualifying_child | ||
| dependent.qualifying_child && dependent.is_claimed_dependent |
There was a problem hiding this comment.
[dust] I'm ok with how this is, but so you know, the qualifying_child column in StateFileDependent is populated from direct_file_json_data during import, and all of the dependents in intake.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.
@mpidcock I didn't know that! When I search the codebase for either is_claimed_dependent or isClaimedDependent, 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
def calculate_line_10_count
@intake.dependents.count do |dependent|
dependent.qualifying_child
end
end
...
def calculate_line_11_count
@intake.dependents.count do |dependent|
!dependent.qualifying_child
end
end
There was a problem hiding this comment.
@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.
Oh ok that makes sense. Thanks!
| it "does not fill in" do | ||
| allow_any_instance_of(Efile::Nj::Nj1040Calculator).to receive(:calculate_line_10_count).and_return 0 |
| @@ -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.
Makes sense to remove this, as it is covered in the calculator tests
Link to pivotal/JIRA issue
https://github.com/newjersey/affordability-pm/issues/224
Is PM acceptance required?
Reminder: merge main into this branch and get green tests before merging to main
What was done?
How to test?
Screenshots
Latifah: