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 1724 NC and NJ to include NRA status in state xml if present on df xml #5486

Conversation

tahsinaislam
Copy link
Contributor

@tahsinaislam tahsinaislam commented Jan 30, 2025

Link to pivotal/JIRA issue

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

Is PM acceptance required? (delete one)

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

What was done?

  • Added in condition to return header to check NRA status for MD NJ and NC when mfs to include spouse section with MRA code instead of SSN
  • Added in condition to NC D400 to not include spouse ssn if spouse has NRA status
  • MD still included ssn if spouse is NRA because mfs status in the xml required spouse ssn to be included

How to test?

  • Updated unit tests for NC D400 and the return header
  • Uploaded a MD persona "Drum_Nra" to test locally

Screenshots (for visual changes)

Screenshot 2025-01-30 at 10 24 10 AM

Copy link

Heroku app: https://gyr-review-app-5486-99a5be932233.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5486 (optionally add --tail)

"no1099Amount": null
}
]
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this persona was added after a conversation with tiffany to test on demo

@@ -393,9 +393,22 @@

context "AZ intake" do
let(:intake) { create(:state_file_az_intake,) }
let(:submission) { create(:efile_submission, data_source: intake) }
let(:doc) { SubmissionBuilder::ReturnHeader.new(submission).document }
it "does not include disaster relief xml" do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this test was not properly testing the xml itself before. Its unrelated to the story

@tahsinaislam tahsinaislam marked this pull request as ready for review January 30, 2025 17:02
@@ -43,15 +43,20 @@ def document
xml.DateSigned date_type_for_timezone(@submission.data_source.primary_esigned_at)&.strftime("%F") if @submission.data_source.primary_esigned_yes?
xml.USPhone @submission.data_source.direct_file_data.phone_number if @submission.data_source.direct_file_data.phone_number.present?
end
if @submission.data_source&.spouse&.ssn.present? && @submission.data_source&.spouse&.first_name.present? && [email protected]_status_mfs?
has_nra_spouse = @intake.check_nra_status? && @intake.direct_file_data.non_resident_alien == "NRA" && @intake.filing_status_mfs?
if @submission.data_source&.spouse&.ssn.present? && @submission.data_source&.spouse&.first_name.present? && ([email protected]_status_mfs? || has_nra_spouse)
Copy link
Contributor

Choose a reason for hiding this comment

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

will @submission.data_source ever return nil? I think we can remove the first &

Copy link
Contributor

Choose a reason for hiding this comment

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

with this addition, when the filer is filing_status_mfs && has_nra_spouse -- we are also newly including the TaxPayerName, DateOfBirth, TaxPayerPIN etc information with the xml. Just making sure that that's a change that prod is expecting and should not be a source of issue downstream

Copy link
Contributor Author

Choose a reason for hiding this comment

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

super good question! talked with tiffany we should include the fields. Things like PIN wont be included in a mfs filing anyways

xml.TaxpayerSSN @submission.data_source.spouse.ssn if @submission.data_source.spouse.ssn.present?
if has_nra_spouse
xml.NRALiteralCd "NRA"
elsif @submission.data_source.spouse.ssn.present?
Copy link
Contributor

Choose a reason for hiding this comment

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

does this need to be @submission.data_source.spouse&.ssn&.present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably not because its after if @submission.data_source.spouse&.ssn&.present? so maybe its redundant?

Copy link
Contributor

Choose a reason for hiding this comment

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

o yea

@tahsinaislam tahsinaislam changed the title FYST 1724 NC MD and NJ to include NRA status in state xml if present on df xml FYST 1724 NC and NJ to include NRA status in state xml if present on df xml Jan 30, 2025
@tahsinaislam tahsinaislam merged commit 964a18d into main Jan 30, 2025
7 checks passed
@tahsinaislam tahsinaislam deleted the FYST-1724-nc-md-include-nra-literal-cd-in-state-xml-if-present-on-df-xml branch January 30, 2025 21:31
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