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: replace all total xp computations with new backend api #2228

Merged
merged 7 commits into from
Sep 29, 2022

Conversation

riccqi
Copy link
Contributor

@riccqi riccqi commented Sep 21, 2022

Description

Implements the new api route introduced to backend in this PR: source-academy/backend#859

The user profile slide over and XP bar in achievement dashboard will now both get the total XP of the user from the new backend GET /user/total_xp route.

  • Added GET_TOTAL_XP and UPDATE_TOTAL_XP actions for this purpose.

Also includes minor cosmetic changes to the user profile slide over, and achievement milestone components.

@martin-henz martin-henz marked this pull request as draft September 28, 2022 14:13
@martin-henz martin-henz marked this pull request as ready for review September 28, 2022 14:15
@coveralls
Copy link

coveralls commented Sep 28, 2022

Pull Request Test Coverage Report for Build 3144282862

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 12 of 23 (52.17%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 33.518%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/commons/achievement/AchievementOverview.tsx 0 1 0.0%
src/commons/application/reducers/SessionsReducer.ts 0 1 0.0%
src/commons/sagas/BackendSaga.ts 1 5 20.0%
src/commons/sagas/RequestsSaga.ts 1 6 16.67%
Totals Coverage Status
Change from base Build 3135571773: -0.02%
Covered Lines: 4311
Relevant Lines: 11988

💛 - Coveralls

Copy link
Member

@shenyih0ng shenyih0ng left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@@ -79,6 +81,8 @@ export const fetchAssessment = (id: number) => action(FETCH_ASSESSMENT, id);

export const fetchAssessmentOverviews = () => action(FETCH_ASSESSMENT_OVERVIEWS);

export const getTotalXp = () => action(GET_TOTAL_XP);
Copy link
Member

Choose a reason for hiding this comment

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

just a nit, maybe fetchTotalXp would be a little better? This is so that it is consistent with fetchGrading and fetchAssessmentOverviews.

@martin-henz martin-henz merged commit c4042f5 into master Sep 29, 2022
@RichDom2185 RichDom2185 deleted the feat/integrate-totalxp-route branch February 5, 2024 15:58
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.

4 participants