Skip to content

Sohail: Phase 1 - Fix Personal Max Record badge#2116

Merged
one-community merged 1 commit intodevelopmentfrom
Sohail-fix-personal-max-badge
Apr 9, 2026
Merged

Sohail: Phase 1 - Fix Personal Max Record badge#2116
one-community merged 1 commit intodevelopmentfrom
Sohail-fix-personal-max-badge

Conversation

@sohailuddinsyed
Copy link
Copy Markdown
Contributor

Description

This PR fixes critical issues with the Personal Max badge assignment and update logic that were causing the badge to never/incorrectly update when a user broke their personal weekly hours record.

Fixes: Personal Max badge assignment and update logic (Priority: High)

Original PR: #1193

Issues Fixed:

  1. The badge count was being incremented each time a record was broken, when only one Personal Max badge should ever exist per user.
  2. A race condition caused personalBestMaxHrs to be updated by $max earlier in the weekly flow before checkPersonalMax ran, meaning the comparison lastWeek > personalBest always evaluated to false and the badge never updated.
  3. The earnedDate was being appended to rather than replaced when a new record was set.
  4. The broken condition lastSaved > lastWeek was always false since both values are set to the same timeSpent in the same atomic DB operation.

Main changes explained:

  • Updated src/helpers/userHelper.js: Rewrote checkPersonalMax function with correct badge assignment logic
    • User now has exactly ONE Personal Max badge at all times
    • When a new weekly record is broken, earnedDate is replaced with the current date and personalBestMaxHrs is updated in the same atomic DB operation
    • Record comparison now uses Math.max(...savedTangibleHrs.slice(0, -1)) to derive the previous best from history, bypassing the race condition with personalBestMaxHrs
    • A tie does not count as a new record (strictly greater than)
    • Duplicate badge cleanup retained
    • Badge is automatically added for users who do not yet have one
  • Added src/helpers/__tests__/checkPersonalMax.spec.js: Unit test file covering 12 edge cases, all passing

How to test:

  1. Check out current branch
  2. Run npm install to ensure dependencies are up to date
  3. Run npm run dev to start the server locally
  4. Send POST request to /api/login, save the response auth token as a header in Postman
  5. Log in as an owner user
  6. Connect with our MongoDB server -> UserProfiles Collection -> Your user profile -> Test with different hours in savedTangibleHrs (For assuming hours for the most recent week, assign lastWeekTangibleHours)
  7. Test badge assignment via POST request to /api/badge/awardNewBadges

Important

Replace awardNewBadges in src/helpers/userHelper.js with this before sending an API call to ensure you only test for your dev account:

const awardNewBadges = async () => {
  try {
    const users = await userProfile
      .find({ isActive: true, firstName: 'Sohail', lastName: 'Admin' })
      .populate('badgeCollection.badge');
    const user = users[0];
    const { _id, badgeCollection } = user;
    const personId = mongoose.Types.ObjectId(_id);
    await checkPersonalMax(personId, user, badgeCollection);
    if (cache.hasCache(`user-${_id}`)) {
      cache.removeCache(`user-${_id}`);
    }
  } catch (err) {
    console.log(err);
    logger.logException(err);
  }
};
  1. To run the unit tests:
npx jest src/helpers/__tests__/checkPersonalMax.spec.js --no-coverage --forceExit
  1. Verify the following scenarios:

Test Scenario 1: New User — First Week Logging Hours

  • Set savedTangibleHrs = [10], lastWeekTangibleHrs = 10, personalBestMaxHrs = 0
  • Badge should be added, earnedDate = today, personalBestMaxHrs = 10

Test Scenario 2: Record Broken

  • Set savedTangibleHrs = [10, 8, 15, 12, 20], lastWeekTangibleHrs = 20
  • Badge earnedDate should update to today, personalBestMaxHrs = 20

Test Scenario 3: Record Not Broken / Tied

  • Set lastWeekTangibleHrs to a value equal to or less than the max of prior weeks
  • Badge should remain unchanged

Test Scenario 4: Zero Hours Logged

  • Set lastWeekTangibleHrs = 0
  • Badge should not update

Test Scenario 5: Duplicate Badge Cleanup

  • If user has multiple Personal Max badges, all but one should be removed automatically

Expected Results:

  • ✅ Each user has exactly ONE Personal Max badge
  • ✅ Badge earnedDate is replaced (not appended) when a new record is set
  • personalBestMaxHrs is updated atomically alongside the badge
  • ✅ A tie does not trigger a badge update
  • ✅ Zero hours logged does not trigger a badge update
  • ✅ New users get the badge assigned on their first week with hours
  • ✅ Duplicate badges are cleaned up automatically

Note:

  • The Personal Max badge tracks the highest tangible hours a user has logged in a single week
  • The weekly cron job (awardNewBadges) runs every Sunday at 12 AM and calls checkPersonalMax for each active user
  • The record comparison uses savedTangibleHrs history (last 200 weeks) excluding the current week, not the stored personalBestMaxHrs field, to avoid the race condition with the $max update earlier in the weekly flow

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@Anusha-Gali Anusha-Gali left a comment

Choose a reason for hiding this comment

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

Hi Sohail,

I tried to review your PR locally and i was able to perform the first test sucessfully however for the second test when i have updated the lastWeekTangibleHrs = 20, and ran the postman and the npm test, the personalBestMaxHrs did not update to 20 from 10. As you cam see in the below test 2 screenshots, i had provided the before test run and after test run mongo db screenshots, even though the batches got assigned, the value did not change in personalBestMaxHrs.

Test 1
Image
Image
Image
Image
Image
Image

Test 2
Image
Image
Image
Image
Image
Image

@sohailuddinsyed
Copy link
Copy Markdown
Contributor Author

Hi Sohail,

I tried to review your PR locally and i was able to perform the first test sucessfully however for the second test when i have updated the lastWeekTangibleHrs = 20, and ran the postman and the npm test, the personalBestMaxHrs did not update to 20 from 10. As you cam see in the below test 2 screenshots, i had provided the before test run and after test run mongo db screenshots, even though the batches got assigned, the value did not change in personalBestMaxHrs.

Hi @Anusha-Gali ,
Thank you for the review. It looks like the issue you encountered is due to the manually assigned database state in Test Scenario 2.

In my logic, the record comparison uses Math.max(...savedTangibleHrs.slice(0, -1)) to derive the previous best. This assumes that the current week's hours (20) are at the end of the array (since MongoDB's $push appends to the end).

In your screenshot/values, you assigned SavedTangibleHours = [..., 20, 12, 15, 8, 10].

  1. The 20 is at the beginning of the array, so slice(0, -1) did not remove it.
  2. This caused the logic to compare lastWeek (20) against a previousMax of 20 (from the start of the array).
  3. Since 20 is not strictly greater than 20, the update for personalBestMaxHrs was correctly skipped.

To test this correctly, please ensure that the savedTangibleHrs array either does not contain the 20 yet, OR that the 20 is the last element in the array. This will allow the slice(0, -1) logic to correctly identify the previous best (15 in your case) and trigger the update to 20.

Copy link
Copy Markdown

@Anusha-Gali Anusha-Gali left a comment

Choose a reason for hiding this comment

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

Hi Sohail,

I have reviewed your PR locally and below are my findings. Thank you for the help.

Before test
Image
Image

Scenario 1:
Image
Image
Image
Image
Image

Scenario 2
Image
Image
Image
Image
Image
Image

Scenario 3
Image
Image
Image
Image
Image

Scenario 4
Image
Image
Image
Image
Image

Scenario 5
Image

@one-community
Copy link
Copy Markdown
Member

Thank you all, merging!

@one-community one-community merged commit c21d374 into development Apr 9, 2026
3 checks passed
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.

3 participants