Skip to content

Commit ef11511

Browse files
committed
Handling CarrierWave/BrowseEverything controller logic
Prior to this commit, the Hyrax::WoksControllerBehavior's Valkyrie branching logic ignored files selected via BrowseEverything. With this change the controller normalizes(-ish) the two methods of file upload: 1) CarrierWave 2) BrowseEverything An immediate follow-up step is to update `Hyrax::UploadedFile.perform_ingest_later`. In it's present implementation, that method assumes a CarrierWave object. Adding a switching case for UploadedFiles that have a `url_of_remote_file_for_ingest` (e.g., a file selected via BrowseEverything) instead of the assumptive `file` attribute (e.g., a file uploaded via CarrierWave). Relates to #4195
1 parent e3943b6 commit ef11511

File tree

6 files changed

+59
-4
lines changed

6 files changed

+59
-4
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class AddUrlToHyraxUploadedFiles < ActiveRecord::Migration[5.2]
2+
def change
3+
add_column :uploaded_files, :url_of_remote_file_for_ingest, :text
4+
end
5+
end

.dassie/db/schema.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#
1111
# It's strongly recommended that you check this file into your version control system.
1212

13-
ActiveRecord::Schema.define(version: 2020_08_21_212903) do
13+
ActiveRecord::Schema.define(version: 2021_07_16_130305) do
1414

1515
# These are extensions that must be enabled in order to support this database
1616
enable_extension "plpgsql"
@@ -480,6 +480,7 @@
480480
t.string "file_set_uri"
481481
t.datetime "created_at", null: false
482482
t.datetime "updated_at", null: false
483+
t.text "url_of_remote_file_for_ingest"
483484
t.index ["file_set_uri"], name: "index_uploaded_files_on_file_set_uri"
484485
t.index ["user_id"], name: "index_uploaded_files_on_user_id"
485486
end

.regen

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
36
1+
37

app/controllers/concerns/hyrax/works_controller_behavior.rb

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ def create_work
202202

203203
@curation_concern =
204204
form.validate(params[hash_key_for_curation_concern]) &&
205+
normalize_uploaded_files! &&
205206
transactions['change_set.create_work']
206207
.with_step_args('work_resource.add_to_parent' => { parent_id: params[:parent_id], user: current_user },
207208
'work_resource.add_file_sets' => { uploaded_files: uploaded_files, file_set_params: params[hash_key_for_curation_concern][:file_set] },
@@ -219,6 +220,7 @@ def update_work
219220

220221
@curation_concern =
221222
form.validate(params[hash_key_for_curation_concern]) &&
223+
normalize_uploaded_files! &&
222224
transactions['change_set.update_work']
223225
.with_step_args('work_resource.add_file_sets' => { uploaded_files: uploaded_files, file_set_params: params[hash_key_for_curation_concern][:file_set] })
224226
.call(form).value!
@@ -369,6 +371,44 @@ def attributes_for_actor # rubocop:disable Metrics/MethodLength
369371
attributes
370372
end
371373

374+
##
375+
# There are two means of "uploading" files:
376+
#
377+
# 1. via CarrierWave (e.g., upload in the HTML form)
378+
# 2. via BrowseEverything (e.g., select files and later upload
379+
# those URLs)
380+
#
381+
# This method seeks to somewhat normalize the uploaded files by
382+
# creating Hyrax::UploadedFile entries for the BrowseEverthing
383+
# files. Doing this minimizes branching logic up until the
384+
# application stack is ready to attach the binary to the
385+
# corresponding Hyrax::FileSet.
386+
#
387+
# @note at this point, this is a Valkyrie only path. Perhaps a
388+
# later refactor will go down the ActiveFedora path.
389+
def normalize_uploaded_files!
390+
# If they selected a BrowseEverything file, but then clicked the
391+
# remove button, it will still show up in `selected_files`, but
392+
# it will no longer be in uploaded_files. By checking the
393+
# intersection, we get the files they added via BrowseEverything
394+
# that they have not removed from the upload widget.
395+
uploaded_files = params.fetch(:uploaded_files, [])
396+
params.fetch(:selected_files, {}).each_pair do |_, value|
397+
next unless uploaded_files.include?(value[:url])
398+
399+
# This logic mimics the behavior of CarrierWave, namely creating an ActiveRecord record for later
400+
# ingest. In this case, we're swapping out the given URL for the record ID. Downstream, we'll
401+
# act on the URL.
402+
uploaded_file = UploadedFile.create!(user: current_user, url_of_remote_file_for_ingest: value[:url])
403+
uploaded_files.delete(value[:url])
404+
uploaded_files << uploaded_file.to_param
405+
end
406+
# Need to ensure that we swap out the URLs for the UploadedFile's we created above. The uploaded_files local
407+
# variable is a copy of the params[:uploaded_file] (e.g., they don't have the same object_id)
408+
params[:uploaded_files] = uploaded_files unless uploaded_files.empty?
409+
true
410+
end
411+
372412
def after_create_response
373413
respond_to do |wants|
374414
wants.html do
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class AddUrlToHyraxUploadedFiles < ActiveRecord::Migration<%= migration_version %>
2+
def change
3+
add_column :uploaded_files, :url_of_remote_file_for_ingest, :text
4+
end
5+
end

spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,18 +124,22 @@
124124

125125
context 'and files' do
126126
let(:uploads) { FactoryBot.create_list(:uploaded_file, 2, user: user) }
127+
let(:url_to_file) { "https://github.com/samvera/hyrax/browse-everything-url.txt" }
128+
let(:selected_files) { { "1" => { url: url_to_file } } }
127129

128130
it 'attaches the files' do
129131
params = { test_simple_work: { title: 'comet in moominland' },
130-
uploaded_files: uploads.map(&:id) }
132+
# Note this is a mix of BrowseEverything files and those "uploaded" via CarrierWave.
133+
uploaded_files: uploads.map(&:id) + [url_to_file],
134+
selected_files: selected_files }
131135

132136
get :create, params: params
133137

134138
expect(flash[:notice]).to be_html_safe
135139
expect(flash[:notice]).to eq "Your files are being processed by Hyrax in the background. " \
136140
"The metadata and access controls you specified are being applied. " \
137141
"You may need to refresh this page to see these updates."
138-
expect(assigns(:curation_concern)).to have_file_set_members(be_persisted, be_persisted)
142+
expect(assigns(:curation_concern)).to have_file_set_members(be_persisted, be_persisted, be_persisted)
139143
end
140144

141145
let(:uploads) { FactoryBot.create_list(:uploaded_file, 2, user: user) }

0 commit comments

Comments
 (0)