-
Notifications
You must be signed in to change notification settings - Fork 8
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
Renew/Reschedule Canceled Events Issue 986 #1091
Conversation
…vTeam/celts into 986-Renew-Reschedule
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 think you need to validate the start and end times of the event. It should not allow the start time to be after the end time
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.
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 find it confusing to have both "Save" and "Renew Event" buttons when editing a canceled event. As in which button to click on since they both are saving changes. Also, Why do we need to save changes for canceled events? In my opinion, I would think having just the Renew Event button should be good for canceled events instead of confusing the user. |
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.
Will look back over code more in depth once the errors are resolved, overall looks alright, will say it would be great if some more comments and docstrings could be added in. Some of the copying stuff is not the most intuitive. Good work though!
…vTeam/celts into 986-Renew-Reschedule
…ts into 986-Renew-Reschedule
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.
All pretty small things, I think otherwise it looks and works really well!
…vTeam/celts into 986-Renew-Reschedule
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.
Code looks good but it breaks because of a missing import.
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.
Everything seems to work and look good, couldn't break it 😞 I would just talk to Brian about whether a new issue is necessary to renew past events. Nice work!
Stale
Issue:
There was a request to have a button to be able to renew/reschedule a canceled event.
Fix:
Added modal which allows supervisors to reschedule previously canceled events. We take data from the page of the canceled event and send that data to a route which uses the current event's information to copy the event contents over.
Test:
Fixes #986