diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index 1269759fc2b3c9ebb92af1c062e346560686a3b5..3b764433c013bdc98c248b1b3457f30a9772dca0 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -1,6 +1,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationController before_action :check_merge_requests_available! before_action :merge_request + before_action :commit before_action :authorize_read_merge_request! private @@ -9,6 +10,11 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont @issuable = @merge_request ||= @project.merge_requests.find_by!(iid: params[:id]) 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 params.require(:merge_request).permit(merge_request_params_attributes) end @@ -28,7 +34,6 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont :task_num, :title, :discussion_locked, - label_ids: [] ] end diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 42a6e5be14f359c95bff3f16db0342a6d716c687..1312c83373ffa68a5b71c7fdf287ceeb9b9e7744 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -22,13 +22,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic def define_diff_vars @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff.order_id_desc - if commit_id = params[:commit_id].presence - @commit = @merge_request.target_project.commit(commit_id) - @compare = @commit - else - @compare = find_merge_request_diff_compare - end - + @compare = commit || find_merge_request_diff_compare return render_404 unless @compare @diffs = @compare.diffs(diff_options) diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 5b2c58d193d5fa8e617ebb83e0700035a15b5793..004aaeb2c56093f4a4e2dbf162b73b303ce74486 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -101,6 +101,30 @@ module MergeRequestsHelper }.merge(merge_params_ee(merge_request)) 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) {} end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 434dda89db0eeac5f9ebffb0385a5b58959b79aa..9f05535d4d4b73aa56ad6babe2e15a554b78828a 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -6,7 +6,7 @@ module MergeRequests @oldrev, @newrev = oldrev, newrev @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 # empty diff during a manual merge close_merge_requests diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index f90c6fcafb855d0b959663fdd34502d6158fbe69..385b34120ef5b0fc7ab2becdfdea9b79b4aa2c74 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -671,8 +671,8 @@ module SystemNoteService def merge_request_commit_url(merge_request, commit) url_helpers.diffs_namespace_project_merge_request_url( - project.namespace, - project, + merge_request.target_project.namespace, + merge_request.target_project, merge_request.iid, commit_id: commit.id ) diff --git a/app/views/projects/merge_requests/diffs/_not_all_comments_displayed.html.haml b/app/views/projects/merge_requests/diffs/_not_all_comments_displayed.html.haml index 26988d3491731ed444b78ef4fb4ee7813a20e069..60c419a3cda2d1af17ba4239007d42853490e627 100644 --- a/app/views/projects/merge_requests/diffs/_not_all_comments_displayed.html.haml +++ b/app/views/projects/merge_requests/diffs/_not_all_comments_displayed.html.haml @@ -13,7 +13,7 @@ - else 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 Show latest version = "of the diff" if @commit diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index 4e3f6f9edc9d85792ed01695fb220b63f93bdbc3..0d7abe8137f32d202dc735ea049d54f72efc9361 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -38,21 +38,21 @@ .nav-links.scrolling-tabs %ul.merge-request-tabs %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 %span.badge= @merge_request.related_notes.user.count - if @merge_request.source_project %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 %span.badge= @commits_count - if @pipelines.any? %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 %span.badge.js-pipelines-mr-count= @pipelines.size %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 %span.badge= @merge_request.diff_size #resolve-count-app.line-resolve-all-container.prepend-top-10{ "v-cloak" => true } diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index fd90c0d8bada9ef480fc11ae53da306a9ba4a7d9..a5b603d6bff3a08dadf21124b4130bd01457ba51 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -132,6 +132,21 @@ describe Projects::CommitController do expect(response).to be_success 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 describe 'GET branches' do