diff --git a/app/controllers/hyrax/batch_edits_controller.rb b/app/controllers/hyrax/batch_edits_controller.rb index 3b705fa408..d70b974169 100644 --- a/app/controllers/hyrax/batch_edits_controller.rb +++ b/app/controllers/hyrax/batch_edits_controller.rb @@ -50,7 +50,7 @@ def update_document(obj) obj.attributes = work_params(admin_set_id: obj.admin_set_id).except(*visibility_params) obj.date_modified = Time.current.ctime - InheritPermissionsJob.perform_now(obj) + InheritPermissionsJob.perform_now(obj, use_valkyrie: false) VisibilityCopyJob.perform_now(obj) obj.save diff --git a/app/jobs/inherit_permissions_job.rb b/app/jobs/inherit_permissions_job.rb index e429cb81fa..b95cfe73ad 100644 --- a/app/jobs/inherit_permissions_job.rb +++ b/app/jobs/inherit_permissions_job.rb @@ -4,21 +4,71 @@ class InheritPermissionsJob < Hyrax::ApplicationJob # Perform the copy from the work to the contained filesets # # @param work containing access level and filesets - def perform(work) - work.file_sets.each do |file| - attribute_map = work.permissions.map(&:to_hash) + # @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 + af_perform(work) + end + end + + private + + # Return array of hashes representing permissions without their :access_to objects + # @param permissions [Permission] + # @return [Array] + def permissions_map(permissions) + permissions.collect { |p| { agent: agent_object(p.agent), mode: p.mode } } + end + + # Returns a list of member file_sets for a work + # @param work [Resource] + # @return [Array] + def file_sets_for(work) + Hyrax.query_service.custom_queries.find_child_filesets(resource: work) + end + + # Converts string representation of Permission.agent to either User or Hyrax::Group + # @param agent [String] + # @return [User] or [Hyrax::Group] + def agent_object(agent) + return Hyrax::Group.new(agent.sub(Hyrax::Group.name_prefix, '')) if agent.starts_with?(Hyrax::Group.name_prefix) + User.find_by_user_key(agent) + end - # copy and removed access to the new access with the delete flag - file.permissions.map(&:to_hash).each do |perm| - unless attribute_map.include?(perm) - perm[:_destroy] = true - attribute_map << perm + # Perform the copy from the work to the contained filesets + # + # @param work containing access level and filesets + def af_perform(work) + attribute_map = work.permissions.map(&:to_hash) + work.file_sets.each do |file| + # copy and removed access to the new access with the delete flag + file.permissions.map(&:to_hash).each do |perm| + unless attribute_map.include?(perm) + perm[:_destroy] = true + attribute_map << perm + end end + # apply the new and deleted attributes + file.permissions_attributes = attribute_map + file.save! end + end - # apply the new and deleted attributes - file.permissions_attributes = attribute_map - file.save! + # Perform the copy from the work to the contained filesets + # + # @param work containing access level and filesets + def valkyrie_perform(work) + work_permissions = permissions_map(work.permission_manager.acl.permissions) + file_sets_for(work).each do |file| + file_acl = Hyrax::AccessControlList.new(resource: file) + file_permissions = permissions_map(file_acl.permissions) + # grant new work permissions to member file_sets + (work_permissions - file_permissions).each { |perm| file_acl.grant(perm[:mode]).to(perm[:agent]).save } + # remove permissions that are not on work from member file_sets + (file_permissions - work_permissions).each { |perm| file_acl.revoke(perm[:mode]).from(perm[:agent]).save } + file_acl.save + end end - end end diff --git a/spec/jobs/inherit_permissions_job_spec.rb b/spec/jobs/inherit_permissions_job_spec.rb index 90ce80b44f..d4fc4ba9ff 100644 --- a/spec/jobs/inherit_permissions_job_spec.rb +++ b/spec/jobs/inherit_permissions_job_spec.rb @@ -2,92 +2,208 @@ let(:user) { create(:user) } let(:work) { create(:work_with_one_file, user: user) } - before do - work.permissions.build(name: name, type: type, access: access) - work.save - end + context "when use_valkyrie is false" do + before do + work.permissions.build(name: name, type: type, access: access) + work.save + end + + context "when edit people change" do + let(:name) { 'abc@123.com' } + let(:type) { 'person' } + let(:access) { 'edit' } + + it 'copies permissions to its contained files' do + # 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) - context "when edit people change" do - let(:name) { 'abc@123.com' } - let(:type) { 'person' } - let(:access) { 'edit' } + file_sets = work.reload.file_sets + expect(file_sets.count).to eq 1 + expect(file_sets[0].edit_users).to match_array [user.to_s, "abc@123.com"] + end + + context "when people should be removed" do + before do + file_set = work.file_sets.first + file_set.permissions.build(name: "remove_me", type: type, access: access) + file_set.save + end + + it 'copies permissions to its contained files' do + # 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"] - it 'copies permissions to its contained files' do - # 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) - work.reload.file_sets.each do |file| - expect(file.edit_users).to match_array [user.to_s, "abc@123.com"] + file_sets = work.reload.file_sets + expect(file_sets.count).to eq 1 + expect(file_sets[0].edit_users).to match_array [user.to_s, "abc@123.com"] + end end end - context "when people should be removed" do - before do - file_set = work.file_sets.first - file_set.permissions.build(name: "remove_me", type: type, access: access) - file_set.save + context "when read people change" do + let(:name) { 'abc@123.com' } + let(:type) { 'person' } + let(:access) { 'read' } + + it 'copies permissions to its contained files' do + # 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) + + file_sets = work.reload.file_sets + expect(file_sets.count).to eq 1 + expect(file_sets[0].read_users).to match_array ["abc@123.com"] + expect(file_sets[0].edit_users).to match_array [user.to_s] end + end + + context "when read groups change" do + let(:name) { 'my_read_group' } + let(:type) { 'group' } + let(:access) { 'read' } + + it 'copies permissions to its contained files' do + # 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) + + file_sets = work.reload.file_sets + expect(file_sets.count).to eq 1 + expect(file_sets[0].read_groups).to match_array ["my_read_group"] + expect(file_sets[0].edit_users).to match_array [user.to_s] + end + end + + context "when edit groups change" do + let(:name) { 'my_edit_group' } + let(:type) { 'group' } + let(:access) { 'edit' } it 'copies permissions to its contained files' do # 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"] + expect(work.file_sets.first.read_groups).to eq [] - described_class.perform_now(work) - work.reload.file_sets.each do |file| - expect(file.edit_users).to match_array [user.to_s, "abc@123.com"] - end + described_class.perform_now(work, use_valkyrie: false) + + file_sets = work.reload.file_sets + expect(file_sets.count).to eq 1 + expect(file_sets[0].edit_groups).to match_array ["my_edit_group"] + expect(file_sets[0].edit_users).to match_array [user.to_s] end end end - context "when read people change" do - let(:name) { 'abc@123.com' } - let(:type) { 'person' } - let(:access) { 'read' } + 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) } + + before do + resource.member_ids = Array(file_set.id) + file_set.permission_manager.acl.grant(:edit).to(user).save + end + + context "when edit people change" do + it 'copies permissions to its contained files' do + resource.permission_manager.acl.grant(:edit).to(user2).save + expect(resource.edit_users).to match_array [user.to_s, user2.to_s] + + # files have the depositor as the edit user 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] + + described_class.perform_now(resource, use_valkyrie: true) - it 'copies permissions to its contained files' do - # files have the depositor as the edit user to begin with - expect(work.file_sets.first.read_users).to eq [] + # files have both edit users 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, user2.to_s] + end + end - described_class.perform_now(work) - work.reload.file_sets.each do |file| - expect(file.read_users).to match_array ["abc@123.com"] - expect(file.edit_users).to match_array [user.to_s] + 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] + + # 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) + + # 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 end - end - context "when read groups change" do - let(:name) { 'my_read_group' } - let(:type) { 'group' } - let(:access) { 'read' } + context "when read people change" do + it 'copies permissions to its contained files' do + resource.permission_manager.acl.grant(:read).to(user2).save + expect(resource.read_users).to match_array [user2.to_s] - it 'copies permissions to its contained files' do - # files have the depositor as the edit user to begin with - expect(work.file_sets.first.read_groups).to eq [] + # files have no read 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].read_users.to_a).to be_empty - described_class.perform_now(work) - work.reload.file_sets.each do |file| - expect(file.read_groups).to match_array ["my_read_group"] - expect(file.edit_users).to match_array [user.to_s] + described_class.perform_now(resource, use_valkyrie: true) + + # files have the specified read user + file_sets = Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) + expect(file_sets.count).to eq 1 + expect(file_sets[0].read_users.to_a).to match_array [user2.to_s] end end - end - context "when edit groups change" do - let(:name) { 'my_edit_group' } - let(:type) { 'group' } - let(:access) { 'edit' } + context "when read groups change" do + let(:group) { Hyrax::Group.new('my_read_group') } + + it 'copies permissions to its contained files' do + resource.permission_manager.acl.grant(:read).to(group).save + + # 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) + + # files have the specified read group + 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] + expect(file_sets[0].read_groups).to match_array ["my_read_group"] + end + end + + context "when edit groups change" do + let(:group) { Hyrax::Group.new('my_edit_group') } + + it 'copies permissions to its contained files' do + resource.permission_manager.acl.grant(:edit).to(group).save + + # work has the specified edit group to begin with + expect(resource.edit_groups).to match_array ["my_edit_group"] - it 'copies permissions to its contained files' do - # files have the depositor as the edit user to begin with - expect(work.file_sets.first.read_groups).to eq [] + described_class.perform_now(resource, use_valkyrie: true) - described_class.perform_now(work) - work.reload.file_sets.each do |file| - expect(file.edit_groups).to match_array ["my_edit_group"] - expect(file.edit_users).to match_array [user.to_s] + # files have the specified edit group + 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] + expect(file_sets[0].edit_groups).to match_array ["my_edit_group"] end end end