Skip to content

Conversation

@meisekimiu
Copy link
Contributor

@meisekimiu meisekimiu commented Nov 12, 2024

Prefer archivematica access copies over older converted access copies in the file viewer. This is accomplished with a centralized helper function that picks the proper access copy in our specific priority order. This function is then used in various file viewer components.

This also includes the change to the download options, since it depends on the new FileVO type definitions.

Resolves PER-9873 + PER-9902.

Steps to test:

  1. If the environment you're testing on includes archivematica-generated access copies, then you can actually test this functionality. Make sure to verify new access copies are downloadable as well.
  2. If not, verify that unit tests pass and that file viewing for all types (image, document, audio, video, are there others?) is otherwise not affected. Verify that downloading them still functions as well.

@meisekimiu meisekimiu force-pushed the archivematica-access-files branch from 3cfdd25 to d6bb9a5 Compare November 12, 2024 16:32
@codecov
Copy link

codecov bot commented Nov 12, 2024

Codecov Report

Attention: Patch coverage is 82.75862% with 5 lines in your changes missing coverage. Please review.

Project coverage is 42.71%. Comparing base (3b1f4d5) to head (284ae00).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/app/shared/components/video/video.component.ts 71.42% 1 Missing and 1 partial ⚠️
...nents/download-button/download-button.component.ts 0.00% 1 Missing ⚠️
...file-list-controls/file-list-controls.component.ts 0.00% 1 Missing ⚠️
src/app/shared/components/audio/audio.component.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #488      +/-   ##
==========================================
+ Coverage   42.53%   42.71%   +0.17%     
==========================================
  Files         357      359       +2     
  Lines       10984    10974      -10     
  Branches     1795     1793       -2     
==========================================
+ Hits         4672     4687      +15     
+ Misses       6151     6130      -21     
+ Partials      161      157       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Copy link
Member

@liam-lloyd liam-lloyd left a comment

Choose a reason for hiding this comment

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

In testing this, it looks like the Archivematica access copy is available via the download button in the record info panel on the right of the screen, but not via the download button at the top of the file list.

@meisekimiu
Copy link
Contributor Author

In testing this, it looks like the Archivematica access copy is available via the download button in the record info panel on the right of the screen, but not via the download button at the top of the file list.

I believe this should be fixed now

@meisekimiu
Copy link
Contributor Author

I decided to refactor the "cursed" pattern I was using earlier since it ended up being used in two places now. This should make those calls easier to read while allowing us to continue to use this same prioritization behavior in each case, and hopefully future cases if they arise.

@meisekimiu meisekimiu requested a review from k8lyn6 November 18, 2024 22:54
@k8lyn6 k8lyn6 requested a review from yeslikesolo November 19, 2024 14:14
@k8lyn6
Copy link
Collaborator

k8lyn6 commented Nov 19, 2024

@yeslikesolo to test this, please check all formats listed as accepted file types in this KB article to make sure they are 1. viewable (in both the preview info tab and full screen!) and 2. downloadable (from the button at the top and the button in the info tab). If you need test files, you can find some here.

@meisekimiu
Copy link
Contributor Author

To clarify a bit, you probably only need to test the various general types of files ("Photo, Audio, Video, Document" etc) and not each specific format (though different kinds of documents might be helpful since they go through multiple processing steps)

@meisekimiu meisekimiu force-pushed the archivematica-access-files branch from dee55c2 to 7aa07eb Compare November 19, 2024 20:55
Create a helper method that fetches the access file for a given record.
Also finally create an internal type for FileVO, which is called
`PermanentFile`.

This algorithm prioritizes Archivematica copies first, then other
coverted copies (where it prioritizes PDFs over other formats since
documents have intermediate conversion formats), before falling back to
the original file.

PER-9873: Update clients to favor Archivematica access copies
Use the `GetAccessFile` method in various file viewer components. In
particular:
  - The code that fetches the PDF url in the file-viewer component
  - The code that fetches the proper audio file
  - The code that fetches the proper video file
  - The code that fetches which image to use in OpenSeaDragon

PER-9873: Update clients to favor Archivematica access copies
This is a small adjustment to look for Archivematica Access files in
download options. It is part of the same branch as PER-9873 so we can
use the new enum for file formats.

PER-9902: Include Archivematica access copies in the download options
Add a method onto RecordVO that gets a list of info about download
options from its list of FileVOs. This function is duplicated across the
codebase for each of the download button components so adding this as a
method onto RecordVO centralizes this logic in one place. Then use this
method in both download buttons.

PER-9902: Include Archivematica access copies in the download options
When sorting through FileVOs a hard-to-read pattern emerged multiple
times that involved subtracting booleans that were converted to numbers.
Extract this logic into its own helper function so we can continue to
use this helpful pattern but it also is more readable and more reusable.

PER-9902: Include Archivematica access copies in the download options
@meisekimiu meisekimiu force-pushed the archivematica-access-files branch from 7aa07eb to 284ae00 Compare November 19, 2024 21:05
@meisekimiu meisekimiu merged commit 4e181f1 into main Nov 19, 2024
4 checks passed
@meisekimiu meisekimiu deleted the archivematica-access-files branch November 19, 2024 21: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