From 70d3c1aed79b4e37137f1cb7434abb6194cf6014 Mon Sep 17 00:00:00 2001 From: Daniel Pierce Date: Thu, 19 Oct 2023 13:32:22 -0400 Subject: [PATCH 1/4] Fix valkyrie resource json response --- .../concerns/hyrax/works_controller_behavior.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/hyrax/works_controller_behavior.rb b/app/controllers/concerns/hyrax/works_controller_behavior.rb index 3ded64a870..35ddd02810 100644 --- a/app/controllers/concerns/hyrax/works_controller_behavior.rb +++ b/app/controllers/concerns/hyrax/works_controller_behavior.rb @@ -79,7 +79,7 @@ def show wants.html { presenter && parent_presenter } wants.json do # load @curation_concern manually because it's skipped for html - @curation_concern = Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: params[:id]) + @curation_concern = load_curation_concern curation_concern # This is here for authorization checks (we could add authorize! but let's use the same method for CanCanCan) render :show, status: :ok end @@ -145,6 +145,14 @@ def manifest private + def load_curation_concern + if Hyrax.config.disable_wings + Hyrax.query_service.find_by(id: params[:id]) + else + Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: params[:id]) + end + end + def iiif_manifest_builder self.class.iiif_manifest_builder || (Flipflop.cache_work_iiif_manifest? ? Hyrax::CachingIiifManifestBuilder.new : Hyrax::ManifestBuilderService.new) From 13c8ac9bcfd8a513d05aa7f89da655dcc90980f9 Mon Sep 17 00:00:00 2001 From: Daniel Pierce Date: Thu, 19 Oct 2023 13:35:41 -0400 Subject: [PATCH 2/4] Move alternate response formats to individual methods, mark graph dependent responses as not implemented in valkyrie. --- .../hyrax/works_controller_behavior.rb | 40 +++++++++++++++++-- .../hyrax/works_controller_behavior_spec.rb | 6 +-- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/app/controllers/concerns/hyrax/works_controller_behavior.rb b/app/controllers/concerns/hyrax/works_controller_behavior.rb index 35ddd02810..13776415f0 100644 --- a/app/controllers/concerns/hyrax/works_controller_behavior.rb +++ b/app/controllers/concerns/hyrax/works_controller_behavior.rb @@ -84,9 +84,6 @@ def show render :show, status: :ok end additional_response_formats(wants) - wants.ttl { render body: presenter.export_as_ttl, mime_type: Mime[:ttl] } - wants.jsonld { render body: presenter.export_as_jsonld, mime_type: Mime[:jsonld] } - wants.nt { render body: presenter.export_as_nt, mime_type: Mime[:nt] } end end # rubocop:enable Metrics/AbcSize @@ -434,6 +431,13 @@ def after_destroy_response(title) end def additional_response_formats(format) + respond_to_endnote(format) + respond_to_ttl(format) + respond_to_jsonld(format) + respond_to_nt(format) + end + + def respond_to_endnote(format) format.endnote do send_data(presenter.solr_document.export_as_endnote, type: "application/x-endnote-refer", @@ -441,6 +445,36 @@ def additional_response_formats(format) end end + def respond_to_ttl(format) + format.ttl do + if presenter.valkyrie_presenter? + render plain: "Error: Not Implemented", status: :not_implemented + else + render body: presenter.export_as_ttl, mime_type: Mime[:ttl] + end + end + end + + def respond_to_jsonld(format) + format.jsonld do + if presenter.valkyrie_presenter? + render plain: "Error: Not Implemented", status: :not_implemented + else + render body: presenter.export_as_jsonld, mime_type: Mime[:jsonld] + end + end + end + + def respond_to_nt(format) + format.nt do + if presenter.valkyrie_presenter? + render plain: "Error: Not Implemented", status: :not_implemented + else + render body: presenter.export_as_nt, mime_type: Mime[:nt] + end + end + end + def save_permissions @saved_permissions = case curation_concern diff --git a/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb b/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb index 37f21b4b75..0634dfd2ae 100644 --- a/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb +++ b/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb @@ -478,21 +478,21 @@ expect(response.status).to eq 200 end - it 'resolves ntriples' do + it 'resolves ntriples', :active_fedora do get :show, params: { id: work.id }, format: :nt expect(RDF::Reader.for(:ntriples).new(response.body).objects) .to include(RDF::Literal(title.first)) end - it 'resolves turtle' do + it 'resolves turtle', :active_fedora do get :show, params: { id: work.id }, format: :ttl expect(RDF::Reader.for(:ttl).new(response.body).objects) .to include(RDF::Literal(title.first)) end - it 'resolves jsonld' do + it 'resolves jsonld', :active_fedora do get :show, params: { id: work.id }, format: :jsonld expect(RDF::Reader.for(:jsonld).new(response.body).objects) From ed57c87979f79583dcfa428157ca57b232ddd4fb Mon Sep 17 00:00:00 2001 From: Daniel Pierce Date: Thu, 19 Oct 2023 13:39:00 -0400 Subject: [PATCH 3/4] Fix work_controller_behavior_spec for koppie --- .../hyrax/works_controller_behavior_spec.rb | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb b/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb index 0634dfd2ae..13fcf7ee58 100644 --- a/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb +++ b/spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb @@ -8,10 +8,15 @@ let(:work) { FactoryBot.valkyrie_create(:hyrax_work, alternate_ids: [id], title: title) } let(:id) { '123' } + let(:work_route_path) do + "#{work.model_name.singular_route_key}_path" + end + let(:main_app_routes) do ActionDispatch::Routing::RouteSet.new.tap do |r| r.draw do # draw minimal routes for this controller mixin mount Hyrax::Engine, at: '/' + namespaced_resources 'hyrax/test/simple_work', except: [:index] namespaced_resources 'hyrax/test/simple_work_legacy', except: [:index] devise_for :users resources :solr_documents, only: [:show], path: '/catalog', controller: 'catalog' @@ -74,7 +79,7 @@ get :create, params: { test_simple_work: { title: 'comet in moominland' } } expect(response) - .to redirect_to paths.hyrax_test_simple_work_legacy_path(id: assigns(:curation_concern).id, locale: :en) + .to redirect_to paths.send(work_route_path, id: assigns(:curation_concern).id, locale: :en) end it 'sets current user as depositor' do @@ -221,9 +226,8 @@ get :create, params: params expect(flash[:notice]).to be_html_safe - expect(flash[:notice]).to eq "Your files are being processed by Hyrax in the background. " \ - "The metadata and access controls you specified are being applied. " \ - "You may need to refresh this page to see these updates." + expect(flash[:notice]).to eq I18n.t("hyrax.works.create.after_create_html", + application_name: I18n.t('hyrax.product_name', default: 'Hyrax')) expect(assigns(:curation_concern)).to have_file_set_members(be_persisted, be_persisted) end @@ -449,7 +453,7 @@ expect(assigns[:form]) .to have_attributes(depositor: user.user_key, - admin_set_id: AdminSet::DEFAULT_ID) + admin_set_id: Hyrax::AdminSetCreateService.find_or_create_default_admin_set.id.to_s) end it 'renders form' do @@ -540,7 +544,7 @@ describe '#update' do it 'redirects to new user login' do - patch :update, params: { id: id } + patch :update, params: { id: work.id } expect(response).to redirect_to paths.new_user_session_path(locale: :en) end @@ -551,16 +555,16 @@ before { Hyrax::AdminSetCreateService.find_or_create_default_admin_set } it 'redirects to updated work' do - patch :update, params: { id: id, test_simple_work: { title: 'new title' } } + patch :update, params: { id: work.id, test_simple_work: { title: 'new title' } } expect(response) - .to redirect_to paths.hyrax_test_simple_work_legacy_path(id: id, locale: :en) + .to redirect_to paths.send(work_route_path, id: work.id, locale: :en) end it 'updates the work metadata' do - patch :update, params: { id: id, test_simple_work: { title: 'new title' } } + patch :update, params: { id: work.id, test_simple_work: { title: 'new title' } } - expect(Hyrax.query_service.find_by(id: id)) + expect(Hyrax.query_service.find_by(id: work.id)) .to have_attributes title: contain_exactly('new title') end @@ -568,7 +572,7 @@ let(:uploads) { FactoryBot.create_list(:uploaded_file, 2, user: user) } it 'attaches the files' do - params = { id: id, test_simple_work: { title: 'comet in moominland' }, + params = { id: work.id, test_simple_work: { title: 'comet in moominland' }, uploaded_files: uploads.map(&:id) } get :update, params: params @@ -576,7 +580,7 @@ end it 'sets the file visibility' do - params = { id: id, + params = { id: work.id, test_simple_work: { title: 'comet in moominland', file_set: [{ uploaded_file_id: uploads.first.id, visibility: 'open' }, { uploaded_file_id: uploads.second.id, visibility: 'open' }] }, @@ -591,13 +595,13 @@ let(:update_params) { { title: 'new title', visibility: 'open' } } it 'can make work public' do - patch :update, params: { id: id, test_simple_work: update_params } + patch :update, params: { id: work.id, test_simple_work: update_params } expect(Hyrax::VisibilityReader.new(resource: assigns(:curation_concern)).read).to eq 'open' end it 'saves the visibility' do - patch :update, params: { id: id, test_simple_work: update_params } + patch :update, params: { id: work.id, test_simple_work: update_params } expect(Hyrax::AccessControlList(assigns[:curation_concern]).permissions) .to include(have_attributes(mode: :read, agent: 'group/public')) @@ -608,7 +612,7 @@ let(:update_params) { { title: 'comet in moominland', permissions_attributes: { "0" => { type: 'person', access: 'read', name: 'foo@bar.com' } } } } it 'saves the visibility' do - post :update, params: { id: id, test_simple_work: update_params } + post :update, params: { id: work.id, test_simple_work: update_params } expect(Hyrax::AccessControlList(assigns[:curation_concern]).permissions) .to include(have_attributes(mode: :read, agent: 'foo@bar.com')) @@ -627,7 +631,7 @@ end it 'stays on the edit form and flashes an error message' do - patch :update, params: { id: id, test_simple_work: update_params } + patch :update, params: { id: work.id, test_simple_work: update_params } expect(flash[:error]).to eq error_msg expect(response).to render_template(:edit) @@ -645,7 +649,7 @@ end it 'stays on the edit form and flashes an error message' do - patch :update, params: { id: id, test_simple_work: update_params } + patch :update, params: { id: work.id, test_simple_work: update_params } expect(flash[:error]).to eq error_msg expect(response).to render_template(:edit) From e6f621e2fa387a7e84c9f01ea7f01bda87a1463e Mon Sep 17 00:00:00 2001 From: Daniel Pierce Date: Thu, 19 Oct 2023 14:09:00 -0400 Subject: [PATCH 4/4] cleanup presenter double --- .../hyrax/generic_works_controller_spec.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/spec/controllers/hyrax/generic_works_controller_spec.rb b/spec/controllers/hyrax/generic_works_controller_spec.rb index ba1061754d..15820ecf6e 100644 --- a/spec/controllers/hyrax/generic_works_controller_spec.rb +++ b/spec/controllers/hyrax/generic_works_controller_spec.rb @@ -173,13 +173,16 @@ end context "ttl" do - let(:presenter) { double } + let(:presenter) do + double("Presenter Double", + export_as_ttl: 'ttl graph', + 'editor?': true, + to_model: stub_model(GenericWork), + 'valkyrie_presenter?': false) + end before do allow(controller).to receive(:presenter).and_return(presenter) - allow(presenter).to receive(:export_as_ttl).and_return("ttl graph") - allow(presenter).to receive(:editor?).and_return(true) - allow(presenter).to receive(:to_model).and_return(stub_model(GenericWork)) end it 'renders a turtle file' do