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

[Maintenance] fixing budget migration #4644

Merged
merged 4 commits into from
Mar 28, 2023
Merged

Conversation

thetif
Copy link
Contributor

@thetif thetif commented Mar 28, 2023

Resolves budget migration issue

Description

I changed up the migration so that it does HITECH and MMIS separately because it seems to be having an issue setting the correct discriminator. I also changed the discriminators in the schemas to use constants.

This pull request is ready to code review when

  • Pull request has been labeled, if applicable with feature, content, bug,
    tests, refactor

This pull request is ready to test when

  • Code has been reviewed by someone other than the original author

@thetif thetif requested review from tbolt, amyd11 and Sun-Mountain March 28, 2023 15:59
Copy link
Contributor

@Sun-Mountain Sun-Mountain left a comment

Choose a reason for hiding this comment

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

🚀

@codecov
Copy link

codecov bot commented Mar 28, 2023

Codecov Report

Merging #4644 (c71bd99) into main (153d84a) will decrease coverage by 6.37%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4644      +/-   ##
==========================================
- Coverage   94.32%   87.95%   -6.37%     
==========================================
  Files         277      274       -3     
  Lines        8808     8603     -205     
  Branches     1773     1773              
==========================================
- Hits         8308     7567     -741     
- Misses        476      998     +522     
- Partials       24       38      +14     
Flag Coverage Δ
api ∅ <ø> (∅)
common 99.33% <100.00%> (+<0.01%) ⬆️
web 87.33% <ø> (-6.73%) ⬇️
Impacted Files Coverage Δ
common/utils/constants.js 100.00% <100.00%> (ø)

... and 25 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

{ _id: apd.budget },
{ ...budget }
{ ...budget, __t: BUDGET_TYPE[apdType] }
Copy link
Contributor

Choose a reason for hiding this comment

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

is this why it is/was failing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so because I think it wasn't properly setting the discrimination so it was only saving what is in the base budget

@cms-eapd-bot
Copy link

cms-eapd-bot commented Mar 28, 2023

This deploy was cleaned up.

@thetif thetif merged commit 47ef941 into main Mar 28, 2023
@thetif thetif deleted the tforkner/fix-budget-migration branch March 28, 2023 17:05
@thetif thetif added the maintenance PR label for code maintenance and misc changes label Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance PR label for code maintenance and misc changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants