From 2bb392df96ec31d0d19c00b8e789f75923922758 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Thu, 10 Jan 2019 14:03:12 -0600 Subject: [PATCH] Return 422 rather than 500 when failing to open a version --- .rubocop_todo.yml | 28 +++++------ app/controllers/versions_controller.rb | 19 +++++++ spec/controllers/versions_controller_spec.rb | 53 +++++++++++--------- 3 files changed, 61 insertions(+), 39 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d055529066..4f0fafb6fa 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2018-11-30 16:59:33 -0600 using RuboCop version 0.57.2. +# on 2019-01-10 14:06:50 -0600 using RuboCop version 0.57.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -88,14 +88,14 @@ Lint/UriEscapeUnescape: - 'app/controllers/sdr_controller.rb' - 'spec/controllers/sdr_controller_spec.rb' -# Offense count: 11 +# Offense count: 10 Metrics/AbcSize: - Max: 32 + Max: 30 # Offense count: 1 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 128 + Max: 121 # Offense count: 6 # Configuration parameters: CountComments. @@ -180,13 +180,12 @@ RSpec/EmptyLineAfterSubject: Exclude: - 'spec/models/symphony_reader_spec.rb' -# Offense count: 27 +# Offense count: 26 # Configuration parameters: Max. RSpec/ExampleLength: Exclude: - 'spec/controllers/objects_controller_spec.rb' - 'spec/controllers/sdr_controller_spec.rb' - - 'spec/controllers/versions_controller_spec.rb' - 'spec/controllers/workflows_controller_spec.rb' - 'spec/dor/goobi_spec.rb' - 'spec/dor/service_item_spec.rb' @@ -198,16 +197,15 @@ RSpec/ExpectInHook: Exclude: - 'spec/dor/update_marc_record_service_spec.rb' -# Offense count: 92 +# Offense count: 77 # Configuration parameters: AssignmentOnly. RSpec/InstanceVariable: Exclude: - - 'spec/dor/goobi_spec.rb' - 'spec/dor/registration_response_spec.rb' - 'spec/dor/service_item_spec.rb' - 'spec/dor/update_marc_record_service_spec.rb' -# Offense count: 26 +# Offense count: 21 # Configuration parameters: EnforcedStyle. # SupportedStyles: have_received, receive RSpec/MessageSpies: @@ -218,10 +216,10 @@ RSpec/MessageSpies: - 'spec/dor/goobi_spec.rb' - 'spec/dor/update_marc_record_service_spec.rb' -# Offense count: 49 +# Offense count: 51 # Configuration parameters: AggregateFailuresByDefault. RSpec/MultipleExpectations: - Max: 6 + Max: 5 # Offense count: 7 # Configuration parameters: Strict, EnforcedStyle. @@ -308,7 +306,7 @@ Style/GuardClause: Exclude: - 'config/initializers/okcomputer.rb' -# Offense count: 45 +# Offense count: 42 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, UseHashRocketsWithSymbolValues, PreferHashRocketsForNonAlnumEndingSymbols. # SupportedStyles: ruby19, hash_rockets, no_mixed_keys, ruby19_no_mixed_keys @@ -317,16 +315,14 @@ Style/HashSyntax: - 'app/controllers/sdr_controller.rb' - 'app/models/dor/goobi.rb' - 'spec/controllers/objects_controller_spec.rb' - - 'spec/controllers/versions_controller_spec.rb' - 'spec/dor/registration_response_spec.rb' - 'spec/dor/service_item_spec.rb' - 'spec/rails_helper.rb' -# Offense count: 3 +# Offense count: 2 # Cop supports --auto-correct. Style/IfUnlessModifier: Exclude: - - 'app/controllers/versions_controller.rb' - 'app/models/dor/goobi.rb' - 'config/initializers/okcomputer.rb' @@ -351,7 +347,7 @@ Style/PercentLiteralDelimiters: - 'spec/dor/service_item_spec.rb' - 'spec/dor/update_marc_record_service_spec.rb' -# Offense count: 12 +# Offense count: 13 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline. # SupportedStyles: single_quotes, double_quotes diff --git a/app/controllers/versions_controller.rb b/app/controllers/versions_controller.rb index 51ecfcad6b..7c386e395c 100644 --- a/app/controllers/versions_controller.rb +++ b/app/controllers/versions_controller.rb @@ -4,6 +4,8 @@ class VersionsController < ApplicationController def create Dor::VersionService.open(@item, open_params) render plain: @item.current_version + rescue Dor::Exception => e + render build_error(e) end def current @@ -17,6 +19,23 @@ def close_current private + # JSON-API error response + def build_error(err) + { + json: { + errors: [ + { + "status": '422', + "title": 'Unable to open version', + "detail": err.message + } + ] + }, + content_type: 'application/vnd.api+json', + status: :unprocessable_entity + } + end + def open_params params.permit( :assume_accessioned, diff --git a/spec/controllers/versions_controller_spec.rb b/spec/controllers/versions_controller_spec.rb index b99caca50c..9d6c838b4f 100644 --- a/spec/controllers/versions_controller_spec.rb +++ b/spec/controllers/versions_controller_spec.rb @@ -49,34 +49,41 @@ end let(:opening_user_name) { 'foo' } - # rubocop:disable RSpec/ExpectInHook before do - expect(Dor::Config.workflow.client).to receive(:get_lifecycle).with('dor', item.pid, 'accessioned').and_return(true) - expect(Dor::Config.workflow.client).to receive(:get_active_lifecycle).with('dor', item.pid, 'submitted').and_return(nil) - expect(Dor::Config.workflow.client).to receive(:get_active_lifecycle).with('dor', item.pid, 'opened').and_return(nil) - expect(Sdr::Client).to receive(:current_version).and_return(1) - - allow(fake_events_ds).to receive(:add_event) - allow(item).to receive(:events).and_return(fake_events_ds) - allow(item).to receive(:save) - # Do not test workflow side effects in dor-services-app; that is dor-services' responsibility - allow(Dor::CreateWorkflowService).to receive(:create_workflow) + allow(item).to receive(:current_version).and_return('2') end - # rubocop:enable RSpec/ExpectInHook - it 'opens a new object version when posted to' do - post :create, params: { object_id: item.pid }, as: :json - expect(response.body).to eq('2') + context 'when opening a version succeedes' do + before do + # Do not test version service side effects in dor-services-app; that is dor-services' responsibility + allow(Dor::VersionService).to receive(:open) + end + + it 'opens a new object version when posted to' do + post :create, params: { object_id: item.pid }, as: :json + expect(response.body).to eq('2') + expect(response).to be_successful + end + + it 'forwards optional params to the Dor::VersionService#open method' do + post :create, params: { object_id: item.pid }, body: open_params.to_json, as: :json + expect(Dor::VersionService).to have_received(:open).with(item, open_params) + expect(response.body).to eq('2') + expect(response).to be_successful + end end - it 'forwards optional params to the Dor::VersionService#open method' do - expect(Dor::VersionService).to receive(:open).with( - item, - open_params - ).and_call_original - post :create, params: { object_id: item.pid }, body: open_params.to_json, as: :json - expect(fake_events_ds).to have_received(:add_event).with('open', opening_user_name, 'Version 2 opened') - expect(response.body).to eq('2') + context 'when opening a version fails' do + before do + # Do not test version service side effects in dor-services-app; that is dor-services' responsibility + allow(Dor::VersionService).to receive(:open).and_raise(Dor::Exception, 'Object net yet accessioned') + end + + it 'returns an error' do + post :create, params: { object_id: item.pid }, as: :json + expect(response.body).to eq('{"errors":[{"status":"422","title":"Unable to open version","detail":"Object net yet accessioned"}]}') + expect(response.status).to eq 422 + end end end end