Make the overrides minimal. Add unit tests

app/overrides/controllers/concerns/hyrax/works_controller_behavior_override.rb

@@ -28,27 +28,16 @@ def available_admin_sets
 
   private
   # [hyc-override] Capture whether the work had an embargo before changes are saved
+  alias_method :original_save_permissions, :save_permissions
   def save_permissions
+    original_save_permissions
     @original_embargo_state = curation_concern.under_embargo?
-    @saved_permissions =
-      case curation_concern
-      when ActiveFedora::Base
-        curation_concern.permissions.map(&:to_hash)
-      else
-        Hyrax::AccessControl.for(resource: curation_concern).permissions
-      end
   end
 
   # [hyc-override] Return true if the permissions have changed or the embargo state has changed
+  alias_method :original_permissions_changed?, :permissions_changed?
   def permissions_changed?
-    perms_changed = @saved_permissions !=
-      case curation_concern
-      when ActiveFedora::Base
-        curation_concern.permissions.map(&:to_hash)
-      else
-        Hyrax::AccessControl.for(resource: curation_concern).permissions
-      end
-    perms_changed || @original_embargo_state != curation_concern.under_embargo?
+    original_permissions_changed? || @original_embargo_state != curation_concern.under_embargo?
   end
 
   # [hyc-override] Special permissions for admins indicating they aren't constrained by the admin set

spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb

@@ -10,6 +10,12 @@
     self.curation_concern_type = General
   end
 
+  before do
+    ActiveFedora::Cleaner.clean!
+    Blacklight.default_index.connection.delete_by_query('*:*')
+    Blacklight.default_index.connection.commit
+  end
+
   describe '#available_admin_sets' do
     context 'with a logged in user' do
       before { sign_in user }
@@ -26,9 +32,6 @@
         let(:workflow) { Sipity::Workflow.find_by!(name: 'default', permission_template: permission_template) }
 
         before do
-          ActiveFedora::Cleaner.clean!
-          Blacklight.default_index.connection.delete_by_query('*:*')
-          Blacklight.default_index.connection.commit
           Hyrax::PermissionTemplateAccess.create(permission_template: permission_template,
                                              agent_type: 'user',
                                              agent_id: user.user_key,
@@ -61,4 +64,45 @@
       end
     end
   end
+
+  describe '#permissions_changed?' do
+    let(:user) { FactoryBot.create(:user) }
+    let(:work) {
+        General.new(title: ['test work'])
+      }
+
+    context 'with no new permissions or embargo' do
+      it 'returns false' do
+        allow(work).to receive(:under_embargo?).and_return(false)
+        controller.instance_variable_set(:@curation_concern, work)
+
+        controller.send(:save_permissions)
+
+        expect(controller.send(:permissions_changed?)).to be false
+      end
+    end
+
+    context 'with an embargo added' do
+      it 'returns true' do
+        allow(work).to receive(:under_embargo?).and_return(true)
+        controller.instance_variable_set(:@curation_concern, work)
+
+        controller.send(:save_permissions)
+
+        expect(controller.send(:permissions_changed?)).to be true
+      end
+    end
+
+    context 'with new permissions but no new embargo' do
+      it 'returns true' do
+        allow(work).to receive(:under_embargo?).and_return(false)
+        allow(controller).to receive(:original_permissions_changed?).and_return(true)
+        controller.instance_variable_set(:@curation_concern, work)
+
+        controller.send(:save_permissions)
+
+        expect(controller.send(:permissions_changed?)).to be true
+      end
+    end
+  end
 end