-
Notifications
You must be signed in to change notification settings - Fork 28
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
Roberto-Fixes assign badge delay #562
Conversation
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.
Hi, I test this pr and it works as intended. Great Work!
video2699265447.mp4
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.
Hey @robertoooc! Tested your changes and they LGTM! Commented a full review on the frontend PR.
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.
Hi @robertoooc, I tested your changes, and all the fixes in this PR work as intended. Detailed review left on FE PR #1390. Great work!
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.
Test video and comment left in FE PR. Nice work! I worked on a similar issue related to delay update and I also found it was related to cache.
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.
Hi, I left my comment on FE.
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.
Hi @robertoooc, I tested your changes, and all the fixes in this PR are working as intended. Detailed review on Frontend PR #1390.
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.
Review left on FE PR #1390.
Hi, I left my comment on FE#1390. |
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.
Hi there, I ledge a comment on FE#1390
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.
Hello, I left my comment on FE#1390.
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.
Backend looks good, left a change request on FE #1390.
Hi, I test this pr and it works as intended. I left my comment on FE#1390. |
Hey, I have left a detailed review on PR 1390. Thank you |
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.
Clean code once again. Cache is handled exactly as desired. No console errors detected during testing either.
Badges updated after 1 refresh as mentioned in the testing instructions. I refreshed from the user's profile page in order to see if the changes took place. Also logged out and under a different user with the new badge still persisting. Fantastic job!
Description
When assigning badges to users specifically on the badgemagement, there was either a delayed response which would take minutes to update the user profile or it wouldn't update at all.
Related PRS (if any):
To test this backend PR you need to checkout the #1390 frontend PR.
…
Main changes explained:
…
How to test:
npm install
,npm run build
, andnpm start
to run this PR locallyNote:
Please make sure to run
npm run build
, if not the code will not be compiled and changes will not be shown.