Merge branch 'all-commit-messages' into 'master'

Merge/squash commit templates: add %{all_commits} variable

See merge request gitlab-org/gitlab!81097

doc/user/project/merge_requests/commit_templates.md

@@ -69,6 +69,7 @@ GitLab creates a squash commit message with this template:
 > - [Added](https://gitlab.com/gitlab-org/gitlab/-/issues/346805) `first_commit` and `first_multiline_commit` variables in GitLab 14.6.
 > - [Added](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75639) `url`, `approved_by`, and `merged_by` variables in GitLab 14.7.
 > - [Added](https://gitlab.com/gitlab-org/gitlab/-/issues/20421) `co_authored_by` variable in GitLab 14.7.
+> - [Added](https://gitlab.com/gitlab-org/gitlab/-/issues/26303) `all_commits` variable in GitLab 14.9.
 
 Commit message templates support these variables:
 
@@ -86,6 +87,7 @@ Commit message templates support these variables:
 | `%{approved_by}`   | Line-separated list of the merge request approvers. This value is not updated until the first page refresh after an approval. | `Approved-by: Sidney Jones <sjones@example.com>` <br> `Approved-by: Zhang Wei <zwei@example.com>` |
 | `%{merged_by}`     | User who merged the merge request. | `Alex Garcia <agarcia@example.com>` |
 | `%{co_authored_by}` | Names and emails of commit authors in a `Co-authored-by` Git commit trailer format. Limited to authors of 100 most recent commits in merge request. | `Co-authored-by: Zane Doe <zdoe@example.com>` <br> `Co-authored-by: Blake Smith <bsmith@example.com>` |
+| `%{all_commits}`   | Messages from all commits in the merge request. Limited to 100 most recent commits. Skips commit bodies exceeding 100KiB and merge commit messages. | `* Feature introduced` <br><br> `This commit implements feature` <br> `Changelog:added` <br><br> `* Bug fixed` <br><br> `* Documentation improved` <br><br>`This commit introduced better docs.`|
 
 Any line containing only an empty variable is removed. If the line to be removed is both
 preceded and followed by an empty line, the preceding empty line is also removed.

lib/gitlab/merge_requests/commit_message_generator.rb

@@ -50,6 +50,19 @@ module Gitlab
                        .except(commit_author&.commit_email_or_default)
                        .map { |author_email, author_name| "Co-authored-by: #{author_name} <#{author_email}>" }
                        .join("\n")
+        end,
+        'all_commits' => -> (merge_request, _, _) do
+          merge_request
+            .recent_commits
+            .without_merge_commits
+            .map do |commit|
+              if commit.safe_message&.bytesize&.>(100.kilobytes)
+                "* #{commit.title}\n\n-- Skipped commit body exceeding 100KiB in size."
+              else
+                "* #{commit.safe_message&.strip}"
+              end
+            end
+            .join("\n\n")
         end
       }.freeze
 

spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb

@@ -403,6 +403,90 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
       end
     end
 
+    context 'when project has commit template with all_commits' do
+      let(message_template_name) { "All commits:\n%{all_commits}" }
+
+      it 'returns all commit messages' do
+        expect(result_message).to eq <<~MSG.rstrip
+          All commits:
+          * Feature added
+
+          Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
+        MSG
+      end
+
+      context 'with 2 commits' do
+        let(:source_branch) { 'fix' }
+
+        it 'returns both messages' do
+          expect(result_message).to eq <<~MSG.rstrip
+            All commits:
+            * Test file for directories with a leading dot
+
+            * JS fix
+
+            Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
+          MSG
+        end
+      end
+
+      context 'with over 100 commits' do
+        let(:source_branch) { 'signed-commits' }
+
+        it 'returns first 100 commits skipping merge commit' do
+          expected_message = <<~MSG
+            All commits:
+            * Multiple signatures commit
+
+            * Add conflicting file
+
+            * Add conflicting file
+
+          MSG
+          expected_message += (5..100).to_a.reverse
+                                .map { |n| "* Unrelated signed commit #{n} to exceed page size of endpoint\n\n" }
+                                .join.rstrip
+          expect(result_message).to eq expected_message
+        end
+      end
+
+      context 'when branch has no unmerged commits' do
+        let(:source_branch) { 'v1.1.0' }
+
+        it 'is an empty string' do
+          expect(result_message).to eq "All commits:\n"
+        end
+      end
+
+      context 'when branch has commit with message over 100kb' do
+        let(:source_branch) { 'add_commit_with_5mb_subject' }
+
+        it 'skips commit body' do
+          expect(result_message).to eq <<~MSG.rstrip
+            All commits:
+            * Commit with 5MB text subject
+
+            -- Skipped commit body exceeding 100KiB in size.
+
+            * Correct test_env.rb path for adding branch
+
+            * Add file with a _flattable_ path
+
+
+            (cherry picked from commit ce369011c189f62c815f5971d096b26759bab0d1)
+
+            * Add file larger than 1 mb
+
+            In order to test Max File Size push rule we need a file larger than 1 MB
+
+            * LFS tracks "*.lfs" through .gitattributes
+
+            * Update README.md to include `Usage in testing and development`
+          MSG
+        end
+      end
+    end
+
     context 'user' do
       subject { described_class.new(merge_request: merge_request, current_user: nil) }
 
@@ -466,6 +550,7 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
         approved_by:%{approved_by}
         merged_by:%{merged_by}
         co_authored_by:%{co_authored_by}
+        all_commits:%{all_commits}
       MSG
 
       it 'uses custom template' do
@@ -486,6 +571,9 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
           approved_by:
           merged_by:#{current_user.name} <#{current_user.commit_email_or_default}>
           co_authored_by:Co-authored-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
+          all_commits:* Feature added
+
+          Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
         MSG
       end
     end

spec/services/projects/branches_by_mode_service_spec.rb

@@ -13,20 +13,22 @@ RSpec.describe Projects::BranchesByModeService do
 
   describe '#execute' do
     context 'page is passed' do
-      let(:params) { { page: 4, mode: 'all', offset: 3 } }
+      let(:page) { (TestEnv::BRANCH_SHA.length.to_f / Kaminari.config.default_per_page).ceil }
+      let(:params) { { page: page, mode: 'all', offset: page - 1 } }
 
       it 'uses offset pagination' do
         expect(finder).to receive(:fetch_branches_via_offset_pagination).and_call_original
 
         branches, prev_page, next_page = subject
+        remaining = TestEnv::BRANCH_SHA.length % Kaminari.config.default_per_page
 
-        expect(branches.size).to eq(11)
+        expect(branches.size).to eq(remaining > 0 ? remaining : 20)
         expect(next_page).to be_nil
-        expect(prev_page).to eq("/#{project.full_path}/-/branches/all?offset=2&page=3")
+        expect(prev_page).to eq("/#{project.full_path}/-/branches/all?offset=#{page - 2}&page=#{page - 1}")
       end
 
       context 'but the page does not contain any branches' do
-        let(:params) { { page: 10, mode: 'all' } }
+        let(:params) { { page: 100, mode: 'all' } }
 
         it 'uses offset pagination' do
           expect(finder).to receive(:fetch_branches_via_offset_pagination).and_call_original
@@ -61,9 +63,10 @@ RSpec.describe Projects::BranchesByModeService do
         expect(finder).to receive(:fetch_branches_via_offset_pagination).and_call_original
 
         branches, prev_page, next_page = subject
+        expected_page_token = ERB::Util.url_encode(TestEnv::BRANCH_SHA.sort[19][0])
 
         expect(branches.size).to eq(20)
-        expect(next_page).to eq("/#{project.full_path}/-/branches/all?offset=1&page_token=conflict-resolvable")
+        expect(next_page).to eq("/#{project.full_path}/-/branches/all?offset=1&page_token=#{expected_page_token}")
         expect(prev_page).to be_nil
       end
     end
@@ -75,26 +78,31 @@ RSpec.describe Projects::BranchesByModeService do
 
       it 'returns branches for the first page' do
         branches, prev_page, next_page = subject
+        expected_page_token = ERB::Util.url_encode(TestEnv::BRANCH_SHA.sort[19][0])
 
         expect(branches.size).to eq(20)
-        expect(next_page).to eq("/#{project.full_path}/-/branches/all?offset=1&page_token=conflict-resolvable")
+        expect(next_page).to eq("/#{project.full_path}/-/branches/all?offset=1&page_token=#{expected_page_token}")
         expect(prev_page).to be_nil
       end
 
       context 'when second page is requested' do
-        let(:params) { { page_token: 'conflict-resolvable', mode: 'all', sort: 'name_asc', offset: 1 } }
+        let(:page_token) { 'conflict-resolvable' }
+        let(:params) { { page_token: page_token, mode: 'all', sort: 'name_asc', offset: 1 } }
 
         it 'returns branches for the first page' do
           branches, prev_page, next_page = subject
+          branch_index = TestEnv::BRANCH_SHA.sort.find_index { |a| a[0] == page_token }
+          expected_page_token = ERB::Util.url_encode(TestEnv::BRANCH_SHA.sort[20 + branch_index][0])
 
           expect(branches.size).to eq(20)
-          expect(next_page).to eq("/#{project.full_path}/-/branches/all?offset=2&page_token=improve%2Fawesome&sort=name_asc")
+          expect(next_page).to eq("/#{project.full_path}/-/branches/all?offset=2&page_token=#{expected_page_token}&sort=name_asc")
           expect(prev_page).to eq("/#{project.full_path}/-/branches/all?offset=0&page=1&sort=name_asc")
         end
       end
 
       context 'when last page is requested' do
-        let(:params) { { page_token: 'signed-commits', mode: 'all', sort: 'name_asc', offset: 4 } }
+        let(:page_token) { TestEnv::BRANCH_SHA.sort[-16][0] }
+        let(:params) { { page_token: page_token, mode: 'all', sort: 'name_asc', offset: 4 } }
 
         it 'returns branches after the specified branch' do
           branches, prev_page, next_page = subject

spec/support/helpers/test_env.rb

@@ -80,7 +80,8 @@ module TestEnv
     'invalid-utf8-diff-paths'            => '99e4853',
     'compare-with-merge-head-source'     => 'f20a03d',
     'compare-with-merge-head-target'     => '2f1e176',
-    'trailers'                           => 'f0a5ed6'
+    'trailers'                           => 'f0a5ed6',
+    'add_commit_with_5mb_subject'        => '8cf8e80'
   }.freeze
 
   # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily