Merge pull request #4863 from sul-dlss/skip_open_check

app/services/update_object_service.rb

@@ -10,17 +10,19 @@ class UpdateObjectService
   # @param [Cocina::Models::DRO|Collection|AdminPolicy|DROWithMetadata|CollectionWithMetadata|AdminPolicyWithMetadata] cocina_object
   # @param [#create] event_factory creates events
   # @param [boolean] skip_lock do not perform an optimistic lock check
+  # @param [boolean] skip_open_check do not check that the object has an open version
   # @raise [Cocina::ValidationError] raised when validation of the Cocina object fails.
   # @raise [CocinaObjectNotFoundError] raised if the cocina object does not already exist in the datastore.
   # @raise [StateLockError] raised if optimistic lock failed.
   # @return [Cocina::Models::DRO, Cocina::Models::Collection, Cocina::Models::AdminPolicy] normalized cocina object
-  def self.update(cocina_object, event_factory: EventFactory, skip_lock: false)
-    new(cocina_object:, skip_lock:, event_factory:).update
+  def self.update(cocina_object, event_factory: EventFactory, skip_lock: false, skip_open_check: false)
+    new(cocina_object:, skip_lock:, event_factory:, skip_open_check:).update
   end
 
-  def initialize(cocina_object:, skip_lock:, event_factory: EventFactory)
+  def initialize(cocina_object:, skip_lock:, skip_open_check:, event_factory: EventFactory)
     @cocina_object = cocina_object
     @skip_lock = skip_lock
+    @skip_open_check = skip_open_check
     @event_factory = event_factory
   end
 
@@ -55,7 +57,7 @@ def update
 
   private
 
-  attr_reader :cocina_object, :skip_lock, :event_factory
+  attr_reader :cocina_object, :skip_lock, :event_factory, :skip_open_check
 
   delegate :version, to: :cocina_object
 
@@ -64,7 +66,7 @@ def druid
   end
 
   def notify_unless_open_version
-    return if VersionService.open?(druid:, version:)
+    return if skip_open_check || VersionService.open?(druid:, version:)
 
     Honeybadger.notify('Updating repository item without an open version', context: { druid:, version: })
   end

app/services/version_service.rb

@@ -85,7 +85,8 @@ def open(cocina_object:, description:, opening_user_name:, assume_accessioned:,
 
     update_cocina_object = cocina_object
 
-    update_cocina_object = UpdateObjectService.update(cocina_object.new(version: new_object_version.version)) if cocina_object.version != new_object_version.version
+    # Skipping open check since not yet opened.
+    update_cocina_object = UpdateObjectService.update(cocina_object.new(version: new_object_version.version), skip_open_check: true) if cocina_object.version != new_object_version.version
 
     workflow_client.create_workflow_by_name(druid, 'versioningWF', version: new_object_version.version.to_s)
 

bin/migrate-cocina

@@ -90,7 +90,7 @@ def perform_migrate(migrator_class:, obj:, mode:)
 
   ## Note: because we are now storing the cocina for every version we cannot remediate cocina breaking changes for a particular cocina version here.
   if mode == :migrate
-    updated_cocina_object = UpdateObjectService.update(updated_cocina_object)
+    updated_cocina_object = UpdateObjectService.update(updated_cocina_object, skip_open_check: !migrator.version?)
     Publish::MetadataTransferService.publish(updated_cocina_object) if migrator.publish?
     close_version(cocina_object: updated_cocina_object, version_description: migrator.version_description) unless updated_cocina_object.version == current_object_version
   end

spec/services/update_object_service_spec.rb

@@ -4,7 +4,7 @@
 
 RSpec.describe UpdateObjectService do
   include Dry::Monads[:result]
-  let(:store) { described_class.new(cocina_object:, skip_lock: true) }
+  let(:store) { described_class.new(cocina_object:, skip_lock: true, skip_open_check: false) }
   let(:open) { true }
 
   describe '#update' do
@@ -41,7 +41,7 @@
       end
 
       context 'when checking lock succeeds' do
-        let(:store) { described_class.new(cocina_object:, skip_lock: false) }
+        let(:store) { described_class.new(cocina_object:, skip_lock: false, skip_open_check: false) }
 
         let(:ar_cocina_object) { create(:ar_dro) }
         let(:lock) { "#{ar_cocina_object.external_identifier}=0" }
@@ -72,7 +72,7 @@
       end
 
       context 'when checking lock fails' do
-        let(:store) { described_class.new(cocina_object:, skip_lock: false) }
+        let(:store) { described_class.new(cocina_object:, skip_lock: false, skip_open_check: false) }
         let!(:ar_cocina_object) { create(:ar_dro) }
         let(:lock) { '64e8320d19d62ddb73c501276c5655cf' }
 
@@ -90,7 +90,7 @@
 
       context 'when version is not open' do
         let(:open) { false }
-        let(:store) { described_class.new(cocina_object:, skip_lock: false) }
+        let(:store) { described_class.new(cocina_object:, skip_lock: false, skip_open_check: false) }
 
         let(:ar_cocina_object) { create(:ar_dro) }
         let(:lock) { "#{ar_cocina_object.external_identifier}=0" }
@@ -110,6 +110,28 @@
                                                              context: { druid: ar_cocina_object.external_identifier, version: 1 })
         end
       end
+
+      context 'when version is not open but skipping open check' do
+        let(:open) { false }
+        let(:store) { described_class.new(cocina_object:, skip_lock: false, skip_open_check: true) }
+
+        let(:ar_cocina_object) { create(:ar_dro) }
+        let(:lock) { "#{ar_cocina_object.external_identifier}=0" }
+
+        let(:cocina_object) do
+          Cocina::Models.with_metadata(ar_cocina_object.to_cocina, lock, created: ar_cocina_object.created_at.utc, modified: ar_cocina_object.updated_at.utc)
+                        .new(label: 'new label')
+        end
+
+        before do
+          allow(Honeybadger).to receive(:notify)
+        end
+
+        it 'does not notify honeybadger' do
+          expect(store.update).to be_a Cocina::Models::DROWithMetadata
+          expect(Honeybadger).not_to have_received(:notify)
+        end
+      end
     end
 
     context 'when object is an AdminPolicy' do

spec/services/version_service_spec.rb

@@ -43,7 +43,7 @@
         expect(Dro.find_by(external_identifier: druid).version).to eq 2
         expect(ObjectVersion.current_version(druid).version).to eq(2)
         expect(workflow_state_service).to have_received(:accessioned?)
-        expect(workflow_state_service).to have_received(:open?).twice
+        expect(workflow_state_service).to have_received(:open?).once
         expect(workflow_state_service).to have_received(:accessioning?)
         expect(workflow_client).to have_received(:create_workflow_by_name).with(druid, 'versioningWF', version: '2')
 
@@ -71,7 +71,7 @@
         expect(ObjectVersion.current_version(druid).version).to eq(2)
         expect(ObjectVersion).not_to have_received(:sync_then_increment_version)
         expect(workflow_state_service).to have_received(:accessioned?)
-        expect(workflow_state_service).to have_received(:open?).twice
+        expect(workflow_state_service).to have_received(:open?).once
         expect(workflow_state_service).to have_received(:accessioning?)
         expect(workflow_client).to have_received(:create_workflow_by_name).with(druid, 'versioningWF', version: '2')