diff --git a/app/controllers/hyrax/dashboard/collections_controller.rb b/app/controllers/hyrax/dashboard/collections_controller.rb index 3485cc3e4d..c592f58452 100644 --- a/app/controllers/hyrax/dashboard/collections_controller.rb +++ b/app/controllers/hyrax/dashboard/collections_controller.rb @@ -109,37 +109,21 @@ def create end def after_update - respond_to do |format| - format.html { redirect_to update_referer, notice: t('hyrax.dashboard.my.action.collection_update_success') } - format.json { render json: @collection, status: :updated, location: dashboard_collection_path(@collection) } - end + Deprecation.warn("Method `#after_update` will be removed in Hyrax 4.0.") + after_update_response # call private method for processing end def after_update_error - form - respond_to do |format| - format.html { render action: 'edit' } - format.json { render json: @collection.errors, status: :unprocessable_entity } - end + Deprecation.warn("Method `#after_update_error` will be removed in Hyrax 4.0.") + after_update_errors(@collection.errors) # call private method for processing end def update - unless params[:update_collection].nil? - process_banner_input - process_logo_input - end - - process_member_changes - - return valkyrie_update if @collection.is_a?(Valkyrie::Resource) - - @collection.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE unless @collection.discoverable? - # we don't have to reindex the full graph when updating collection - @collection.reindex_extent = Hyrax::Adapters::NestingIndexAdapter::LIMITED_REINDEX - if @collection.update(collection_params.except(:members)) - after_update + case @collection + when ActiveFedora::Base + update_active_fedora_collection else - after_update_error + update_valkyrie_collection end end @@ -224,12 +208,39 @@ def create_valkyrie_collection after_create_response end - def valkyrie_update - form.validate(collection_params) && - @collection = transactions['change_set.update_collection'] - .call(form) - .value_or { return after_update_error } - after_update + def update_active_fedora_collection + unless params[:update_collection].nil? + process_banner_input + process_logo_input + end + + process_member_changes + + @collection.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE unless @collection.discoverable? + # we don't have to reindex the full graph when updating collection + @collection.reindex_extent = Hyrax::Adapters::NestingIndexAdapter::LIMITED_REINDEX + if @collection.update(collection_params.except(:members)) + after_update_response + else + after_update_errors(@collection.errors) + end + end + + def update_valkyrie_collection + return after_update_errors(form_err_msg(form)) unless form.validate(collection_params) + + result = transactions['change_set.update_collection'] + .call(form) + @collection = result.value_or { return after_update_errors(result.failure.first) } + + process_member_changes + + unless params[:update_collection].nil? + process_banner_input + process_logo_input + end + + after_update_response end def valkyrie_destroy @@ -574,12 +585,55 @@ def after_create_response add_members_to_collection unless batch.empty? end - def after_create_errors(errors) + def after_create_errors_for_active_fedora(errors) + form + respond_to do |format| + format.html do + flash[:error] = errors.to_s + render action: 'new' + end + format.json { render json: @collection.errors, status: :unprocessable_entity } + end + end + + def after_create_errors(errors) # for valkyrie + return after_create_errors_for_active_fedora(errors) if @collection.is_a? ActiveFedora::Base respond_to do |wants| wants.html do flash[:error] = errors.to_s render 'new', status: :unprocessable_entity end + wants.json do + render_json_response(response_type: :unprocessable_entity, options: { errors: errors }) + end + end + end + + def after_update_response + respond_to do |format| + format.html { redirect_to update_referer, notice: t('hyrax.dashboard.my.action.collection_update_success') } + format.json { render json: @collection, status: :updated, location: dashboard_collection_path(@collection) } + end + end + + def after_update_errors_for_active_fedora(errors) + form + respond_to do |format| + format.html do + flash[:error] = errors.to_s + render action: 'edit' + end + format.json { render json: @collection.errors, status: :unprocessable_entity } + end + end + + def after_update_errors(errors) # for valkyrie + return after_update_errors_for_active_fedora(errors) if @collection.is_a? ActiveFedora::Base + respond_to do |wants| + wants.html do + flash[:error] = errors.to_s + render 'edit', status: :unprocessable_entity + end wants.json { render_json_response(response_type: :unprocessable_entity, options: { errors: errors }) } end end diff --git a/spec/controllers/hyrax/dashboard/collections_controller_spec.rb b/spec/controllers/hyrax/dashboard/collections_controller_spec.rb index 20190e0781..3d1bde0d9c 100644 --- a/spec/controllers/hyrax/dashboard/collections_controller_spec.rb +++ b/spec/controllers/hyrax/dashboard/collections_controller_spec.rb @@ -184,23 +184,29 @@ context "when create fails" do let(:collection) { Collection.new } + let(:error) { "Failed to save collection" } before do allow(controller).to receive(:authorize!) allow(Collection).to receive(:new).and_return(collection) allow(collection).to receive(:save).and_return(false) - - allow(Hyrax.persister) - .to receive(:save) - .with(any_args) - .and_raise(StandardError, 'Failed to save collection') + allow(collection).to receive(:errors).and_return(error) end it "renders the form again" do post :create, params: { collection: collection_attrs } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:ok) expect(response).to render_template(:new) + expect(flash[:error]).to eq error + end + + it "renders json" do + post :create, params: { collection: collection_attrs, format: :json } + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.content_type).to eq "application/json" + expect(response.body).to eq error end end end @@ -339,19 +345,18 @@ end context "when update fails" do - let(:collection) { FactoryBot.valkyrie_create(:hyrax_collection) } + let(:collection) { FactoryBot.create(:collection_lw) } let(:repository) { instance_double(Blacklight::Solr::Repository, search: result) } let(:result) { double(documents: [], total: 0) } + let(:error) { "Failed to save collection" } before do allow(controller).to receive(:authorize!) allow(Collection).to receive(:find).and_return(collection) allow(collection).to receive(:update).and_return(false) allow(controller).to receive(:repository).and_return(repository) - allow(Hyrax.persister) - .to receive(:save) - .with(any_args) - .and_raise(StandardError, 'Failed to save collection') + allow_any_instance_of(::Collection).to receive(:save).and_return(false) + allow(collection).to receive(:errors).and_return(error) end it "renders the form again" do @@ -359,9 +364,20 @@ id: collection, collection: collection_attrs } - expect(response).to be_successful + expect(response).to have_http_status(:ok) expect(response).to render_template(:edit) end + + it "renders json" do + put :update, params: { + id: collection, + collection: collection_attrs, + format: :json + } + expect(response).to have_http_status(:unprocessable_entity) + expect(response.content_type).to eq "application/json" + expect(response.body).to eq error + end end context "updating a collections branding metadata" do diff --git a/spec/controllers/hyrax/dashboard/collections_controller_with_resource_spec.rb b/spec/controllers/hyrax/dashboard/collections_controller_with_resource_spec.rb index b1cdc182b9..811a4c4c83 100644 --- a/spec/controllers/hyrax/dashboard/collections_controller_with_resource_spec.rb +++ b/spec/controllers/hyrax/dashboard/collections_controller_with_resource_spec.rb @@ -189,12 +189,26 @@ expect(flash[:error]).to match(/Failed to save collection/) expect(response).to render_template(:new) end + + it "renders json" do + post :create, params: { collection: collection_attrs, format: :json } + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.content_type).to eq "application/json" + + json_response = JSON.parse(response.body) + expect(json_response["code"]).to eq 422 + expect(json_response["message"]).to eq "Unprocessable Entity" + expect(json_response["description"]).to eq "The resource you attempted to modify cannot be modified according to your request." + expect(json_response["errors"]).to match(/Failed to save collection/) + end end context "in validations" do let(:form) { instance_double(Hyrax::Forms::PcdmCollectionForm, errors: errors) } let(:errors) { instance_double(Reform::Contract::CustomError, messages: messages) } - let(:messages) { { "error" => "Validation error" } } + let(:messages) { { "error" => errmsg } } + let(:errmsg) { "Validation error" } before do allow(controller).to receive(:authorize!) allow(Hyrax::Forms::ResourceForm).to receive(:for).with(collection).and_return(form) @@ -205,10 +219,24 @@ it "renders the form again" do post :create, params: { collection: collection_attrs } + expect(response).to have_http_status(:unprocessable_entity) - expect(flash[:error]).to eq "Validation error" + expect(flash[:error]).to eq errmsg expect(response).to render_template(:new) end + + it "renders json" do + post :create, params: { collection: collection_attrs, format: :json } + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.content_type).to eq "application/json" + + json_response = JSON.parse(response.body) + expect(json_response["code"]).to eq 422 + expect(json_response["message"]).to eq "Unprocessable Entity" + expect(json_response["description"]).to eq "The resource you attempted to modify cannot be modified according to your request." + expect(json_response["errors"]).to eq errmsg + end end end end @@ -360,20 +388,76 @@ context "when update fails" do before do + allow(controller).to receive(:authorize!) collection # ensure the collection is loaded before we stub the persister save - allow(Hyrax.persister) - .to receive(:save) - .with(any_args) - .and_raise(StandardError, 'Failed to save collection') end - it "renders the form again" do - put :update, params: { - id: collection, - collection: collection_attrs - } - expect(response).to be_successful - expect(response).to render_template(:edit) + context "in transaction processing" do + before do + allow(Hyrax.persister) + .to receive(:save) + .with(any_args) + .and_raise(StandardError, 'Failed to save collection') + end + + it "renders the form again" do + put :update, params: { id: collection, collection: collection_attrs } + + expect(response).to have_http_status(:unprocessable_entity) + expect(flash[:error]).to match(/Failed to save collection/) + expect(response).to render_template(:edit) + end + + it "renders json" do + put :update, params: { + id: collection, + collection: collection_attrs, + format: :json + } + expect(response).to have_http_status(:unprocessable_entity) + expect(response.content_type).to eq "application/json" + + json_response = JSON.parse(response.body) + expect(json_response["code"]).to eq 422 + expect(json_response["message"]).to eq "Unprocessable Entity" + expect(json_response["description"]).to eq "The resource you attempted to modify cannot be modified according to your request." + expect(json_response["errors"]).to match(/Failed to save collection/) + end + end + + context "in validations" do + let(:form) { instance_double(Hyrax::Forms::PcdmCollectionForm, errors: errors) } + let(:errors) { instance_double(Reform::Contract::CustomError, messages: messages) } + let(:messages) { { "error" => errmsg } } + let(:errmsg) { "Validation error" } + before do + allow(Hyrax::Forms::ResourceForm).to receive(:for).with(collection).and_return(form) + allow(form).to receive(:validate).with(any_args).and_return(false) + end + + it "renders the form again" do + put :update, params: { id: collection, collection: collection_attrs } + + expect(response).to have_http_status(:unprocessable_entity) + expect(flash[:error]).to eq errmsg + expect(response).to render_template(:edit) + end + + it "renders json" do + put :update, params: { + id: collection, + collection: collection_attrs, + format: :json + } + expect(response).to have_http_status(:unprocessable_entity) + expect(response.content_type).to eq "application/json" + + json_response = JSON.parse(response.body) + expect(json_response["code"]).to eq 422 + expect(json_response["message"]).to eq "Unprocessable Entity" + expect(json_response["description"]).to eq "The resource you attempted to modify cannot be modified according to your request." + expect(json_response["errors"]).to eq errmsg + end end end