Merge branch '24010-Add-named-anchors-for-diffs-in-the-commit-page' into 'master'

Adds hash anchors to diff_files to make possible linking exact file in commit

Closes #24010

See merge request !7298

9 files changed, 37 insertions, 25 deletions

diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js
index 5ca4b34909a..509e3aa362a 100644
--- a/app/assets/javascripts/merge_request_tabs.js
+++ b/app/assets/javascripts/merge_request_tabs.js
@@ -145,7 +145,8 @@
       if (action === 'show') {
         action = 'notes';
       }
-      $(".merge-request-tabs a[data-action='" + action + "']").tab('show').trigger('shown.bs.tab');
+      // important note: the .tab('show') method triggers 'shown.bs.tab' event itself
+      $(".merge-request-tabs a[data-action='" + action + "']").tab('show');
     };
 
     // Replaces the current Merge Request-specific action in the URL with a new one
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index b78cc6585ba..56ced786311 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -42,7 +42,7 @@ class Projects::BlobController < Projects::ApplicationController
     after_edit_path =
       if from_merge_request && @target_branch == @ref
         diffs_namespace_project_merge_request_path(from_merge_request.target_project.namespace, from_merge_request.target_project, from_merge_request) +
-          "#file-path-#{hexdigest(@path)}"
+          "##{hexdigest(@path)}"
       else
         namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @path))
       end
diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml
index c0c07d65daa..307c5a11206 100644
--- a/app/views/notify/repository_push_email.html.haml
+++ b/app/views/notify/repository_push_email.html.haml
@@ -27,9 +27,9 @@
   %h4 #{pluralize @message.diffs_count, "changed file"}:
 
   %ul
-    - @message.diffs.each_with_index do |diff, i|
+    - @message.diffs.each do |diff|
       %li.file-stats
-        %a{href: "#{@message.target_url if @message.disable_diffs?}#diff-#{i}" }
+        %a{href: "#{@message.target_url if @message.disable_diffs?}##{hexdigest(diff.file_path)}" }
           - if diff.deleted_file
             %span.deleted-file
               &minus;
@@ -52,9 +52,10 @@
       %h5 The diff was not included because it is too large.
     - else
       %h4 Changes:
-      - diff_files.each_with_index do |diff_file, i|
-        %li{id: "diff-#{i}"}
-          %a{href: @message.target_url + "#diff-#{i}"}<
+      - diff_files.each do |diff_file|
+        - file_hash = hexdigest(diff_file.file_path)
+        %li{id: file_hash}
+          %a{href: @message.target_url + "##{file_hash}"}<
             - if diff_file.deleted_file
               %strong<
                 = diff_file.old_path
diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml
index 067cf595da3..ab4a2dc36e5 100644
--- a/app/views/projects/diffs/_diffs.html.haml
+++ b/app/views/projects/diffs/_diffs.html.haml
@@ -22,11 +22,12 @@
   = render 'projects/diffs/warning', diff_files: diff_files
 
 .files{ data: { can_create_note: can_create_note } }
-  - diff_files.each_with_index do |diff_file, index|
+  - diff_files.each_with_index do |diff_file|
     - diff_commit = commit_for_diff(diff_file)
     - blob = diff_file.blob(diff_commit)
     - next unless blob
     - blob.load_all_data!(diffs.project.repository) unless blob.only_display_raw?
+    - file_hash = hexdigest(diff_file.file_path)
 
-    = render 'projects/diffs/file', index: index, project: diffs.project,
+    = render 'projects/diffs/file', file_hash: file_hash, project: diffs.project,
       diff_file: diff_file, diff_commit: diff_commit, blob: blob
diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml
index 8f4f9ad4a80..120ba9ffcd2 100644
--- a/app/views/projects/diffs/_file.html.haml
+++ b/app/views/projects/diffs/_file.html.haml
@@ -1,6 +1,6 @@
-.diff-file.file-holder{id: "diff-#{index}", data: diff_file_html_data(project, diff_file.file_path, diff_commit.id)}
+.diff-file.file-holder{id: file_hash, data: diff_file_html_data(project, diff_file.file_path, diff_commit.id)}
   .file-title{id: "file-path-#{hexdigest(diff_file.file_path)}"}
-    = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_commit, project: project, url: "#diff-#{index}"
+    = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_commit, project: project, url: "##{file_hash}"
 
     - unless diff_file.submodule?
       .file-actions.hidden-xs
diff --git a/app/views/projects/diffs/_stats.html.haml b/app/views/projects/diffs/_stats.html.haml
index e751dabdf99..66d6254aa1e 100644
--- a/app/views/projects/diffs/_stats.html.haml
+++ b/app/views/projects/diffs/_stats.html.haml
@@ -9,28 +9,29 @@
     %strong.cred #{diff_files.sum(&:removed_lines)} deletions
   .file-stats.js-toggle-content.hide
     %ul
-      - diff_files.each_with_index do |diff_file, i|
+      - diff_files.each do |diff_file|
+        - file_hash = hexdigest(diff_file.file_path)
         %li
           - if diff_file.deleted_file
             %span.deleted-file
-              %a{href: "#diff-#{i}"}
+              %a{href: "##{file_hash}"}
                 %i.fa.fa-minus
                 = diff_file.old_path
           - elsif diff_file.renamed_file
             %span.renamed-file
-              %a{href: "#diff-#{i}"}
+              %a{href: "##{file_hash}"}
                 %i.fa.fa-minus
                 = diff_file.old_path
                 &rarr;
                 = diff_file.new_path
           - elsif diff_file.new_file
             %span.new-file
-              %a{href: "#diff-#{i}"}
+              %a{href: "##{file_hash}"}
                 %i.fa.fa-plus
                 = diff_file.new_path
           - else
             %span.edit-file
-              %a{href: "#diff-#{i}"}
+              %a{href: "##{file_hash}"}
                 %i.fa.fa-adjust
                 = diff_file.new_path
 
diff --git a/changelogs/unreleased/24010-change-anchor-link-to-mr-diff.yml b/changelogs/unreleased/24010-change-anchor-link-to-mr-diff.yml
new file mode 100644
index 00000000000..33ce18b2141
--- /dev/null
+++ b/changelogs/unreleased/24010-change-anchor-link-to-mr-diff.yml
@@ -0,0 +1,4 @@
+---
+title: Unify anchor link format for MR diff files
+merge_request: 7298
+author: YarNayar
diff --git a/changelogs/unreleased/24010-double-event-trigger.yml b/changelogs/unreleased/24010-double-event-trigger.yml
new file mode 100644
index 00000000000..3c2f20d391f
--- /dev/null
+++ b/changelogs/unreleased/24010-double-event-trigger.yml
@@ -0,0 +1,4 @@
+---
+title: Fix double event and ajax request call on MR page
+merge_request: 7298
+author: YarNayar
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index 2ccab4334eb..f728d243cdc 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -413,37 +413,37 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I click link "Hide inline discussion" of the third file' do
-    page.within '.files [id^=diff]:nth-child(3)' do
+    page.within '.files>div:nth-child(3)' do
       find('.js-toggle-diff-comments').trigger('click')
     end
   end
 
   step 'I click link "Show inline discussion" of the third file' do
-    page.within '.files [id^=diff]:nth-child(3)' do
+    page.within '.files>div:nth-child(3)' do
       find('.js-toggle-diff-comments').trigger('click')
     end
   end
 
   step 'I should not see a comment like "Line is wrong" in the third file' do
-    page.within '.files [id^=diff]:nth-child(3)' do
+    page.within '.files>div:nth-child(3)' do
       expect(page).not_to have_visible_content "Line is wrong"
     end
   end
 
   step 'I should see a comment like "Line is wrong" in the third file' do
-    page.within '.files [id^=diff]:nth-child(3) .note-body > .note-text' do
+    page.within '.files>div:nth-child(3) .note-body > .note-text' do
       expect(page).to have_visible_content "Line is wrong"
     end
   end
 
   step 'I should not see a comment like "Line is wrong here" in the third file' do
-    page.within '.files [id^=diff]:nth-child(3)' do
+    page.within '.files>div:nth-child(3)' do
       expect(page).not_to have_visible_content "Line is wrong here"
     end
   end
 
   step 'I should see a comment like "Line is wrong here" in the third file' do
-    page.within '.files [id^=diff]:nth-child(3) .note-body > .note-text' do
+    page.within '.files>div:nth-child(3) .note-body > .note-text' do
       expect(page).to have_visible_content "Line is wrong here"
     end
   end
@@ -456,7 +456,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
       click_button "Comment"
     end
 
-    page.within ".files [id^=diff]:nth-child(2) .note-body > .note-text" do
+    page.within ".files>div:nth-child(2) .note-body > .note-text" do
       expect(page).to have_content "Line is correct"
     end
   end
@@ -471,7 +471,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I should still see a comment like "Line is correct" in the second file' do
-    page.within '.files [id^=diff]:nth-child(2) .note-body > .note-text' do
+    page.within '.files>div:nth-child(2) .note-body > .note-text' do
       expect(page).to have_visible_content "Line is correct"
     end
   end
@@ -494,7 +494,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I should see comments on the side-by-side diff page' do
-    page.within '.files [id^=diff]:nth-child(2) .parallel .note-body > .note-text' do
+    page.within '.files>div:nth-child(2) .parallel .note-body > .note-text' do
       expect(page).to have_visible_content "Line is correct"
     end
   end