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

Anoushka fix most hrs week badge #1150

Merged
merged 2 commits into from
Jan 12, 2025

Conversation

Anoushka21
Copy link
Contributor

Description

image
The Most hours in a week badge wasn't being awarded correctly and resulted in TypeErrors due to logical errors and lack of error handling in code

Related PRS (if any):

This is a backend PR

Main changes explained:

  • Updated logic of checkMostHrsWeek in userHelper.js
  • Look for the badge "Most hours in a week" in the database instead of the specific user's badge collection.
  • Add error handling if the badge "Most hours in a week" doesn't exist in the database.
  • Find badge "Most hours in a week" in the badgeCollection of the user with maximum hours. Increment badeCount if exists or add new badge

How to test:

  1. check into current branch
  2. Go to routes/badgeRouter.js and uncomment line 8
  3. Go to controller/badgeController.js and uncomment line 6, 16,17,18, 348
  4. Go to helpers/userHelper.js and uncomment line 1631 and line 1634 for ease of testing and logging statements.
  5. Go to Mongodb , userProfiles and search for a document using email field. Eg - { email: "[email protected]" }. In this document update savedTangibleHrs array item to reflect max hours in a week and also update field lastWeekTangibleHrs with same number of hours.
  6. Go to helpers/userHelper.js and update line 2019 with const users = await userProfile.find({email: xxx}).populate('badgeCollection.badge'); and comment line 2025,2026,2028-2032 for faster testing.
  7. do npm install and npm run dev to run this PR locally
  8. Go to Postman and send api request to http://localhost:4500/api/badge/awardBadgesTest with login authorization token.
  9. The API result should say Badges Awarded and on terminal you should see the logging statements and no TypeErrors.

Screenshots or videos of changes:

Screen.Recording.2024-11-15.232411.mp4

Note:

Include the information the reviewers need to know.

@Anoushka21 Anoushka21 added High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible and removed High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible labels Nov 19, 2024
Copy link

@humera314 humera314 left a comment

Choose a reason for hiding this comment

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

Works perfectly well

video2617219546.mp4

Copy link

@Muhideenthegreat Muhideenthegreat left a comment

Choose a reason for hiding this comment

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

Verification

  • Most Hours Per Week badge is now correctly assigned!
Screen.Recording.2024-11-21.at.12.55.58.PM.mov

@Hritvik111 Hritvik111 added the High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible label Nov 23, 2024
Copy link

@AshritaCherlapally AshritaCherlapally left a comment

Choose a reason for hiding this comment

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

Code changes works well, most hrs week badge is getting updated as per the fix.

PR.1150.mov

@mashpotato9
Copy link

code changes work as expected
Screenshot 2024-12-19 at 11 08 26 PM
Screenshot 2024-12-19 at 11 08 34 PM
Screenshot 2024-12-19 at 11 08 39 PM

Copy link

@kobakvantrishvili kobakvantrishvili left a comment

Choose a reason for hiding this comment

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

Works well. The Bug seems to be fixed.

PR 1150 Back test

@one-community
Copy link
Member

Thank you all, merging!

@one-community one-community merged commit d57c0d1 into development Jan 12, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants