Ensure note is promoted to discussion within reply create transaction

Previously a note on a merge request would be committed to the database
before its `type` is promoted to `DiscussionNote`. However, as we saw in
https://gitlab.com/gitlab-org/gitlab/-/issues/301237, if a user replies
to an individual note on the merge request but the UPDATE to the note
fails (e.g. due to a SQL timeout), the note will persist in the database
but have the wrong discussion `type`.

This can block users from being able to merge a merge request because
the backend will detect an unresolved discussion. However, the UI won't
show any unresolved discussions because the first note will erroneously
appear to be an individual note, which cannot be resolved since it's not
a discussion.

To avoid this issue, we now promote the `type` within the save
transaction so that the note will always persist with the right value.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/301237

app/services/notes/create_service.rb

@@ -27,7 +27,10 @@ module Notes
         end
 
         note_saved = note.with_transaction_returning_status do
-          !only_commands && note.save
+          break false if only_commands
+
+          before_save(note)
+          note.save
         end
 
         when_saved(note) if note_saved
@@ -54,12 +57,17 @@ module Notes
       @quick_actions_service ||= QuickActionsService.new(project, current_user)
     end
 
-    def when_saved(note)
+    def before_save(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