Merge pull request #1052 from UNC-Libraries/hyc-1818-share

HYC-1818 - Fix share with user error

app/overrides/actors/hyrax/actors/base_actor_override.rb

@@ -1,22 +1,11 @@
 # frozen_string_literal: true
 # [hyc-override] Overriding to allow updated content jobs to run immediately so file reference isn't lost
-# https://github.com/samvera/hyrax/blob/v3.4.2/app/actors/hyrax/actors/base_actor.rb
+# https://github.com/samvera/hyrax/blob/hyrax-v4.0.0/app/actors/hyrax/actors/base_actor.rb
 Hyrax::Actors::BaseActor.class_eval do
-  alias_method :original_create, :create
-  def create(env)
-    original_create(env) && apply_work_specific_permissions(env)
-  end
-
-  # @param [Hyrax::Actors::Environment] env
-  # @return [Boolean] true if update was successful
+  alias_method :original_update, :update
   def update(env)
-    apply_update_data_to_curation_concern(env)
     # [hyc-override] Log deleted people objects
-    log_deleted_people_objects(env.attributes, env.curation_concern.id)
-    apply_save_data_to_curation_concern(env)
-    # [hyc-override] Apply work specific permissions
-    apply_work_specific_permissions(env)
-    next_actor.update(env) && save(env) && run_callbacks(:after_update_metadata, env)
+    original_update(env) && log_deleted_people_objects(env.attributes, env.curation_concern.id)
   end
 
   private
@@ -49,7 +38,12 @@ def clean_attributes(attributes)
                               else
                                 v
                               end
+        if v['name']&.include?('@')
+          permission_attrs[k]['name'] = ::User.find_by(email: v['name']).uid
+          Rails.logger.debug("BaseActor.clean_attributes removed email suffix, new id is #{permission_attrs[k]['name']}")
+        end
       end
+      # Rails.logger.error("===clean_attributes Setting perms #{permission_attrs}")
       attributes['permissions_attributes'] = permission_attrs
     end
     remove_blank_attributes!(attributes).except('file_set')
@@ -67,46 +61,6 @@ def log_deleted_people_objects(attributes, work_id)
         end
       end
     end
-  end
-
-  # [hyc-override] added this method to allow work-specific permissions to work
-  def apply_work_specific_permissions(env)
-    permissions_attributes = env.attributes['permissions_attributes']
-    return true if permissions_attributes.blank?
-    # File sets don't have admin sets. So updating them independently of their work should skip this update.
-    # It doesn't seem possible for a FileSet to reach here, since they have their own actor that doesn't inherit from BaseActor?
-    return true unless env.curation_concern.respond_to? :admin_set
-
-    workflow = Sipity::Workflow.where(permission_template_id: env.curation_concern.admin_set.permission_template.id,
-                                      active: true).first
-    entity = Sipity::Entity.where(proxy_for_global_id: env.curation_concern.to_global_id.to_s).first_or_create!
-    permissions_attributes.each do |_k, permission|
-      # skip the pre-existing permissions since they have already been applied
-      next unless permission['id'].blank?
-
-      if permission['type'] == 'person'
-        agent_type = 'User'
-        agent_id = ::User.find_by(uid: permission['name'])
-      else
-        agent_type = 'Hyrax::Group'
-        agent_id = permission['name']
-      end
-      agents = [Sipity::Agent.where(proxy_for_id: agent_id, proxy_for_type: agent_type).first_or_create]
-
-      roles = if permission['access'] == 'edit'
-                'approving'
-              else
-                'viewing'
-              end
-      create_workflow_permissions(entity, agents, roles, workflow)
-    end
-  end
-
-  # [hyc-override] added this method to allow work-specific permissions to work
-  def create_workflow_permissions(entity, agents, roles, workflow)
-    Hyrax::Workflow::PermissionGenerator.call(entity: entity,
-                                              agents: agents,
-                                              roles: roles,
-                                              workflow: workflow)
+    true
   end
 end

spec/actors/hyrax/actors/base_actor_spec.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+require 'rails_helper'
+
+require Rails.root.join('app/overrides/actors/hyrax/actors/base_actor_override.rb')
+
+RSpec.describe Hyrax::Actors::BaseActor do
+  before do
+    ActiveFedora::Cleaner.clean!
+    Blacklight.default_index.connection.delete_by_query('*:*')
+    Blacklight.default_index.connection.commit
+    allow(Hyrax::VirusCheckerService).to receive(:file_has_virus?).and_return(false)
+    Sipity::WorkflowState.create(workflow_id: workflow.id, name: 'deposited')
+  end
+
+  let(:depositor) { FactoryBot.create(:user) }
+  let(:admin_set) { AdminSet.create(title: ['an admin set']) }
+
+  let(:permission_template) do
+    Hyrax::PermissionTemplate.create!(source_id: admin_set.id)
+  end
+
+  let(:workflow) do
+    Sipity::Workflow.create(name: 'test', allows_access_grant: true, active: true,
+                            permission_template_id: permission_template.id)
+  end
+
+  describe '#update' do
+    context 'update work with non-admin non-depositor view permission' do
+      let!(:basic_user) { FactoryBot.create(:user) }
+      let(:ability) { ::Ability.new(depositor) }
+
+      let(:work) {
+        General.create(title: ['work for sharing'],
+                            depositor: depositor.email,
+                            admin_set_id: admin_set.id)
+      }
+      let(:file_set1) { FactoryBot.create(:file_set) }
+
+      let!(:entity) { Sipity::Entity.create(proxy_for_global_id: work.to_global_id.to_s, workflow_id: workflow.id) }
+
+      let(:terminator) { Hyrax::Actors::Terminator.new }
+
+      subject(:middleware) do
+        stack = ActionDispatch::MiddlewareStack.new.tap do |middleware|
+          middleware.use Hyrax::Actors::CreateWithFilesActor
+          middleware.use Hyrax::Actors::AddToWorkActor
+          middleware.use Hyrax::Actors::InterpretVisibilityActor
+          middleware.use Hyrax::Actors::GeneralActor
+        end
+        stack.build(terminator)
+      end
+
+      let(:attributes) {
+        {
+          permissions_attributes: {
+            "0": {
+              type: 'person',
+              name: basic_user.email,
+              access: 'read'
+            }
+          }
+        }
+      }
+
+      before do
+        allow(terminator).to receive(:update).and_return(true)
+        work.ordered_members << file_set1
+      end
+
+      it 'adds the user permission to the work' do
+        env = Hyrax::Actors::Environment.new(work, ability, attributes)
+        middleware.update(env)
+
+        user_perm = work.permissions.to_a.find { |perm| perm.agent.first.id == "http://projecthydra.org/ns/auth/person##{basic_user.uid}" }
+        expect(user_perm.mode.first.id).to eq 'http://www.w3.org/ns/auth/acl#Read'
+      end
+    end
+  end
+end

spec/actors/hyrax/actors/honors_thesis_actor_spec.rb

@@ -13,8 +13,8 @@
   end
 
   let(:depositor) {
-    User.create(email: 'test@example.com',
-                uid: '[email protected]',
+    User.create(email: 'test_depositor@example.com',
+                uid: 'test_depositor',
                 password: 'password',
                 password_confirmation: 'password')
   }
@@ -65,17 +65,18 @@
           permissions_attributes: {
             "0": {
               type: 'person',
-              name: 'test@example.com',
+              name: 'test_depositor@example.com',
               access: 'edit'
             }
           }
         }
       }
 
       it 'assigns user agent permissions' do
-        expect(Hyrax::Workflow::PermissionGenerator).to receive(:call)
-
         subject.update(env)
+
+        approver_perm = curation_concern.permissions.to_a.find { |perm| perm.agent.first.id == 'http://projecthydra.org/ns/auth/person#test_depositor' }
+        expect(approver_perm.mode.first.id).to eq 'http://www.w3.org/ns/auth/acl#Write'
       end
     end
 
@@ -93,8 +94,10 @@
       }
 
       it 'assigns group agent permissions' do
-        expect_create_workflow_permissions_called(entity, 'viewing', workflow)
         subject.update(env)
+
+        reader_perm = curation_concern.permissions.to_a.find { |perm| perm.agent.first.id == 'http://projecthydra.org/ns/auth/group#agroup' }
+        expect(reader_perm.mode.first.id).to eq 'http://www.w3.org/ns/auth/acl#Read'
       end
     end
 
@@ -109,6 +112,8 @@
         expect(Hyrax::Workflow::PermissionGenerator).not_to receive(:call)
 
         subject.update(env)
+
+        expect(curation_concern.permissions.to_a).to be_empty
       end
     end
 
@@ -120,17 +125,18 @@
             '0': {
               index: '0',
               type: 'person',
-              name: 'test@example.com',
+              name: 'test_depositor@example.com',
               access: 'edit'
             }
           }
         }
       }
 
       it 'assigns user agent permissions' do
-        expect_create_workflow_permissions_called(entity, 'approving', workflow)
-
         subject.update(env)
+
+        approver_perm = curation_concern.permissions.to_a.find { |perm| perm.agent.first.id == 'http://projecthydra.org/ns/auth/person#test_depositor' }
+        expect(approver_perm.mode.first.id).to eq 'http://www.w3.org/ns/auth/acl#Write'
       end
     end
 
@@ -143,8 +149,6 @@
       }
 
       it 'saves person details to work' do
-        expect(Hyrax::Workflow::PermissionGenerator).not_to receive(:call)
-
         subject.update(env)
 
         first_creator = curation_concern.creators.first
@@ -171,8 +175,6 @@
       }
 
       it 'logs the id of the work containing the deleted person' do
-        expect(Hyrax::Workflow::PermissionGenerator).not_to receive(:call)
-
         subject.update(env)
 
         File.open(ENV['DELETED_PEOPLE_FILE'], 'r') do |file|
@@ -207,17 +209,18 @@
           permissions_attributes: {
             "0": {
               type: 'person',
-              name: 'test@example.com',
+              name: 'test_depositor@example.com',
               access: 'edit'
             }
           }
         }
       }
 
       it 'assigns user agent permissions' do
-        expect_create_workflow_permissions_called(entity, 'approving', workflow)
-
         middleware.create(env)
+
+        approver_perm = curation_concern.permissions.to_a.find { |perm| perm.agent.first.id == 'http://projecthydra.org/ns/auth/person#test_depositor' }
+        expect(approver_perm.mode.first.id).to eq 'http://www.w3.org/ns/auth/acl#Write'
       end
     end
   end