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

Hammer/max downloads #909

Merged
merged 18 commits into from
Mar 11, 2024
Merged

Hammer/max downloads #909

merged 18 commits into from
Mar 11, 2024

Conversation

aaronchongth
Copy link
Member

@aaronchongth aaronchongth commented Feb 28, 2024

What's new

  • Exporting CSV now queries in chunks instead of all in one go
  • New global backdrop loading screen and events to trigger it, lets users know that the exporting is underway (which could take sometime)
  • Made export and refresh buttons more explicit
export-2024-03-08_15.23.58.mp4

Normal resolution
image

Low resolution
image

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>
Copy link

codecov bot commented Feb 28, 2024

Codecov Report

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

Project coverage is 46.76%. Comparing base (fe0e808) to head (efa1cd5).
Report is 68 commits behind head on deploy/hammer.

Files Patch % Lines
...kages/dashboard/src/components/tasks/tasks-app.tsx 0.00% 45 Missing ⚠️
packages/dashboard/src/components/app-base.tsx 0.00% 8 Missing ⚠️
packages/dashboard/src/components/tasks/utils.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           deploy/hammer     #909      +/-   ##
=================================================
- Coverage          49.35%   46.76%   -2.60%     
=================================================
  Files                285      204      -81     
  Lines               7564     6266    -1298     
  Branches            1050      833     -217     
=================================================
- Hits                3733     2930     -803     
+ Misses              3682     3330     -352     
+ Partials             149        6     -143     
Flag Coverage Δ
dashboard 13.74% <0.00%> (-1.32%) ⬇️
react-components ?
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.

packages/api-server/api_server/dependencies.py Outdated Show resolved Hide resolved
packages/api-client/schema/index.ts Outdated Show resolved Hide resolved
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>
@aaronchongth aaronchongth requested a review from koonpeng March 8, 2024 07:24
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
…ck to 31 days of querying

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 March 8, 2024 18:08
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
…t month

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth aaronchongth requested a review from koonpeng March 11, 2024 03:18
return;
}
if (minimal) {
downloadCsvMinimal(now, allTasks, allTaskRequests);
downloadCsvMinimal(now, pastMonthTasks, pastMonthTaskRequests);
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. "download*" is misleading since it doesn't actually download, suggest to rename it to "save*" or add docs.
  2. It's weird that "minimal" contains info that is not in "full" (from what I see, pickup and dropoff), I will say to include the task request in full so it has the same data.
  3. Parsing the task request for pick and dropoff is also a red flag, it is out of scope of this PR so just add a FIXME. In future PR we can look at using task tags to store this information and just dump it, it also solves the issue of requiring double query to get all the required data.

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. Changing it to export, 651562d

  2. Technically, full contains all the information of minimal and much more, since any number of pickups and dropoffs can be parsed from the task phases if needed. And since we will be adding pick and dropoff information in labels soon, I will not make any changes to this.

  3. FIXME was added previously d126767

Copy link
Collaborator

@koonpeng koonpeng Mar 11, 2024

Choose a reason for hiding this comment

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

Technically, full contains all the information of minimal and much more, since any number of pickups and dropoffs can be parsed from the task phases if needed. And since we will be adding pick and dropoff information in labels soon, I will not make any changes to this.

Then we don't need to fetch task requests for "minimal"? Or do you mean that it will be included after we add the tags? If so then add a TODO.

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth aaronchongth requested a review from koonpeng March 11, 2024 06:39
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.

Technically, full contains all the information of minimal and much more, since any number of pickups and dropoffs can be parsed from the task phases if needed. And since we will be adding pick and dropoff information in labels soon, I will not make any changes to this.

Then we don't need to fetch task requests for "minimal"? Or do you mean that it will be included after we add the tags? If so then add a TODO.

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

Technically, full contains all the information of minimal and much more, since any number of pickups and dropoffs can be parsed from the task phases if needed. And since we will be adding pick and dropoff information in labels soon, I will not make any changes to this.

Then we don't need to fetch task requests for "minimal"? Or do you mean that it will be included after we add the tags? If so then add a TODO.

The ticket detailing this is already in our deployment project board, will not be adding additional TODOs here.

@aaronchongth aaronchongth merged commit a0e2f8b into deploy/hammer Mar 11, 2024
2 checks passed
@aaronchongth aaronchongth deleted the hammer/max-downloads branch March 11, 2024 07:42
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