Fyst 1720 md disqualify dependent taxpayers from claiming poverty level credit #5483

mrotondo · 2025-01-30T02:55:50Z

Link to pivotal/JIRA issue

https://codeforamerica.atlassian.net/browse/FYST-1720

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?

The MD 502 calculator is supposed to make certain decisions based on whether the filer will use the filing status "dependent" on their Maryland return. However, this filing status only exists for Maryland, and will never be present on the federal return - we decide whether to use it in MD based on whether the filer's direct file data indicates that they are claimed as a dependent.
This means that the previous implementation of Md502Calculator#filing_status_dependent? would never return true for a real user, since it checked the intake's filing status (which is their federal filing status) for a value it could never have.
This PR changes that method to instead check the direct_file_data.claimed_as_dependent? method, which is the same way we decide to set their MD filing status to "dependent".
Several tests started failing when we made this change, since they were incorrectly setting the federal filing status of test data to 6 (dependent) in their setup methods, which it can never be. These methods (MD502 line 22 and line 42) both relate to EIC and now both return if the filer is claimed as a dependent, which from talking with Courtney I believe is correct, but we should double check.
I think that we need a subsequent change that will uniformize how we use filing status in the calculators. It sounds like we calculate a state filing status that could differ from the federal filing status in at least one other state. IMO we should have a federal_filing_status and state_filing_status method on intakes, the latter of which in MD would return dependent if the filer is claimed as a dependent (as we do in Md502.rb on lines 63-74), and switch our calculators over to use #state_filing_status everywhere (or almost everywhere, if there are calculations that explicitly use federal filing status)
- Notably, there are still a number of tests for Md502Calculator that pass :dependent as a filing status to the StateFileMdIntake constructor, which I believe will never happen in real usage since that is not a valid federal filing status.

How to test?

We need to be careful with this one, since we failed to catch this bug in acceptance previously.
We should reproduce the error on demo with a persona that is claimed as a dependent but has a valid federal filing status and is otherwise eligible for poverty level credit (and EIC, since i suspect we had the same kind of bug with that credit)
We should then use the same persona and do the same checks on this PR's heroku instance and verify that we are not able to claim those credits

… and not a federal filing status, and fix calculator tests and implementation accordingly

github-actions · 2025-01-30T03:00:02Z

Heroku app: https://gyr-review-app-5483-81eb6cd9fb32.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5483 (optionally add --tail)

mrotondo · 2025-01-30T22:05:11Z

spec/lib/efile/md/md502_calculator_spec.rb

-      before do
-        allow_any_instance_of(described_class).to receive(:deduction_method_is_standard?).and_return(false)
-      end
+      let(:line_1b) { 5_000 }


These needed to change because the previous values made the intake ineligible for the credit - the test still passed even when I changed the filing status to not-dependent

mrotondo · 2025-01-30T22:05:17Z

spec/lib/efile/md/md502_calculator_spec.rb

-      let(:filing_status) { "dependent" }
-      let(:line_1b) { 20_000 }
-      let(:line_7) { 15_000 }
+      let(:line_1b) { 5_000 }


Same as above

…calculator filing_status_dependent? to indicate that it is MD specific, and fix test for 502CR calculator to accurately represent a persona whose MD filing status will be dependent

arinchoi03 · 2025-01-30T22:43:51Z

Notably, there are still a number of tests for Md502Calculator that pass :dependent as a filing status to the StateFileMdIntake constructor, which I believe will never happen in real usage since that is not a valid federal filing status.

I'm guessing you're referring to the bunch of tests that do something like this:

      {
        single: 14_600,
        dependent: 14_600,
        married_filing_jointly: 29_200,
        married_filing_separately: 14_600,
        head_of_household: 21_900,
        qualifying_widow: 29_200
      }.each do |filing_status, filing_minimum|
        context "#{filing_status}" do
          let(:intake) { create(:state_file_md_intake, primary_birth_date: primary_birth_date, spouse_birth_date: spouse_birth_date, filing_status: filing_status) }

should we fix this up as a fast follow up to this story? Are we not testing these scenarios properly currently because dependent filing_status doesn't really mean anything -- for instance, we're never entering the if statement here in the test when we determine deduction method?:

      def calculate_deduction_method
        gross_income_amount = if @direct_file_data.claimed_as_dependent?
                                (@direct_file_data.fed_agi + line_or_zero(:MD502_LINE_6)) - line_or_zero(:MD502_LINE_15)
                              else
                                (@direct_file_data.fed_agi - @direct_file_data.fed_taxable_ssb) + line_or_zero(:MD502_LINE_6)
                              end

arinchoi03 · 2025-01-30T22:50:51Z

spec/lib/efile/md/md502_calculator_spec.rb

@@ -1018,30 +1024,31 @@
  end

  describe "#calculate_line_23" do
+    let (:deduction_method_standard) { true}


space after true

arinchoi03 · 2025-01-30T22:51:42Z

spec/lib/efile/md/md502cr_calculator_spec.rb

+        else
+          let(:filing_status) { filing_status }
+          let(:claimed_as_dependent) { false }
+        end


i'm guessing we'll do something similar for the MD502 calculator tests when we have time to fix them up?

Yes, that's my thinking

mrotondo · 2025-01-30T23:06:02Z

should we fix this up as a fast follow up to this story? Are we not testing these scenarios properly currently because dependent filing_status doesn't really mean anything -- for instance, we're never entering the if statement here in the test when we determine deduction method?:

Yes, the tests should be fixed but IMO that should go hand in hand with changing how we check filing status (e.g. the federal_filing_status vs state_filing_status suggestion in the pr description). And that work should be very hand in hand with program & PM to make sure it is acceptance-tested really precisely.

And as for the if block you pasted - we will go into that one because that (direct_file_data.claimed_as_dependent?) is actually the correct check (rather than intake.filing_status == :dependent), but I'd rather that the method do something like intake.state_filing_status == :dependent so that we're not duplicating the logic of checking the direct file data field all over the place.

mrotondo added 2 commits January 28, 2025 17:13

wip

Unverified

This user has not yet uploaded their public signing key.

GPG key ID: ED0851220993C483

Learn about vigilant mode

4dddceb

switch over to mocking DFData for claimed as dependent, for consistency

Loading
Loading status checks…

e2d996d

mrotondo commented Jan 30, 2025

View reviewed changes

arinchoi03 approved these changes Jan 30, 2025

View reviewed changes

mrotondo added 2 commits January 30, 2025 15:07

add a space

Loading
Loading status checks…

c3b2bbb

add avril dep md persona

Loading
Loading status checks…

a9cd8fd

mrotondo merged commit d329ee8 into main Jan 31, 2025
7 checks passed

mrotondo deleted the FYST-1720-md-disqualify-dependent-taxpayers-from-claiming-poverty-level-credit branch January 31, 2025 17:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fyst 1720 md disqualify dependent taxpayers from claiming poverty level credit #5483

Fyst 1720 md disqualify dependent taxpayers from claiming poverty level credit #5483

mrotondo commented Jan 30, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jan 30, 2025

Uh oh!

mrotondo Jan 30, 2025 •

edited

Loading

Uh oh!

mrotondo Jan 30, 2025

Uh oh!

arinchoi03 commented Jan 30, 2025 •

edited

Loading

Uh oh!

arinchoi03 Jan 30, 2025

Uh oh!

arinchoi03 Jan 30, 2025

Uh oh!

mrotondo Jan 30, 2025

Uh oh!

mrotondo commented Jan 30, 2025

Uh oh!

Uh oh!

Fyst 1720 md disqualify dependent taxpayers from claiming poverty level credit #5483

Fyst 1720 md disqualify dependent taxpayers from claiming poverty level credit #5483

Conversation

mrotondo commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to pivotal/JIRA issue

Is PM acceptance required? (delete one)

What was done?

How to test?

Uh oh!

Uh oh!

github-actions bot commented Jan 30, 2025

Uh oh!

Uh oh!

mrotondo Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrotondo Jan 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arinchoi03 commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arinchoi03 Jan 30, 2025

Choose a reason for hiding this comment

Uh oh!

arinchoi03 Jan 30, 2025

Choose a reason for hiding this comment

Uh oh!

mrotondo Jan 30, 2025

Choose a reason for hiding this comment

Uh oh!

mrotondo commented Jan 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrotondo commented Jan 30, 2025 •

edited

Loading

mrotondo Jan 30, 2025 •

edited

Loading

arinchoi03 commented Jan 30, 2025 •

edited

Loading