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

fix: Activity quarter table bug across multiple activities [#3459] #3459

Merged
merged 4 commits into from
Sep 20, 2021

Conversation

thetif
Copy link
Contributor

@thetif thetif commented Sep 2, 2021

resolves #3419

Description-
If you navigated between Activity's Budget and FFP pages the quarterly percentages were always showing the first one you had opened. If you went to a different page between the Budget and FFP pages it would show the correct value. I thought this might be related to #2822. We weren't able to have the useEffect because we didn't have the ability to mask the value properly so it was losing formatting. CMS Design system recently exported the maskValue function they use, so I was able to import it into NumberField and bring back the useEffect.

This pull request changes...

  • adds an useEffect to the NumberField and masks the value when it changes

This pull request also touches…

  • anything else that uses the NumberField

Steps to manually verify this change...

  1. open the APD
  2. open Activity 1: Budget and FFP
  3. change the quarterly percentages
  4. open Activity 2: Budget and FFP
  5. it should not have the same values that were entered for Activity 1
  6. open Key Personnel and Program Management
  7. add a new Key Personnel
  8. for the Does this person have costs directly attributable to this program? select yes
  9. enter a Cost with Benefits and see that it formats it correctly as money
  10. select no on Does this person have costs directly attributable to this program?
  11. select yes on Does this person have costs directly attributable to this program?
  12. verify that a Cost with Benefits is still formatted correctly as money

This pull request is ready to review when...

  • Automated tests are updated (and all tests are passing)

This pull request can be merged when…

  • Code has been reviewed by someone other than the original author
  • QA has verified the accessibility and functionality related to the change
  • Design has approved the experience
  • Product has approved the experience

@thetif thetif requested a review from tbolt September 2, 2021 18:17
@cms-eapd-bot
Copy link

cms-eapd-bot commented Sep 2, 2021

This deploy was cleaned up.

@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2021

Codecov Report

Merging #3459 (cea4dc1) into development (b2b9178) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #3459      +/-   ##
===============================================
- Coverage        88.60%   88.53%   -0.07%     
===============================================
  Files              284      284              
  Lines             5615     5616       +1     
  Branches          1073     1073              
===============================================
- Hits              4975     4972       -3     
- Misses             587      591       +4     
  Partials            53       53              
Impacted Files Coverage Δ
web/src/components/NumberField.js 100.00% <100.00%> (ø)
api/files/local.js 19.04% <0.00%> (-19.05%) ⬇️

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 b2b9178...cea4dc1. Read the comment docs.

Copy link
Contributor

@knollfear knollfear left a comment

Choose a reason for hiding this comment

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

looks good to me. Followed repro instructions and it behaved as expected.

Copy link
Contributor

@jeromeleecms jeromeleecms left a comment

Choose a reason for hiding this comment

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

Looks good. Check budget math on export and total amounts - looks like it matches. Subtotals reflect the calculated amount from the app vs. what is the subtotal calculated from what the data entry is (i.e. you can have something less than 100% represented in the quarterly table and the subtotal will still line up with 100% of the cost of the activity). If I recall that was by design.

Also verified number formatting and that works as advertised.

@thetif thetif changed the title fixes Activity quarter table bug across multiple activities fix: Activity quarter table bug across multiple activities [#3459] Sep 20, 2021
@thetif thetif merged commit 26b43dd into development Sep 20, 2021
@thetif thetif deleted the tforkner/3419-activity-quarter-table-bug branch September 20, 2021 18:56
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.

[Bug] Activity quarter table bug across multiple activities
7 participants