-
Notifications
You must be signed in to change notification settings - Fork 3
Remove schedule generation on user creation #540
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
Conversation
…lly get generated
this-Aditya
left a comment
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 Pauline, please see the comments inline. Other than that, this PR looks good. Thanks.
I'll make sure to add this in the latest version as well
| if (!user.getAttributes().equals(userDto.getAttributes()) | ||
| || !user.getTimezone().equals(userDto.getTimezone()) | ||
| || !user.getEnrolmentDate().equals(userDto.getEnrolmentDate()) | ||
| || !user.getLanguage().equals(userDto.getLanguage())) | ||
| { |
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.
If I'm not wrong, the /schedule endpoint will be called by the questionnaire app. Do we have checks in the app to ensure that this endpoint is only called when updating the user, and only if these properties don’t match?
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.
Oh yes the user will be calling the update endpoint from the app anyway so they can just call the schedule endpoint afterwards. We can make the updates afterwards in a separate PR since we have not enabled the app server scheduling yet for users.
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.
Oh okay, Thanks.
| // Generate schedule for user | ||
| this.scheduleService.generateScheduleForUser(savedUser); |
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 don't see any use of the schedulerService variable, this can be removed.
this-Aditya
left a comment
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.
LGTM, Thanks.
/scheduleendpoint can simply get called