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: Moved enrollments charts to new API endpoint. #1298

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

saleem-latif
Copy link
Contributor

Jira Ticket: ENT-9420

Description:
Updated frontend logic according to the backend changes made in openedx/edx-enterprise-data#489

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

@saleem-latif saleem-latif self-assigned this Sep 12, 2024
@saleem-latif saleem-latif force-pushed the saleem-latif/ENT-9420 branch 2 times, most recently from 7101e5e to c1b14b2 Compare September 12, 2024 11:39
Copy link

codecov bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 95.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 85.60%. Comparing base (115c430) to head (5235e25).
Report is 33 commits behind head on master.

Files with missing lines Patch % Lines
...yticsV2/data/hooks/useEnterpriseEnrollmentsData.js 85.71% 3 Missing ⚠️
...components/AdvanceAnalyticsV2/tabs/Enrollments.jsx 82.35% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1298      +/-   ##
==========================================
+ Coverage   85.52%   85.60%   +0.07%     
==========================================
  Files         564      566       +2     
  Lines       12351    12467     +116     
  Branches     2621     2641      +20     
==========================================
+ Hits        10563    10672     +109     
- Misses       1729     1737       +8     
+ Partials       59       58       -1     

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

docs/decisions/0008-application_state_differenciation.rst Outdated Show resolved Hide resolved
package.json Outdated
@@ -41,13 +41,14 @@
"@tanstack/react-query": "4.36.1",
"@tanstack/react-query-devtools": "4.36.1",
"algoliasearch": "4.8.3",
"axios": "^1.7.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please explain the reason behind this change?

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 was getting an error of missing axios while it being used in the code. So, I added this to fix that.

src/components/AdvanceAnalyticsV2/tabs/AnalyticsTable.jsx Outdated Show resolved Hide resolved
@saleem-latif saleem-latif merged commit 152488a into master Sep 16, 2024
6 checks passed
@saleem-latif saleem-latif deleted the saleem-latif/ENT-9420 branch September 16, 2024 07:53
@muhammad-ammar muhammad-ammar added the plotly-migration Work to migrate from Plotly DASH to PlotlyJS label Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plotly-migration Work to migrate from Plotly DASH to PlotlyJS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants