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

create association in fileset to the file #3842

Merged
merged 1 commit into from
Jun 21, 2019
Merged

Conversation

elrayle
Copy link
Contributor

@elrayle elrayle commented Jun 21, 2019

This fixes some of the remaining issue for PR #3814. Specifically…

  • image is viewable on the work show page
  • thumbnail is shown in dashboard -> works
  • file is downloadable
  • characterization job completes

Still need to write tests

Related to issue: Issue #3813


Created issue for Hydra::Works reporting the code that was originally copied that isn't working as expected...
samvera/hydra-works#369

@elrayle elrayle added the wings label Jun 21, 2019
@elrayle elrayle self-assigned this Jun 21, 2019
@elrayle elrayle force-pushed the wings/files_display branch from a0166e8 to 51ca6b7 Compare June 21, 2019 17:47
@@ -76,10 +76,6 @@ def perform_ingest_file_through_active_fedora(io)

def perform_ingest_file_through_valkyrie(io)
# Skip versioning because versions will be minted by VersionCommitter as necessary during save_characterize_and_record_committer.
storage_adapter = Valkyrie.config.storage_adapter
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to separate method to make rubocop happy about the method size.

saved_node
pathhint = io.uploaded_file.uploader.path if io.uploaded_file # in case next worker is on same filesystem
id = Hyrax.config.translate_uri_to_id.call saved_node.file_identifiers.first
CharacterizeJob.perform_later(file_set, id, pathhint || io.path)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perform characterization after creating the resource.

@@ -45,14 +45,15 @@ def update

# @param [RDF::URI] the identified use of the file (e.g. Valkyrie::Vocab::PCDMUse.OriginalFile, Valkyrie::Vocab::PCDMUse.ThumbnailImage, etc.)
# @param [true, false] update_existing when true, try to retrieve existing element before building one
def find_or_create_file(use, update_existing)
Copy link
Contributor Author

@elrayle elrayle Jun 21, 2019

Choose a reason for hiding this comment

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

This code grabbed from Hydra::Works::AddFileToFileSet #find_or_create_file_for_rdf_uri does not work. It fails to create the #original_file association which is needed in other places in Hyrax (e.g. CharacterizeJob).

Switched to using the code from #find_or_create_file_for_symbol which requires that the URI passed in be converted to a symbol first.

end

def association_type(use)
use.first.to_s.split('#').second.underscore.to_sym
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the conversion from URI to symbol which allows the original_file association to be created.

This fixes some of the remaining issue for PR #3814.  Specifically…
* image is viewable on the work show page
* thumbnail is shown in dashboard -> works
* file will now download
* characterization job completes

Still need to write tests
@elrayle elrayle force-pushed the wings/files_display branch from 51ca6b7 to 8254ccb Compare June 21, 2019 20:31
@@ -16,7 +16,7 @@ def initialize(storage_adapter:, persister:)

# @param io_wrapper [JobIOWrapper] with details about the uploaded file
# @param node [FileNode] the metadata to represent the file
# @param file_set [Valkyrie::Resouce, Hydra::Works::FileSet] the associated FileSet # TODO: Remove Hydra::Works::FileSet as a potential type when valkyrization is complete.
# @param file_set [Valkyrie::Resouce, Hydra::Works::FileSet] the associated FileSet # TODO: WINGS - Remove Hydra::Works::FileSet as a potential type when valkyrization is complete.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Marking TODOs as WINGS related to make them easier to find and resolve.

saved_node = file_set.is_a?(::Valkyrie::Resource) ? attach_file_node_to_valkyrie_file_set(node, file_set) : node

# note the returned saved_node does not yet contain the characterization done in the async job
CharacterizeJob.perform_later(saved_node.file_identifiers.first.to_s) # TODO: What id is the correct one for the file? Check where this is called outside of wings.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this here. It wasn't working and is now done in the file_actor. The file_actor performs this when not working with Valkyrie, so it seems like a consistent place to do it when working with Valkyrie.

Copy link
Contributor

@no-reply no-reply left a comment

Choose a reason for hiding this comment

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

Approving this on the condition that unit tests are forthcoming.

Since it's a wings change, and currently uncalled code, i'm happy to integrate eagerly as long as we are still working on this.

@no-reply no-reply merged commit 16accab into master Jun 21, 2019
@no-reply no-reply deleted the wings/files_display branch June 21, 2019 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants