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

Editing the schedule #770

Merged
merged 43 commits into from
Sep 19, 2023
Merged

Editing the schedule #770

merged 43 commits into from
Sep 19, 2023

Conversation

Angatupyry
Copy link
Collaborator

What's new

  • The editing of an event within the calendar is allowed.
  • Cells are blocked so that no event can be created from them.
  • Only cells with events are enabled.
  • The task can be edited (category, priority, etc.) as well as the schedule (days, time, etc.).
  • The user can:
    • Edit a single event, which causes that event to be removed from the task to which it belonged and another task is created with that single schedule.
    • Edit the entire series, deleting the entire schedule and recreating it, keeping the task.
  • Separate the scheduling logic from the task-app file.
  • Custom hooks are created for later reuse.

The video shows the full functionality. As well as the use of the property "ends on".

Screencast.from.09-14-2023.02.08.07.PM.webm

Self-checks

  • I have prototyped this new feature (if necessary) on Figma
  • I'm familiar with and follow this Typescript guideline
  • I added unit-tests for new components
  • I tried testing edge cases
  • I tested the behavior of the components that interact with the backend, with an e2e test

Discussion

Signed-off-by: angatupyry <[email protected]>
Signed-off-by: angatupyry <[email protected]>
Signed-off-by: angatupyry <[email protected]>
Signed-off-by: angatupyry <[email protected]>
Signed-off-by: angatupyry <[email protected]>
Signed-off-by: angatupyry <[email protected]>
Signed-off-by: angatupyry <[email protected]>
Signed-off-by: angatupyry <[email protected]>
@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Merging #770 (37610d6) into main (c885e3d) will decrease coverage by 0.95%.
The diff coverage is 8.17%.

@@            Coverage Diff             @@
##             main     #770      +/-   ##
==========================================
- Coverage   53.92%   52.97%   -0.95%     
==========================================
  Files         263      268       +5     
  Lines        6639     6792     +153     
  Branches      882      906      +24     
==========================================
+ Hits         3580     3598      +18     
- Misses       2919     3054     +135     
  Partials      140      140              
Flag Coverage Δ
api-server 81.47% <10.34%> (-0.73%) ⬇️
dashboard 17.40% <6.64%> (-0.48%) ⬇️
react-components 51.66% <27.77%> (+0.02%) ⬆️
rmf-auth ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...hboard/src/components/tasks/task-schedule-utils.ts 0.00% <0.00%> (ø)
...s/dashboard/src/components/tasks/task-schedule.tsx 0.00% <0.00%> (ø)
...kages/dashboard/src/components/tasks/tasks-app.tsx 0.00% <ø> (ø)
...ackages/react-components/lib/tasks/create-task.tsx 4.01% <0.00%> (-0.06%) ⬇️
packages/dashboard/src/components/tasks/utils.ts 1.29% <5.55%> (+1.29%) ⬆️
...-server/api_server/routes/tasks/scheduled_tasks.py 59.48% <10.34%> (-16.38%) ⬇️
packages/dashboard/src/hooks/useCreateTaskForm.tsx 30.00% <30.00%> (ø)
packages/dashboard/src/hooks/useFetchUser.tsx 50.00% <50.00%> (ø)
packages/dashboard/src/components/appbar.tsx 33.90% <100.00%> (+4.49%) ⬆️
...ib/tasks/task-schedule-event-edit-delete-popup.tsx 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

Thanks for the huge refactor and implementation effort! I've just left a string of comments mostly related to naming conventions and nitpicks, nothing logic related.

I'll be spending some additional time to test the implementation out separately.

@Angatupyry
Copy link
Collaborator Author

Thanks for these suggestions, Aaron!
In changing so many files I have lost track of some things that were not necessary.

Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

Thanks for all the changes! LGTM!

Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

I think the new schedule time is not being propagated to the schedule editing dialog, it is still using the current time

simplescreenrecorder-2023-09-19_12.39.28.mp4

@Angatupyry
Copy link
Collaborator Author

I think the new schedule time is not being propagated to the schedule editing dialog, it is still using the current time

simplescreenrecorder-2023-09-19_12.39.28.mp4

Done 37610d6 !
Thanks, Aaron!

Screencast.from.09-19-2023.09.36.28.AM.webm

Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

Thanks for getting this epic feature together! LGTM

@aaronchongth aaronchongth merged commit a3f3615 into main Sep 19, 2023
6 checks passed
@aaronchongth aaronchongth deleted the feature/schedule-editing branch September 19, 2023 14:02
@Angatupyry Angatupyry mentioned this pull request Sep 20, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants