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

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

Merged
merged 46 commits into from
May 23, 2024

Conversation

aaronchongth
Copy link
Member

@aaronchongth aaronchongth commented Mar 12, 2024

What's new

Requires open-rmf/rmf_ros2#328 and #903

This PR adds TaskBookingLabel to be used by frontend dashboards to encode additional information required for rendering.

  • Encode json into task booking label for
    • task name (not to be confused with task request category, this is an internal encoding of what task "type" it is). The plan is to encode the default name of the task, or the remapped name (supported in Hammer/demo tasks #925), and the dashboard will render it accordingly, otherwise it will be considered an unknown task and revert to custom
    • warning time (to be populated and used later on, None for now).
    • pickup
    • destination
    • cart ID
  • Clean up workaround of saving pickup and destination when task request is saved
  • Clean up workaround where task requests were queried when tasks were exported
  • Adding task_definition_id field to TaskFavorite,
  • Script for migrating past database to encode labels (TaskState, ScheduledTask)
  • Refactored task state update/create to only lock mutex when needed

Migration (postgres)

Backup database, enter database

pg_dump -U USERNAME -W DBNAME > /tmp/DUMPNAME.sql

Handling additional field in TaskFavorite,

# enter db
psql -U USERNAME -d DATABASENAME

# add column
ALTER TABLE "taskfavorite" ADD COLUMN task_definition_id VARCHAR(255)

Migrate the rest of the models

cd packages/api-server
# modify psql_local_config.py to point to the database

source ../../.venv/bin/activate
source RMF_WORKSPACE/install/setup.bash
export RMF_API_SERVER_CONFIG=/PATH_TO_RMF_WEB/packages/api-server/psql_local_config.py

python3 migrate_db_912.py

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>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
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>
…cription assumptions

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
… request is saved

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

codecov bot commented Mar 12, 2024

Codecov Report

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

Project coverage is 46.34%. Comparing base (fe0e808) to head (0d6f019).
Report is 87 commits behind head on deploy/hammer.

Current head 0d6f019 differs from pull request most recent head 7c2d8ef

Please upload reports for the commit 7c2d8ef to get more accurate results.

Files Patch % Lines
...ackages/react-components/lib/tasks/create-task.tsx 0.00% 36 Missing ⚠️
...-components/lib/tasks/task-booking-label-utils.tsx 14.28% 17 Missing and 1 partial ⚠️
...es/dashboard/src/components/tasks/task-summary.tsx 0.00% 6 Missing ⚠️
...api-server/api_server/models/task_booking_label.py 82.60% 4 Missing ⚠️
...react-components/lib/tasks/task-table-datagrid.tsx 50.00% 2 Missing and 2 partials ⚠️
...ckages/api-server/api_server/routes/tasks/tasks.py 76.92% 3 Missing ⚠️
...kages/dashboard/src/components/tasks/tasks-app.tsx 0.00% 1 Missing ⚠️
packages/dashboard/src/components/tasks/utils.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           deploy/hammer     #912      +/-   ##
=================================================
- Coverage          49.35%   46.34%   -3.01%     
=================================================
  Files                285      299      +14     
  Lines               7564     8681    +1117     
  Branches            1050     1384     +334     
=================================================
+ Hits                3733     4023     +290     
- Misses              3682     4455     +773     
- Partials             149      203      +54     
Flag Coverage Δ
react-components 43.09% <14.70%> (-4.99%) ⬇️
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.

Base automatically changed from hammer/debug-custom-description to deploy/hammer March 13, 2024 10:49
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth aaronchongth requested review from koonpeng and removed request for koonpeng March 13, 2024 14:23
@aaronchongth aaronchongth marked this pull request as draft March 15, 2024 01:46
* Getting ScheduledTask as well

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

* Handle schedules as well

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

* lint

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

---------

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth aaronchongth marked this pull request as ready for review March 20, 2024 01:28
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
… api-client

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
…rom the state, instead of the request

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
…ate when fav task is clicked

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth aaronchongth requested a review from koonpeng May 6, 2024 07:51
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Copy link
Collaborator

@koonpeng koonpeng left a comment

Choose a reason for hiding this comment

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

The api-server should eventually treat the labels as black boxes, i.e. there should be no models for them. For that to happen, we need to support querying and sorting the labels in a generic way.

I can work on it after this is merged, but in order to avoid more migration hell, we should prepare the labels in a new table in this PR.

packages/api-server/api_server/repositories/tasks.py Outdated Show resolved Hide resolved
…lds for sorting and filtering

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

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth aaronchongth merged commit 38d5869 into deploy/hammer May 23, 2024
5 checks passed
@aaronchongth aaronchongth deleted the hammer/use-labels branch May 23, 2024 16:09
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