Merge branch 'sm-cherry-pick-list-commits-in-message' into 'master'

Add 'from commit' information to cherry-picked commits

See merge request !13475

7 files changed, 95 insertions, 4 deletions

diff --git a/app/models/commit.rb b/app/models/commit.rb
index d41c88b4e30..c943365016f 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -251,6 +251,28 @@ class Commit
     project.repository.next_branch("cherry-pick-#{short_id}", mild: true)
   end
 
+  def cherry_pick_description(user)
+    message_body = "(cherry picked from commit #{sha})"
+
+    if merged_merge_request?(user)
+      commits_in_merge_request = merged_merge_request(user).commits
+
+      if commits_in_merge_request.present?
+        message_body << "\n"
+
+        commits_in_merge_request.reverse.each do |commit_in_merge|
+          message_body << "\n#{commit_in_merge.short_id} #{commit_in_merge.title}"
+        end
+      end
+    end
+
+    message_body
+  end
+
+  def cherry_pick_message(user)
+    %Q{#{message}\n\n#{cherry_pick_description(user)}}
+  end
+
   def revert_description(user)
     if merged_merge_request?(user)
       "This reverts merge request #{merged_merge_request(user).to_reference}"
diff --git a/app/models/repository.rb b/app/models/repository.rb
index b3fa51a14f7..5474c8eeb68 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -908,7 +908,7 @@ class Repository
 
       committer = user_to_committer(user)
 
-      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/changelogs/unreleased/sm-cherry-pick-list-commits-in-message.yml b/changelogs/unreleased/sm-cherry-pick-list-commits-in-message.yml
new file mode 100644
index 00000000000..602ca358b8b
--- /dev/null
+++ b/changelogs/unreleased/sm-cherry-pick-list-commits-in-message.yml
@@ -0,0 +1,5 @@
+---
+title: Add 'from commit' information to cherry-picked commits
+merge_request: 13475
+author: Saverio Miroddi
+type: added
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index c18c635d811..11e64a0f877 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -195,6 +195,67 @@ eos
     it { expect(data[:removed]).to eq([]) }
   end
 
+  describe '#cherry_pick_message' do
+    let(:user) { create(:user) }
+
+    context 'of a regular commit' do
+      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)
+        { message: 'Test message', committer: author, author: author }
+      end
+
+      let(:merge_request) do
+        create(:merge_request,
+               source_branch: 'video',
+               target_branch: 'master',
+               source_project: project,
+               author: user)
+      end
+
+      let(:merge_commit) do
+        merge_commit_id = repository.merge(user,
+                                           merge_request.diff_head_sha,
+                                           merge_request,
+                                           commit_options)
+
+        repository.commit(merge_commit_id)
+      end
+
+      context 'that is found' do
+        before do
+          # Artificially mark as completed.
+          merge_request.update(merge_commit_sha: merge_commit.id)
+        end
+
+        it do
+          expected_appended_text = <<~STR.rstrip
+
+            (cherry picked from commit #{merge_commit.sha})
+
+            467dc98f Add new 'videos' directory
+            88790590 Upload new video file
+          STR
+
+          expect(merge_commit.cherry_pick_message(user)).to include(expected_appended_text)
+        end
+      end
+
+      context "that is existing but not found" do
+        it 'does not include details of the merged commits' do
+          expect(merge_commit.cherry_pick_message(user)).to end_with("(cherry picked from commit #{merge_commit.sha})")
+        end
+      end
+    end
+  end
+
   describe '#reverts_commit?' do
     let(:another_commit) { double(:commit, revert_description: "This reverts commit #{commit.sha}") }
     let(:user) { commit.author }
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 34e1a955309..40875c8fb7e 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1379,8 +1379,11 @@ describe Repository, models: true do
       it 'cherry-picks the changes' do
         expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).to be_nil
 
-        repository.cherry_pick(user, pickable_merge, 'improve/awesome')
+        cherry_pick_commit_sha = repository.cherry_pick(user, pickable_merge, 'improve/awesome')
+        cherry_pick_commit_message = project.commit(cherry_pick_commit_sha).message
+
         expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).not_to be_nil
+        expect(cherry_pick_commit_message).to include('cherry picked from')
       end
     end
   end
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index dafe3f466a2..d3b48f948f6 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -804,7 +804,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.message)
+          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 4a4a5dc5c7c..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.message)
+        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