Commit 34fcade1 authored by Douwe Maan's avatar Douwe Maan

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

parent c72abcef
...@@ -50,7 +50,7 @@ module NotesHelper ...@@ -50,7 +50,7 @@ module NotesHelper
def link_to_reply_discussion(discussion, line_type = nil) def link_to_reply_discussion(discussion, line_type = nil)
return unless current_user 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', button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
data: data, title: 'Add a reply' data: data, title: 'Add a reply'
......
...@@ -85,6 +85,12 @@ class Discussion ...@@ -85,6 +85,12 @@ class Discussion
first_note.discussion_id(context_noteable) first_note.discussion_id(context_noteable)
end 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 alias_method :to_param, :id
def diff_discussion? def diff_discussion?
......
---
title: Fix replying to a commit discussion displayed in the context of an MR
merge_request:
author:
...@@ -5,7 +5,7 @@ feature 'Merge Request Discussions', feature: true do ...@@ -5,7 +5,7 @@ feature 'Merge Request Discussions', feature: true do
login_as :admin login_as :admin
end end
context "Diff discussions" do describe "Diff discussions" do
let(:merge_request) { create(:merge_request, importing: true) } let(:merge_request) { create(:merge_request, importing: true) }
let(:project) { merge_request.source_project } let(:project) { merge_request.source_project }
let!(:old_merge_request_diff) { merge_request.merge_request_diffs.create(diff_refs: outdated_diff_refs) } 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 ...@@ -48,4 +48,43 @@ feature 'Merge Request Discussions', feature: true do
end end
end 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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment