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

use the Hyrax::FileSetFileService to determine "primary file" #6752

Merged
merged 1 commit into from
Mar 28, 2024
Merged

Conversation

no-reply
Copy link
Contributor

Hyrax::FileSetFileService was introduced to un-hard code the assumption that the :original_file PCDM Use is the "primary" file for a given file set. use it throughout the code base to avoid requiring that assumption in downstream apps.

this should allow downstream apps to wire in a new service and resolve the "primary file" for each object according to local requirements. e.g. it may be desirable for apps to treat "preservation file" or "service file" as "primary"

Changes proposed in this pull request:

  • adds support for overriding the "primary" file resolution behavior

@samvera/hyrax-code-reviewers

@no-reply no-reply force-pushed the primary branch 2 times, most recently from b9ec2c9 to 106a22c Compare March 27, 2024 17:29
`Hyrax::FileSetFileService` was introduced to un-hard code the assumption that
the `:original_file` PCDM Use is the "primary" file for a given file set. use it
throughout the code base to avoid requiring that assumption in downstream apps.
#
# @return [Hyrax::FileMetadata]
def self.primary_file_for(file_set:, query_service: Hyrax.query_service)
new(file_set: file_set, query_service: query_service).primary_file
Copy link
Collaborator

Choose a reason for hiding this comment

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

The fallback in #primary_file (at least) does

fallback_id = file_set.file_ids.first

This should probably use a query service too instead, yeah? (Maybe we need a “first file metadata in file set” query?)

I’d also very much like to get rid of the dependence on file_set.original_file_id in that method (which I think suffers from the same modelling issues that file_ids does), but I couldn’t figure out a way how when I wrote that method originally. Is there a query we could use for that instead?

(The difference between the service and any original file query is, and should remain, that the service has fallback behaviour if there is no original file, whereas the query would return nil.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think you're probably right, on both points.

i remember the #primary_file setup being pretty fussy though (between Wings and PG), and i think for our purposes it's moot anyway, since we want to override this whole service in comet. getting this much through unblocks that, and next steps can be explored from there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

re: original_file_id, i think custom_queries.find_original_file(file_set: file_set) is the preferred approach, and is executed in the other branch. but i presume there's a reason for original_file_id getting used preferentially.

@dlpierce dlpierce merged commit 84513ee into main Mar 28, 2024
6 checks passed
@dlpierce dlpierce deleted the primary branch March 28, 2024 19:22
@dlpierce dlpierce added the notes-minor Release Notes: Non-breaking features label Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes-minor Release Notes: Non-breaking features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants