Merge branch 'mycroft-gitlab' into 'master'

Display merged commit sha in fast-forward merge mode

See merge request gitlab-org/gitlab!41369

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

@@ -45,8 +45,8 @@ export default class MergeRequestStore {
     this.mergeTrainWhenPipelineSucceedsDocsPath = data.merge_train_when_pipeline_succeeds_docs_path;
     this.mergeStatus = data.merge_status;
     this.commitMessage = data.default_merge_commit_message;
-    this.shortMergeCommitSha = data.short_merge_commit_sha;
-    this.mergeCommitSha = data.merge_commit_sha;
+    this.shortMergeCommitSha = data.short_merged_commit_sha;
+    this.mergeCommitSha = data.merged_commit_sha;
     this.commitMessageWithDescription = data.default_merge_commit_message_with_description;
     this.commitsCount = data.commits_count;
     this.divergedCommitsCount = data.diverged_commits_count;
@@ -135,7 +135,7 @@ export default class MergeRequestStore {
     this.createIssueToResolveDiscussionsPath = data.create_issue_to_resolve_discussions_path;
     this.mergePath = data.merge_path;
     this.canMerge = Boolean(data.merge_path);
-    this.mergeCommitPath = data.merge_commit_path;
+    this.mergeCommitPath = data.merged_commit_path;
     this.canPushToSourceBranch = data.can_push_to_source_branch;
 
     if (data.work_in_progress !== undefined) {

app/models/merge_request.rb

@@ -1473,6 +1473,19 @@ class MergeRequest < ApplicationRecord
     Commit.truncate_sha(merge_commit_sha) if merge_commit_sha
   end
 
+  def merged_commit_sha
+    return unless merged?
+
+    sha = merge_commit_sha || squash_commit_sha || diff_head_sha
+    sha.presence
+  end
+
+  def short_merged_commit_sha
+    if sha = merged_commit_sha
+      Commit.truncate_sha(sha)
+    end
+  end
+
   def can_be_reverted?(current_user)
     return false unless merge_commit
     return false unless merged_at

app/serializers/merge_request_poll_cached_widget_entity.rb

@@ -3,8 +3,8 @@
 class MergeRequestPollCachedWidgetEntity < IssuableEntity
   expose :auto_merge_enabled
   expose :state
-  expose :merge_commit_sha
-  expose :short_merge_commit_sha
+  expose :merged_commit_sha
+  expose :short_merged_commit_sha
   expose :merge_error
   expose :public_merge_status, as: :merge_status
   expose :merge_user_id
@@ -56,9 +56,9 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity
     presenter(merge_request).target_branch_tree_path
   end
 
-  expose :merge_commit_path do |merge_request|
-    if merge_request.merge_commit_sha
-      project_commit_path(merge_request.project, merge_request.merge_commit_sha)
+  expose :merged_commit_path do |merge_request|
+    if sha = merge_request.merged_commit_sha
+      project_commit_path(merge_request.project, sha)
     end
   end
 

changelogs/unreleased/display-merged-commit-sha-in-fast-forward-mode.yml

+---
+title: Display merged commit sha in fast-forward merge mode
+merge_request: 41369
+author: Mycroft Kang @TaehyeokKang
+type: added

spec/features/merge_request/user_accepts_merge_request_spec.rb

@@ -12,12 +12,38 @@ RSpec.describe 'User accepts a merge request', :js, :sidekiq_might_not_need_inli
     sign_in(user)
   end
 
-  it 'presents merged merge request content' do
+  context 'presents merged merge request content' do
+    it 'when merge method is set to merge commit' do
       visit(merge_request_path(merge_request))
 
       click_button('Merge')
 
-    expect(page).to have_content("The changes were merged into #{merge_request.target_branch} with #{merge_request.short_merge_commit_sha}")
+      expect(page).to have_content("The changes were merged into #{merge_request.target_branch} with #{merge_request.short_merged_commit_sha}")
+    end
+
+    context 'when merge method is set to fast-forward merge' do
+      let(:project) { create(:project, :public, :repository, merge_requests_ff_only_enabled: true) }
+
+      it 'accepts a merge request with rebase and merge' do
+        merge_request = create(:merge_request, :rebased, source_project: project)
+
+        visit(merge_request_path(merge_request))
+
+        click_button('Merge')
+
+        expect(page).to have_content("The changes were merged into #{merge_request.target_branch} with #{merge_request.short_merged_commit_sha}")
+      end
+
+      it 'accepts a merge request with squash and merge' do
+        merge_request = create(:merge_request, :rebased, source_project: project, squash: true)
+
+        visit(merge_request_path(merge_request))
+
+        click_button('Merge')
+
+        expect(page).to have_content("The changes were merged into #{merge_request.target_branch} with #{merge_request.short_merged_commit_sha}")
+      end
+    end
   end
 
   context 'with removing the source branch' do

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

@@ -7,8 +7,8 @@
     "title": { "type": "string" },
     "auto_merge_enabled": { "type": "boolean" },
     "state": { "type": "string" },
-    "merge_commit_sha": { "type": ["string", "null"] },
-    "short_merge_commit_sha": { "type": ["string", "null"] },
+    "merged_commit_sha": { "type": ["string", "null"] },
+    "short_merged_commit_sha": { "type": ["string", "null"] },
     "merge_error": { "type": ["string", "null"] },
     "merge_status": { "type": "string" },
     "merge_user_id": { "type": ["integer", "null"] },
@@ -40,7 +40,7 @@
     "diverged_commits_count": { "type": "integer" },
     "target_branch_commits_path": { "type": "string" },
     "target_branch_tree_path": { "type": "string" },
-    "merge_commit_path": { "type": ["string", "null"] },
+    "merged_commit_path": { "type": ["string", "null"] },
     "source_branch_with_namespace_link": { "type": "string" },
     "source_branch_path": { "type": "string" }
   }

spec/models/merge_request_spec.rb

@@ -2160,6 +2160,60 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
   end
 
+  describe '#merged_commit_sha' do
+    it 'returns nil when not merged' do
+      expect(subject.merged_commit_sha).to be_nil
+    end
+
+    context 'when the MR is merged' do
+      let(:sha) { 'f7ce827c314c9340b075657fd61c789fb01cf74d' }
+
+      before do
+        subject.mark_as_merged!
+      end
+
+      it 'returns merge_commit_sha when there is a merge_commit_sha' do
+        subject.update_attribute(:merge_commit_sha, sha)
+
+        expect(subject.merged_commit_sha).to eq(sha)
+      end
+
+      it 'returns squash_commit_sha when there is a squash_commit_sha' do
+        subject.update_attribute(:squash_commit_sha, sha)
+
+        expect(subject.merged_commit_sha).to eq(sha)
+      end
+
+      it 'returns diff_head_sha when there are no merge_commit_sha and squash_commit_sha' do
+        allow(subject).to receive(:diff_head_sha).and_return(sha)
+
+        expect(subject.merged_commit_sha).to eq(sha)
+      end
+    end
+  end
+
+  describe '#short_merged_commit_sha' do
+    context 'when merged_commit_sha is nil' do
+      before do
+        allow(subject).to receive(:merged_commit_sha).and_return(nil)
+      end
+
+      it 'returns nil' do
+        expect(subject.short_merged_commit_sha).to be_nil
+      end
+    end
+
+    context 'when merged_commit_sha is present' do
+      before do
+        allow(subject).to receive(:merged_commit_sha).and_return('f7ce827c314c9340b075657fd61c789fb01cf74d')
+      end
+
+      it 'returns shortened merged_commit_sha' do
+        expect(subject.short_merged_commit_sha).to eq('f7ce827c')
+      end
+    end
+  end
+
   describe '#can_be_reverted?' do
     subject { create(:merge_request, source_project: create(:project, :repository)) }