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

feat: show all enterprise budgets regardless of plan and route correctly #1042

Merged
merged 2 commits into from
Sep 27, 2023

Conversation

jajjibhai008
Copy link
Contributor

@jajjibhai008 jajjibhai008 commented Sep 27, 2023

Description
This PR covers up the following points

  • All budgets of ecommerce offers and learner-credit offers are showing regardless of plan.
  • View Budget button takes the learner to the detail page along with offerId/budgetId
  • Create two new routes
    • for e-commerce /:enterpriseSlug/admin/learner-credit/:offerId
    • for learner_credit budget /:enterpriseSlug/admin/learner-credit/:budgetId
  • Fetch the enrollments on the basis of budget_id if present otherwise, fetch enrollments on the base of offer_id on the detail page
  • The “Budgets” link in the breadcrumbs on the LCM UI relies on react-router’s Link component and avoids a full-page refresh.
  • Ensure dollar amounts are displayed as monetary (e.g., $18,500.25) rather than just floats without any formatting.
  • Ensure LCM is wrapped in a fluid container for more left/right whitespace padding in the page contents. It should be consistent with other page’s throughout the MFE (e.g., see “Highlights” page’s left/right padding).

JIRA -> [ENT-7672, ENT-7673, ENT-7674, ENT-7565]

Here is the updated screen short
screencapture-localhost-1991-test-enterprise-admin-learner-credit-2023-09-25-19_13_50

For all changes

  • Ensure adequate tests are in place (or reviewed existing tests cover changes)

Only if submitting a visual change

  • Ensure to attach screenshots
  • Ensure to have UX team confirm screenshots

jajjibhai008 and others added 2 commits September 27, 2023 12:20
fix: full page refresh issue when clicking 'Budgets', added test coverage and fixed lint issues

feat: show all enterprise budgets regardless of plan and route correctly

fix: moved LCM routes to separate file

fix: resolve spacing and repetitions issue

fix: resolve spacing and repetitions issue

fix: improved test coverage

refactor: improve code efficiency and readability

fix: incorporate adam's suggestions and nits

fix: incorporated adam's feedback

fix: incorporate adam's feedback
feat: show all enterprise budgets regardless of plan and route correctly
@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

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

Comparison is base (e12f09a) 82.90% compared to head (5a6502c) 82.91%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1042      +/-   ##
==========================================
+ Coverage   82.90%   82.91%   +0.01%     
==========================================
  Files         412      415       +3     
  Lines        8902     8933      +31     
  Branches     1825     1832       +7     
==========================================
+ Hits         7380     7407      +27     
- Misses       1482     1484       +2     
- Partials       40       42       +2     
Files Coverage Δ
...c/components/EnterpriseApp/EnterpriseAppRoutes.jsx 65.00% <100.00%> (ø)
src/components/EnterpriseApp/data/constants.js 100.00% <100.00%> (ø)
...omponents/EnterpriseSubsidiesContext/data/hooks.js 94.64% <100.00%> (-1.08%) ⬇️
.../learner-credit-management/MultipleBudgetsPage.jsx 81.81% <100.00%> (ø)
...components/learner-credit-management/data/hooks.js 90.76% <100.00%> (+0.60%) ⬆️
...components/learner-credit-management/data/utils.js 96.00% <100.00%> (+3.14%) ⬆️
src/components/learner-credit-management/index.jsx 100.00% <100.00%> (ø)
...nts/learner-credit-management/BudgetDetailPage.jsx 92.85% <92.85%> (ø)
...onents/learner-credit-management/BudgetCard-V2.jsx 80.00% <50.00%> (-20.00%) ⬇️
...earner-credit-management/MultipleBudgetsPicker.jsx 50.00% <0.00%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

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

Copy link
Contributor

@muhammad-ammar muhammad-ammar left a comment

Choose a reason for hiding this comment

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

LGTM

@jajjibhai008 jajjibhai008 merged commit 1d57072 into master Sep 27, 2023
5 checks passed
@jajjibhai008 jajjibhai008 deleted the eahmadjaved/ENT-7660 branch September 27, 2023 08:57
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.

2 participants