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.

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

[FYST-1882] Update ID disability logic to only ask questions about filers who fall within 62-65 age range #5664

Merged
merged 12 commits into from
Mar 3, 2025

Conversation

arinchoi03
Copy link
Contributor

@arinchoi03 arinchoi03 commented Feb 28, 2025

Link to pivotal/JIRA issue

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?

  • Only showing the four options view "Yes, I am", "Yes, my spouse is", "Yes, we both are", and "No, neither of us is" if both filers are between 62-65. Previously, we showed it for any mfj filer who had eligible 1099Rs
  • added a bunch of _between_62_and_65_years_old? methods. Seems a bit overkill but they all do get used 🤷 shorter naming suggestions will be welcome

How to test?

  • Describe the testing approach taken to verify the changes, including:
    • Unit/integration/manual tests
    • Test data used:
      • Pangolin: primary between 62-65; spouse filer under 62
      • Potato MFJ: primary 65+; only spouse between 62-65
      • Charlie Swann: Single filer, 62-65 yo
      • TBD MFJ persona that has two filers both between 62-65 yo
  • Specify any relevant testing environments used (e.g., development, staging, demo, Heroku).
  • Risk Assessment
    • This was a quick change (quick request, quick implementation)

Screenshots (for visual changes)

Potato MFJ (only shows q about spouse):
Screenshot 2025-02-28 at 3 28 12 PM

Pangolin (only shows q about primary):
Screenshot 2025-02-28 at 3 29 37 PM

Charlie Swan (only q about primary):
Screenshot 2025-02-28 at 3 30 52 PM

Barrel Roll (both filers are 62-65):
Screenshot 2025-03-03 at 8 59 35 AM

…w spouse-specific screen if only spouse 62-65
Copy link

Heroku app: https://gyr-review-app-5664-7dd2fe6afd19.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5664 (optionally add --tail)


def should_check_spouse_disabled?
intake.filing_status_mfj? && !intake.all_filers_between_62_and_65_years_old? && intake.spouse_between_62_and_65_years_old?
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i thought about moving these to the intake level, but feel like the names could be a bit misleading/confusing and the use case for these methods are really limited to this presence check.

@arinchoi03 arinchoi03 marked this pull request as ready for review February 28, 2025 23:19
@arinchoi03 arinchoi03 changed the title Only show mfj_disability questions if mfj & all filers are 62-65; sho… [FYST-1882] Update ID disability logic to only ask questions about filers who fall within 62-65 age range Feb 28, 2025
@anisharamnani
Copy link
Contributor

anisharamnani commented Mar 2, 2025

[dust] the dustiest of dust, but for MD, we bold the person who is eligible for the disability. For example, “Does your spouse meet the qualifications to be classified as disabled?” Especially since the title says “You might be eligible for the Idaho Retirement Deduction” This could also be done when we know what happens if both filers are between 62 and 65. Or, it could be skipped as a suggestion completely! But, I thought I’d call out the differences in design.

image

let(:primary_birth_date) { senior_dob }
let(:spouse_birth_date) { not_senior_dob }

context "when primary_disabled is blank" do
Copy link
Contributor

Choose a reason for hiding this comment

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

i believe this should be: context "when spouse_disabled is blank” do & the same for lines 87 and 96

if primary_disabled == "no" || %w[spouse none].include?(mfj_disability)
primary_followups = @intake.filer_1099_rs(:primary).map(&:state_specific_followup).compact
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 this is more for my own understanding, but why do you delete the followups?

Copy link
Contributor Author

@arinchoi03 arinchoi03 Mar 3, 2025

Choose a reason for hiding this comment

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

this is due to:
https://github.com/codeforamerica/vita-min/blob/main/app/lib/efile/id/id39_r_calculator.rb#L87-L95

If the user goes through the flow first for the 62-65 year old filer, answer "yes" for disability, then answer "eligible_income_source_yes?" for that eligible 1099R, but then comes back via the review page & then answers "no" for disabled. Without the clean up, we will add up all the income for 1099Rs, even though the filer may have changed their answer to no for disability.

I suppose another way to handle it is to:

  1. keep the followup (whatever the answer was -- eligible/not eligible)
  2. check if the filer is associated with a particular filer who is either disabled/over 65(senior) in the calculation

Pros of current approach:

  • we really don't need that eligible_income_source_yes? information unless the filer is disabled
  • it feels like a waste/unnecessary data to have records of if the 62-65 filer is not disabled in the first place. The followup only has that one piece of information (eligible_income_source)

Cons of current approach:

  • the calculations may benefit from double checking whether the 1099R comes from a disabled filer anyway?
    • to that end, i rather now think we should use eligible_1099rs instead of state_file_1099rs for calculations, which takes into account whether the person_qualifies? or not even though we may keep the cleanup logic

Another unrelated refactor I'm looking at is using eligible_1099rs method in the self.show? method of RetirementIncomeSubtractionController

This way we can clean up the unnecessary data and clear up the logic in the calculations to mirror what's happening in the flow. What do you think? Happy to create a chore & clean up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

quick followup here #5665 while my mind is on it but no need to get it through review or anything, unless we feel like we have loads of time to run through this quickly before release.

Copy link
Contributor

@anisharamnani anisharamnani left a comment

Choose a reason for hiding this comment

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

i don’t think any of my questions or comments are blocking, thanks @arinchoi03 for working on this! disability logic is 🤯

@arinchoi03 arinchoi03 merged commit 49aed7b into main Mar 3, 2025
7 checks passed
@arinchoi03 arinchoi03 deleted the FYST-1882-id-update-idaho-retirement-deduction-logic branch March 3, 2025 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants