Merging with squash_commit_message should only be passed if squash button is active
From gitaly#2395 (closed), we noticed that a merge attempt attempted to squash a 1-commit merge request for some reason. In the backend, there is an optimization to avoid squashing a 1-commit merge request if the squash message is blank: https://gitlab.com/gitlab-org/gitlab/blob/fee383f37d1b69442230f345471a36b2d1199392/app/services/merge_requests/squash_service.rb#L10-12
When there is only 1 commit in a merge request and the merge request options have been configured to Squash
, the Merge
button is active, but the Squash commits
button is not:
However, if you edit the merge request, you can see that it has been activated:
In 407fd2ed, I believe we added support for overriding a commit message.
What's confusing is that when a user pushes Merge
, squash_commit_message
is always passed, even though it is not configurable in this case where there's only a single commit. This resulted in the backend attempting to squash commits unnecessarily. I believe the default set of options is set in https://gitlab.com/gitlab-org/gitlab/blob/e55c6602483cfb0e531407e1521bf7476f0a47f7/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue#L149.
In https://gitlab.com/gitlab-org/gitlab-ee/blob/e55c6602483cfb0e531407e1521bf7476f0a47f7/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue#L109 we have logic to hide the squash button. I think the easiest thing to do here is to omit the squash_commit_message
argument if shouldShowSquashBeforeMerge()
returns false
. Perhaps something like this?
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue
index d230ac566de..1f8ee9deb74 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue
@@ -145,9 +145,12 @@ export default {
commit_message: this.commitMessage,
auto_merge_strategy: useAutoMerge ? this.mr.preferredAutoMergeStrategy : undefined,
should_remove_source_branch: this.removeSourceBranch === true,
- squash: this.squashBeforeMerge,
- squash_commit_message: this.squashCommitMessage,
- };
+ squash: this.squashBeforeMerge,
+ }
+
+ if (this.shouldShowSquashBeforeMerge) {
+ options.squash_commit_message = this.squashCommitMessage;
+ }
this.isMakingRequest = true;
this.service
/cc: @ntepluhina