Commit cd08aa66 authored by Alain Takoudjou's avatar Alain Takoudjou

NXD Add support for apply patch in Merge request

If there is only 1 commit is present in MR, apply patch buttun is enabled
allow to merge the MR in a single commit.
parent 3b6ff550
...@@ -173,6 +173,13 @@ export default { ...@@ -173,6 +173,13 @@ export default {
return this.mr.commitsCount; return this.mr.commitsCount;
}, },
ApplyPatchButtonText() {
return __('Apply Patch ');
},
isApplyPatchAllowed() {
return this.mr.commitsCount !== 1 || this.isMergeButtonDisabled;
},
preferredAutoMergeStrategy() { preferredAutoMergeStrategy() {
if (this.glFeatures.mergeRequestWidgetGraphql) { if (this.glFeatures.mergeRequestWidgetGraphql) {
return MergeRequestStore.getPreferredAutoMergeStrategy( return MergeRequestStore.getPreferredAutoMergeStrategy(
...@@ -329,7 +336,7 @@ export default { ...@@ -329,7 +336,7 @@ export default {
: this.mr.commitMessageWithDescription; : this.mr.commitMessageWithDescription;
this.commitMessage = includeDescription ? commitMessageWithDescription : commitMessage; this.commitMessage = includeDescription ? commitMessageWithDescription : commitMessage;
}, },
handleMergeButtonClick(useAutoMerge, mergeImmediately = false) { handleMergeButtonClick(useAutoMerge, mergeImmediately = false, apply_patch = false) {
if (mergeImmediately) { if (mergeImmediately) {
this.isMergingImmediately = true; this.isMergingImmediately = true;
} }
...@@ -343,6 +350,7 @@ export default { ...@@ -343,6 +350,7 @@ export default {
auto_merge_strategy: useAutoMerge ? this.preferredAutoMergeStrategy : undefined, auto_merge_strategy: useAutoMerge ? this.preferredAutoMergeStrategy : undefined,
should_remove_source_branch: this.removeSourceBranch === true, should_remove_source_branch: this.removeSourceBranch === true,
squash: this.squashBeforeMerge, squash: this.squashBeforeMerge,
apply_patch: apply_patch,
}; };
// If users can't alter the squash message (e.g. for 1-commit merge requests), // If users can't alter the squash message (e.g. for 1-commit merge requests),
...@@ -525,6 +533,18 @@ export default { ...@@ -525,6 +533,18 @@ export default {
@mergeImmediately="onMergeImmediatelyConfirmation" @mergeImmediately="onMergeImmediatelyConfirmation"
/> />
</gl-dropdown> </gl-dropdown>
<gl-button
size="medium"
category="primary"
class="accept-merge-request"
data-testid="apply-button"
:variant="mergeButtonVariant"
:disabled="isApplyPatchAllowed"
:loading="isMakingRequest"
data-qa-selector="apply_patch_button"
@click="handleMergeButtonClick(false, false, true)"
>{{ ApplyPatchButtonText }}</gl-button
>
</gl-button-group> </gl-button-group>
<div <div
v-if="shouldShowMergeControls" v-if="shouldShowMergeControls"
......
...@@ -374,7 +374,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -374,7 +374,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end end
def merge_params def merge_params
params.permit(merge_params_attributes) params.permit(merge_params_attributes, :apply_patches)
end end
def merge_params_attributes def merge_params_attributes
......
...@@ -1311,7 +1311,8 @@ class MergeRequest < ApplicationRecord ...@@ -1311,7 +1311,8 @@ class MergeRequest < ApplicationRecord
end end
message << "#{description}" if include_description && description.present? message << "#{description}" if include_description && description.present?
message << "See merge request #{to_reference(full: true)}" #message << "See merge request #{to_reference(full: true)}"
message << "/reviewed-on #{Gitlab::UrlBuilder.build(self)}"
message.join("\n\n") message.join("\n\n")
end end
......
...@@ -84,7 +84,15 @@ module MergeRequests ...@@ -84,7 +84,15 @@ module MergeRequests
def commit def commit
log_info("Git merge started on JID #{merge_jid}") log_info("Git merge started on JID #{merge_jid}")
commit_id = try_merge
mr_1patch = merge_request.commits.size == 1
do_apply = params[:apply_patches] || mr_1patch
do_merge = !do_apply
patch = merge_request.commits.reverse.first
commit_id = do_merge ? try_merge \
: repository.cherry_pick(current_user, repository.commit(patch.sha), merge_request.target_branch, commit_message,
start_branch_name: nil, start_project: project, dry_run: false)
if commit_id if commit_id
log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}")
...@@ -99,7 +107,7 @@ module MergeRequests ...@@ -99,7 +107,7 @@ module MergeRequests
def try_merge def try_merge
repository.merge(current_user, source, merge_request, commit_message).tap do repository.merge(current_user, source, merge_request, commit_message).tap do
merge_request.update_column(:squash_commit_sha, source) if merge_request.squash_on_merge? merge_request.update_column(:squashoptions_commit_sha, source) if merge_request.squash_on_merge?
end end
rescue Gitlab::Git::PreReceiveError => e rescue Gitlab::Git::PreReceiveError => e
raise MergeError, raise MergeError,
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
- nonce = SecureRandom.hex - nonce = SecureRandom.hex
- descriptions = local_assigns.slice(:message_with_description, :message_without_description) - descriptions = local_assigns.slice(:message_with_description, :message_without_description)
= label_tag "commit_message-#{nonce}", class: 'col-form-label col-sm-2' do = label_tag "commit_message-#{nonce}", class: 'col-form-label col-sm-2' do
#{ _('Commit message') } local_assigns[:label_text] || #{ _('Commit message') }
.col-sm-10 .col-sm-10
.commit-message-container .commit-message-container
.max-width-marker .max-width-marker
......
...@@ -73,6 +73,7 @@ module QA ...@@ -73,6 +73,7 @@ module QA
view 'app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue' do view 'app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue' do
element :merge_button element :merge_button
element :apply_patch_button
element :fast_forward_message_content element :fast_forward_message_content
element :merge_moment_dropdown element :merge_moment_dropdown
element :merge_immediately_menu_item element :merge_immediately_menu_item
...@@ -255,10 +256,12 @@ module QA ...@@ -255,10 +256,12 @@ module QA
# Waits up 60 seconds and raises an error if unable to merge # Waits up 60 seconds and raises an error if unable to merge
def wait_until_ready_to_merge def wait_until_ready_to_merge
has_element?(:merge_button) has_element?(:merge_button)
has_element?(:apply_patch_button)
# The merge button is enabled via JS # The merge button is enabled via JS
wait_until(reload: false) do wait_until(reload: false) do
!find_element(:merge_button).disabled? !find_element(:merge_button).disabled?
!find_element(:apply_patch_button).disabled?
end end
end end
......
  • @tomo @jerome @kirr This commit is adding apply patch button for MR with 1 commit.

    As we discussed with @tomo, I'm going to enable it for MR with more than one commits and it will do Fast Forward Merge, by calling https://lab.nexedi.com/nexedi/gitlab-ce/-/blob/v13.12.15-nxd/lib/gitlab/git/repository.rb#L614. It's not possible with ff_merge to update commit message, gitlab add a link to the MR in the commit page (see this one: slapos@9227dd59).

  • @alain.takoudjou, @tomo, thanks for heads up and for bringing "Apply patch" back. It is really good to see.

    I have one suggestion: for multipatch case instead of going through fast-forward merge, we could loop through all patches in the merge-request and cherry-pick them one by one like we already do for single-patch case. Then adding /reviewed-on ... line to each patch should be relatively straightforward.

    Thanks, once again, for reviwing the "apply patch" story.
    Kirill

  • for multipatch case instead of going through fast-forward merge, we could loop through all patches in the merge-request and cherry-pick them one by one ...

    Yes maybe better because ff need rebase, @tomo ?

Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment