-
Notifications
You must be signed in to change notification settings - Fork 30
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
Fix personal max record badge #1193
base: development
Are you sure you want to change the base?
Conversation
… current weeks hours.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unclear to me how to test this PR. I don't know where exactly to modify the backend code and I'm also not sure what HTTP request I should send out. More clarity or a video demonstration would be good. I have no complaints with the actual code that's modified as it makes sense and is readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Personal Max Record Badge is not awarded to my test email address after following those steps. I have attached the exact steps and the resulting output.
If the user has no badges or joined recently, then there won’t be any badges, and this loop won’t execute at all:
for (let i = 0; i < badgeCollection.length; i += 1) {
if (badgeCollection[i].badge?.type === 'Personal Max') {
As a result, the record for 'Personal Max' won’t be created.
The same situation applies to a new user with a default lastWeekTangibleHrs. In that case, this condition will fail:
user.lastWeekTangibleHrs &&
user.lastWeekTangibleHrs >= user.personalBestMaxHrs
Additionally, if badgeOfType is undefined, this line will result in an error:
badgeOfType.earnedDate.unshift(moment().format('MMM-DD-YYYY'));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the PR but the personal max is not working as mentioned in the description. Also, instead of naming in a generic way please use appropriate method name like mergeHours
or something feasible to you instead of mergeArrays.
Also do not add comments in the code.
Description
Fixes:
Related PRS (if any):
Related backend PR: PR #947
To test this PR use frontend development.
Main changes explained:
Time Entries
anduser.savedTangibleHrs
How to test:
npm install
and...
to run this PR locallyAdd intangible Time Entry
Note: earnedDate will showcase the current date (Sunday) and not the previous dates, this is because
user.savedTangibleHrs
does not have any dates associated with its hours, whereasTime Entries
does. Due to this, previous dates are not accounted for.Screenshots or videos of changes:
Modify the backend with the following to test this PR:
Verify
Note:
Will need to use Postman to POST and GET requests and MongoDB to update savedTangibleHrs and verify that the badge collection has the badge.