diff --git a/app/controllers/versions_controller.rb b/app/controllers/versions_controller.rb index 48a13dd53..23077e58b 100644 --- a/app/controllers/versions_controller.rb +++ b/app/controllers/versions_controller.rb @@ -47,23 +47,18 @@ def build_error(msg, err) def open_params params.permit( :assume_accessioned, - vers_md_upd_info: [ - :description, - :opening_user_name, - :significance - ] - ).to_h.deep_symbolize_keys + :description, + :opening_user_name, + :significance + ).to_h.symbolize_keys end def close_params - symbolized_hash = params.permit( + params.permit( :description, :significance, :start_accession, :version_num ).to_h.symbolize_keys - # Downstream code expects the significance value to be a symbol - symbolized_hash[:significance] = symbolized_hash[:significance].to_sym if symbolized_hash.key?(:significance) - symbolized_hash end end diff --git a/app/services/version_service.rb b/app/services/version_service.rb index 9e05e658f..333879f9c 100644 --- a/app/services/version_service.rb +++ b/app/services/version_service.rb @@ -21,7 +21,9 @@ def initialize(work) # Increments the version number and initializes versioningWF for the object # @param [Hash] opts optional params # @option opts [Boolean] :assume_accessioned If true, does not check whether object has been accessioned. - # @option opts [Hash] :vers_md_upd_info If present, used to add to the events datastream and set the desc and significance on the versionMetadata datastream + # @option opts [String] :significance set significance (major/minor/patch) of version change + # @option opts [String] :description set description of version change + # @option opts [String] :opening_user_name add opening username to the events datastream # @raise [Dor::Exception] if the object hasn't been accessioned, or if a version is already opened def open(opts = {}) sdr_version = try_to_get_current_version(opts[:assume_accessioned]) @@ -32,12 +34,13 @@ def open(opts = {}) Dor::Config.workflow.client.create_workflow_by_name(work.pid, 'versioningWF') - vmd_upd_info = opts[:vers_md_upd_info] - return unless vmd_upd_info + return if (opts.keys & open_options_requiring_work_save).empty? - work.events.add_event('open', vmd_upd_info[:opening_user_name], "Version #{vmd_ds.current_version_id} opened") - vmd_ds.update_current_version(description: vmd_upd_info[:description], significance: vmd_upd_info[:significance].to_sym) - work.save + work.events.add_event('open', opts[:opening_user_name], "Version #{vmd_ds.current_version_id} opened") if opts[:opening_user_name] + + vmd_ds.update_current_version(description: opts[:description], significance: opts[:significance].to_sym) if opts[:description] && opts[:significance] + + work.save! end # Determines whether a new version can be opened for an object. @@ -117,4 +120,10 @@ def accessioning? end attr_reader :work + + private + + def open_options_requiring_work_save + [:opening_user_name, :significance, :description] + end end diff --git a/spec/controllers/versions_controller_spec.rb b/spec/controllers/versions_controller_spec.rb index 4ff618c00..74ed82223 100644 --- a/spec/controllers/versions_controller_spec.rb +++ b/spec/controllers/versions_controller_spec.rb @@ -35,7 +35,7 @@ it 'forwards optional params to the VersionService#close method' do post :close_current, params: { object_id: item.pid }, body: %( {"description": "some text", "significance": "major"} ), as: :json expect(response.body).to match(/version 1 closed/) - expect(VersionService).to have_received(:close).with(item, description: 'some text', significance: :major) + expect(VersionService).to have_received(:close).with(item, description: 'some text', significance: 'major') end end @@ -63,11 +63,9 @@ let(:open_params) do { assume_accessioned: false, - vers_md_upd_info: { - significance: 'minor', - description: 'bar', - opening_user_name: opening_user_name - } + significance: 'minor', + description: 'bar', + opening_user_name: opening_user_name } end let(:opening_user_name) { 'foo' } diff --git a/spec/services/version_service_spec.rb b/spec/services/version_service_spec.rb index 6407edb56..bd9f05265 100644 --- a/spec/services/version_service_spec.rb +++ b/spec/services/version_service_spec.rb @@ -48,23 +48,23 @@ expect(workflow_client).to have_received(:create_workflow_by_name).with(obj.pid, 'versioningWF') end - it 'includes vers_md_upd_info' do - vers_md_upd_info = { significance: 'real_major', description: 'same as it ever was', opening_user_name: 'sunetid' } + it 'includes options' do + options = { significance: 'real_major', description: 'same as it ever was', opening_user_name: 'sunetid' } cur_vers = '2' allow(vmd_ds).to receive(:current_version).and_return(cur_vers) - allow(obj).to receive(:save) + allow(obj).to receive(:save!) - expect(ev_ds).to receive(:add_event).with('open', vers_md_upd_info[:opening_user_name], "Version #{cur_vers} opened") - expect(vmd_ds).to receive(:update_current_version).with(description: vers_md_upd_info[:description], significance: vers_md_upd_info[:significance].to_sym) - expect(obj).to receive(:save) + expect(ev_ds).to receive(:add_event).with('open', options[:opening_user_name], "Version #{cur_vers} opened") + expect(vmd_ds).to receive(:update_current_version).with(description: options[:description], significance: options[:significance].to_sym) + expect(obj).to receive(:save!) - described_class.open(obj, vers_md_upd_info: vers_md_upd_info) + described_class.open(obj, **options) end - it "doesn't include vers_md_upd_info" do + it "doesn't include options" do expect(ev_ds).not_to receive(:add_event) expect(vmd_ds).not_to receive(:update_current_version) - expect(obj).not_to receive(:save) + expect(obj).not_to receive(:save!) open end