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-1721 AZ include qual child dependent only if filer is hoh #5475

Conversation

tahsinaislam
Copy link
Contributor

Link to pivotal/JIRA issue

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

Is PM acceptance required? (delete one)

  • Yes - don't merge until JIRA issue is accepted!

What was done?

  • Added a condition to make sure filer is hoh before adding qualifying dependent info

How to test?

  • Updated units test

Copy link

Heroku app: https://gyr-review-app-5475-603a50cda1ce.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5475 (optionally add --tail)

@tahsinaislam tahsinaislam marked this pull request as ready for review January 29, 2025 17:02
Copy link
Contributor

@arinchoi03 arinchoi03 left a comment

Choose a reason for hiding this comment

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

one question/suggestion

@@ -44,7 +44,7 @@ def documents_wrapper
xml_doc = build_xml_doc("Form140") do |xml|
xml.LNPriorYrs sanitize_for_xml(@submission.data_source.prior_last_names)
xml.FilingStatus filing_status
if hoh_qualifying_person.present?
if hoh_qualifying_person.present? && @submission.data_source.filing_status_hoh?
Copy link
Contributor

@arinchoi03 arinchoi03 Jan 29, 2025

Choose a reason for hiding this comment

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

can hoh_qualifying_person exist if filing status is not hoh?
i.e. can we fold in this check into the hoh_qualifying_person logic?
I guess I'm seeing the test coverage and wondering if this is a likely scenario that a :single filer will have an hoh qualifying person.

just return false unless @submission.data_source.filing_status_hoh? in the method perhaps

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 we had a hoh qualifiying dependent on a single filer

Copy link
Contributor

@arinchoi03 arinchoi03 left a comment

Choose a reason for hiding this comment

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

ty for clarifying! I should've read the story a little more closely

@tahsinaislam tahsinaislam merged commit 2ae4a0c into main Jan 29, 2025
7 checks passed
@tahsinaislam tahsinaislam deleted the FYST-1721-az-do-not-include-qual-child-dependent-name-unless-filer-is-ho-h branch January 29, 2025 20:53
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