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

fix: 3 bugs in the provisioning tool #369

Merged
merged 2 commits into from
Dec 5, 2023
Merged

fix: 3 bugs in the provisioning tool #369

merged 2 commits into from
Dec 5, 2023

Conversation

pwnage101
Copy link
Contributor

@pwnage101 pwnage101 commented Dec 5, 2023 •
edited
Loading

  1. We no longer double-multiply the subsidy balance by 100, leading to a higher subsidy starting balance than intended.
  2. catalogUuid no longer is interpreted as invalid when a predefined query is selected.
  3. On the edit view, the budget header now correctly reflects the catalog selection instead of always displaying "Budget". Now it displays "Open Courses budget", "Unique/Curated budget" , etc. depending on the catalog selection.

ENT-7922

pwnage101
pwnage101 commented Dec 5, 2023
View reviewed changes
src/data/services/SubsidyApiService.js
* @param {String} enterpriseUUID
* @param {String} startDate
* @param {String} endDate
* @param {Number} startingBalance - The initial balance of the new subsidy in USD Cents (integer).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our component-level tests don't cover testing this layer, so they don't assert what's really being sent in the POST request. The least I can do is document the expected parameters.

@codecov Codecov
Copy link

codecov bot commented Dec 5, 2023 •
edited
Loading

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (8195709) 88.09% compared to head (4b0f9aa) 88.02%.
Report is 2 commits behind head on master.

❗ Current head 4b0f9aa differs from pull request most recent head 52b4d75. Consider uploading reports for the commit 52b4d75 to get more accurate results

Files Patch % Lines
...ningForm/PolicyForm/ProvisioningFormPolicyType.jsx 91.66% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #369      +/-   ##
==========================================
- Coverage   88.09%   88.02%   -0.08%     
==========================================
  Files         161      159       -2     
  Lines        3361     3349      -12     
  Branches      825      826       +1     
==========================================
- Hits         2961     2948      -13     
- Misses        396      397       +1     
  Partials        4        4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

brobro10000
brobro10000 approved these changes Dec 5, 2023
View reviewed changes
Copy link
Member

@brobro10000 brobro10000 left a comment

Choose a reason for hiding this comment

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

LGTM. I would just double check the price validation in stage for the startingBalance on the subsidy. 👍🏽 Great job.

@pwnage101 pwnage101 force-pushed the pwnage101/ENT-7922-2 branch from da68aee to 76bc033 Compare December 5, 2023 22:20
@pwnage101
Copy link
Contributor Author

pwnage101 commented Dec 5, 2023 •
edited
Loading

FYI @brobro10000 I did just push one more commit to fix the new "policy type" field component to be supported in the edit view. It was pretty straightforward and mostly copy+pasting from another component to add that support.

@pwnage101 pwnage101 force-pushed the pwnage101/ENT-7922-2 branch from 4b0f9aa to c5788f2 Compare December 5, 2023 23:42
@pwnage101
fix: 3 bugs in the provisioning tool
5167c0c 
1. We no longer double-multiply the subsidy balance by 100, leading to a higher subsidy starting balance than intended.
2. catalogUuid no longer is interpreted as invalid when a predefined query is selected.
3. On the edit view, the budget header now correctly reflects the catalog selection.

ENT-7922
@pwnage101
feat: policy type field now supported in edit view (readonly).
52b4d75 
ENT-7922
@pwnage101 pwnage101 force-pushed the pwnage101/ENT-7922-2 branch from c5788f2 to 52b4d75 Compare December 5, 2023 23:47
@pwnage101
Copy link
Contributor Author

pwnage101 commented Dec 5, 2023 •
edited
Loading

Also added a commit to display the policy type in the edit view (readonly).
image

FYI @katrinan029

@pwnage101 pwnage101 merged commit 439e611 into master Dec 5, 2023
3 checks passed
@pwnage101 pwnage101 deleted the pwnage101/ENT-7922-2 branch December 6, 2023 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brobro10000 brobro10000 brobro10000 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.
2 participants
@pwnage101 @brobro10000