corrects the url building

10 files changed, 45 insertions, 43 deletions

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: '<pre>This is a test</pre>', short_id: '12345678')
-      escaped = '* 12345678 - &lt;pre&gt;This is a test&lt;&#x2F;pre&gt;'
+      escaped = '&lt;pre&gt;This is a test&lt;&#x2F;pre&gt;'
+
+      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([commit])).to eq([escaped])
+      expect(described_class.new_commit_summary(merge_request, [commit])).to all(match(url))
     end
   end