Merge pull request #5141 from sul-dlss/t5140-user_version_mode

Support new user version mode for update if exists.

app/controllers/versions_controller.rb

@@ -91,6 +91,7 @@ def close_params
       :user_name,
       :user_versions
     ).to_h.symbolize_keys
+    new_params[:user_version_mode] = new_params.delete(:user_versions).to_sym if new_params.key?(:user_versions)
     boolean_param(new_params, :start_accession)
   end
 

app/services/version_service.rb

@@ -1,11 +1,14 @@
 # frozen_string_literal: true
 
 # Open and close versions
+# rubocop:disable Metrics/ClassLength
 class VersionService
   class VersioningError < StandardError; end
 
   class CocinaObjectNotFoundError < VersioningError; end
 
+  DEFAULT_USER_VERSION_MODE = :update_if_existing
+
   # @param [String] druid of the item
   # @param [Integer] version of the item
   # @raise [CocinaObjectNotFoundError] if the object is not found
@@ -38,12 +41,12 @@ def self.can_open?(druid:, version:, assume_accessioned: false)
   # @param [String] description describes the version change
   # @param [String] user_name add username to the events datastream
   # @param [Boolean] start_accession (true) set to true if you want accessioning to start, false otherwise
-  # @param [String] user_versions (none) one of none, new, update
-  def self.close(druid:, version:, description: nil, user_name: nil, start_accession: true, user_versions: 'none')
+  # @param [Symbol] :user_version_mode :create, :update, :update_if_existing (default), or :none (do nothing) with user_versions on close
+  def self.close(druid:, version:, description: nil, user_name: nil, start_accession: true, user_version_mode: DEFAULT_USER_VERSION_MODE)
     new(druid:, version:).close(description:,
                                 user_name:,
                                 start_accession:,
-                                user_versions:)
+                                user_version_mode:)
   end
 
   # @param [String] druid of the item
@@ -115,14 +118,14 @@ def can_open?(assume_accessioned: false)
   # @param [String] :description describes the version change
   # @param [String] :user_name add username to the events datastream
   # @param [Boolean] :start_accession set to true if you want accessioning to start (default), false otherwise
-  # @param [String] :user_versions create, update, or do nothing with user_versions on close
+  # @param [Symbol] :user_version_mode :none (do nothing), :new, :update, or :update_if_existing (default) with user_versions on close
   # @raise [VersionService::VersioningError] if the object hasn't been opened for versioning, or if accessionWF has
   #   already been instantiated or the current version is missing a description
   # @raise [ArgumentError] if user_versions is not one of none, new, update
-  def close(description:, user_name:, start_accession: true, user_versions: 'none')
-    user_version_options = %w[none new update]
+  def close(description:, user_name:, start_accession: true, user_version_mode: DEFAULT_USER_VERSION_MODE)
+    user_version_mode_options = %i[none new update update_if_existing]
 
-    raise ArgumentError, "user_version must be one of #{user_version_options.join(', ')}" unless user_version_options.include?(user_versions)
+    raise ArgumentError, "user_version_mode must be one of #{user_version_mode_options.join(', ')}" unless user_version_mode_options.include?(user_version_mode)
 
     ensure_closeable!
 
@@ -133,7 +136,7 @@ def close(description:, user_name:, start_accession: true, user_versions: 'none'
 
     EventFactory.create(druid:, event_type: 'version_close', data: { who: user_name, version: version.to_s })
 
-    update_user_version(user_versions:, repository_object:)
+    update_user_version(user_version_mode:, repository_object:)
   end
 
   # Determines whether a version can be closed for an object.
@@ -195,15 +198,28 @@ def ensure_closeable!
     raise VersionService::VersioningError, "accessionWF already created for versioned object #{druid}" if accessioning?
   end
 
-  def update_user_version(user_versions:, repository_object:)
-    return if user_versions == 'none'
-    return UserVersionService.create(druid:, version: repository_object.last_closed_version.version) if user_versions == 'new'
+  def update_user_version(user_version_mode:, repository_object:)
+    case user_version_mode
+    when :new
+      create_user_version(repository_object)
+    when :update
+      no_user_versions?(repository_object) ? create_user_version(repository_object) : move_user_version(repository_object)
+    when :update_if_existing
+      move_user_version(repository_object) unless no_user_versions?(repository_object)
+    end
+    # :none falls through and does nothing
+  end
 
-    # If called with 'update', but there are no user versions to update, create a new user version.
-    return UserVersionService.create(druid:, version: repository_object.last_closed_version.version) if repository_object.user_versions.empty?
+  def no_user_versions?(repository_object)
+    repository_object.user_versions.empty?
+  end
+
+  def create_user_version(repository_object)
+    UserVersionService.create(druid:, version: repository_object.last_closed_version.version)
+  end
 
-    max_user_version = repository_object.user_versions.maximum(:version)
-    UserVersionService.move(druid:, version: repository_object.last_closed_version.version, user_version: max_user_version)
+  def move_user_version(repository_object)
+    UserVersionService.move(druid:, version: repository_object.last_closed_version.version, user_version: repository_object.head_user_version)
   end
 
   def check_version!(current_version:)
@@ -216,3 +232,4 @@ def check_version!(current_version:)
     raise VersionService::VersioningError, "Version from Preservation is out of sync. Preservation expects #{preservation_version} but current version is #{current_version}"
   end
 end
+# rubocop:enable Metrics/ClassLength

openapi.yml

@@ -1028,6 +1028,7 @@ paths:
               - "none"
               - "new"
               - "update"
+              - "update_if_existing"
   "/v1/objects/{object_id}/versions":
     get:
       tags:

spec/services/version_service_spec.rb

@@ -208,12 +208,12 @@
                             description:,
                             user_name: 'jcoyne',
                             start_accession:,
-                            user_versions: user_version_param)
+                            user_version_mode:)
     end
 
     let(:version) { 2 }
     let(:start_accession) { true }
-    let(:user_version_param) { 'none' }
+    let(:user_version_mode) { :none }
     let(:description) { 'closing text' }
     let(:workflow_client) do
       instance_double(Dor::Workflow::Client, create_workflow_by_name: true)
@@ -251,7 +251,7 @@
       end
 
       context 'when user_version is new' do
-        let(:user_version_param) { 'new' }
+        let(:user_version_mode) { :new }
 
         it 'creates a new user version' do
           close
@@ -262,7 +262,7 @@
       end
 
       context 'when user_version is new and there is an existing user version' do
-        let(:user_version_param) { 'new' }
+        let(:user_version_mode) { :new }
         let(:version) { 3 }
 
         before do
@@ -271,7 +271,7 @@
                                 description:,
                                 user_name: 'jcoyne',
                                 start_accession:,
-                                user_versions: 'new')
+                                user_version_mode: :new)
           allow(Cocina::ObjectValidator).to receive(:new).and_return(instance_double(Cocina::ObjectValidator, validate: true))
           allow(Preservation::Client.objects).to receive(:current_version).and_return(2)
           allow(workflow_state_service).to receive_messages(accessioned?: true, accessioning?: false)
@@ -291,7 +291,7 @@
       end
 
       context 'when user_version is update' do
-        let(:user_version_param) { 'update' }
+        let(:user_version_mode) { :update }
         let(:version) { 3 }
 
         before do
@@ -300,7 +300,7 @@
                                 description:,
                                 user_name: 'jcoyne',
                                 start_accession:,
-                                user_versions: 'new')
+                                user_version_mode: :new)
           allow(Cocina::ObjectValidator).to receive(:new).and_return(instance_double(Cocina::ObjectValidator, validate: true))
           allow(Preservation::Client.objects).to receive(:current_version).and_return(2)
           allow(workflow_state_service).to receive_messages(accessioned?: true, accessioning?: false)
@@ -321,7 +321,7 @@
       end
 
       context 'when user_version is update but there is no previous user version' do
-        let(:user_version_param) { 'update' }
+        let(:user_version_mode) { :update }
         let(:version) { 3 }
 
         before do
@@ -330,7 +330,7 @@
                                 description:,
                                 user_name: 'jcoyne',
                                 start_accession:,
-                                user_versions: 'none')
+                                user_version_mode: :none)
           allow(Cocina::ObjectValidator).to receive(:new).and_return(instance_double(Cocina::ObjectValidator, validate: true))
           allow(Preservation::Client.objects).to receive(:current_version).and_return(2)
           allow(workflow_state_service).to receive_messages(accessioned?: true, accessioning?: false)
@@ -348,6 +348,64 @@
           expect(repository_object.last_closed_version.user_versions.first.version).to eq 1
         end
       end
+
+      context 'when user_version is update_if_existing' do
+        let(:user_version_mode) { :update_if_existing }
+        let(:version) { 3 }
+
+        before do
+          described_class.close(druid:,
+                                version: 2,
+                                description:,
+                                user_name: 'jcoyne',
+                                start_accession:,
+                                user_version_mode: :new)
+          allow(Cocina::ObjectValidator).to receive(:new).and_return(instance_double(Cocina::ObjectValidator, validate: true))
+          allow(Preservation::Client.objects).to receive(:current_version).and_return(2)
+          allow(workflow_state_service).to receive_messages(accessioned?: true, accessioning?: false)
+          allow(cocina_object).to receive(:version).and_return(2)
+          described_class.open(cocina_object:, description: 'same as it ever was', opening_user_name: 'sunetid')
+          repository_object.versions.last.update!(closed_at: nil)
+          repository_object.head_version = repository_object.versions.last
+          repository_object.last_closed_version = repository_object.versions.first
+        end
+
+        it 'moves the user version' do
+          close
+          expect(repository_object.reload.last_closed_version).to be_present
+          expect(repository_object.last_closed_version.user_versions.count).to eq 1
+          expect(repository_object.last_closed_version.user_versions.first.version).to eq 1
+          expect(repository_object.versions.find_by(version: 2).user_versions.count).to eq 0
+        end
+      end
+
+      context 'when user_version is update_if_existing but there is no previous user version' do
+        let(:user_version_mode) { :update_if_existing }
+        let(:version) { 3 }
+
+        before do
+          described_class.close(druid:,
+                                version: 2,
+                                description:,
+                                user_name: 'jcoyne',
+                                start_accession:,
+                                user_version_mode: :none)
+          allow(Cocina::ObjectValidator).to receive(:new).and_return(instance_double(Cocina::ObjectValidator, validate: true))
+          allow(Preservation::Client.objects).to receive(:current_version).and_return(2)
+          allow(workflow_state_service).to receive_messages(accessioned?: true, accessioning?: false)
+          allow(cocina_object).to receive(:version).and_return(2)
+          described_class.open(cocina_object:, description: 'same as it ever was', opening_user_name: 'sunetid')
+          repository_object.versions.last.update!(closed_at: nil)
+          repository_object.head_version = repository_object.versions.last
+          repository_object.last_closed_version = repository_object.versions.first
+        end
+
+        it 'does nothing' do
+          close
+          expect(repository_object.last_closed_version.user_versions).to be_empty
+          expect(repository_object.user_versions).to be_empty
+        end
+      end
     end
 
     context 'when start_accession is false' do