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

Enable editing Valkyrie resources with children #5516

Merged
merged 2 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/helpers/hyrax/membership_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def work_members_json(resource)
Hyrax.custom_queries.find_child_works(resource: resource).map do |member|
{ id: member.id.to_s,
label: member.title.first,
path: url_for(member) }
path: main_app.url_for([member, { only_path: true }]) }
end.to_json
end
end
Expand Down
1 change: 1 addition & 0 deletions app/indexers/hyrax/file_set_indexer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def generate_solr_document # rubocop:disable Metrics/AbcSize, Metrics/MethodLeng
solr_doc['original_checksum_tesim'] = object.original_checksum
solr_doc['alpha_channels_ssi'] = object.alpha_channels
solr_doc['original_file_id_ssi'] = original_file_id
solr_doc['generic_type_si'] = 'FileSet'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generic_type field is used in PcdmMemberPresenterFactory. This is called from:

  • Hyrax::WorkShowPresenter #member_presenter_factory which only calls it for Valkyrie::Resource
  • Hyrax::WorkFormHelper #form_file_set_select_for(parent:) which is called from several views.
    • select representative_id in _form_representative.html.erb
    • select rendering_ids in _form_rendering.html.erb
    • select thumbnail_id in _form_thumbnail.html.erb

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with Main and with this PR's branch. The behavior for GenericWork is the same for both branches. GenericWork uses a different member_presenter_factory that does not depend on generic_type_si, so the addition of this index field has no impact on non-valkyrie works.

end
end

Expand Down
4 changes: 3 additions & 1 deletion app/presenters/hyrax/pcdm_member_presenter_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ def presenter_for(document:, ability:)
case document['has_model_ssim'].first
when Hyrax::FileSet.name
Hyrax::FileSetPresenter.new(document, ability)
when ::FileSet.name
Hyrax::FileSetPresenter.new(document, ability)
else
Hyrax::WorkShowPresenter.new(document, ability)
end
Expand All @@ -111,7 +113,7 @@ def query_docs(generic_type: nil, ids: object.member_ids)
query += "{!term f=generic_type_si}#{generic_type}" if generic_type

Hyrax::SolrService
.post(query, rows: 10_000)
.post(q: query, rows: 10_000)
.fetch('response')
.fetch('docs')
end
Expand Down
13 changes: 13 additions & 0 deletions spec/features/edit_work_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,17 @@
expect(page).to have_selector('#new_group_name_skel', text: 'librarians admin donor')
end
end

context 'with a parent Valkyrie resource' do
let(:monograph) { FactoryBot.valkyrie_create(:monograph, :with_member_works) }

before do
sign_in user_admin
end

it 'displays an edit page with a relationships tab' do
visit edit_hyrax_monograph_path(monograph)
expect(page).to have_link("Relationships")
end
end
end
209 changes: 151 additions & 58 deletions spec/presenters/hyrax/pcdm_member_presenter_factory_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

RSpec.describe Hyrax::PcdmMemberPresenterFactory, index_adapter: :solr_index, valkyrie_adapter: :test_adapter do
RSpec.describe Hyrax::PcdmMemberPresenterFactory do
subject(:factory) { described_class.new(solr_doc, ability) }
let(:ability) { :FAKE_ABILITY }
let(:ids) { [] }
Expand All @@ -9,8 +9,7 @@
RSpec::Matchers.define :be_presenter_for do |expected|
match do |actual|
actual.id == expected.id &&
(actual.solr_document['has_model_ssim'].first ==
expected.class.name)
actual.model_name.name == expected.model_name.name
end
end

Expand All @@ -33,92 +32,186 @@
end
end

describe '#file_set_presenters' do
it 'is empty' do
expect(factory.file_set_presenters.to_a).to be_empty
context 'with ActiveFedora index adapter' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to see this tested with both AF and Valkyrie.

describe '#file_set_presenters' do
it 'is empty' do
expect(factory.file_set_presenters.to_a).to be_empty
end

context 'with members' do
include_context 'with members'

it 'builds only file_set presenters' do
expect(factory.file_set_presenters)
.to contain_exactly(*file_sets.map { |fs| be_presenter_for(fs) })
end

it 'gives members in order' do
expect(factory.file_set_presenters.map(&:id)).to eq file_sets.map(&:id)
end
end
end

context 'with members' do
include_context 'with members'
describe '#member_presenters' do
it 'is empty' do
expect(factory.member_presenters.to_a).to be_empty
end

it 'builds only file_set presenters' do
expect(factory.file_set_presenters)
.to contain_exactly(*file_sets.map { |fs| be_presenter_for(fs) })
it 'accepts bespoke member ids' do
fs = FactoryBot.valkyrie_create(:hyrax_file_set)
Hyrax.index_adapter.save(resource: fs)

expect(factory.member_presenters([fs.id]).to_a)
.to contain_exactly(be_presenter_for(fs))
end

it 'gives members in order' do
expect(factory.file_set_presenters.map(&:id)).to eq file_sets.map(&:id)
it 'raises an error if given an unindexed id' do
expect { factory.member_presenters(['FAKE_ID']).to_a }
.to raise_error ArgumentError
end
end
end

describe '#member_presenters' do
it 'is empty' do
expect(factory.member_presenters.to_a).to be_empty
end
context 'with members' do
include_context 'with members'

it 'accepts bespoke member ids' do
fs = FactoryBot.valkyrie_create(:hyrax_file_set)
Hyrax.index_adapter.save(resource: fs)
it 'builds all member presenters' do
members = file_sets + works

expect(factory.member_presenters([fs.id]).to_a)
.to contain_exactly(be_presenter_for(fs))
end
expect(factory.member_presenters)
.to contain_exactly(*members.map { |fs| be_presenter_for(fs) })
end

it 'raises an error if given an unindexed id' do
expect { factory.member_presenters(['FAKE_ID']).to_a }
.to raise_error ArgumentError
it 'builds member presenters with appropriate classes' do
expect(factory.member_presenters)
.to contain_exactly(an_instance_of(Hyrax::WorkShowPresenter),
an_instance_of(Hyrax::WorkShowPresenter),
an_instance_of(Hyrax::FileSetPresenter),
an_instance_of(Hyrax::FileSetPresenter))
end

it 'gives members in order' do
expect(factory.member_presenters.map(&:id)).to eq ids
end
end
end

context 'with members' do
include_context 'with members'
describe '#ordered_ids' do
its(:ordered_ids) { is_expected.to eq ids }

it 'builds all member presenters' do
members = file_sets + works
context 'with members' do
include_context 'with members'

expect(factory.member_presenters)
.to contain_exactly(*members.map { |fs| be_presenter_for(fs) })
its(:ordered_ids) { is_expected.to eq ids }
end
end

it 'builds member presenters with appropriate classes' do
expect(factory.member_presenters)
.to contain_exactly(an_instance_of(Hyrax::WorkShowPresenter),
an_instance_of(Hyrax::WorkShowPresenter),
an_instance_of(Hyrax::FileSetPresenter),
an_instance_of(Hyrax::FileSetPresenter))
describe '#work_presenters' do
it 'is empty' do
expect(factory.work_presenters.to_a).to be_empty
end

it 'gives members in order' do
expect(factory.member_presenters.map(&:id)).to eq ids
context 'with members' do
include_context 'with members'

it 'builds only work presenters' do
expect(factory.work_presenters)
.to contain_exactly(*works.map { |fs| be_presenter_for(fs) })
end

it 'gives members in order' do
expect(factory.work_presenters.map(&:id)).to eq works.map(&:id)
end
end
end
end

describe '#ordered_ids' do
its(:ordered_ids) { is_expected.to eq ids }
context 'with Valkyrie index adapter', index_adapter: :solr_index, valkyrie_adapter: :test_adapter do
describe '#file_set_presenters' do
it 'is empty' do
expect(factory.file_set_presenters.to_a).to be_empty
end

context 'with members' do
include_context 'with members'
context 'with members' do
include_context 'with members'

its(:ordered_ids) { is_expected.to eq ids }
it 'builds only file_set presenters' do
expect(factory.file_set_presenters)
.to contain_exactly(*file_sets.map { |fs| be_presenter_for(fs) })
end

it 'gives members in order' do
expect(factory.file_set_presenters.map(&:id)).to eq file_sets.map(&:id)
end
end
end
end

describe '#work_presenters' do
it 'is empty' do
expect(factory.work_presenters.to_a).to be_empty
describe '#member_presenters' do
it 'is empty' do
expect(factory.member_presenters.to_a).to be_empty
end

it 'accepts bespoke member ids' do
fs = FactoryBot.valkyrie_create(:hyrax_file_set)
Hyrax.index_adapter.save(resource: fs)

expect(factory.member_presenters([fs.id]).to_a)
.to contain_exactly(be_presenter_for(fs))
end

it 'raises an error if given an unindexed id' do
expect { factory.member_presenters(['FAKE_ID']).to_a }
.to raise_error ArgumentError
end

context 'with members' do
include_context 'with members'

it 'builds all member presenters' do
members = file_sets + works

expect(factory.member_presenters)
.to contain_exactly(*members.map { |fs| be_presenter_for(fs) })
end

it 'builds member presenters with appropriate classes' do
expect(factory.member_presenters)
.to contain_exactly(an_instance_of(Hyrax::WorkShowPresenter),
an_instance_of(Hyrax::WorkShowPresenter),
an_instance_of(Hyrax::FileSetPresenter),
an_instance_of(Hyrax::FileSetPresenter))
end

it 'gives members in order' do
expect(factory.member_presenters.map(&:id)).to eq ids
end
end
end

context 'with members' do
include_context 'with members'
describe '#ordered_ids' do
its(:ordered_ids) { is_expected.to eq ids }

context 'with members' do
include_context 'with members'

it 'builds only work presenters' do
expect(factory.work_presenters)
.to contain_exactly(*works.map { |fs| be_presenter_for(fs) })
its(:ordered_ids) { is_expected.to eq ids }
end
end

describe '#work_presenters' do
it 'is empty' do
expect(factory.work_presenters.to_a).to be_empty
end

context 'with members' do
include_context 'with members'

it 'builds only work presenters' do
expect(factory.work_presenters)
.to contain_exactly(*works.map { |fs| be_presenter_for(fs) })
end

it 'gives members in order' do
expect(factory.work_presenters.map(&:id)).to eq works.map(&:id)
it 'gives members in order' do
expect(factory.work_presenters.map(&:id)).to eq works.map(&:id)
end
end
end
end
Expand Down