Commit f7a9ced2 authored by ⛄️ Sean McGivern ⛄️'s avatar ⛄️ Sean McGivern ⛄️

Merge branch 'commit-diff-discussion-in-mr-context-fix-links' into 'master'

fix the commit diff discussion sending the wrong url

See merge request gitlab-org/gitlab-ce!15988
parents 08ee1e29 a794b161
...@@ -22,12 +22,9 @@ class DiffDiscussion < Discussion ...@@ -22,12 +22,9 @@ class DiffDiscussion < Discussion
def merge_request_version_params def merge_request_version_params
return unless for_merge_request? return unless for_merge_request?
return {} if active?
if on_merge_request_commit? version_params.tap do |params|
{ commit_id: commit_id } params[:commit_id] = commit_id if on_merge_request_commit?
else
noteable.version_params_for(position.diff_refs)
end end
end end
...@@ -37,4 +34,12 @@ class DiffDiscussion < Discussion ...@@ -37,4 +34,12 @@ class DiffDiscussion < Discussion
position: position.to_json position: position.to_json
) )
end end
private
def version_params
return {} if active?
noteable.version_params_for(position.diff_refs)
end
end end
...@@ -41,6 +41,7 @@ describe NotesHelper do ...@@ -41,6 +41,7 @@ describe NotesHelper do
describe '#discussion_path' do describe '#discussion_path' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:anchor) { discussion.line_code }
context 'for a merge request discusion' do context 'for a merge request discusion' do
let(:merge_request) { create(:merge_request, source_project: project, target_project: project, importing: true) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project, importing: true) }
...@@ -151,6 +152,15 @@ describe NotesHelper do ...@@ -151,6 +152,15 @@ describe NotesHelper do
expect(helper.discussion_path(discussion)).to be_nil expect(helper.discussion_path(discussion)).to be_nil
end end
end end
context 'for a contextual commit discussion' do
let(:commit) { merge_request.commits.last }
let(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, commit_id: commit.id).to_discussion }
it 'returns the merge request diff discussion scoped in the commit' do
expect(helper.discussion_path(discussion)).to eq(diffs_project_merge_request_path(project, merge_request, commit_id: commit.id, anchor: anchor))
end
end
end end
context 'for a commit discussion' do context 'for a commit discussion' do
...@@ -160,7 +170,7 @@ describe NotesHelper do ...@@ -160,7 +170,7 @@ describe NotesHelper do
let(:discussion) { create(:diff_note_on_commit, project: project).to_discussion } let(:discussion) { create(:diff_note_on_commit, project: project).to_discussion }
it 'returns the commit path with the line code' do it 'returns the commit path with the line code' do
expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: discussion.line_code)) expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: anchor))
end end
end end
...@@ -168,7 +178,7 @@ describe NotesHelper do ...@@ -168,7 +178,7 @@ describe NotesHelper do
let(:discussion) { create(:legacy_diff_note_on_commit, project: project).to_discussion } let(:discussion) { create(:legacy_diff_note_on_commit, project: project).to_discussion }
it 'returns the commit path with the line code' do it 'returns the commit path with the line code' do
expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: discussion.line_code)) expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: anchor))
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