Correct the cherry-pick message for merge commits

The list of commits must be generated from the merge request,
not from a diff of the branches.

5 files changed, 45 insertions, 23 deletions

diff --git a/app/models/commit.rb b/app/models/commit.rb
index f36e07fc3ca..5f59d088cd2 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -251,19 +251,16 @@ class Commit
     project.repository.next_branch("cherry-pick-#{short_id}", mild: true)
   end
 
-  def cherry_pick_description(start_project, start_branch_name)
+  def cherry_pick_description(user)
     message_buffer = "(cherry picked from commit #{sha})"
 
-    if merge_commit?
-      compare = CompareService.new(project, sha).execute(start_project, start_branch_name)
-
-      # Ignore the merge commit.
-      commits_in_merge = compare.commits[0..-2]
+    if merged_merge_request?(user)
+      commits_in_merge_request = merged_merge_request(user).commits
 
-      if commits_in_merge.present?
+      if commits_in_merge_request.present?
         message_buffer << "\n"
 
-        commits_in_merge.each do |commit_in_merge|
+        commits_in_merge_request.each do |commit_in_merge|
           message_buffer << "\n#{commit_in_merge.short_id} #{commit_in_merge.title}"
         end
       end
@@ -272,8 +269,8 @@ class Commit
     message_buffer
   end
 
-  def cherry_pick_message(start_project, start_branch_name)
-    %Q{#{message}\n\n#{cherry_pick_description(start_project, start_branch_name)}}
+  def cherry_pick_message(user)
+    %Q{#{message}\n\n#{cherry_pick_description(user)}}
   end
 
   def revert_description(user)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index c11f9d53497..7e38c06e6db 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -879,9 +879,7 @@ class Repository
 
       committer = user_to_committer(user)
 
-      commit_message = commit.cherry_pick_message(start_project, start_branch_name)
-
-      create_commit(message: commit_message,
+      create_commit(message: commit.cherry_pick_message(user),
                     author: {
                         email: commit.author_email,
                         name: commit.author_name,
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 9711222b3a1..98054b91c29 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -196,25 +196,52 @@ eos
   end
 
   describe '#cherry_pick_message' do
-    let(:regular_commit) { project.commit('video') }
-    let(:merge_commit) { project.commit('wip') }
+    let(:user) { create(:user) }
 
     context 'of a regular commit' do
-      it { expect(regular_commit.cherry_pick_message(project, 'master')).to include("\n\n(cherry picked from commit 88790590ed1337ab189bccaa355f068481c90bec)") }
+      let(:commit) { project.commit('video') }
+
+      it { expect(commit.cherry_pick_message(user)).to include("\n\n(cherry picked from commit 88790590ed1337ab189bccaa355f068481c90bec)") }
     end
 
     context 'of a merge commit' do
+      let(:repository) { project.repository }
+
+      let(:commit_options) do
+        author = repository.user_to_committer(user)
+        commit_options = { message: 'Test message', committer: author, author: author }
+      end
+
+      let(:merge_commit) do
+        merge_request = create(:merge_request,
+                               source_branch: 'feature',
+                               target_branch: 'master',
+                               source_project: project,
+                               author: user)
+
+        merge_commit_id = repository.merge(user,
+                                           merge_request.diff_head_sha,
+                                           merge_request,
+                                           commit_options)
+
+        merge_commit = repository.commit(merge_commit_id)
+
+        # Manually mark as completed.
+        #
+        merge_request.update(merge_commit_sha: merge_commit_id)
+
+        merge_commit
+      end
+
       it do
         expected_appended_text = <<~STR.rstrip
 
+          (cherry picked from commit #{merge_commit.sha})
 
-          (cherry picked from commit b9238ee5bf1d7359dd3b8c89fd76c1c7f8b75aba)
-
-          6d664995 This commit will be fixupped against
-          64117577 fixup! This commit will be fixupped against
+          0b4bc9a4 Feature added
         STR
 
-        expect(merge_commit.cherry_pick_message(project, 'master')).to include(expected_appended_text)
+        expect(merge_commit.cherry_pick_message(user)).to include(expected_appended_text)
       end
     end
   end
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 1fba7fcc10a..57baf9cd3b3 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -791,7 +791,7 @@ describe API::Commits do
           expect(response).to have_gitlab_http_status(201)
           expect(response).to match_response_schema('public_api/v4/commit/basic')
           expect(json_response['title']).to eq(commit.title)
-          expect(json_response['message']).to eq(commit.cherry_pick_message(project, branch))
+          expect(json_response['message']).to eq(commit.cherry_pick_message(user))
           expect(json_response['author_name']).to eq(commit.author_name)
           expect(json_response['committer_name']).to eq(user.name)
         end
diff --git a/spec/requests/api/v3/commits_spec.rb b/spec/requests/api/v3/commits_spec.rb
index 9edf21719cd..8fb96b3c7c5 100644
--- a/spec/requests/api/v3/commits_spec.rb
+++ b/spec/requests/api/v3/commits_spec.rb
@@ -474,7 +474,7 @@ describe API::V3::Commits do
 
         expect(response).to have_http_status(201)
         expect(json_response['title']).to eq(master_pickable_commit.title)
-        expect(json_response['message']).to eq(master_pickable_commit.cherry_pick_message(project, 'master'))
+        expect(json_response['message']).to eq(master_pickable_commit.cherry_pick_message(user))
         expect(json_response['author_name']).to eq(master_pickable_commit.author_name)
         expect(json_response['committer_name']).to eq(user.name)
       end