[#12045] Instructor copying sessions: preserve time when auto-changing dates #12051
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zhaojj2209
requested changes
Feb 4, 2023
There was a problem hiding this comment.
InstructorHomePageE2ETest is failing due to the changes made in this PR, do look into it.
Also, before opening a PR, we would appreciate if you could indicate interest in fixing the issue in the issue thread itself first, and check that there are no other people working to fix the issue.
|
Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Closing due to inactivity. Feel free to reopen this PR if you would like to continue working on it. |
|
Hi @zhaojj2209, I would like to reopen this PR but I think I don't have the right to reopen it. Could you reopen it for me, please? |
domlimm
approved these changes
Mar 1, 2023
zhaojj2209
approved these changes
Mar 1, 2023
zhaojj2209
added
s.ToMerge
samuelfangjw
added
the
c.Feature
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #12045
Outline of Solution
When copying a feedback session, if the dates aren't in the allowed time period, the new date will use your local time to generate a valid date range. For the opening time, the new date is +2 days from now and for the closing time the new date is +7 days from now. I kept this logic and added an extra logic to set the time to the original one instead of your local time.
I also changed some variable names as the names were saying "roundedUp", but what is really happening is a round down with the "startOf" method of moment.js. So I changed the last part to "roundedDown".
I didn't changed the logic to keep the time for dates that surpasses the upper bound limit because that could lead to some corner case problems if the original time is bigger than the local time (the date would be bigger than the limit).