refactor PermisionsController to use load_resource

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

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
 

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

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)