Backport add_to_work_actor permission check

Completes #3848

In the case that a request to the actor stack tries to add a work to a parent
that the current ability cannot edit, we want to avoid doing so.

Previously, we allowed the stack to carry on creating the new work, only to fail
at the end--with an incomplete work. This leaves the application data in an
undesirable state.

app/actors/hyrax/actors/add_to_work_actor.rb

@@ -5,7 +5,8 @@ class AddToWorkActor < AbstractActor
       # @return [Boolean] true if create was successful
       def create(env)
         work_ids = env.attributes.delete(:in_works_ids)
-        next_actor.create(env) && add_to_works(env, work_ids)
+
+        can_edit_works?(env, work_ids) && next_actor.create(env) && add_to_works(env, work_ids)
       end
 
       # @param [Hyrax::Actors::Environment] env
@@ -21,6 +22,15 @@ def can_edit_both_works?(env, work)
           env.current_ability.can?(:edit, work) && env.current_ability.can?(:edit, env.curation_concern)
         end
 
+        def can_edit_works?(env, work_ids)
+          unless Array(work_ids).all? { |work_id| env.current_ability.can?(:edit, work_id) }
+            add_permissions_error(env.curation_concern)
+            return false
+          end
+
+           true
+        end
+
         def add_to_works(env, new_work_ids)
           return true if new_work_ids.nil?
           cleanup_ids_to_remove_from_curation_concern(env, new_work_ids)
@@ -39,16 +49,25 @@ def cleanup_ids_to_remove_from_curation_concern(env, new_work_ids)
 
         def add_new_work_ids_not_already_in_curation_concern(env, new_work_ids)
           # add to new so long as the depositor for the parent and child matches, otherwise inject an error
-          (new_work_ids - env.curation_concern.in_works_ids).each do |work_id|
-            work = ::ActiveFedora::Base.find(work_id)
+          new_works_for(env, new_work_ids).each do |work|
             if can_edit_both_works?(env, work)
               work.ordered_members << env.curation_concern
               work.save!
             else
-              env.curation_concern.errors[:in_works_ids] << "Works can only be related to each other if user has ability to edit both."
+              add_permissions_error(env.curation_concern)
             end
           end
         end
+
+        def new_works_for(env, new_work_ids)
+           (new_work_ids - env.curation_concern.in_works_ids).map do |work_id|
+            ::ActiveFedora::Base.find(work_id)
+          end
+        end
+
+        def add_permissions_error(work)
+          work.errors[:in_works_ids] << "Works can only be related to each other if user has ability to edit both."
+        end
     end
   end
 end

spec/features/actor_stack_spec.rb

@@ -26,6 +26,58 @@ def create(env)
         .to true
     end
 
+    context 'when adding to a work' do
+      let(:other_work) { FactoryBot.create(:work, user: user) }
+      let(:attributes) { { 'in_works_ids' => [other_work.id] } }
+
+      context 'when the user cannot edit the parent work' do
+        let(:other_work) { FactoryBot.create(:work, user: other_user) }
+        let(:other_user) { FactoryBot.create(:user) }
+
+        it 'fails' do
+          expect { actor.create(env) }
+            .not_to change { other_work.reload.members.to_a }
+            .from be_empty
+        end
+
+        it 'does not create the work' do
+          expect { actor.create(env) }
+            .not_to change { work.persisted? }
+            .from false
+        end
+      end
+
+      it 'adds the work to the parent' do
+        expect { actor.create(env) }
+          .to change { other_work.reload.members.to_a }
+          .from(be_empty)
+          .to contain_exactly(work)
+      end
+    end
+
+    context 'when noids are disabled' do
+      let(:uuid_regex) { /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/ }
+
+      it 'uses the fedora assigned uuids' do
+        expect { actor.create(env) }
+          .to change { env.curation_concern.id }
+          .to uuid_regex
+      end
+    end
+
+    context 'when noids are enabled' do
+      before(:context) { Hyrax.config.enable_noids = true }
+      after(:context)  { Hyrax.config.enable_noids = false }
+
+      let(:noid_regex) { /^[0-9a-z]+$/ }
+
+      it 'assigns noids' do
+        expect { actor.create(env) }
+          .to change { env.curation_concern.id }
+          .to noid_regex
+      end
+    end
+
     context 'when failing on the way back up the actor stack' do
       before { stack.insert_before(Hyrax::Actors::ModelActor, delayed_failure_actor) }