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-1717] Add exemption amounts for Dependent Taxpayers #5481

Merged

Conversation

DrewProebstel
Copy link
Contributor

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?

  • Modified the 502 xml filler so that the required fields are always present.

How to test?

  • Go through MD with a Dependent Tax Payer

Copy link

Heroku app: https://gyr-review-app-5481-9939f3a5ea01.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5481 (optionally add --tail)

@@ -244,7 +244,7 @@

context "exemptions stuff" do
Copy link
Contributor

Choose a reason for hiding this comment

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

can we rename this to context "exemptions" 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.

you got it

xml.Count calculated_fields.fetch(:MD502_LINE_D_COUNT_TOTAL)
xml.Amount calculated_fields.fetch(:MD502_LINE_D_AMOUNT_TOTAL)
end
if has_dependent_exemption?
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

xml.Count calculated_fields.fetch(:MD502_LINE_A_COUNT)
xml.Amount calculated_fields.fetch(:MD502_LINE_A_AMOUNT)
end
if has_exemptions?
Copy link
Contributor

Choose a reason for hiding this comment

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

can you also add a test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 have a few clarifying questions and requests for tests.

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.

thanks so much for walking me through this!

@DrewProebstel DrewProebstel merged commit f3497f2 into main Jan 30, 2025
7 checks passed
@DrewProebstel DrewProebstel deleted the FYST-1717-add-exemption-amounts-for-dependent-taxpayers branch January 30, 2025 21:05
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