From 256f77bb348bad8c97f7da66bbb1a56aaee6c07a Mon Sep 17 00:00:00 2001 From: Justin Coyne 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