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

NJ 140 - fix XML errors and add schema validation to all XML tests #4925

Merged
merged 5 commits into from
Oct 31, 2024

Conversation

aloverso
Copy link
Contributor

Link to pivotal/JIRA issue

https://github.com/newjersey/affordability-pm/issues/140

Is PM acceptance required? (delete one)

  • No - merge after code review approval

What was done?

  • Fix error introduced by W2 changes (need for to_i)
  • Add tests that would have caught this error to nj_return_xml_spec, automatically sync W2s in NJ intake
  • Add tests that will catch future issues in XML including client questions by adding XML validation to all nj1040 specs

How to test?

n/a

Screenshots (for visual changes)

Before:

113:0: ERROR: Element '{http://www.irs.gov/efile}WagesSalariesTips': '50000.0' is not a valid value of the atomic type '{http://www.irs.gov/efile}NJAmountNNType'.
115:0: ERROR: Element '{http://www.irs.gov/efile}TotalIncome': '50000.0' is not a valid value of the atomic type '{http://www.irs.gov/efile}NJAmountNNType'.
116:0: ERROR: Element '{http://www.irs.gov/efile}GrossIncome': '50000.0' is not a valid value of the atomic type '{http://www.irs.gov/efile}NJAmountNNType'.
120:0: ERROR: Element '{http://www.irs.gov/efile}TaxableIncome': '45000.0' is not a valid value of the atomic type '{http://www.irs.gov/efile}NJAmountNNType'.
124:0: ERROR: Element '{http://www.irs.gov/efile}NewJerseyTaxableIncome': '45000.0' is not a valid value of the atomic type '{http://www.irs.gov/efile}NJAmountNNType'.

@aloverso aloverso force-pushed the nj-140-xml-schema-validation branch 2 times, most recently from 2b1f9c7 to 3700b58 Compare October 29, 2024 17:11
Copy link

Heroku app: https://gyr-review-app-4925-0e89a5f11bd7.herokuapp.com/
View logs: heroku logs --app gyr-review-app-4925 (optionally add --tail)

Copy link
Contributor

@jachan jachan left a comment

Choose a reason for hiding this comment

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

Looks great. Appreciate you adding muni code by default, definitely simplifies future testing. Seeing errors in the CircleCI tests related to state wages, is the calculation still happening correctly?

@jenny-heath
Copy link
Contributor

@aloverso looks like the issue was that one of the W2s in the arizona fixture for :df_data_2_w2s lacked a LocalityNm (and actually just doesn't have contents in W2LocalTaxGrp). according to our validations on the model it's valid to be missing those things, so i just added in an if statement. i'd like to flesh out the test a little more but i think what i'll do as a follow-up to this is check with our product people to get a more comprehensive idea of what we care about re: building the w2s, now that we have them all in the db. then can refactor it out into a separate spec for return_w2.rb (which i've been meaning to do anyway).

@aloverso aloverso merged commit 34667d5 into main Oct 31, 2024
7 checks passed
@aloverso aloverso deleted the nj-140-xml-schema-validation branch October 31, 2024 18:03
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.

4 participants