Commit c77083a4 authored by Micaël Bergeron's avatar Micaël Bergeron

fix the commit diff discussion sending the wrong url

it should now send you to the merge request diff path scoped to the commit.
parent c9041508
...@@ -22,12 +22,10 @@ class DiffDiscussion < Discussion ...@@ -22,12 +22,10 @@ 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 do |params|
{ commit_id: commit_id } params[:commit_id] = commit_id if on_merge_request_commit?
else params
noteable.version_params_for(position.diff_refs)
end end
end end
...@@ -37,4 +35,15 @@ class DiffDiscussion < Discussion ...@@ -37,4 +35,15 @@ class DiffDiscussion < Discussion
position: position.to_json position: position.to_json
) )
end end
private
def version_params
params = if active?
{}
else
noteable.version_params_for(position.diff_refs)
end
yield params
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