diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index 3b764433c01..793ae03fb88 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -1,7 +1,6 @@ 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 @@ -10,11 +9,6 @@ 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 diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 1312c83373f..07bf9db5a34 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -4,6 +4,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic include RendersNotes before_action :apply_diff_view_cookie! + before_action :commit before_action :define_diff_vars before_action :define_diff_comment_vars @@ -28,6 +29,13 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic @diffs = @compare.diffs(diff_options) end + def commit + return nil unless commit_id = params[:commit_id].presence + return nil unless @merge_request.all_commit_shas.include?(commit_id) + + @commit ||= @project.commit(commit_id) + end + def find_merge_request_diff_compare @merge_request_diff = if diff_id = params[:diff_id].presence diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 361d56b211c..2d304f7eb91 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -231,9 +231,9 @@ module CommitsHelper def commit_path(project, commit, merge_request: nil) if merge_request&.persisted? - diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, commit_id: commit.id) + diffs_project_merge_request_path(project, merge_request, commit_id: commit.id) else - namespace_project_commit_path(project.namespace, project, commit.id) + project_commit_path(project, commit) end end end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 385b34120ef..5f8a1bf07e2 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -670,11 +670,10 @@ module SystemNoteService end def merge_request_commit_url(merge_request, commit) - url_helpers.diffs_namespace_project_merge_request_url( - merge_request.target_project.namespace, + url_helpers.diffs_project_merge_request_url( merge_request.target_project, - merge_request.iid, - commit_id: commit.id + merge_request, + commit_id: commit ) end end diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index f2414d43578..09934c09865 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -87,6 +87,6 @@ This commit is part of merge request = succeed '.' do - = link_to @merge_request.to_reference, namespace_project_merge_request_path(@project.namespace, @project, @merge_request) + = link_to @merge_request.to_reference, diffs_project_merge_request_path(@project, @merge_request, commit_id: @commit.id) Comments created here will be created in the context of that merge request. diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 022ded21362..45b4ef12ec9 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -4,13 +4,7 @@ - ref = local_assigns.fetch(:ref) { merge_request&.source_branch } - link = commit_path(project, commit, merge_request: merge_request) -- if @note_counts - - note_count = @note_counts.fetch(commit.id, 0) -- else - - notes = commit.notes - - note_count = notes.user.count - -- cache_key = [project.full_path, commit.id, current_application_settings, note_count, @path.presence, current_controller?(:commits), merge_request, I18n.locale] +- cache_key = [project.full_path, commit.id, current_application_settings, @path.presence, current_controller?(:commits), merge_request.iid, view_details, I18n.locale] - cache_key.push(commit.status(ref)) if commit.status(ref) = cache(cache_key, expires_in: 1.day) do @@ -55,4 +49,4 @@ = link_to_browse_code(project, commit) - if view_details && merge_request - = link_to "View details", namespace_project_commit_path(project.namespace, project, commit.id, merge_request_iid: merge_request.iid), class: "btn btn-default" + = link_to "View details", project_commit_path(project, commit.id, merge_request_iid: merge_request.iid), class: "btn btn-default" diff --git a/app/views/projects/merge_requests/diffs/_different_base.html.haml b/app/views/projects/merge_requests/diffs/_different_base.html.haml index aeeaa053c5f..0e57066f9c9 100644 --- a/app/views/projects/merge_requests/diffs/_different_base.html.haml +++ b/app/views/projects/merge_requests/diffs/_different_base.html.haml @@ -4,8 +4,8 @@ = icon('info-circle') Selected versions have different base commits. Changes will include - = link_to namespace_project_compare_path(@project.namespace, @project, from: @start_version.base_commit_sha, to: @merge_request_diff.base_commit_sha) do + = link_to project_compare_path(@project, from: @start_version.base_commit_sha, to: @merge_request_diff.base_commit_sha) do new commits from = succeed '.' do - %code= @merge_request.target_branch + %code.ref-name= @merge_request.target_branch diff --git a/app/views/projects/merge_requests/diffs/_diffs.html.haml b/app/views/projects/merge_requests/diffs/_diffs.html.haml index 63f6c3e6716..60c91024b23 100644 --- a/app/views/projects/merge_requests/diffs/_diffs.html.haml +++ b/app/views/projects/merge_requests/diffs/_diffs.html.haml @@ -6,11 +6,11 @@ - if @merge_request_diff&.empty? .nothing-here-block = image_tag 'illustrations/merge_request_changes_empty.svg' - %p - Nothing to merge from - %strong= @merge_request.source_branch - into - %strong= @merge_request.target_branch + = succeed '.' do + No changes between + %span.ref-name= @merge_request.source_branch + and + %span.ref-name= @merge_request.target_branch %p= link_to 'Create commit', project_new_blob_path(@project, @merge_request.source_branch), class: 'btn btn-save' - else - diff_viewable = @merge_request_diff ? @merge_request_diff.collected? || @merge_request_diff.overflow? : true 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 60c419a3cda..e4a1dc786b9 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 @@ -1,19 +1,17 @@ - if @commit || @start_version || (@merge_request_diff && !@merge_request_diff.latest?) .mr-version-controls - .content-block.comments-disabled-notif + .content-block.comments-disabled-notif.clearfix = icon('info-circle') - Not all comments are displayed because you're = succeed '.' do - if @commit - viewing only the changes in commit - - = link_to @commit.short_id, diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request, commit_id: @commit.id), class: "commit-sha" - - elsif @start_version - comparing two versions of the diff + Only comments from the following commit are shown below - else - viewing an old version of the diff - - .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 + Not all comments are displayed because you're + - if @start_version + comparing two versions of the diff + - else + viewing an old version of the diff + .pull-right + = link_to diffs_project_merge_request_path(@project, @merge_request), class: 'btn btn-sm' do + Show latest version + = "of the diff" if @commit diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index a918383ecd2..148f81b6a58 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -690,11 +690,20 @@ describe SystemNoteService do end describe '.new_commit_summary' do + let(:merge_request) { create(:merge_request, :simple, target_project: project, source_project: project) } + it 'escapes HTML titles' do commit = double(title: '
This is a test
', short_id: '12345678') - escaped = '* 12345678 - <pre>This is a test</pre>' + escaped = '<pre>This is a test</pre>' - expect(described_class.new_commit_summary([commit])).to eq([escaped]) + expect(described_class.new_commit_summary(merge_request, [commit])).to all(match(%r[- #{escaped}])) + end + + it 'contains the MR diffs commit url' do + commit = merge_request.commits.last + url = %r[/merge_requests/#{merge_request.iid}/diffs\?commit_id=#{commit.id}] + + expect(described_class.new_commit_summary(merge_request, [commit])).to all(match(url)) end end