Merge branch 'mr-branch-link-use-tree' into 'master'

MR branch link now links to tree instead of commits

Closes #34243

See merge request !12763

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.js

@@ -92,13 +92,13 @@ export default {
             :class="{'label-truncated has-tooltip': isBranchTitleLong(mr.targetBranch)}"
             :title="isBranchTitleLong(mr.targetBranch) ? mr.targetBranch : ''"
             data-placement="bottom">
-            <a :href="mr.targetBranchPath">{{mr.targetBranch}}</a>
+            <a :href="mr.targetBranchTreePath">{{mr.targetBranch}}</a>
           </span>
         </strong>
         <span
           v-if="shouldShowCommitsBehindText"
           class="diverged-commits-count">
-          ({{mr.divergedCommitsCount}} {{commitsText}} behind)
+          (<a :href="mr.targetBranchPath">{{mr.divergedCommitsCount}} {{commitsText}} behind</a>)
         </span>
       </div>
     </div>

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -48,6 +48,7 @@ export default class MergeRequestStore {
     this.sourceBranchLink = data.source_branch_with_namespace_link;
     this.mergeError = data.merge_error;
     this.targetBranchPath = data.target_branch_commits_path;
+    this.targetBranchTreePath = data.target_branch_tree_path;
     this.conflictResolutionPath = data.conflict_resolution_path;
     this.cancelAutoMergePath = data.cancel_merge_when_pipeline_succeeds_path;
     this.removeWIPPath = data.remove_wip_path;

app/presenters/merge_request_presenter.rb

@@ -76,6 +76,12 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
     end
   end
 
+  def target_branch_tree_path
+    if target_branch_exists?
+      project_tree_path(project, target_branch)
+    end
+  end
+
   def target_branch_commits_path
     if target_branch_exists?
       project_commits_path(project, target_branch)
@@ -94,7 +100,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
 
     if source_branch_exists?
       namespace = link_to(namespace, project_path(source_project))
-      branch = link_to(branch, project_commits_path(source_project, source_branch))
+      branch = link_to(branch, project_tree_path(source_project, source_branch))
     end
 
     if for_fork?

app/serializers/merge_request_entity.rb

@@ -97,6 +97,10 @@ class MergeRequestEntity < IssuableEntity
     presenter(merge_request).target_branch_commits_path
   end
 
+  expose :target_branch_tree_path do |merge_request|
+    presenter(merge_request).target_branch_tree_path
+  end
+
   expose :new_blob_path do |merge_request|
     if can?(current_user, :push_code, merge_request.project)
       project_new_blob_path(merge_request.project, merge_request.source_branch)

changelogs/unreleased/mr-branch-link-use-tree.yml

+---
+title: MR branch link now links to tree instead of commits
+merge_request:
+author:

lib/gitlab/git/repository.rb

@@ -930,7 +930,7 @@ module Gitlab
 
         return unless commit_object && commit_object.type == :COMMIT
 
-        gitmodules = gitaly_commit_client.tree_entry(ref, '.gitmodules', Blob::MAX_DATA_DISPLAY_SIZE)
+        gitmodules = gitaly_commit_client.tree_entry(ref, '.gitmodules', Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE)
         found_module = GitmodulesParser.new(gitmodules.data).parse[path]
 
         found_module && found_module['url']

spec/fixtures/api/schemas/entities/merge_request.json

@@ -75,6 +75,7 @@
       "additionalProperties": false
     },
     "target_branch_commits_path": { "type": "string" },
+    "target_branch_tree_path": { "type": "string" },
     "source_branch_path": { "type": "string" },
     "conflict_resolution_path": { "type": ["string", "null"] },
     "cancel_merge_when_pipeline_succeeds_path": { "type": "string" },

spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js

@@ -54,6 +54,7 @@ describe('MRWidgetHeader', () => {
       sourceBranch: 'mr-widget-refactor',
       sourceBranchLink: `<a href="${sourceBranchPath}">mr-widget-refactor</a>`,
       targetBranchPath: 'foo/bar/commits-path',
+      targetBranchTreePath: 'foo/bar/tree/path',
       targetBranch: 'master',
       isOpen: true,
       emailPatchesPath: '/mr/email-patches',
@@ -69,12 +70,14 @@ describe('MRWidgetHeader', () => {
       expect(el.classList.contains('mr-source-target')).toBeTruthy();
       const sourceBranchLink = el.querySelectorAll('.label-branch')[0];
       const targetBranchLink = el.querySelectorAll('.label-branch')[1];
+      const commitsCount = el.querySelector('.diverged-commits-count');
 
       expect(sourceBranchLink.textContent).toContain(mr.sourceBranch);
       expect(targetBranchLink.textContent).toContain(mr.targetBranch);
       expect(sourceBranchLink.querySelector('a').getAttribute('href')).toEqual(sourceBranchPath);
-      expect(targetBranchLink.querySelector('a').getAttribute('href')).toEqual(mr.targetBranchPath);
-      expect(el.querySelector('.diverged-commits-count').textContent).toContain('12 commits behind');
+      expect(targetBranchLink.querySelector('a').getAttribute('href')).toEqual(mr.targetBranchTreePath);
+      expect(commitsCount.textContent).toContain('12 commits behind');
+      expect(commitsCount.querySelector('a').getAttribute('href')).toEqual(mr.targetBranchPath);
 
       expect(el.textContent).toContain('Check out branch');
       expect(el.querySelectorAll('.dropdown li a')[0].getAttribute('href')).toEqual(mr.emailPatchesPath);

spec/presenters/merge_request_presenter_spec.rb

@@ -332,7 +332,31 @@ describe MergeRequestPresenter do
       end
     end
 
-    context 'when target branch does not exists' do
+    context 'when target branch does not exist' do
+      it 'returns nil' do
+        allow(resource).to receive(:target_branch_exists?) { false }
+
+        is_expected.to be_nil
+      end
+    end
+  end
+
+  describe '#target_branch_tree_path' do
+    subject do
+      described_class.new(resource, current_user: user)
+        .target_branch_tree_path
+    end
+
+    context 'when target branch exists' do
+      it 'returns path' do
+        allow(resource).to receive(:target_branch_exists?) { true }
+
+        is_expected
+          .to eq("/#{resource.target_project.full_path}/tree/#{resource.target_branch}")
+      end
+    end
+
+    context 'when target branch does not exist' do
       it 'returns nil' do
         allow(resource).to receive(:target_branch_exists?) { false }
 
@@ -355,7 +379,7 @@ describe MergeRequestPresenter do
       end
     end
 
-    context 'when source branch does not exists' do
+    context 'when source branch does not exist' do
       it 'returns nil' do
         allow(resource).to receive(:source_branch_exists?) { false }
 
@@ -363,4 +387,17 @@ describe MergeRequestPresenter do
       end
     end
   end
+
+  describe '#source_branch_with_namespace_link' do
+    subject do
+      described_class.new(resource, current_user: user).source_branch_with_namespace_link
+    end
+
+    it 'returns link' do
+      allow(resource).to receive(:source_branch_exists?) { true }
+
+      is_expected
+        .to eq("<a href=\"/#{resource.source_project.full_path}/tree/#{resource.source_branch}\">#{resource.source_branch}</a>")
+    end
+  end
 end

spec/serializers/merge_request_entity_spec.rb

@@ -47,7 +47,7 @@ describe MergeRequestEntity do
                                :cancel_merge_when_pipeline_succeeds_path,
                                :create_issue_to_resolve_discussions_path,
                                :source_branch_path, :target_branch_commits_path,
-                               :commits_count)
+                               :target_branch_tree_path, :commits_count)
   end
 
   it 'has email_patches_path' do