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.

Sign up for GitHub

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

Jump to bottom

NJ 118 - Lines 45, 49, 50, 54: Calculating Total Tax Due #5023

Merged
merged 6 commits into from
Nov 26, 2024
Merged

NJ 118 - Lines 45, 49, 50, 54: Calculating Total Tax Due #5023

merged 6 commits into from
Nov 26, 2024

Conversation

aloverso
Copy link
Contributor

Link to pivotal/JIRA issue

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

Is PM acceptance required? (delete one)

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

What was done?

  • Basic arithmetic calculations for lines 45, 49, 50, 54
  • Add to XML and PDF

How to test?

zeus_many_w2s will get you some numbers in here. Especially if you add sales/use tax and property tax credit

Screenshots (for visual changes)

image

image

@aloverso aloverso force-pushed the nj-118-total-tax-due branch from 084fd99 to b95e2a7 Compare November 22, 2024 19:44
@github-actions GitHub Actions
Copy link

Heroku app: https://gyr-review-app-5023-8ce2170788d2.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5023 (optionally add --tail)

mmazanec22
mmazanec22 approved these changes Nov 22, 2024
View reviewed changes
app/lib/pdf_filler/nj1040_pdf.rb Show resolved Hide resolved
spec/lib/efile/nj/nj1040_calculator_spec.rb Outdated Show resolved Hide resolved
spec/lib/pdf_filler/nj1040_pdf_spec.rb Outdated Show resolved Hide resolved
spec/lib/submission_builder/ty2024/states/nj/documents/nj1040_spec.rb Show resolved Hide resolved
mluedke2
mluedke2 approved these changes Nov 22, 2024
View reviewed changes
Copy link
Contributor

@mluedke2 mluedke2 left a comment

Choose a reason for hiding this comment

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

one pebble otherwise 👍 from me!

embarnard
embarnard requested changes Nov 25, 2024
View reviewed changes
Copy link
Contributor

@embarnard embarnard left a comment

Choose a reason for hiding this comment

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

Overall, this looks good! I left some notes about calling lines in the calculator from the set lines rather than using the calculator methods directly. Let me know if you'd like to discuss this further—happy to do a re-review once those adjustments are made!

app/lib/efile/nj/nj1040_calculator.rb Show resolved Hide resolved
app/lib/efile/nj/nj1040_calculator.rb Show resolved Hide resolved
app/lib/efile/nj/nj1040_calculator.rb Show resolved Hide resolved
app/lib/efile/nj/nj1040_calculator.rb Show resolved Hide resolved
spec/lib/submission_builder/ty2024/states/nj/documents/nj1040_spec.rb Show resolved Hide resolved
app/lib/efile/nj/nj1040_calculator.rb Show resolved Hide resolved
@aloverso aloverso force-pushed the nj-118-total-tax-due branch from e6241d4 to 48953e8 Compare November 26, 2024 18:33
@aloverso aloverso merged commit ba01d9d into main Nov 26, 2024
7 checks passed
@aloverso aloverso deleted the nj-118-total-tax-due branch November 26, 2024 19:01
@aloverso aloverso mentioned this pull request Nov 26, 2024
Merged
squanto added a commit that referenced this pull request Nov 27, 2024
@aloverso @squanto
NJ 118 - Lines 45, 49, 50, 54: Calculating Total Tax Due (#5023)
9aa9f70 
Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>
squanto added a commit that referenced this pull request Dec 3, 2024
@squanto @mrotondo
@anisharamnani @tahsinaislam @jnf @mpidcock @aloverso @powersurge360
[Fyst-294] add notification preferences page (#5027)
c9a27cc 
* Add state file md notification prefs

Co-authored-by: Mike Rotondo <mrotondo@codeforamerica.org>

* Add ny opt in columns

Co-authored-by: Mike Rotondo <mrotondo@codeforamerica.org>

* Add state file az intake fields

Co-authored-by: Mike Rotondo <mrotondo@codeforamerica.org>

* Add id and nc forms

Co-authored-by: Mike Rotondo <mrotondo@codeforamerica.org>

* Fill out specs and add nj

Co-authored-by: Mike Rotondo <mrotondo@codeforamerica.org>

* fix failing specs

Co-authored-by: Mike Rotondo <mrotondo@codeforamerica.org>

* Fix data transfer spec

Co-authored-by: Mike Rotondo <mrotondo@codeforamerica.org>

* Normalize i18n

Co-authored-by: Mike Rotondo <mrotondo@codeforamerica.org>

* Add specs for form

Co-authored-by: Mike Rotondo <mrotondo@codeforamerica.org>

* reference ID 2024 Schema (#5004)

Co-authored-by: Tahsina Islam <tislam@codeforamerica.org>
Co-authored-by: Anisha Ramnani <aramnani@codeforamerica.org>

* [FYST-1217] Tweak a default parameter so it renders as html (#5028)

* tweak the default render call for landing page bullets
* hint for the linter; because the key is used as a default, the i18n linter doesn't recognize it as being used
Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* Change content for political subdivisions (#5019)

Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* Upload additional DF XMLs & JSONs for NC (#5032)

Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* NJ 118 - Lines 45, 49, 50, 54: Calculating Total Tax Due (#5023)

Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* Adds persona import/export as a CLI (#5037)

Adds persona import/export CLI tool
Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* Fyst 1220 upload DF xmls & jsons for AZ (#5035)

* johnny

* alexis

* shell
Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* normailze number before validation

Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* Update content and spacing

Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* Preparse phone number and tighten checkboxes

Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* Normalize i18n

Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* Validate at least one is provided

Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* Fix nj acceptance spec

Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

* normalize i18n

Co-authored-by: Hugo Melo <hmelo@codeforamerica.org>

---------

Co-authored-by: Mike Rotondo <mrotondo@codeforamerica.org>
Co-authored-by: Anisha Ramnani <anisha.ramnani@gmail.com>
Co-authored-by: Tahsina Islam <tislam@codeforamerica.org>
Co-authored-by: Anisha Ramnani <aramnani@codeforamerica.org>
Co-authored-by: jey <jnf@users.noreply.github.com>
Co-authored-by: Martha Pidcock <122047394+mpidcock@users.noreply.github.com>
Co-authored-by: Anne LoVerso <anne.loverso@gmail.com>
Co-authored-by: Kevin Diale <kdiale@codeforamerica.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mluedke2 mluedke2 mluedke2 approved these changes

@mmazanec22 mmazanec22 mmazanec22 approved these changes

@embarnard embarnard embarnard approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aloverso @mluedke2 @mmazanec22 @embarnard