-
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
Jiayu Fix bug manual add lost volunteer time #977
Jiayu Fix bug manual add lost volunteer time #977
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.
I have tested the PR and it works as expected. I have left a comment on 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.
Great job on this everything works fine
Everything is working fine. I have left a comment on the front-end PR. Review |
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 functionality works and I have reviewed the PR for the front-end PR OneCommunityGlobal/HighestGoodNetworkApp#2319 (review)
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.
I have tested the PR and it works as expected. I have left a comment on frontend
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 Jiayu,
This functionality works as expected. Dropped a video implementation on the related front end PR
OneCommunityGlobal/HighestGoodNetworkApp#2319 (review)
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.
Changes work as expected. Great work.
PR.2319.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.
Hello,
Great work, but I have observed while adding lost time for team, steps to reproduce:
Add lost time -> select team -> and type a new team name -> it will ask for create new team with the name you have given, while it doesn't make sense to add lost time for a team that has not been created, I'm not sure that option might be at a wrong place, correct me if I'm wrong.
Screen.Recording.2024-06-29.at.4.56.32.PM.mov
Screen.Recording.2024-06-29.at.4.58.04.PM.mov
Screen.Recording.2024-06-29.at.5.03.15.PM.mov
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.
Found an issue that might not be related to this PR, hence approving it.
Please report this to concerned team, Thanks @peter6866
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.
this PR working as intended and i left comment on frontend PR.
OneCommunityGlobal/HighestGoodNetworkApp#2319 (review)
Thank you all, merging! |
Description
Related PRS (if any):
This backend PR is related to FE# 2319.
Main changes explained:
timeEntryController.js
to allow adding/editing/deleting non-user time entries for projects and teams.How to test:
npm install
andnpm run build && npm run start
to run this PR locallyScreenshots or videos of changes:
Before Changes
After Changes
https://www.loom.com/share/df6c6c8043ad405088c3766672dec080?sid=e4b769b9-4c1a-4245-9f1e-735f2f9c4342