diff --git a/app/controllers/hyrax/permissions_controller.rb b/app/controllers/hyrax/permissions_controller.rb index ed85b7e1a6..51bba25386 100644 --- a/app/controllers/hyrax/permissions_controller.rb +++ b/app/controllers/hyrax/permissions_controller.rb @@ -1,6 +1,9 @@ # frozen_string_literal: true module Hyrax class PermissionsController < ApplicationController + load_resource class: ActiveFedora::Base, instance_name: :curation_concern + + attr_reader :curation_concern helper_method :curation_concern def confirm @@ -28,9 +31,5 @@ def copy_access InheritPermissionsJob.perform_later(curation_concern) redirect_to [main_app, curation_concern], notice: I18n.t("hyrax.upload.change_access_flash_message") end - - def curation_concern - @curation_concern ||= Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: params[:id], use_valkyrie: false) - end end end diff --git a/app/jobs/inherit_permissions_job.rb b/app/jobs/inherit_permissions_job.rb index 9e6d772576..c41091b92f 100644 --- a/app/jobs/inherit_permissions_job.rb +++ b/app/jobs/inherit_permissions_job.rb @@ -5,12 +5,16 @@ class InheritPermissionsJob < Hyrax::ApplicationJob # Perform the copy from the work to the contained filesets # # @param work containing access level and filesets - # @param use_valkyrie [Boolean] whether to use valkyrie support - def perform(work, use_valkyrie: Hyrax.config.use_valkyrie?) - if use_valkyrie - valkyrie_perform(work) - else + # @param use_valkyrie [Boolean] whether to use valkyrie support (deprecated) + def perform(work, use_valkyrie: :DEFAULT) + Deprecation.warn("use_valkyrie argument is deprecated, behavior depends on deserialized resource") unless + use_valkyrie == :DEFAULT + + case work + when ActiveFedora::Base af_perform(work) + else + valkyrie_perform(work) end end diff --git a/spec/controllers/hyrax/permissions_controller_spec.rb b/spec/controllers/hyrax/permissions_controller_spec.rb index 22c1c71d02..54f612af13 100644 --- a/spec/controllers/hyrax/permissions_controller_spec.rb +++ b/spec/controllers/hyrax/permissions_controller_spec.rb @@ -1,57 +1,51 @@ # frozen_string_literal: true RSpec.describe Hyrax::PermissionsController do - let(:user) { create(:user) } + let(:user) { FactoryBot.create(:user) } - before do - sign_in user - allow(ActiveFedora::Base).to receive(:find).with(work.id).and_return(work) - end - - describe '#confirm' do - let(:work) { build(:generic_work, user: user, id: 'abc') } + before { sign_in user } - before do - # /~https://github.com/samvera/active_fedora/issues/1251 - allow(work).to receive(:persisted?).and_return(true) - end + context 'with legacy AF models' do + describe '#confirm_access' do + let(:work) { FactoryBot.create(:generic_work, user: user) } - it 'draws the page' do - get :confirm, params: { id: work } - expect(response).to be_successful + it 'draws the page' do + get :confirm_access, params: { id: work } + expect(response).to be_successful + end end - end - describe '#copy' do - let(:work) { create(:generic_work, user: user) } + describe '#copy' do + let(:work) { FactoryBot.create(:generic_work, user: user) } - it 'adds a worker to the queue' do - expect(VisibilityCopyJob).to receive(:perform_later).with(work) - post :copy, params: { id: work } - expect(response).to redirect_to main_app.hyrax_generic_work_path(work, locale: 'en') - expect(flash[:notice]).to eq 'Updating file permissions. This may take a few minutes. You may want to refresh your browser or return to this record later to see the updated file permissions.' - end - end + it 'adds a worker to the queue' do + expect { post :copy, params: { id: work } } + .to have_enqueued_job(VisibilityCopyJob) + .with(work) - describe '#confirm_access' do - let(:work) { create(:work_with_one_file, user: user) } - - it 'draws the page' do - get :confirm_access, params: { id: work } - expect(response).to be_successful + expect(response).to redirect_to main_app.hyrax_generic_work_path(work, locale: 'en') + expect(flash[:notice]).to eq 'Updating file permissions. This may take a few minutes. You may want to refresh your browser or return to this record later to see the updated file permissions.' + end end - end - describe '#copy_access' do - let(:work) { create(:work_with_one_file, user: user) } - - it 'adds a worker to the queue' do - expect(VisibilityCopyJob).to receive(:perform_later).with(work) - expect(InheritPermissionsJob).to receive(:perform_later).with(work) - post :copy_access, params: { id: work } - expect(response).to redirect_to main_app.hyrax_generic_work_path(work, locale: 'en') - expect(flash[:notice]).to eq 'Updating file access levels. This may take a few minutes. ' \ - 'You may want to refresh your browser or return to this record ' \ - 'later to see the updated file access levels.' + describe '#copy_access' do + let(:work) { FactoryBot.create(:work_with_one_file, user: user) } + + it 'adds VisibilityCopyJob to the queue' do + expect { post :copy_access, params: { id: work } } + .to have_enqueued_job(VisibilityCopyJob) + .with(work) + + expect(response).to redirect_to main_app.hyrax_generic_work_path(work, locale: 'en') + expect(flash[:notice]).to eq 'Updating file access levels. This may take a few minutes. ' \ + 'You may want to refresh your browser or return to this record ' \ + 'later to see the updated file access levels.' + end + + it 'adds InheritPermisionsJob to the queue' do + expect { post :copy_access, params: { id: work } } + .to have_enqueued_job(InheritPermissionsJob) + .with(work) + end end end end diff --git a/spec/jobs/inherit_permissions_job_spec.rb b/spec/jobs/inherit_permissions_job_spec.rb index c699b6548f..8f4189ca5a 100644 --- a/spec/jobs/inherit_permissions_job_spec.rb +++ b/spec/jobs/inherit_permissions_job_spec.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true RSpec.describe InheritPermissionsJob do - let(:user) { create(:user) } - let(:work) { create(:work_with_one_file, user: user) } + let(:user) { FactoryBot.create(:user) } + let(:work) { FactoryBot.create(:work_with_one_file, user: user) } - context "when use_valkyrie is false" do + context "when using a legacy AF resource" do before do work.permissions.build(name: name, type: type, access: access) work.save @@ -18,7 +18,7 @@ # files have the depositor as the edit user to begin with expect(work.file_sets.first.edit_users).to eq [user.to_s] - described_class.perform_now(work, use_valkyrie: false) + described_class.perform_now(work) file_sets = work.reload.file_sets expect(file_sets.count).to eq 1 @@ -36,7 +36,7 @@ # files have the depositor as the edit user to begin with expect(work.file_sets.first.edit_users).to eq [user.to_s, "remove_me"] - described_class.perform_now(work, use_valkyrie: false) + described_class.perform_now(work) file_sets = work.reload.file_sets expect(file_sets.count).to eq 1 @@ -54,7 +54,7 @@ # files have no read users to begin with expect(work.file_sets.first.read_users).to eq [] - described_class.perform_now(work, use_valkyrie: false) + described_class.perform_now(work) file_sets = work.reload.file_sets expect(file_sets.count).to eq 1 @@ -72,7 +72,7 @@ # files have no read groups to begin with expect(work.file_sets.first.read_groups).to eq [] - described_class.perform_now(work, use_valkyrie: false) + described_class.perform_now(work) file_sets = work.reload.file_sets expect(file_sets.count).to eq 1 @@ -90,7 +90,7 @@ # files have the depositor as the edit user to begin with expect(work.file_sets.first.read_groups).to eq [] - described_class.perform_now(work, use_valkyrie: false) + described_class.perform_now(work) file_sets = work.reload.file_sets expect(file_sets.count).to eq 1 @@ -100,10 +100,10 @@ end end - context "when use_valkyrie is true" do - let(:resource) { valkyrie_create(:hyrax_work, edit_users: [user]) } - let(:file_set) { valkyrie_create(:hyrax_file_set) } - let(:user2) { create(:user) } + context "when passed a valkyrie model", valkyrie_adapter: :test_adapter do + let(:resource) { FactoryBot.valkyrie_create(:hyrax_work, edit_users: [user]) } + let(:file_set) { FactoryBot.valkyrie_create(:hyrax_file_set) } + let(:user2) { FactoryBot.create(:user) } before do resource.member_ids = Array(file_set.id) @@ -120,7 +120,7 @@ expect(file_sets.count).to eq 1 expect(file_sets[0].edit_users).to match_array [user.to_s] - described_class.perform_now(resource, use_valkyrie: true) + described_class.perform_now(resource) # files have both edit users from parent resource file_sets = Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) @@ -130,24 +130,22 @@ end context "when people should be removed" do - context "when use_valkyrie is true" do - it 'copies permissions to its contained files' do - file_set.permission_manager.acl.grant(:edit).to(user2).save - # work has the depositor as the edit user to begin with - expect(resource.edit_users).to match_array [user.to_s] + it 'copies permissions to its contained files' do + file_set.permission_manager.acl.grant(:edit).to(user2).save + # work has the depositor as the edit user to begin with + expect(resource.edit_users).to match_array [user.to_s] - # files have the depositor and extra user as the edit users to begin with - file_sets = Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) - expect(file_sets.count).to eq 1 - expect(file_sets[0].edit_users).to match_array [user.to_s, user2.to_s] + # files have the depositor and extra user as the edit users to begin with + file_sets = Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) + expect(file_sets.count).to eq 1 + expect(file_sets[0].edit_users).to match_array [user.to_s, user2.to_s] - described_class.perform_now(resource, use_valkyrie: true) + described_class.perform_now(resource) - # files have single edit user from parent resource - file_sets = Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) - expect(file_sets.count).to eq 1 - expect(file_sets[0].edit_users).to match_array [user.to_s] - end + # files have single edit user from parent resource + file_sets = Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) + expect(file_sets.count).to eq 1 + expect(file_sets[0].edit_users).to match_array [user.to_s] end end @@ -161,7 +159,7 @@ expect(file_sets.count).to eq 1 expect(file_sets[0].read_users.to_a).to be_empty - described_class.perform_now(resource, use_valkyrie: true) + described_class.perform_now(resource) # files have the specified read user file_sets = Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) @@ -179,7 +177,7 @@ # work has the specified read group to begin with expect(resource.read_groups).to match_array ["my_read_group"] - described_class.perform_now(resource, use_valkyrie: true) + described_class.perform_now(resource) # files have the specified read group file_sets = Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) @@ -198,7 +196,7 @@ # work has the specified edit group to begin with expect(resource.edit_groups).to match_array ["my_edit_group"] - described_class.perform_now(resource, use_valkyrie: true) + described_class.perform_now(resource) # files have the specified edit group file_sets = Hyrax.query_service.custom_queries.find_child_filesets(resource: resource)