diff --git a/.rubocop_fixme.yml b/.rubocop_fixme.yml index e86d00e2bd..fb52bcc4db 100644 --- a/.rubocop_fixme.yml +++ b/.rubocop_fixme.yml @@ -29,6 +29,8 @@ Metrics/ModuleLength: - 'app/models/concerns/hyrax/ability.rb' - 'app/services/hyrax/workflow/permission_query.rb' - 'spec/services/hyrax/workflow/permission_query_spec.rb' + # TODO: extract CollectionAccessControls or something, so we don't have to skip this check? + - 'app/models/concerns/hyrax/collection_behavior.rb' Metrics/MethodLength: Exclude: diff --git a/.solr_wrapper b/.solr_wrapper index 1935e70ac2..a425df6d1d 100644 --- a/.solr_wrapper +++ b/.solr_wrapper @@ -1,6 +1,5 @@ # Place any default configuration for solr_wrapper here # port: 8983 -version: 6.6.1 collection: persist: true dir: solr/config/ diff --git a/app/models/concerns/hyrax/collection_behavior.rb b/app/models/concerns/hyrax/collection_behavior.rb index bd04794c3b..8537542168 100644 --- a/app/models/concerns/hyrax/collection_behavior.rb +++ b/app/models/concerns/hyrax/collection_behavior.rb @@ -126,10 +126,18 @@ def permission_template # Calculate and update who should have edit access based on who # has "manage" access in the PermissionTemplateAccess def update_access_controls! - update!(edit_users: permission_template.agent_ids_for(access: 'manage', agent_type: 'user'), - edit_groups: permission_template.agent_ids_for(access: 'manage', agent_type: 'group'), - read_users: permission_template.agent_ids_for(access: 'view', agent_type: 'user'), - read_groups: (permission_template.agent_ids_for(access: 'view', agent_type: 'group') + visibility_group).uniq) + edit_users = permission_template.agent_ids_for(access: 'manage', agent_type: 'user') + edit_groups = permission_template.agent_ids_for(access: 'manage', agent_type: 'group') + read_users = permission_template.agent_ids_for(access: 'view', agent_type: 'user') + read_groups = (permission_template.agent_ids_for(access: 'view', agent_type: 'group') + visibility_group).uniq + update!(edit_users: edit_users, + edit_groups: edit_groups, + read_users: read_users, + read_groups: read_groups) + # added because the collection indexing happens before Hydra::AccessControls::Permission is + # saved, and the after_update_index callback in the nested_relationship_reindexer removes the + # permissions from the Solr document. + update_index end private diff --git a/app/services/hyrax/adapters/nesting_index_adapter.rb b/app/services/hyrax/adapters/nesting_index_adapter.rb index 85abae6fff..997782fa13 100644 --- a/app/services/hyrax/adapters/nesting_index_adapter.rb +++ b/app/services/hyrax/adapters/nesting_index_adapter.rb @@ -19,7 +19,10 @@ def self.find_preservation_document_by(id:) def self.find_preservation_parent_ids_for(id:) # Not everything is guaranteed to have library_collection_ids # If it doesn't have it, what do we do? - fedora_object = ActiveFedora::Base.find(id) + fedora_object = ActiveFedora::Base.uncached do + fedora_object = ActiveFedora::Base.find(id) + end + if fedora_object.respond_to?(:member_of_collection_ids) fedora_object.member_of_collection_ids else @@ -65,7 +68,9 @@ def self.each_perservation_document_id_and_parent_ids(&block) # @param pathnames [Array] # @return Hash - the attributes written to the indexing layer def self.write_document_attributes_to_index_layer(id:, parent_ids:, ancestors:, pathnames:) - solr_doc = ActiveFedora::Base.find(id).to_solr # What is the current state of the solr document + solr_doc = ActiveFedora::Base.uncached do + ActiveFedora::Base.find(id).to_solr # What is the current state of the solr document + end # Now add the details from the nesting indexor to the document solr_doc[solr_field_name_for_storing_ancestors] = ancestors diff --git a/spec/factories/collections_factory.rb b/spec/factories/collections_factory.rb index c8970286ee..ecb5c52812 100644 --- a/spec/factories/collections_factory.rb +++ b/spec/factories/collections_factory.rb @@ -18,11 +18,25 @@ end after(:create) do |collection, evaluator| - if evaluator.with_permission_template + # create the permission template if it was requested, OR if + # nested reindexing is included (so we can apply the user's + # permissions). + if evaluator.with_permission_template || RSpec.current_example.metadata[:with_nested_reindexing] attributes = { source_id: collection.id, source_type: 'collection' } attributes = evaluator.with_permission_template.merge(attributes) if evaluator.with_permission_template.respond_to?(:merge) create(:permission_template, attributes) unless Hyrax::PermissionTemplate.find_by(source_id: collection.id) end + # Nested indexing requires that the user's permissions be saved + # on the Fedora object... if simply in local memory, they are + # lost when the adapter pulls the object from Fedora to reindex. + if RSpec.current_example.metadata[:with_nested_reindexing] + create(:permission_template_access, + :manage, + permission_template: collection.permission_template, + agent_type: 'user', + agent_id: evaluator.user.user_key) + collection.update_access_controls! + end end factory :public_collection, traits: [:public] diff --git a/spec/factories/generic_works.rb b/spec/factories/generic_works.rb index 37b60c09c7..41210136d8 100644 --- a/spec/factories/generic_works.rb +++ b/spec/factories/generic_works.rb @@ -18,6 +18,10 @@ end end + after(:create) do |work, _evaluator| + work.save! if work.member_of_collections.present? + end + title ["Test title"] visibility Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE diff --git a/spec/models/collection_spec.rb b/spec/models/collection_spec.rb index 392a986f2b..d39081bcf0 100644 --- a/spec/models/collection_spec.rb +++ b/spec/models/collection_spec.rb @@ -243,13 +243,27 @@ class Member < ActiveFedora::Base end end - describe 'factories' do - it 'will create a permission_template when one is requested' do - expect { create(:collection, with_permission_template: true) }.to change { Hyrax::PermissionTemplate.count }.by(1) + context 'collection factory' do + describe 'when creating permission templates' do + it 'will create a permission_template when one is requested' do + expect { create(:collection, with_permission_template: true) }.to change { Hyrax::PermissionTemplate.count }.by(1) + end + + it 'will not create a permission_template by default' do + expect { create(:collection) }.not_to change { Hyrax::PermissionTemplate.count } + end end - it 'will not create a permission_template by default' do - expect { create(:collection) }.not_to change { Hyrax::PermissionTemplate.count } + describe 'when including nesting indexing', with_nested_indexing: true do + # Nested indexing requires that the user's permissions be saved + # on the Fedora object... if simply in local memory, they are + # lost when the adapter pulls the object from Fedora to reindex. + let(:user) { create(:user) } + let(:collection) { create(:collection, user: user) } + + it 'will authorize the creating user' do + expect(user.can?(:edit, collection)).to be true + end end end