Add 'Download' button to record set attachment viewer #6052

alesan99 · 2025-01-08T18:45:48Z

Fixes #609

This PR adds two new features to speed up attachment downloading.

When viewing the attachment gallery for a record set, a 'download all' button is now available. Clicking it should start downloading a zip file containing all the attachments.
I also added new small download buttons to each individual attachment, which speeds up downloading a selection of attachments. This was applied universally.

Additional Notes:
The URL to download all the attachments is attachment_gw/download_all/. Couldn't come up with a better name, so if you have a better name let me know!
If you download multiple attachments with the same name they will be renamed to OriginalName_AssetServerName.EXT. I can also change this if it doesn't seem good enough.

Checklist

Self-review the PR after opening it to make sure the changes look good and
self-explanatory (or properly documented)
Add automated tests
Add relevant issue to release milestone
Add relevant documentation (Tester - Dev)

Testing instructions

Test 'Download All' button:

Use a database that has a lot of attachments
In QB, run a Query on Collection Objects that have attachments
Browse in Forms
Click the attachment gallery icon on the header
Click the 'Download All' button
Download is successful
Verify that the zip file contains all the requested attachments with the correct filenames.
Make sure the download all button is only enabled when there are multiple attachments to download.
With many attachments, make sure the download all button doesn't break when not all attachments have been loaded yet.
Try out the button in different attachments galleries, like in individual records or in record sets.
Try to download multiple attachments with the same file name. Make sure they all get downloaded correctly. They should be named fileName_2.EXT and so on.
Test small download buttons:
Verify that the download button works in different contexts (Attachment viewer, Record set attachment gallery, etc.)

Triggered by 902b762 on branch refs/heads/issue-609

Triggered by dd48454 on branch refs/heads/issue-609

grantfitzsimmons · 2025-01-08T19:42:54Z

🐐

Fix trying to download unavailable attachments WIP checkboxes?

Fix translations

specifyweb/attachment_gw/views.py

Fix downloading files with the same filename Add ability to recieve and download binary files on the frontend

alesan99 · 2025-01-15T19:42:08Z

Looking for feedback on these👇! This PR ended up being a bit bigger than I thought so I want to make sure everything is right,

Dev Note:
To my dismay, it looks like receiving zip files with a POST request wasn't possible with the existing utilities :(. The zip file would get turned into a string at multiple points of the process.
I had to change the ajax fetch utility a bit (and downloadFile). Once a response is read with response.text(), it can no longer be read with response.blob(), so I couldn't extract the original binary data further in the process.
I prioritized minimizing code impact mainly, but the code still might not be ideal.
I'm not comfortable messing with such important code, so I am looking on feedback on those changes especially.

Additional Notes:
The URL to download all the attachments is attachment_gw/download_all/. Couldn't come up with a better name, so if you have a better name let me know!
If you download multiple attachments with the same name they will be renamed to OriginalName_AssetServerName.EXT. I can also change this if it doesn't seem good enough.

maxpatiiuk

great job on a complex feature!

the changes in ajax() look good - this is exactly how I would have done it 🤗

specifyweb/frontend/js_src/lib/components/Attachments/RecordSetAttachment.tsx

maxpatiiuk · 2025-01-20T03:36:33Z

specifyweb/frontend/js_src/lib/components/Attachments/RecordSetAttachment.tsx

-            <Button.DialogClose>{commonText.close()}</Button.DialogClose>
+            <>
+              <Button.Info
+                disabled={fetchedCount.current !== records.length || records.length <= 1}


it may not be clear to user that all attachments need to be bulked fetched by front-end for them to be downloadable
could you add a title={} with explanation to this button when it is disabled?

OR, call back-end API with record set id and let back-end retrieve list of attachments

specifyweb/frontend/js_src/lib/components/Molecules/FilePicker.tsx

specifyweb/frontend/js_src/lib/utils/ajax/index.ts

Triggered by f8a5f8f on branch refs/heads/issue-609

WIP add tool tip to download all button when disabled WIP use record set name as file name

…issue-609

Triggered by e598352 on branch refs/heads/issue-609

sharadsw

Looks good to me. Good job!

Improve handling of files with same file name

…issue-609

Triggered by 8b1dea4 on branch refs/heads/issue-609

pashiav

Download is successful
Verify that the zip file contains all the requested attachments with the correct filenames.
Make sure the download all button is only enabled when there are multiple attachments to download.
With many attachments, make sure the download all button doesn't break when not all attachments have been loaded yet.
Try to download multiple attachments with the same file name. Make sure they all get downloaded correctly. They should be named OriginalFileName_InternalFileName.EXT.
Test small download buttons:
Verify that the download button works in different contexts (Attachment viewer, Record set attachment gallery, etc.)

Great work so far!

For:

Make sure the download all button is only enabled when there are multiple attachments to download.

The Download All button is a little confusing because it's disabled for a singular attachment/record. It says it downloads all attachments when you hover over it, so I think it should include and work for singular attachments too. Removing the button when there is only one attachment could work as well. @specify/ux-testing what do you think?

Also I found that if you have 1 record with 1 attachment, but select multiple records which do not have any attachments, the Download All button is functional and will create a zip file of only 1 attachment.

To reproduce:

Select a record with 1 attachment in QB
Select other records with no attachments in addition to it
Click Browse in Forms -> Attachment Gallery icon -> Download All
See a zip file has been downloaded with 1 attachment

Screen.Recording.2025-01-22.at.11.43.30.AM.mov

Areyes42

Download is successful
Verify that the zip file contains all the requested attachments with the correct filenames.
Make sure the download all button is only enabled when there are multiple attachments to download.
With many attachments, make sure the download all button doesn't break when not all attachments have been loaded yet.
Try out the button in different attachments galleries, like in individual records or in record sets.
Try to download multiple attachments with the same file name. Make sure they all get downloaded correctly. They should be named fileName_2.EXT and so on.
Test small download buttons:
Verify that the download button works in different contexts (Attachment viewer, Record set attachment gallery, etc.)

Great work so far, Alejandro!

For #6052 (review), I agree that keeping the Download All option for records with only one attachment is more consistent, even if it feels redundant. I also noticed that when browsing a single form with multiple attachments, the Download All button is also disabled.

Additionally, I kept running into a crash dialog while using the Download All button on the KUFish databases. Initially, I thought it might be related to the asset server configuration, preventing it from properly fetching the assets. However, I was able to download each asset individually without issues, so I'm thinking it might be another issue. The errors only occurred when trying to download all attachments at once.

Screen.Recording.2025-01-22.at.2.10.14.PM.mov

Specify 7 Crash Report - 2025-01-22T20_11_48.389Z.txt

alesan99 added 5 commits January 6, 2025 11:30

WIP

902b762

Lint code with ESLint and Prettier

9410f27

Triggered by 902b762 on branch refs/heads/issue-609

WIP use attachmentLocations instead

dd48454

Lint code with ESLint and Prettier

551bf58

Triggered by dd48454 on branch refs/heads/issue-609

Add functional download all button and archive notification

8e9154b

alesan99 changed the title ~~Add 'Download All' button to record set attachment viewer~~ Add 'Download' button to record set attachment viewer Jan 8, 2025

alesan99 added 2 commits January 8, 2025 15:28

Add download button to individual attachment cells

d529864

Fix trying to download unavailable attachments WIP checkboxes?

Fix missing icon

e7b0fc5

Fix translations

grantfitzsimmons reviewed Jan 13, 2025

View reviewed changes

specifyweb/attachment_gw/views.py Outdated Show resolved Hide resolved

alesan99 added 3 commits January 15, 2025 12:47

Stream zip file to client instead of using a notification

fcc557e

Fix downloading files with the same filename Add ability to recieve and download binary files on the frontend

Merge branch 'production' into issue-609

2ef03dc

Cleanup imports

5a5a746

alesan99 marked this pull request as ready for review January 15, 2025 19:40

alesan99 requested a review from a team January 15, 2025 19:40

CarolineDenis requested a review from maxpatiiuk January 15, 2025 19:43

maxpatiiuk approved these changes Jan 20, 2025

View reviewed changes

alesan99 added 5 commits January 21, 2025 10:49

Misc. code improvements

f8a5f8f

Lint code with ESLint and Prettier

930ea7c

Triggered by f8a5f8f on branch refs/heads/issue-609

Add loading bar

503338c

WIP add tool tip to download all button when disabled WIP use record set name as file name

Merge branch 'issue-609' of https://github.com/specify/specify7 into …

e598352

…issue-609

Lint code with ESLint and Prettier

3c2aa19

Triggered by e598352 on branch refs/heads/issue-609

CarolineDenis requested a review from a team January 22, 2025 00:02

CarolineDenis added this to the 7.9.11.1 milestone Jan 22, 2025

CarolineDenis requested a review from sharadsw January 22, 2025 14:58

sharadsw approved these changes Jan 22, 2025

View reviewed changes

alesan99 added 3 commits January 22, 2025 10:08

Use record set name as zip file name

6b12230

Improve handling of files with same file name

Merge branch 'issue-609' of https://github.com/specify/specify7 into …

8b1dea4

…issue-609

Lint code with ESLint and Prettier

fd3caf7

Triggered by 8b1dea4 on branch refs/heads/issue-609

pashiav requested changes Jan 22, 2025

View reviewed changes

pashiav requested a review from a team January 22, 2025 17:52

Areyes42 requested changes Jan 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 'Download' button to record set attachment viewer #6052

Add 'Download' button to record set attachment viewer #6052

alesan99 commented Jan 8, 2025 •

edited

Loading

grantfitzsimmons commented Jan 8, 2025

alesan99 commented Jan 15, 2025

maxpatiiuk left a comment

maxpatiiuk Jan 20, 2025 •

edited

Loading

sharadsw left a comment

pashiav left a comment

Areyes42 left a comment

Add 'Download' button to record set attachment viewer #6052

Are you sure you want to change the base?

Add 'Download' button to record set attachment viewer #6052

Conversation

alesan99 commented Jan 8, 2025 • edited Loading

Checklist

Testing instructions

grantfitzsimmons commented Jan 8, 2025

alesan99 commented Jan 15, 2025

maxpatiiuk left a comment

Choose a reason for hiding this comment

maxpatiiuk Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

sharadsw left a comment

Choose a reason for hiding this comment

pashiav left a comment

Choose a reason for hiding this comment

Areyes42 left a comment

Choose a reason for hiding this comment

alesan99 commented Jan 8, 2025 •

edited

Loading

maxpatiiuk Jan 20, 2025 •

edited

Loading