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

Fix/request date export #690

Merged
merged 2 commits into from
Mar 20, 2023
Merged

Conversation

aaronchongth
Copy link
Member

@aaronchongth aaronchongth commented Mar 17, 2023

What's new

Related to #689

  • Adds Date column to minimal export
  • Refactored task repository save_task_state
    • creating a new task state model explicitly setting request time, if it is a new task
    • updating all other fields when task already exists, leaving out request time and ID

To check exports, go to download button and select minimal export

To check if the request time stays the same, add these lines after here, outside of the else scope

        # debug
        debug_task_state = await DbTaskState.get_or_none(id_=task_state.booking.id)
        if debug_task_state is None:
            print('oh no something went wrong')
        else:
            print('request_time: {}'.format(debug_task_state.unix_millis_request_time))

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

@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #690 (2a38ce1) into deploy/hammer (9b0d007) will increase coverage by 0.16%.
The diff coverage is 60.22%.

@@                Coverage Diff                @@
##           deploy/hammer     #690      +/-   ##
=================================================
+ Coverage          59.30%   59.47%   +0.16%     
=================================================
  Files                281      282       +1     
  Lines               5932     6003      +71     
  Branches             801      789      -12     
=================================================
+ Hits                3518     3570      +52     
- Misses              2260     2279      +19     
  Partials             154      154              
Flag Coverage Δ
api-server 84.71% <94.64%> (+0.22%) ⬆️
dashboard 20.28% <0.00%> (-0.39%) ⬇️

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%> (ø)
packages/dashboard/src/components/tasks/utils.ts 0.00% <0.00%> (ø)
...ackages/react-components/lib/tasks/create-task.tsx 1.49% <ø> (+0.15%) ⬆️
...react-components/lib/tasks/task-table-datagrid.tsx 61.53% <ø> (ø)
...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 95.06% <100.00%> (+0.46%) ⬆️
...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

@Angatupyry
Copy link
Collaborator

LGTM!
I think it was something we needed when we started adding those fields that should not be updated. So, thanks for these changes!
The only question I have is whether we should separate the update and create methods in different endpoints, but let's discuss that at our next meeting.

@aaronchongth aaronchongth merged commit a1ea778 into deploy/hammer Mar 20, 2023
@aaronchongth aaronchongth deleted the fix/request-date-export branch March 20, 2023 12:39
aaronchongth added a commit that referenced this pull request Apr 16, 2023
* Added date column to minimal export, only update fields of task state when it has not been created before

Signed-off-by: Aaron Chong <[email protected]>

* Use create and get_or_none, and save model if it exists

Signed-off-by: Aaron Chong <[email protected]>

---------

Signed-off-by: Aaron Chong <[email protected]>
(cherry picked from commit a1ea778)
Signed-off-by: Aaron Chong <[email protected]>
@aaronchongth aaronchongth mentioned this pull request Apr 16, 2023
5 tasks
aaronchongth added a commit that referenced this pull request Jul 4, 2023
* Date column in Tasks section (#689)

* Add unix-millis-request-time field in taskstate

Signed-off-by: fierro <[email protected]>

* Generate taskstate model according to the rmf_api_msgs changes

Signed-off-by: fierro <[email protected]>

* Generate api client to get the new field

Signed-off-by: fierro <[email protected]>

* Save unix request time as now() and use previous value if the method is update

Signed-off-by: fierro <[email protected]>

* Show unix_millis_request_time in dashboard

Signed-off-by: fierro <[email protected]>

* Add Unix_millis_request_time into booking instead of taskstate directly

Signed-off-by: fierro <[email protected]>

* Access to unix_millis_request_time field through booking

Signed-off-by: fierro <[email protected]>

* Get previous task_state and if is not none use its request_time value

Signed-off-by: fierro <[email protected]>

---------

Signed-off-by: fierro <[email protected]>
(cherry picked from commit 7ea986d)
Signed-off-by: Aaron Chong <[email protected]>

* Fix/request date export (#690)

* Added date column to minimal export, only update fields of task state when it has not been created before

Signed-off-by: Aaron Chong <[email protected]>

* Use create and get_or_none, and save model if it exists

Signed-off-by: Aaron Chong <[email protected]>

---------

Signed-off-by: Aaron Chong <[email protected]>
(cherry picked from commit a1ea778)
Signed-off-by: Aaron Chong <[email protected]>

* Feature/task initiator (#692)

* Add initiator column in taskstate model

Signed-off-by: fierro <[email protected]>

* Initiator field has been added on client side

Signed-off-by: fierro <[email protected]>

* Set initiator field with the previous value if its is updated or use self.user.username if its is created

Signed-off-by: fierro <[email protected]>

* Add initiator as column in datagrid and show it in TaskSection

Signed-off-by: fierro <[email protected]>

* Populate initiator field with self.user.name when task is created

Signed-off-by: fierro <[email protected]>

* Add initiator column to minimal export

Signed-off-by: fierro <[email protected]>

* Set initiator column to minimal export

Signed-off-by: fierro <[email protected]>

* In update method, fill the initiator field with the previous db_task_state value

Signed-off-by: fierro <[email protected]>

---------

Signed-off-by: fierro <[email protected]>
(cherry picked from commit 86700da)
Signed-off-by: Aaron Chong <[email protected]>

* Adding user and request time to requests before submitting

Signed-off-by: Aaron Chong <[email protected]>

* api-server, use requester instead of initiator for task_request and task_state

Signed-off-by: Aaron Chong <[email protected]>

* Initiator to Requester

Signed-off-by: Aaron Chong <[email protected]>

* Update api-client

Signed-off-by: Aaron Chong <[email protected]>

* Using N/A instead of unknown for fields

Signed-off-by: Aaron Chong <[email protected]>

* pylint

Signed-off-by: Aaron Chong <[email protected]>

* Changing N/A to unknown

Signed-off-by: Aaron Chong <[email protected]>

* Adding try catch for username promise

Signed-off-by: Aaron Chong <[email protected]>

---------

Signed-off-by: fierro <[email protected]>
Signed-off-by: Aaron Chong <[email protected]>
Co-authored-by: César Rolón <[email protected]>
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