From 256f77bb348bad8c97f7da66bbb1a56aaee6c07a Mon Sep 17 00:00:00 2001
From: Justin Coyne <jcoyne@justincoyne.com>
Date: Thu, 28 Mar 2024 09:09:18 -0500
Subject: [PATCH] Notify honeybadger when an update is issued on a non-opened
 object

Fixes #4759
---
 app/services/update_object_service.rb       | 23 ++++++++++++++-----
 spec/requests/update_metadata_spec.rb       |  1 +
 spec/services/update_object_service_spec.rb | 25 +++++++++++++++++++++
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/app/services/update_object_service.rb b/app/services/update_object_service.rb
index b2db4c91e..023ae5499 100644
--- a/app/services/update_object_service.rb
+++ b/app/services/update_object_service.rb
@@ -26,6 +26,7 @@ def initialize(cocina_object:, skip_lock:, event_factory: EventFactory)
 
   def update
     Cocina::ObjectValidator.validate(cocina_object)
+    notify_unless_open_version
 
     # If this is a collection and the title has changed, then reindex the children.
     update_items = need_to_update_members?
@@ -36,18 +37,18 @@ def update
     cocina_object_without_metadata = Cocina::Models.without_metadata(cocina_object)
 
     # TODO: after migration to RepositoryObject we can remove these nil checks
-    RepositoryObject.find_by(external_identifier: cocina_object.externalIdentifier)&.head_version&.update!(RepositoryObjectVersion.to_model_hash(cocina_object_without_metadata))
+    RepositoryObject.find_by(external_identifier: druid)&.head_version&.update!(RepositoryObjectVersion.to_model_hash(cocina_object_without_metadata))
 
-    event_factory.create(druid: cocina_object.externalIdentifier, event_type: 'update', data: { success: true, request: cocina_object_without_metadata.to_h })
+    event_factory.create(druid:, event_type: 'update', data: { success: true, request: cocina_object_without_metadata.to_h })
 
     # Broadcast this update action to a topic
     Notifications::ObjectUpdated.publish(model: cocina_object_with_metadata)
 
     # Update all items in the collection if necessary
-    PublishItemsModifiedJob.perform_later(cocina_object.externalIdentifier) if update_items
+    PublishItemsModifiedJob.perform_later(druid) if update_items
     cocina_object_with_metadata
   rescue Cocina::ValidationError => e
-    event_factory.create(druid: cocina_object.externalIdentifier, event_type: 'update',
+    event_factory.create(druid:, event_type: 'update',
                          data: { success: false, error: e.message, request: Cocina::Models.without_metadata(cocina_object).to_h })
     raise
   end
@@ -56,9 +57,21 @@ def update
 
   attr_reader :cocina_object, :skip_lock, :event_factory
 
+  delegate :version, to: :cocina_object
+
+  def druid
+    cocina_object.externalIdentifier
+  end
+
+  def notify_unless_open_version
+    return if VersionService.open?(druid:, version:)
+
+    Honeybadger.notify('Updating repository item without an open version', context: { druid:, version: })
+  end
+
   def need_to_update_members?
     cocina_object.collection? &&
-      Cocina::Models::Builders::TitleBuilder.build(CocinaObjectStore.find(cocina_object.externalIdentifier).description.title) !=
+      Cocina::Models::Builders::TitleBuilder.build(CocinaObjectStore.find(druid).description.title) !=
         Cocina::Models::Builders::TitleBuilder.build(cocina_object.description.title)
   end
 end
diff --git a/spec/requests/update_metadata_spec.rb b/spec/requests/update_metadata_spec.rb
index 932bb56d2..ed4c45950 100644
--- a/spec/requests/update_metadata_spec.rb
+++ b/spec/requests/update_metadata_spec.rb
@@ -91,6 +91,7 @@
     allow(Cocina::ObjectValidator).to receive(:validate)
 
     allow(EventFactory).to receive(:create)
+    allow(VersionService).to receive(:open?).and_return(true)
   end
 
   it 'updates the object' do
diff --git a/spec/services/update_object_service_spec.rb b/spec/services/update_object_service_spec.rb
index 4ccc69e0a..8619e1f0d 100644
--- a/spec/services/update_object_service_spec.rb
+++ b/spec/services/update_object_service_spec.rb
@@ -5,10 +5,12 @@
 RSpec.describe UpdateObjectService do
   include Dry::Monads[:result]
   let(:store) { described_class.new(cocina_object:, skip_lock: true) }
+  let(:open) { true }
 
   describe '#update' do
     before do
       allow(Cocina::ObjectValidator).to receive(:validate)
+      allow(VersionService).to receive(:open?).and_return(open)
     end
 
     context 'when object is a DRO' do
@@ -85,6 +87,29 @@
           expect { store.update }.to raise_error(CocinaObjectStore::StaleLockError)
         end
       end
+
+      context 'when version is not open' do
+        let(:open) { false }
+        let(:store) { described_class.new(cocina_object:, skip_lock: false) }
+
+        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 'notifies honeybadger' do
+          expect(store.update).to be_a Cocina::Models::DROWithMetadata
+          expect(Honeybadger).to have_received(:notify).with('Updating repository item without an open version',
+                                                             context: { druid: ar_cocina_object.external_identifier, version: 1 })
+        end
+      end
     end
 
     context 'when object is an AdminPolicy' do