Custom task type support, refactor tasks to be singular, cleaned up #903

aaronchongth · 2024-02-18T14:41:25Z

What's new

Custom compose task type
Cleaned up and refactored create-task form to support singular tasks only (removing legacy support for batch task upload)

The pasted component must be a valid json to be submitted, and is only the Description component of the task request, while the task category is compose,

{
  category: 'compose',
  ...
  description: PASTE-IN-WHATS-NEEDED-HERE
}

For example, https://gist.github.com/aaronchongth/992f1de8a36188ed22da0e228358d4ae or https://gist.github.com/mxgrey/b7a8ede7075cf7764e622ce8e2996888

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

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

codecov · 2024-02-18T14:50:13Z

Codecov Report

Attention: Patch coverage is 1.06383% with 93 lines in your changes are missing coverage. Please review.

Project coverage is 26.91%. Comparing base (fe0e808) to head (8c7739f).
Report is 69 commits behind head on deploy/hammer.

Files	Patch %	Lines
...ackages/react-components/lib/tasks/create-task.tsx	1.09%	90 Missing ⚠️
...hboard/src/components/tasks/task-schedule-utils.ts	0.00%	3 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##           deploy/hammer     #903       +/-   ##
==================================================
- Coverage          49.35%   26.91%   -22.45%     
==================================================
  Files                285      157      -128     
  Lines               7564     5273     -2291     
  Branches            1050     1353      +303     
==================================================
- Hits                3733     1419     -2314     
+ Misses              3682     3655       -27     
- Partials             149      199       +50

Flag	Coverage Δ
api-server	`?`
dashboard	`13.74% <0.00%> (-1.33%)`	⬇️
react-components	`44.44% <1.09%> (-3.63%)`	⬇️
rmf-auth	`?`

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

packages/react-components/lib/tasks/create-task.tsx

koonpeng · 2024-03-01T05:51:54Z

packages/react-components/lib/tasks/create-task.tsx

-    updateTasks();
+  };
+
+  const handleCustomTaskDescriptionChange = (newDesc: CustomTaskDescription) => {


Is appears to be a more stripped down version of legacyHandleTaskDescriptionChange as well. Also is it intended that this does not update the favorite task buffer?

Also is it intended that this does not update the favorite task buffer?

Yeah, I wasn't planning to support saving custom tasks as favorite tasks.

Since I'm re-visiting the task description changes, I'll see if saving it as a favorite task can be supported easily.

To support favorite tasks for this custom compose, I'd probably like to properly define the model types, so I added a fixme for later

Favorite task is probably the most useful with custom tasks, I think we should support it.

Yes, but probably not at the moment. We will keep it in view until we finish the more critical items towards ongoing projects and release. For this PR, the cleanup to using single requests is required for the next few features

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

…ting custom tasks Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

…s be compose, display description for short description Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

koonpeng · 2024-03-05T08:54:21Z

packages/react-components/lib/tasks/create-task.tsx

+    console.error(JSON.stringify(taskRequest.description));
+    return JSON.stringify(taskRequest.description);


trying to JSON.stringify might throw an error. You probably want to either return undefined or let the error propagate up or return a fallback value.

On another note, this piece of code looks very specific, it makes a lot of assumption about the task without checking if it is correct. It should

Check the category first before attempting to destructure the data.

Check if the phases has the expected data (does each phase have a category?)

Check if the activities has the correct data.

Handle any error from the above checks.

iirc taskRequest.description.category is not strictly defined in rmf, it is making assumptions on the deployment as well. So this part needs a FIXME, i.e. the data need to come from rmf.

trying to JSON.stringify might throw an error. You probably want to either return undefined or let the error propagate up or return a fallback value.

thanks for catching that, will handle that exception.

On another note, this piece of code looks very specific

Yup, generally every single line of code that is task request related is hand-crafted based on know-how at this point, until we move towards validating with schemas we get from RMF.

The schemas are well defined and have not needed much changes so far (thankfully), so all this is a stopgap until the schemas solution rolls in, to get things working on our various projects.

I will add the FIXMEs where needed, but this should not hinder getting features that are urgent in.

koonpeng · 2024-03-05T09:11:57Z

packages/react-components/lib/tasks/create-task.tsx

+      const deliveryDropoff: DropoffActivity = {
+        category: 'perform_action',
+        description: {
+          unix_millis_action_duration_estimate: 60000,


Not a blocker since it's the same as the previous code, but shouldn't this be set by rmf?

No, it's meant to be set by the user, estimating how long each perform action is supposed to take to help RMF plan. It was set manually here, because it is thankfully an invariant (for picking up or dropping off carts in the current project).

But it was also a feature request that we don't bog down the users with too many inputs to fill in, so this is the estimate we chose to use statically.

Add another FIXME to this, this design isn't good as we are making assumptions on the deployment again.

03aa973

When I introduce back the demos task requests into this, I will refactor them similarly to https://github.com/open-rmf/rmf-web/pull/893/files#diff-70a086cc44648bbf06fb810e18f59a99cfcf139952b6eda2fe1447b0faf31a7c, where they are easier to maintain, testable and have better separation of concern

…cription assumptions Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

Custom task type support, refactor tasks to be singular, cleaned up

9daeb5d

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

lint

81f0bd3

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

aaronchongth requested a review from koonpeng March 1, 2024 05:03

koonpeng requested changes Mar 1, 2024

View reviewed changes

aaronchongth added 5 commits March 1, 2024 18:31

Make custom task always available

a159492

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

Merge branch 'deploy/hammer' into hammer/debug-custom-description

0880120

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

Check exception for TypeError

b3bdf92

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

Unifying handleTaskDescriptionChange, added FIXME for allowing favori…

279b2b1

…ting custom tasks Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

Renaming custom to custom_compose, since the task category will alway…

fab9d5d

…s be compose, display description for short description Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

aaronchongth requested a review from koonpeng March 4, 2024 13:44

koonpeng requested changes Mar 5, 2024

View reviewed changes

Catch possible exception with JSON.stringify, add fixmes for task des…

03aa973

…cription assumptions Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

aaronchongth requested a review from koonpeng March 6, 2024 10:36

Merge branch 'deploy/hammer' into hammer/debug-custom-description

8c7739f

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

aaronchongth mentioned this pull request Mar 12, 2024

Use task request labels to encode information required by dashboard #912

Merged

5 tasks

koonpeng approved these changes Mar 13, 2024

View reviewed changes

aaronchongth merged commit c4228f4 into deploy/hammer Mar 13, 2024
3 checks passed

aaronchongth deleted the hammer/debug-custom-description branch March 13, 2024 10:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom task type support, refactor tasks to be singular, cleaned up #903

Custom task type support, refactor tasks to be singular, cleaned up #903

aaronchongth commented Feb 18, 2024 •

edited

Loading

codecov bot commented Feb 18, 2024 •

edited

Loading

koonpeng Mar 1, 2024

aaronchongth Mar 4, 2024

aaronchongth Mar 4, 2024

koonpeng Mar 5, 2024

aaronchongth Mar 5, 2024

koonpeng Mar 5, 2024

aaronchongth Mar 5, 2024

aaronchongth Mar 6, 2024

koonpeng Mar 5, 2024

aaronchongth Mar 5, 2024

koonpeng Mar 6, 2024

aaronchongth Mar 6, 2024

		console.error(JSON.stringify(taskRequest.description));
		return JSON.stringify(taskRequest.description);

Custom task type support, refactor tasks to be singular, cleaned up #903

Custom task type support, refactor tasks to be singular, cleaned up #903

Conversation

aaronchongth commented Feb 18, 2024 • edited Loading

What's new

Self-checks

codecov bot commented Feb 18, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronchongth commented Feb 18, 2024 •

edited

Loading

codecov bot commented Feb 18, 2024 •

edited

Loading