Commit 142edf0a authored by micael.bergeron's avatar micael.bergeron

diff notes created in merge request on a commit have the right context

add a spec for commit merge request diff notes
parent e4eba908
class Projects::MergeRequests::ApplicationController < Projects::ApplicationController class Projects::MergeRequests::ApplicationController < Projects::ApplicationController
before_action :check_merge_requests_available! before_action :check_merge_requests_available!
before_action :merge_request before_action :merge_request
before_action :commit
before_action :authorize_read_merge_request! before_action :authorize_read_merge_request!
private private
...@@ -9,6 +10,11 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont ...@@ -9,6 +10,11 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
@issuable = @merge_request ||= @project.merge_requests.find_by!(iid: params[:id]) @issuable = @merge_request ||= @project.merge_requests.find_by!(iid: params[:id])
end end
def commit
return nil unless commit_id = params[:commit_id].presence
@commit ||= merge_request.target_project.commit(commit_id)
end
def merge_request_params def merge_request_params
params.require(:merge_request).permit(merge_request_params_attributes) params.require(:merge_request).permit(merge_request_params_attributes)
end end
...@@ -28,7 +34,6 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont ...@@ -28,7 +34,6 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
:task_num, :task_num,
:title, :title,
:discussion_locked, :discussion_locked,
label_ids: [] label_ids: []
] ]
end end
......
...@@ -22,13 +22,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic ...@@ -22,13 +22,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
def define_diff_vars def define_diff_vars
@merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff.order_id_desc @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff.order_id_desc
if commit_id = params[:commit_id].presence @compare = commit || find_merge_request_diff_compare
@commit = @merge_request.target_project.commit(commit_id)
@compare = @commit
else
@compare = find_merge_request_diff_compare
end
return render_404 unless @compare return render_404 unless @compare
@diffs = @compare.diffs(diff_options) @diffs = @compare.diffs(diff_options)
......
...@@ -101,6 +101,30 @@ module MergeRequestsHelper ...@@ -101,6 +101,30 @@ module MergeRequestsHelper
}.merge(merge_params_ee(merge_request)) }.merge(merge_params_ee(merge_request))
end end
def tab_link_for(tab, options={}, &block)
data_attrs = {
action: tab.to_s,
target: "##{tab.to_s}",
toggle: options.fetch(:force_link, false) ? '' : 'tab'
}
url = case tab
when :show
data_attrs.merge!(target: '#notes')
project_merge_request_path(@project, @merge_request)
when :commits
commits_project_merge_request_path(@project, @merge_request)
when :pipelines
pipelines_project_merge_request_path(@project, @merge_request)
when :diffs
diffs_project_merge_request_path(@project, @merge_request)
else
raise "Cannot create tab #{tab}."
end
link_to(url, data: data_attrs, &block)
end
def merge_params_ee(merge_request) def merge_params_ee(merge_request)
{} {}
end end
......
...@@ -6,7 +6,7 @@ module MergeRequests ...@@ -6,7 +6,7 @@ module MergeRequests
@oldrev, @newrev = oldrev, newrev @oldrev, @newrev = oldrev, newrev
@branch_name = Gitlab::Git.ref_name(ref) @branch_name = Gitlab::Git.ref_name(ref)
find_new_commits Gitlab::GitalyClient.allow_n_plus_1_calls(&method(:find_new_commits))
# Be sure to close outstanding MRs before reloading them to avoid generating an # Be sure to close outstanding MRs before reloading them to avoid generating an
# empty diff during a manual merge # empty diff during a manual merge
close_merge_requests close_merge_requests
......
...@@ -671,8 +671,8 @@ module SystemNoteService ...@@ -671,8 +671,8 @@ module SystemNoteService
def merge_request_commit_url(merge_request, commit) def merge_request_commit_url(merge_request, commit)
url_helpers.diffs_namespace_project_merge_request_url( url_helpers.diffs_namespace_project_merge_request_url(
project.namespace, merge_request.target_project.namespace,
project, merge_request.target_project,
merge_request.iid, merge_request.iid,
commit_id: commit.id commit_id: commit.id
) )
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
- else - else
viewing an old version of the diff viewing an old version of the diff
.pull-right .text-right
= link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-sm' do = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-sm' do
Show latest version Show latest version
= "of the diff" if @commit = "of the diff" if @commit
...@@ -38,21 +38,21 @@ ...@@ -38,21 +38,21 @@
.nav-links.scrolling-tabs .nav-links.scrolling-tabs
%ul.merge-request-tabs %ul.merge-request-tabs
%li.notes-tab %li.notes-tab
= link_to project_merge_request_path(@project, @merge_request), data: { target: 'div#notes', action: 'show', toggle: 'tab' } do = tab_link_for :show, force_link: @commit.present? do
Discussion Discussion
%span.badge= @merge_request.related_notes.user.count %span.badge= @merge_request.related_notes.user.count
- if @merge_request.source_project - if @merge_request.source_project
%li.commits-tab %li.commits-tab
= link_to commits_project_merge_request_path(@project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do = tab_link_for :commits do
Commits Commits
%span.badge= @commits_count %span.badge= @commits_count
- if @pipelines.any? - if @pipelines.any?
%li.pipelines-tab %li.pipelines-tab
= link_to pipelines_project_merge_request_path(@project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do = tab_link_for :pipelines do
Pipelines Pipelines
%span.badge.js-pipelines-mr-count= @pipelines.size %span.badge.js-pipelines-mr-count= @pipelines.size
%li.diffs-tab %li.diffs-tab
= link_to diffs_project_merge_request_path(@project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do = tab_link_for :diffs do
Changes Changes
%span.badge= @merge_request.diff_size %span.badge= @merge_request.diff_size
#resolve-count-app.line-resolve-all-container.prepend-top-10{ "v-cloak" => true } #resolve-count-app.line-resolve-all-container.prepend-top-10{ "v-cloak" => true }
......
...@@ -132,6 +132,21 @@ describe Projects::CommitController do ...@@ -132,6 +132,21 @@ describe Projects::CommitController do
expect(response).to be_success expect(response).to be_success
end end
end end
context 'in the context of a merge_request' do
let(:merge_request) { create(:merge_request, source_project: project) }
let(:commit) { merge_request.commits.first }
it 'prepare diff notes in the context of the merge request' do
go(id: commit.id, merge_request_iid: merge_request.iid)
expect(assigns(:new_diff_note_attrs)).to eq({ noteable_type: 'MergeRequest',
noteable_id: merge_request.id,
commit_id: commit.id
})
expect(response).to be_ok
end
end
end end
describe 'GET branches' do describe 'GET branches' do
......
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