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

Support multiple years #778

Merged
merged 27 commits into from
Jan 1, 2022

Conversation

zakpatterson
Copy link
Collaborator

@zakpatterson zakpatterson commented Dec 15, 2021

Add form implementations for 2021
Work on 2019 form implementations is going on here: zakpatterson#91

What kind of change does this PR introduce?

  • Feature
  • Docs
  • Code style update
  • Refactor
  • Build-related changes

After a lot of experimentation in #662, I suggest we maintain a monorepo structure.

I believe the benefits of splitting the repository will never outweigh the costs. The initial motivations of splitting the repo involved being able to release the forms implementations to NPM for potential use in other projects, and maintaining one coherent view of the forms implementations, using git to jump between years.

The first sign this was a bad idea was the need for core project that the forms branches and the main repo could access. Splitting the code on tax year using git submodules means that now there's 2N+2 git modules that can potentially get out of sync, as each year requires a copy of core, and the main project requires a copy of core. This can be mitigated with scripts, but we're not getting anything for that complexity.

We will still be able to release from a single directory in our source tree down the line, so I think even with a monorepo structure we can release our form implementations. Splitting is not necessary.

While having separate branches of our forms implementation would solve the second goal, it comes at considerable cost as each year's branch will continue to diverge from each other. To mitigate this we will require PRs to bring them closer together, example. Alternatively, in a single repository, we can simply diff the year directories to see how they are drifting, and adjust accordingly, which should be less cumbersome.

Anyway, the experiment on #662 was valuable and the form implementations developed there were easily copied into this project. At this point, however, I propose we just reorganize the source tree and leave a single repository.

Closes #662
Fixes #753

@codecov
Copy link

codecov bot commented Dec 15, 2021

Codecov Report

Merging #778 (25a9e1b) into master (1668ddc) will increase coverage by 0.56%.
The diff coverage is 70.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #778      +/-   ##
==========================================
+ Coverage   69.96%   70.52%   +0.56%     
==========================================
  Files         116      194      +78     
  Lines        3815     6158    +2343     
  Branches      595     1010     +415     
==========================================
+ Hits         2669     4343    +1674     
- Misses       1132     1800     +668     
- Partials       14       15       +1     
Impacted Files Coverage Δ
src/components/DataPropagator.tsx 0.00% <0.00%> (ø)
src/components/GettingStarted.tsx 37.50% <ø> (ø)
src/components/Main.tsx 0.00% <0.00%> (ø)
src/components/Menu.tsx 93.33% <ø> (-0.42%) ⬇️
src/components/Patterns.ts 87.50% <0.00%> (-6.25%) ⬇️
src/components/SaveToFile.tsx 0.00% <0.00%> (ø)
src/components/TaxPayer/PersonFields.tsx 25.00% <0.00%> (ø)
src/components/debug.tsx 0.00% <0.00%> (ø)
src/components/deductions/F1098eInfo.tsx 16.00% <ø> (ø)
src/components/income/F1099Info.tsx 56.17% <ø> (ø)
... and 188 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1668ddc...25a9e1b. Read the comment docs.

Add implementations for 2021
@zakpatterson zakpatterson force-pushed the feature-years-monorepo branch 2 times, most recently from 7893bfe to a7556e2 Compare December 15, 2021 14:38
@zakpatterson zakpatterson changed the title Move files to support multiple years Support multiple years Dec 16, 2021
@thegrims
Copy link
Collaborator

image

filing status is invalidated when year is switched back to year with save state, resets after page refresh

image

@thegrims
Copy link
Collaborator

Getting this error when trying to print federal 1040 forms
image

@thegrims
Copy link
Collaborator

thegrims commented Dec 22, 2021

Comment has been resolved by Zak

recording

when the state is Illinois, an error is thrown when navigating to /createpdf

Browser metadata
Path:      /createpdf
Browser:   Chrome 96.0.4664.110 on Windows 10
Viewport:  1536 x 754 @1.25x
Language:  en-US
Cookies:   Enabled

Open in BrowserStack

Open Deploy Preview · Mark as Unresolved

@thegrims
Copy link
Collaborator

image

Copy link
Collaborator

@thegrims thegrims left a comment

Choose a reason for hiding this comment

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

See above comments

@zakpatterson zakpatterson reopened this Dec 27, 2021
@thegrims thegrims dismissed their stale review December 27, 2021 22:21

changes needed per review

@thegrims
Copy link
Collaborator

Yes, the EIC and IL 1040V seem like current issues in production. Seems like we're not handling refundable credits correctly.

I'll handle the state warnings and merge, then solve the other two things separately.

I don't think the same EIC issue is showing up in prod
image

@zakpatterson
Copy link
Collaborator Author

I think I got the display of the proper EIC credit, as well as a more helpful message about no filing states.

image

@zakpatterson
Copy link
Collaborator Author

Fixed up the IL1040 line 10b issue here for efficiency.

@thegrims
Copy link
Collaborator

thegrims commented Dec 29, 2021

Comment has been resolved by Zak

Error thrown when navigating to the /createpdf page and no information is inputted previously

recording

Browser metadata
Path:      /createpdf
Browser:   Chrome 96.0.4664.110 on Windows 10
Viewport:  1536 x 754 @1.25x
Language:  en-US
Cookies:   Enabled

Open in BrowserStack

Open Deploy Preview · Mark as Unresolved

@thegrims
Copy link
Collaborator

image
Error message

@thegrims
Copy link
Collaborator

thegrims commented Dec 30, 2021

Should ask if fincen is required if you have a financial interest in an asset located in a foreign country is checked

image

@thegrims
Copy link
Collaborator

thegrims commented Dec 30, 2021

Comment has been resolved by Zak

Saved 1099-R information is incorrectly pulled into new 1099 form input

recording

Browser metadata
Path:      /income/f1099s
Browser:   Chrome 96.0.4664.110 on Windows 10
Viewport:  1536 x 754 @1.25x
Language:  en-US
Cookies:   Enabled

Open in BrowserStack

Open Deploy Preview · Mark as Unresolved

@thegrims
Copy link
Collaborator

image
Additional child tax credit on line 15 on form 8812 should be $1125 instead of empty

@thegrims
Copy link
Collaborator

this problem appears to be in prod as well

@zakpatterson
Copy link
Collaborator Author

As much as this PR has turned into a rewrite of the code, I think it's time to consider merging it so we can deal with other issues without conflicts. Let me know what you think. I'm open to cleaning it up in some way if it would help.

@thegrims
Copy link
Collaborator

As much as this PR has turned into a rewrite of the code, I think it's time to consider merging it so we can deal with other issues without conflicts. Let me know what you think. I'm open to cleaning it up in some way if it would help.

That makes sense

@zakpatterson
Copy link
Collaborator Author

Really good QA / review work, lets gooooo 2021...

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.

W2 Medicare wages allowed to be greater than actual wages
2 participants