Merge branch 'allow-squashing-single-commit' into 'master'

Allow squashing in MRs with single commit

See merge request gitlab-org/gitlab!74464

app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue

@@ -287,7 +287,7 @@ export default {
         return false;
       }
 
-      return enableSquashBeforeMerge && this.commitsCount > 1;
+      return enableSquashBeforeMerge;
     },
     shouldShowMergeControls() {
       if (this.glFeatures.restructuredMrWidget) {

app/services/merge_requests/squash_service.rb

@@ -5,7 +5,7 @@ module MergeRequests
     def execute
       # If performing a squash would result in no change, then
       # immediately return a success message without performing a squash
-      if merge_request.commits_count < 2 && message.nil?
+      if merge_request.commits_count == 1 && message == merge_request.first_commit.safe_message
         return success(squash_sha: merge_request.diff_head_sha)
       end
 
@@ -17,7 +17,7 @@ module MergeRequests
     private
 
     def squash!
-      squash_sha = repository.squash(current_user, merge_request, message || merge_request.default_squash_commit_message)
+      squash_sha = repository.squash(current_user, merge_request, message)
 
       success(squash_sha: squash_sha)
     rescue StandardError => e
@@ -39,7 +39,7 @@ module MergeRequests
     end
 
     def message
-      params[:squash_commit_message].presence
+      params[:squash_commit_message].presence || merge_request.default_squash_commit_message
     end
   end
 end

spec/features/merge_request/user_squashes_merge_request_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe 'User squashes a merge request', :js do
                                                   committer_name: user.name)
 
       merge_commit = an_object_having_attributes(sha: a_string_matching(/\h{40}/),
-                                                 message: a_string_starting_with("Merge branch 'csv' into 'master'"),
+                                                 message: a_string_starting_with("Merge branch '#{source_branch}' into 'master'"),
                                                  author_name: user.name,
                                                  committer_name: user.name)
 
@@ -57,34 +57,34 @@ RSpec.describe 'User squashes a merge request', :js do
   end
 
   context 'when the MR has only one commit' do
-    let(:source_branch) { 'master' }
-    let(:target_branch) { 'branch-merged' }
-    let(:protected_source_branch) { true }
+    let(:source_branch) { 'feature' }
+    let(:target_branch) { 'master' }
     let(:source_sha) { project.commit(source_branch).sha }
     let(:target_sha) { project.commit(target_branch).sha }
 
     before do
-      merge_request = create(:merge_request, source_project: project, target_project: project, source_branch: source_branch, target_branch: target_branch, squash: true)
-
-      visit project_merge_request_path(project, merge_request)
+      visit project_new_merge_request_path(project, merge_request: { target_branch: target_branch, source_branch: source_branch })
+      check 'merge_request[squash]'
+      click_on 'Create merge request'
+      wait_for_requests
     end
 
-    it 'accepts the merge request without issuing a squash request', :sidekiq_inline do
-      expect_next_instance_of(Gitlab::GitalyClient::OperationService) do |instance|
-        expect(instance).not_to receive(:user_squash)
+    context 'when squash message differs from existing commit message' do
+      before do
+        accept_mr
       end
 
-      expect(project.repository.ancestor?(source_branch, target_branch)).to be_falsey
-      expect(page).not_to have_field('squash')
-
-      accept_mr
-
-      expect(page).to have_content('Merged')
+      include_examples 'squash'
+    end
 
-      latest_target_commits = project.repository.commits_between(source_sha, target_sha).map(&:raw)
+    context 'when squash message is the same as existing commit message' do
+      before do
+        click_button("Modify commit messages")
+        fill_in('Squash commit message', with: project.commit(source_branch).safe_message)
+        accept_mr
+      end
 
-      expect(latest_target_commits.count).to eq(1)
-      expect(project.repository.ancestor?(source_branch, target_branch)).to be_truthy
+      include_examples 'no squash'
     end
   end
 

spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js

@@ -503,10 +503,10 @@ describe('ReadyToMerge', () => {
         expect(findCheckboxElement().exists()).toBeFalsy();
       });
 
-      it('should not be rendered when there is only 1 commit', () => {
+      it('should be rendered when there is only 1 commit', () => {
         createComponent({ mr: { commitsCount: 1, enableSquashBeforeMerge: true } });
 
-        expect(findCheckboxElement().exists()).toBeFalsy();
+        expect(findCheckboxElement().exists()).toBe(true);
       });
 
       describe('squash options', () => {

spec/services/merge_requests/merge_to_ref_service_spec.rb

@@ -150,7 +150,10 @@ RSpec.describe MergeRequests::MergeToRefService do
           merge_request.update!(squash: true)
         end
 
-        it_behaves_like 'MergeService for target ref'
+        it_behaves_like 'successfully merges to ref with merge method' do
+          let(:first_parent_ref) { 'refs/heads/master' }
+          let(:target_ref) { merge_request.merge_ref_path }
+        end
 
         it 'does not squash before merging' do
           expect(MergeRequests::SquashService).not_to receive(:new)

spec/services/merge_requests/squash_service_spec.rb

@@ -55,18 +55,26 @@ RSpec.describe MergeRequests::SquashService do
         expect(merge_request).to receive(:commits_count).at_least(:once).and_return(1)
       end
 
-      it 'will skip performing the squash, as the outcome would be the same' do
-        expect(merge_request.target_project.repository).not_to receive(:squash)
+      it 'will still perform the squash' do
+        expect(merge_request.target_project.repository).to receive(:squash).and_return('sha')
 
         service.execute
       end
 
-      it 'will still perform the squash when a custom squash commit message has been provided' do
-        service = described_class.new(project: project, current_user: user, params: { merge_request: merge_request, squash_commit_message: 'A custom commit message' })
+      context 'when squash message matches commit message' do
+        let(:service) { described_class.new(project: project, current_user: user, params: { merge_request: merge_request, squash_commit_message: merge_request.first_commit.safe_message }) }
 
-        expect(merge_request.target_project.repository).to receive(:squash).and_return('sha')
+        it 'returns that commit SHA' do
+          result = service.execute
 
-        service.execute
+          expect(result).to match(status: :success, squash_sha: merge_request.diff_head_sha)
+        end
+
+        it 'does not perform any git actions' do
+          expect(repository).not_to receive(:squash)
+
+          service.execute
+        end
       end
     end
 
@@ -113,17 +121,7 @@ RSpec.describe MergeRequests::SquashService do
     context 'when there is only one commit in the merge request' do
       let(:merge_request) { merge_request_with_one_commit }
 
-      it 'returns that commit SHA' do
-        result = service.execute
-
-        expect(result).to match(status: :success, squash_sha: merge_request.diff_head_sha)
-      end
-
-      it 'does not perform any git actions' do
-        expect(repository).not_to receive(:popen)
-
-        service.execute
-      end
+      include_examples 'the squash succeeds'
     end
 
     context 'when squashing only new files' do