scheduling frontend

[koonpeng]

What's new

UI for creating scheduled task and rendering it in a calendar. Also includes changes in the backend to support querying scheduled tasks.

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

[codecov]

Codecov Report

Merging #700 (dc3cd88) into main (161e7e3) will decrease coverage by 0.87%.
The diff coverage is 17.52%.

@@            Coverage Diff             @@
##             main     #700      +/-   ##
==========================================
- Coverage   57.23%   56.37%   -0.87%     
==========================================
  Files         290      293       +3     
  Lines        6681     6954     +273     
  Branches      907      954      +47     
==========================================
+ Hits         3824     3920      +96     
- Misses       2691     2844     +153     
- Partials      166      190      +24     

Flag	Coverage Δ
api-server	82.98% <94.87%> (-0.36%)	⬇️
dashboard	18.46% <3.73%> (-0.88%)	⬇️
react-components	58.10% <2.56%> (-0.45%)	⬇️

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

Impacted Files	Coverage Δ
packages/dashboard/src/components/app-events.ts	100.00% <ø> (ø)
...kages/dashboard/src/components/tasks/tasks-app.tsx	0.00% <0.00%> (ø)
...react-components/lib/tasks/task-table-datagrid.tsx	61.53% <ø> (ø)
...ackages/react-components/lib/tasks/create-task.tsx	1.44% <2.56%> (+0.13%)	⬆️
packages/dashboard/src/components/appbar.tsx	30.15% <16.66%> (-4.94%)	⬇️
...-server/api_server/routes/tasks/scheduled_tasks.py	90.00% <92.30%> (-0.33%)	⬇️
...pi_server/models/tortoise_models/scheduled_task.py	87.93% <100.00%> (+5.23%)	⬆️

... and 15 files with indirect coverage changes

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

[aaronchongth]

This looks awesome! Thanks for handling this massive feature. I noticed a few things that need modifications on regarding the frontend, that I thought would be best to mention first. I will then continue to review the code while you can check out my comments.

Most of these comments will be based on what our figma design, I'll DM a link over shortly.

• The scheduling feature should be a second sub tab, under Tasks, called Calendar, while our original table will be under the first sub tab, called Queue. Users will need to select the sub tabs to change the view between our task queue table and the calendar view of scheduled tasks
• For task creation, it looks like daily scheduling is turned on by default, unless a user deselects all days or checks Now. Scheduling would ideally be a decision made only at the end of creating a task (whether to schedule or send it out once), which should open up a separate dialog that contains the components you've written so far for selecting days-of-the-week, and future features of selecting ending dates, etc. This will help keep the task creation form less cluttered and easy for us to add more features next.

Let me know if these modifications is possible with our current implementation, thanks!

[Angatupyry]

@aaronchongth
schedule package provides the clear function that receives a job tag.
I have added a tag to the job in the to_job() function which is the id of the schedule.
clear function removes jobs by iterating through each job using its tag which is a Hashable.

Both schedule.clear() and schedule.clean_job() functions iterate through a list of jobs.

This has O(n) complexity, which implies that the execution time when deleting schedules is proportional to the size of the input. It doesn't seem very inefficient to me unless we are considering a large amount of data.

[Angatupyry]

The api-server error is because pydantic warning Access to a protected member _id of a client class (protected-access)
Why we called _id instead of id here ?

[aaronchongth]

@Angatupyry, thank you so much for the help! I can verify that the deletion bug has been fixed.

This has O(n) complexity, which implies that the execution time when deleting schedules is proportional to the size of the input. It doesn't seem very inefficient to me unless we are considering a large amount of data.

Yup, I'm willing to live with that. We can further optimize this when we implement end-date for schedules, and have to handle schedules that have already past.

The api-server error is because pydantic warning Access to a protected member _id of a client class (protected-access)
Why we called _id instead of id here ?

Gotcha! I was actually referring to a stalling error, but it looks like it may have already been resolved, perhaps it was a github runner issue. The member is called _id to be considered a protected member of the Model, and hence will the interface will not include the member, https://github.com/open-rmf/rmf-web/blob/kp/scheduling-frontend/packages/api-client/lib/openapi/api.ts#L299-L330

I've added a getter to access the protected member instead, this resolves the linting error

[aaronchongth]

I'm liking how this is looking! Thanks for all the help so far @koonpeng and @Angatupyry! LGTM

@Angatupyry can you do another round of review for this? Probably not a good idea for me to approve and just merge what I wrote 🤣

[Angatupyry]

I'm liking how this is looking! Thanks for all the help so far @koonpeng and @Angatupyry! LGTM

@Angatupyry can you do another round of review for this? Probably not a good idea for me to approve and just merge what I wrote 🤣

haha! Thanks, Aaron!
I will!

[Angatupyry]

LGTM!

[Angatupyry]

@aaronchongth

The only thing I could say is that maybe it doesn't make much sense that the start date could be less than the current date.
Perhaps it could be restricted as a minimum acceptable to today.
But it is a detail that does not affect functionality.

[aaronchongth]

Yeah I understand what you mean. Well in the spirit of mimicking a modern calendar system, we should also allow handling of schedules in the past then. Thanks for the work and review from all! Merging it! 💥