Skip to content

Commit

Permalink
update NestedCollectionPersistenceService to use CollectionMemberService
Browse files Browse the repository at this point in the history
Since CollectionMemberService publishes `'object.metadata.updated'` and message published wants to attribute the action to a user, the persistence service methods added a `user:` parameter which defaults to nil for backwards compatibility.  Callers of these methods within Hyrax code were updated to pass a user or updated to call CollectionmemberService directly to avoid having to create an instance of the parent collection.
  • Loading branch information
elrayle committed Aug 17, 2021
1 parent 963be88 commit 8e296e9
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 27 deletions.
6 changes: 4 additions & 2 deletions app/controllers/hyrax/dashboard/collections_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,10 @@ def default_collection_type
end

def link_parent_collection(parent_id)
parent = Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: parent_id, use_valkyrie: false)
Hyrax::Collections::NestedCollectionPersistenceService.persist_nested_collection_for(parent: parent, child: @collection)
child = collection.respond_to?(:valkyrie_resource) ? collection.valkyrie_resource : collection
Hyrax::Collections::CollectionMemberService.add_member(collection_id: parent_id,
new_member: child,
user: current_user)
end

def uploaded_files(uploaded_file_ids)
Expand Down
4 changes: 2 additions & 2 deletions app/forms/hyrax/forms/dashboard/nest_collection_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def initialize(parent: nil, child: nil, context:, query_service: default_query_s

def save
return false unless valid?
persistence_service.persist_nested_collection_for(parent: parent, child: child)
persistence_service.persist_nested_collection_for(parent: parent, child: child, user: context.current_user)
end

##
Expand Down Expand Up @@ -83,7 +83,7 @@ def validate_add

def remove
if context.can? :edit, parent
persistence_service.remove_nested_relationship_for(parent: parent, child: child)
persistence_service.remove_nested_relationship_for(parent: parent, child: child, user: context.current_user)
else
errors.add(:parent, :cannot_remove_relationship)
false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,23 @@ module NestedCollectionPersistenceService
# Responsible for persisting the relationship between the parent and the child.
# @see Hyrax::Collections::NestedCollectionQueryService
#
# @param parent [::Collection]
# @param child [::Collection]
# @param parent [Hyrax::PcdmCollection | ::Collection]
# @param child [Hyrax::PcdmCollection | ::Collection]
# @param user [::User] current logged in user (defaults=nil for backward compatibility)
# @note There is odd permission arrangement based on the NestedCollectionQueryService:
# You can nest the child within a parent if you can edit the parent and read the child.
# See https://wiki.lyrasis.org/display/samvera/Samvera+Tech+Call+2017-08-23 for tech discussion.
# @note Adding the member_of_collections method doesn't trigger reindexing of the child so we have to do it manually.
# However it save and reindexes the parent unnecessarily!!
def self.persist_nested_collection_for(parent:, child:)
parent.reindex_extent = Hyrax::Adapters::NestingIndexAdapter::LIMITED_REINDEX
child.member_of_collections.push(parent)
child.update_nested_collection_relationship_indices
def self.persist_nested_collection_for(parent:, child:, user: nil)
child_resource = child.respond_to?(:valkyrie_resource) ? child.valkyrie_resource : child
Hyrax::Collections::CollectionMemberService.add_member(collection_id: parent.id, new_member: child_resource, user: user)
end

# @note Removing the member_of_collections method doesn't trigger reindexing of the child so we have to do it manually.
# However it doesn't save and reindex the parent, as it does when a parent is added!!
def self.remove_nested_relationship_for(parent:, child:)
child.member_of_collections.delete(parent)
child.update_nested_collection_relationship_indices
# @param parent [Hyrax::PcdmCollection | ::Collection]
# @param child [Hyrax::PcdmCollection | ::Collection]
# @param user [::User] current logged in user (defaults=nil for backward compatibility)
def self.remove_nested_relationship_for(parent:, child:, user: nil)
child_resource = child.respond_to?(:valkyrie_resource) ? child.valkyrie_resource : child
Hyrax::Collections::CollectionMemberService.remove_member(collection_id: parent.id, member: child_resource, user: user)
true
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
collection: collection_attrs, parent_id: parent_collection.id
}
end.to change { Collection.count }.by(1)
expect(assigns[:collection].member_of_collections).to eq [parent_collection]
expect(assigns[:collection].reload.member_of_collections).to eq [parent_collection]
end
end

Expand Down
10 changes: 7 additions & 3 deletions spec/forms/hyrax/forms/dashboard/nest_collection_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
end

let(:child) { FactoryBot.create(:collection) }
let(:context) { double('Context') }
let(:context) { double('Context', current_user: user) }
let(:nesting_depth_result) { true }
let(:parent) { FactoryBot.create(:collection) }
let(:user) { create(:user) }

let(:persistence_service) do
double(Hyrax::Collections::NestedCollectionPersistenceService,
Expand Down Expand Up @@ -91,7 +92,7 @@
it "returns the result of the given persistence_service's call to persist_nested_collection_for" do
expect(persistence_service)
.to receive(:persist_nested_collection_for)
.with(parent: parent, child: child)
.with(parent: parent, child: child, user: user)
.and_return(:persisted)

expect(form.save).to eq :persisted
Expand Down Expand Up @@ -173,7 +174,10 @@
end

it "returns the result of the given persistence_service's call to remove_nested_relationship_for" do
expect(persistence_service).to receive(:remove_nested_relationship_for).with(parent: parent, child: child).and_return(:persisted)
expect(persistence_service)
.to receive(:remove_nested_relationship_for)
.with(parent: parent, child: child, user: user)
.and_return(:persisted)

expect(form.remove).to eq :persisted
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
# frozen_string_literal: true
RSpec.describe Hyrax::Collections::NestedCollectionPersistenceService, with_nested_reindexing: true do
let(:parent) { build(:collection_lw) }
let(:child) { create(:collection) }
let(:parent) { FactoryBot.valkyrie_create(:hyrax_collection) }
let(:child) { FactoryBot.valkyrie_create(:hyrax_collection) }

describe '.persist_nested_collection_for' do
subject { described_class.persist_nested_collection_for(parent: parent, child: child) }

it 'creates the relationship between parent and child' do
subject
expect(parent.member_objects).to eq([child])
expect(child.member_of_collections).to eq([parent])
expect(Hyrax.custom_queries.find_parent_collection_ids(resource: child)).to eq [parent.id]
expect(Hyrax.custom_queries.find_child_collection_ids(resource: parent)).to eq [child.id]
end
end

Expand All @@ -22,8 +22,8 @@

it 'removes the relationship between parent and child' do
subject
expect(parent.member_objects).to eq([])
expect(child.member_of_collections).to eq([])
expect(Hyrax.custom_queries.find_parent_collection_ids(resource: child)).to eq []
expect(Hyrax.custom_queries.find_child_collection_ids(resource: parent)).to eq []
end
end
end

0 comments on commit 8e296e9

Please sign in to comment.