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

1608 download filtered results UI modal #1625

Merged
merged 43 commits into from
Jun 8, 2021

Conversation

sergioloporto
Copy link
Contributor

@sergioloporto sergioloporto commented Mar 4, 2021

#1608

Changes

  • User has the ability to download full dataset that is stored in S3 and updated daily (for now only in .csv)
  • User has the ability to download filtered data instantly if applied query returns less than 10000 cases (csv, tsv and json)
  • For bigger queries background worker will fetch the results and after compressing it upload to AWS S3 to newly created covid19-filtered-downloads bucket. The file will be automatically removed after 3 days. The user will receive an email containing pre signed url to this file (also valid for 3 days) allowing him to download the results. (csv, tsv and json)

Email that is being sent with the download link
Screen Shot 2021-06-01 at 11 47 12

Alert on data page informing user that the email was sent
Screen Shot 2021-06-03 at 10 37 48

@sergioloporto sergioloporto marked this pull request as draft March 4, 2021 09:49
@codecov-io
Copy link

codecov-io commented Mar 18, 2021

Codecov Report

Merging #1625 (71ecf37) into main (1064662) will increase coverage by 26.80%.
The diff coverage is 81.55%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #1625       +/-   ##
===========================================
+ Coverage   55.99%   82.80%   +26.80%     
===========================================
  Files          69       57       -12     
  Lines        2343     1803      -540     
  Branches      724      339      -385     
===========================================
+ Hits         1312     1493      +181     
+ Misses       1031      310      -721     
Impacted Files Coverage Δ
...rving/data-service/src/controllers/preprocessor.ts 100.00% <ø> (ø)
...ata-serving/data-service/src/geocoding/geocoder.ts 100.00% <ø> (ø)
...rification/curator-service/api/src/model/source.ts 100.00% <ø> (ø)
verification/curator-service/api/src/model/user.ts 100.00% <ø> (ø)
...rator-service/api/src/clients/aws-lambda-client.ts 89.58% <20.00%> (ø)
...ation/curator-service/api/src/controllers/cases.ts 60.33% <28.57%> (ø)
...cation/curator-service/api/src/controllers/auth.ts 55.00% <33.33%> (ø)
data-serving/data-service/src/geocoding/mapbox.ts 85.07% <50.00%> (ø)
data-serving/data-service/src/controllers/case.ts 80.47% <91.66%> (ø)
verification/curator-service/api/src/index.ts 87.80% <100.00%> (ø)
... and 123 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a92bbec...71ecf37. Read the comment docs.

@codecov-commenter
Copy link

codecov-commenter commented May 17, 2021

Codecov Report

Merging #1625 (66df6a6) into main (0649012) will decrease coverage by 24.89%.
The diff coverage is 39.28%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #1625       +/-   ##
===========================================
- Coverage   81.19%   56.29%   -24.90%     
===========================================
  Files          56       71       +15     
  Lines        1723     2581      +858     
  Branches      315      826      +511     
===========================================
+ Hits         1399     1453       +54     
- Misses        324     1128      +804     
Impacted Files Coverage Δ
...urator-service/ui/src/components/LinelistTable.tsx 51.63% <33.78%> (ø)
...-service/ui/src/components/SnackbarAlert/index.tsx 71.42% <71.42%> (ø)
...fication/curator-service/ui/src/components/App.tsx 78.70% <100.00%> (ø)
...ice/ui/src/components/VerificationStatusHeader.tsx 100.00% <100.00%> (ø)
...rving/data-service/src/controllers/preprocessor.ts
data-serving/data-service/src/model/travel.ts
...on/curator-service/api/src/clients/email-client.ts
...ation/curator-service/api/src/controllers/users.ts
...fication/curator-service/api/src/model/schedule.ts
...ata-serving/data-service/src/model/transmission.ts
... and 118 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7940055...66df6a6. Read the comment docs.

@maciej-zarzeczny maciej-zarzeczny marked this pull request as ready for review May 27, 2021 09:07
@maciej-zarzeczny maciej-zarzeczny removed their request for review May 27, 2021 09:08
@maciej-zarzeczny maciej-zarzeczny self-assigned this May 27, 2021
Comment on lines +77 to +80
if (req.body.query && req.body.caseIds) {
res.status(400).json({ message: 'Bad request' });
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, good to test this constraint before continuing

Comment on lines 117 to 131
if (queryLimit) {
matchingCases = await Case.aggregate(casesIgnoringExcluded)
.collation({
locale: 'en_US',
strength: 2,
})
.limit(queryLimit);
} else {
matchingCases = await Case.aggregate(
casesIgnoringExcluded,
).collation({
locale: 'en_US',
strength: 2,
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal, but could be changed to:

let query:any = Case.aggregate(casesIgnoringExcluded)collation({locale: 'en_US', strength: 2, });
if (queryLimit) { query = query.limit(queryLimit); }
const matchingCases = await query;

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the code will be cleaner, thank you for spotting that!

Comment on lines +102 to +104
<p>Please visit the link below to download list of cases in response to your query <strong>${workerData.query}</strong>.</p>

<a href="${signedUrl}">Click here to download the data</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, a much better way of handling the large files

Copy link
Contributor

@iamleeg iamleeg left a comment

Choose a reason for hiding this comment

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

Nice work, thank you!

@iamleeg iamleeg merged commit 98d5354 into main Jun 8, 2021
@iamleeg iamleeg deleted the 1608-download-filtered-results-ui-modal branch June 8, 2021 15: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.

5 participants