Merge branch 'sh-fix-discussion-note-promotion-failure-handling' into 'master'

Ensure note is promoted to discussion within reply create transaction

See merge request gitlab-org/gitlab!53542

app/services/notes/create_service.rb

@@ -27,7 +27,11 @@ module Notes
         end
 
         note_saved = note.with_transaction_returning_status do
-          !only_commands && note.save
+          break false if only_commands
+
+          note.save.tap do
+            update_discussions(note)
+          end
         end
 
         when_saved(note) if note_saved
@@ -54,12 +58,17 @@ module Notes
       @quick_actions_service ||= QuickActionsService.new(project, current_user)
     end
 
-    def when_saved(note)
+    def update_discussions(note)
+      # Ensure that individual notes that are promoted into discussions are
+      # updated in a transaction with the note creation to avoid inconsistencies:
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/301237
       if note.part_of_discussion? && note.discussion.can_convert_to_discussion?
         note.discussion.convert_to_discussion!.save
         note.clear_memoization(:discussion)
       end
+    end
 
+    def when_saved(note)
       todo_service.new_note(note, current_user)
       clear_noteable_diffs_cache(note)
       Suggestions::CreateService.new(note).execute

changelogs/unreleased/sh-fix-discussion-note-promotion-failure-handling.yml

+---
+title: Ensure note is promoted to discussion within reply create transaction
+merge_request: 53542
+author:
+type: fixed

spec/services/notes/create_service_spec.rb

@@ -459,6 +459,26 @@ RSpec.describe Notes::CreateService do
             .and change { existing_note.updated_at }
       end
 
+      context 'failure in when_saved' do
+        let(:service) { described_class.new(project, user, reply_opts) }
+
+        it 'converts existing note to DiscussionNote' do
+          expect do
+            existing_note
+
+            allow(service).to receive(:when_saved).and_raise(ActiveRecord::StatementInvalid)
+
+            travel_to(Time.current + 1.minute) do
+              service.execute
+            rescue ActiveRecord::StatementInvalid
+            end
+
+            existing_note.reload
+          end.to change { existing_note.type }.from(nil).to('DiscussionNote')
+            .and change { existing_note.updated_at }
+        end
+      end
+
       it 'returns a DiscussionNote with its parent discussion refreshed correctly' do
         discussion_notes = subject.discussion.notes