Merge branch 'osw-41401-render-mr-commit-sha-instead-diffs' into 'master'

Render MR commit SHA instead "diffs" when viable on GFM

See merge request gitlab-org/gitlab-ce!18141

changelogs/unreleased/osw-41401-render-mr-commit-sha-instead-diffs.yml

+---
+title: Render MR commit SHA instead "diffs" when viable
+merge_request:
+author:
+type: added

lib/banzai/filter/abstract_reference_filter.rb

@@ -171,7 +171,7 @@ module Banzai
           end
 
           if object
-            title = object_link_title(object)
+            title = object_link_title(object, matches)
             klass = reference_class(object_sym)
 
             data = data_attributes_for(link_content || match, parent, object,
@@ -216,7 +216,7 @@ module Banzai
         extras
       end
 
-      def object_link_title(object)
+      def object_link_title(object, matches)
         object.title
       end
 

lib/banzai/filter/commit_range_reference_filter.rb

@@ -34,7 +34,7 @@ module Banzai
                                         range.to_param.merge(only_path: context[:only_path]))
       end
 
-      def object_link_title(range)
+      def object_link_title(range, matches)
         nil
       end
     end

lib/banzai/filter/label_reference_filter.rb

@@ -77,7 +77,7 @@ module Banzai
         CGI.unescapeHTML(text.to_s)
       end
 
-      def object_link_title(object)
+      def object_link_title(object, matches)
         # use title of wrapped element instead
         nil
       end

lib/banzai/filter/merge_request_reference_filter.rb

@@ -17,10 +17,19 @@ module Banzai
                                             only_path: context[:only_path])
       end
 
+      def object_link_title(object, matches)
+        object_link_commit_title(object, matches) || super
+      end
+
       def object_link_text_extras(object, matches)
         extras = super
 
+        if commit_ref = object_link_commit_ref(object, matches)
+          return extras.unshift(commit_ref)
+        end
+
         path = matches[:path] if matches.names.include?("path")
+
         case path
         when '/diffs'
           extras.unshift "diffs"
@@ -38,6 +47,36 @@ module Banzai
           .where(iid: ids.to_a)
           .includes(target_project: :namespace)
       end
+
+      private
+
+      def object_link_commit_title(object, matches)
+        object_link_commit(object, matches)&.title
+      end
+
+      def object_link_commit_ref(object, matches)
+        object_link_commit(object, matches)&.short_id
+      end
+
+      def object_link_commit(object, matches)
+        return unless matches.names.include?('query') && query = matches[:query]
+
+        # Removes leading "?". CGI.parse expects "arg1&arg2&arg3"
+        params = CGI.parse(query.sub(/^\?/, ''))
+
+        return unless commit_sha = params['commit_id']&.first
+
+        if commit = find_commit_by_sha(object, commit_sha)
+          Commit.from_hash(commit.to_hash, object.project)
+        end
+      end
+
+      def find_commit_by_sha(object, commit_sha)
+        @all_commits ||= {}
+        @all_commits[object.id] ||= object.all_commits
+
+        @all_commits[object.id].find { |commit| commit.sha == commit_sha }
+      end
     end
   end
 end

lib/banzai/filter/milestone_reference_filter.rb

@@ -84,7 +84,7 @@ module Banzai
         end
       end
 
-      def object_link_title(object)
+      def object_link_title(object, matches)
         nil
       end
     end

spec/lib/banzai/filter/merge_request_reference_filter_spec.rb

@@ -196,6 +196,41 @@ describe Banzai::Filter::MergeRequestReferenceFilter do
     end
   end
 
+  context 'URL reference for a commit' do
+    let(:mr) { create(:merge_request, :with_diffs) }
+    let(:reference) do
+      urls.project_merge_request_url(mr.project, mr) + "/diffs?commit_id=#{mr.diff_head_sha}"
+    end
+    let(:commit) { mr.commits.find { |commit| commit.sha == mr.diff_head_sha } }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href'))
+        .to eq reference
+    end
+
+    it 'has valid text' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.text).to eq("See #{mr.to_reference(full: true)} (#{commit.short_id})")
+    end
+
+    it 'has valid title attribute' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('title')).to eq(commit.title)
+    end
+
+    it 'ignores invalid commit short_ids on link text' do
+      invalidate_commit_reference =
+        urls.project_merge_request_url(mr.project, mr) + "/diffs?commit_id=12345678"
+      doc = reference_filter("See #{invalidate_commit_reference}")
+
+      expect(doc.text).to eq("See #{mr.to_reference(full: true)} (diffs)")
+    end
+  end
+
   context 'cross-project URL reference' do
     let(:namespace) { create(:namespace, name: 'cross-reference') }
     let(:project2)  { create(:project, :public, namespace: namespace) }