Date column in Tasks section

[Angatupyry]

What's new

Date column has been added in taskstate

This field is saved when the user creates a new task. It allows us to know when a task was created.
This field cannot be updated by the user.

• Related Date Request and Initiator field in task_state rmf_api_msgs#34

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 #689 (2a0785a) into deploy/hammer (9b0d007) will decrease coverage by 0.75%.
The diff coverage is 28.40%.

@@                Coverage Diff                @@
##           deploy/hammer     #689      +/-   ##
=================================================
- Coverage          59.30%   58.55%   -0.75%     
=================================================
  Files                281      282       +1     
  Lines               5932     6093     +161     
  Branches             801      824      +23     
=================================================
+ Hits                3518     3568      +50     
- Misses              2260     2370     +110     
- Partials             154      155       +1     

Flag	Coverage Δ
api-server	84.69% <94.11%> (+0.20%)	⬆️
dashboard	20.32% <0.00%> (-0.35%)	⬇️
react-components	58.33% <2.06%> (-2.40%)	⬇️

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

Impacted Files	Coverage Δ
...rver/api_server/models/tortoise_models/__init__.py	100.00% <ø> (ø)
...kages/dashboard/src/components/tasks/tasks-app.tsx	0.00% <0.00%> (ø)
...ackages/react-components/lib/tasks/create-task.tsx	1.31% <1.05%> (-0.04%)	⬇️
...react-components/lib/tasks/task-table-datagrid.tsx	60.97% <50.00%> (-0.57%)	⬇️
...i-server/api_server/routes/tasks/favorite_tasks.py	91.42% <91.42%> (ø)
...api-server/api_server/models/rmf_api/task_state.py	100.00% <100.00%> (ø)
...-server/api_server/models/tortoise_models/tasks.py	100.00% <100.00%> (ø)
...ckages/api-server/api_server/repositories/tasks.py	94.80% <100.00%> (+0.21%)	⬆️
...ges/api-server/api_server/routes/tasks/__init__.py	100.00% <100.00%> (ø)

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

[aaronchongth]

Upon going through it in detail, I believe we might not need to update rmf_api_msgs, and the pydantic model as well, just an additional field to the tortoise model is sufficient.

It looks like we just need to modify https://github.com/open-rmf/rmf-web/blob/main/packages/api-server/api_server/repositories/tasks.py#L29-L46, where instead of ttm.TaskState.update_or_create, we can get_or_none, if it is not none, we copy the existing state's unix_millis_request_time, and update the model with that same request time.

Otherwise, we create a new one with the current time stamp for the request time. We can be sure that the first state we get to save is right after the request has been dispatched, since it is saved in this post call and this post call

This approach should be used for the upcoming column of task initiator too, using the dashboard's current user. Let me know what you think about this approach

[Angatupyry]

Upon going through it in detail, I believe we might not need to update rmf_api_msgs, and the pydantic model as well, just an additional field to the tortoise model is sufficient.

It looks like we just need to modify https://github.com/open-rmf/rmf-web/blob/main/packages/api-server/api_server/repositories/tasks.py#L29-L46, where instead of ttm.TaskState.update_or_create, we can get_or_none, if it is not none, we copy the existing state's unix_millis_request_time, and update the model with that same request time.

Otherwise, we create a new one with the current time stamp for the request time. We can be sure that the first state we get to save is right after the request has been dispatched, since it is saved in this post call and this post call

This approach should be used for the upcoming column of task initiator too, using the dashboard's current user. Let me know what you think about this approach

Hey, Aaron. I think your approach is ok and that's exactly what I'm trying to do here

async def save_task_state(self, task_state: TaskState) -> None:
        datetime_request_time = (
            datetime.fromtimestamp(task_state.unix_millis_request_time / 1000)
            if task_state.unix_millis_request_time
            else datetime.now()
        )
        task_state.unix_millis_request_time = (
            task_state.unix_millis_request_time
            if task_state.unix_millis_request_time
            else round(time.time() * 1000)
        )
        await ttm.TaskState.update_or_create(
            {
                "data": task_state.json(),
                "category": task_state.category.__root__
                if task_state.category
                else None,
                "assigned_to": task_state.assigned_to.name
                if task_state.assigned_to
                else None,
                "unix_millis_start_time": task_state.unix_millis_start_time
                and datetime.fromtimestamp(task_state.unix_millis_start_time / 1000),
                "unix_millis_finish_time": task_state.unix_millis_finish_time
                and datetime.fromtimestamp(task_state.unix_millis_finish_time / 1000),
                "status": task_state.status if task_state.status else None,
                "unix_millis_request_time": datetime_request_time,
            },
            id_=task_state.booking.id,
        )

But I also need to update the data field so that it can be displayed later in the dashboard.
And regarding displaying in the datagrid, how could we access the field without it existing here? The API client creates the interfaces depending on that class.
I consider that to save the data in the database the tortoise field is enough but to show it on the dashboard I consider that we also need the field in rmf_api (which does not necessarily have to depend on rmf_msgs_api and)

[aaronchongth]

Doh! Thanks for explaining, I totally forgot that we are using the data JSON field, we should fix that at some point, I'll add a backburner task for that.

I've left some comments and suggestions, let me know what you think

[aaronchongth]

Thanks for the quick rework! Tested and working great, LGTM