Merge branch 'dm-comment-on-mr-commit-discussion' into 'master'

Fix replying to a commit discussion displayed in the context of an MR

Closes #33201

See merge request !11847

app/helpers/notes_helper.rb

@@ -50,7 +50,7 @@ module NotesHelper
   def link_to_reply_discussion(discussion, line_type = nil)
     return unless current_user
 
-    data = { discussion_id: discussion.id, line_type: line_type }
+    data = { discussion_id: discussion.reply_id, line_type: line_type }
 
     button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
                            data: data, title: 'Add a reply'

app/models/discussion.rb

@@ -85,6 +85,12 @@ class Discussion
     first_note.discussion_id(context_noteable)
   end
 
+  def reply_id
+    # To reply to this discussion, we need the actual discussion_id from the database,
+    # not the potentially overwritten one based on the noteable.
+    first_note.discussion_id
+  end
+
   alias_method :to_param, :id
 
   def diff_discussion?

changelogs/unreleased/dm-comment-on-mr-commit-discussion.yml

@@ -0,0 +1,4 @@
+---
+title: Fix replying to a commit discussion displayed in the context of an MR
+merge_request:
+author:

spec/features/merge_requests/discussion_spec.rb

@@ -5,7 +5,7 @@ feature 'Merge Request Discussions', feature: true do
     login_as :admin
   end
 
-  context "Diff discussions" do
+  describe "Diff discussions" do
     let(:merge_request) { create(:merge_request, importing: true) }
     let(:project) { merge_request.source_project }
     let!(:old_merge_request_diff) { merge_request.merge_request_diffs.create(diff_refs: outdated_diff_refs) }
@@ -48,4 +48,43 @@ feature 'Merge Request Discussions', feature: true do
       end
     end
   end
+
+  describe 'Commit comments displayed in MR context', :js do
+    let(:merge_request) { create(:merge_request) }
+    let(:project) { merge_request.project }
+
+    shared_examples 'a functional discussion' do
+      let(:discussion_id) { note.discussion_id(merge_request) }
+
+      it 'is displayed' do
+        expect(page).to have_css(".discussion[data-discussion-id='#{discussion_id}']")
+      end
+
+      it 'can be replied to' do
+        within(".discussion[data-discussion-id='#{discussion_id}']") do
+          click_button 'Reply...'
+          fill_in 'note[note]', with: 'Test!'
+          click_button 'Comment'
+
+          expect(page).to have_css('.note', count: 2)
+        end
+      end
+    end
+
+    before(:each) do
+      visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+    end
+
+    context 'a regular commit comment' do
+      let(:note) { create(:note_on_commit, project: project) }
+
+      it_behaves_like 'a functional discussion'
+    end
+
+    context 'a commit diff comment' do
+      let(:note) { create(:diff_note_on_commit, project: project) }
+
+      it_behaves_like 'a functional discussion'
+    end
+  end
 end