Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update NestedCollectionPersistenceService to use CollectionMemberService #5076

Merged
merged 5 commits into from
Aug 17, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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]
elrayle marked this conversation as resolved.
Show resolved Hide resolved
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 []
elrayle marked this conversation as resolved.
Show resolved Hide resolved
expect(Hyrax.custom_queries.find_child_collection_ids(resource: parent)).to eq []
elrayle marked this conversation as resolved.
Show resolved Hide resolved
end
end
end