Fix single use links for Valkyrie. #6226
Conversation
Closes #6204 This also adds a shortcut to allow creating a work that has a file, including a FileSet, FileMetadata, and File Identifier. I'm not sure it's the best, but it works really well for this test!
| evaluator.uploaded_files.each do |file| | ||
| allow(Hyrax.config.characterization_service).to receive(:run).and_return(true) | ||
| # I don't love this - we might want to just run background jobs so | ||
| # this is more real, but we'd have to stub some things. |
There was a problem hiding this comment.
could we do
ActiveJob::Base.queue_adapter.filter += [ValkyrieIngestJob]Hyrax already makes use of the ActiveJob test adapter. i guess the real implementation is probably a bit fussy, since we need to reach into config and then reset it to be really clean:
old_perform_enqueued_setting = ActiveJob::Base.queue_adapter.perform_enqueued_jobs
old_job_filter = ActiveJob::Base.queue_adapter.filter
ActiveJob::Base.queue_adapter.perform_enqueued_jobs = true
ActiveJob::Base.queue_adapter.filter += [ValkyrieIngestJob]
allow(Hyrax.config.characterization_service).to receive(:run).and_return(true)
Hyrax::WorkUploadsHandler.new(work: work).add(files: evaluator.uploaded_files).attach
ActiveJob::Base.queue_adapter.perform_enqueued_jobs = old_perform_enqueued_setting
ActiveJob::Base.queue_adapter.filter = old_job_filtermaybe more than is worth messing with?
the stub on Hyrax.config.characterization_service feels like a code smell to me. currently, if i publish file.uploaded from any synchronous code, i'll trigger a long running characterization_service.run inline. since any app can publish that event from any code, that might not be the best.
this issue is probably out of scope here, but fixing it would let us rely on the ActiveJob adapter filter for the characterization behavior too, which would offer a lot of flexiblitiy.
There was a problem hiding this comment.
I had a version that did perform_enqueued jobs
The big problem is just fits tries to run and it's not installed.
There was a problem hiding this comment.
does the stub not take effect through the adapter?
| @@ -66,7 +80,7 @@ def not_found_exception | |||
| end | |||
|
|
|||
| def asset | |||
| @asset ||= Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: single_use_link.item_id, use_valkyrie: false) | |||
| @asset ||= Hyrax.query_service.find_by(id: single_use_link.item_id) | |||
spec/factories/hyrax_file_set.rb
Outdated
| @@ -24,7 +25,7 @@ | |||
| .assign_access_for(visibility: evaluator.visibility_setting) | |||
| end | |||
| file_set.file_ids = evaluator.files.map(&:id) if evaluator.files | |||
| file_set.original_file_id = evaluator.original_file.id if evaluator.original_file | |||
| file_set.original_file_id = evaluator&.original_file&.id || evaluator.files&.first&.id | |||
There was a problem hiding this comment.
i'm not sure i'm following this. is it used somewhere i'm missing?
There was a problem hiding this comment.
Oh this might be left over from a previous try.
|
approved because all the suggestions are related to test setup. both the implementation and the test setup are clearly improved here, to me. |
Closes #6204
This also adds a shortcut to allow creating a work that has a file, including a FileSet, FileMetadata, and File Identifier. I'm not sure it's the best, but it works really well for this test!