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

Huijie fix add lost hours editing deleting #1115

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

metaphor987
Copy link
Contributor

@metaphor987 metaphor987 commented Sep 22, 2024

Description

1

Related PRS (if any):

This backend PR is related to the frontend PR 2710.

Main changes explained:

The error when adding, editing, or deleting lost hours is caused by a Mongoose validation error for users with an empty jobTitle field. Setting the validateModifiedOnly option to true when saving updates to the user profile can ensure that only modified fields are validated.

How to test:

  1. check into current branch
  2. do npm install, run run build and npm start to run this PR locally
  3. Clear site data/cache
  4. log as admin or owner user (admin user do not have permission for adding lost hours)
  5. go to Reports -> Reports -> Add Lost Time, or Show Person Lost Time, verify that the following errors do not occur:
  6. owners do not get a 500 error when adding a person's lost time:
    error image before the PR
    2
  7. Do not get an error when updating a person's lost time
    error image before the PR
    2
  8. Do not get an error when deleting a person's lost time
    error image before the PR:
    2
  9. owners do not get an error when adding a lost time for team or project:
    error image before the PR:
    2
    Thank you!

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

@Ankuriboh Ankuriboh left a comment

Choose a reason for hiding this comment

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

Easy fix that only validates modified fields. Can confirm that new data is stored in database.

Copy link

@javid679 javid679 left a comment

Choose a reason for hiding this comment

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

Logged in as DevAdmin, can confirm data is stored in the database, below is the video for reference:

PR1115_.screenshot.mp4

@sheetalmangate
Copy link

sheetalmangate commented Oct 20, 2024

As per user story code changes look good but I could not able to test this PR locally

PR_2710_error

Copy link

@hqzhu0913 hqzhu0913 left a comment

Choose a reason for hiding this comment

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

Both people and team edit time work as expected. But there is no adding function as explained.
Screenshot 2024-10-22 at 11 58 07 AM
Screenshot 2024-10-22 at 11 58 48 AM
Screenshot 2024-10-22 at 11 52 05 AM
Screenshot 2024-10-22 at 11 52 30 AM

@hqzhu0913 hqzhu0913 self-requested a review October 22, 2024 19:02
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.

6 participants