From 6700f2264c2a22bb4400e914b8f5e37cc8f69b4c Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 11 Dec 2020 00:36:02 -0800 Subject: [PATCH 1/4] Add a quick action for /rebase This will make it easier for people to bring their merge requests up-to-date. --- .../quick_actions/merge_request_actions.rb | 31 ++++++++++++ locale/gitlab.pot | 12 +++++ .../user_uses_quick_actions_spec.rb | 1 + .../rebase_quick_action_shared_examples.rb | 47 +++++++++++++++++++ 4 files changed, 91 insertions(+) create mode 100644 spec/support/shared_examples/quick_actions/merge_request/rebase_quick_action_shared_examples.rb diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb index aafd493549b001..5be3ccf573c01c 100644 --- a/lib/gitlab/quick_actions/merge_request_actions.rb +++ b/lib/gitlab/quick_actions/merge_request_actions.rb @@ -38,6 +38,37 @@ module MergeRequestActions @updates[:merge] = params[:merge_request_diff_head_sha] end + types MergeRequest + desc do + _('Rebase source branch') + end + explanation do + _('Rebase source branch on the target branch.') + end + condition do + merge_request = quick_action_target + + next false unless merge_request.source_branch_exists? + + access_check = ::Gitlab::UserAccess + .new(current_user, container: merge_request.source_project) + + access_check.can_push_to_branch?(merge_request.source_branch) + end + command :rebase do + result = MergeRequests::RebaseService.new(quick_action_target.project, current_user) + .execute(quick_action_target) + + branch = quick_action_target.source_branch + + @execution_message[:rebase] = + if result[:status] == :success + _('Scheduled a rebase of branch %{branch}.') + else + _('Failed to schedule a rebase of %{branch}.') + end % { branch: branch } + end + desc 'Toggle the Draft status' explanation do noun = quick_action_target.to_ability_name.humanize(capitalize: false) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0a9615bf3697a4..8d648c5507d6c4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11880,6 +11880,9 @@ msgstr "" msgid "Failed to save preferences." msgstr "" +msgid "Failed to schedule a rebase of %{branch}." +msgstr "" + msgid "Failed to set due date because the date format is invalid." msgstr "" @@ -22975,6 +22978,12 @@ msgstr "" msgid "Rebase in progress" msgstr "" +msgid "Rebase source branch" +msgstr "" + +msgid "Rebase source branch on the target branch." +msgstr "" + msgid "Receive alerts from manually configured Prometheus servers." msgstr "" @@ -24328,6 +24337,9 @@ msgstr "" msgid "Scheduled Deletion At - %{permanent_deletion_time}" msgstr "" +msgid "Scheduled a rebase of branch %{branch}." +msgstr "" + msgid "Scheduled to merge this merge request (%{strategy})." msgstr "" diff --git a/spec/features/merge_request/user_uses_quick_actions_spec.rb b/spec/features/merge_request/user_uses_quick_actions_spec.rb index 04a2e046f42d91..b48659353ec01c 100644 --- a/spec/features/merge_request/user_uses_quick_actions_spec.rb +++ b/spec/features/merge_request/user_uses_quick_actions_spec.rb @@ -41,5 +41,6 @@ end it_behaves_like 'merge quick action' + it_behaves_like 'rebase quick action' end end diff --git a/spec/support/shared_examples/quick_actions/merge_request/rebase_quick_action_shared_examples.rb b/spec/support/shared_examples/quick_actions/merge_request/rebase_quick_action_shared_examples.rb new file mode 100644 index 00000000000000..b21a35424feed2 --- /dev/null +++ b/spec/support/shared_examples/quick_actions/merge_request/rebase_quick_action_shared_examples.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'rebase quick action' do + context 'when updating the description' do + before do + sign_in(user) + visit edit_project_merge_request_path(project, merge_request) + end + + it 'rebases the MR', :sidekiq_inline do + fill_in('Description', with: '/rebase') + click_button('Save changes') + + expect(page).not_to have_content('commit behind the target branch') + expect(merge_request.reload).not_to be_merged + end + end + + context 'when creating a new note' do + context 'when the current user can rebase the MR' do + before do + sign_in(user) + visit project_merge_request_path(project, merge_request) + end + + it 'rebase the MR', :sidekiq_inline do + add_note("/rebase") + + expect(page).to have_content "Scheduled a rebase of branch #{merge_request.source_branch}." + end + end + + context 'when the current user cannot rebase the MR' do + before do + project.add_guest(guest) + sign_in(guest) + visit project_merge_request_path(project, merge_request) + end + + it 'does not rebase the MR' do + add_note("/rebase") + + expect(page).not_to have_content 'Your commands have been executed!' + end + end + end +end -- GitLab From 69b88a0e4b0235be6d6d699b087089c99d4bad37 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 27 Dec 2020 13:56:31 -0800 Subject: [PATCH 2/4] Prevent simultaneous use of /rebase and /merge actions Since /rebase and /merge actions are not atomic, we may run into confusing race conditions if these are applied together. For now, we just ignore the /merge and proceed with the rebase. --- app/services/merge_requests/update_service.rb | 6 ++++++ lib/gitlab/quick_actions/merge_request_actions.rb | 3 +++ .../merge_request/rebase_quick_action_shared_examples.rb | 8 ++++++++ 3 files changed, 17 insertions(+) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 826a48fdcf922d..e1216c36823b65 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -126,6 +126,12 @@ def create_branch_change_note(issuable, branch_type, old_branch, new_branch) override :handle_quick_actions def handle_quick_actions(merge_request) super + + # Ignore /merge if /rebase is used to avoid an unexpected race + params.delete(:merge) if params[:rebase] && params[:merge] + # Rebase is handled in MergeRequestActions to provide user feedback + params.delete(:rebase) + merge_from_quick_action(merge_request) if params[:merge] end diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb index 5be3ccf573c01c..d39a00d86f53c3 100644 --- a/lib/gitlab/quick_actions/merge_request_actions.rb +++ b/lib/gitlab/quick_actions/merge_request_actions.rb @@ -56,6 +56,9 @@ module MergeRequestActions access_check.can_push_to_branch?(merge_request.source_branch) end command :rebase do + # This will be used to avoid simultaneous /merge and /rebase actions + @updates[:rebase] = true + result = MergeRequests::RebaseService.new(quick_action_target.project, current_user) .execute(quick_action_target) diff --git a/spec/support/shared_examples/quick_actions/merge_request/rebase_quick_action_shared_examples.rb b/spec/support/shared_examples/quick_actions/merge_request/rebase_quick_action_shared_examples.rb index b21a35424feed2..2e8db16ad68fd9 100644 --- a/spec/support/shared_examples/quick_actions/merge_request/rebase_quick_action_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/merge_request/rebase_quick_action_shared_examples.rb @@ -14,6 +14,14 @@ expect(page).not_to have_content('commit behind the target branch') expect(merge_request.reload).not_to be_merged end + + it 'ignores /merge if /rebase is specified', :sidekiq_inline do + fill_in('Description', with: "/merge\n/rebase") + click_button('Save changes') + + expect(page).not_to have_content('commit behind the target branch') + expect(merge_request.reload).not_to be_merged + end end context 'when creating a new note' do -- GitLab From 0b69b7112fcf35501681c12ea05f68519755c52f Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 27 Dec 2020 21:36:47 -0800 Subject: [PATCH 3/4] Make rebase from quick actions async Also move rebase functionality into `MergeRequests::UpdateService` to ensure merges/rebases are handled in the same location. --- app/services/merge_requests/update_service.rb | 16 ++++++++++++---- .../quick_actions/merge_request_actions.rb | 10 +--------- locale/gitlab.pot | 3 --- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index e1216c36823b65..8e894773f5e876 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -127,14 +127,22 @@ def create_branch_change_note(issuable, branch_type, old_branch, new_branch) def handle_quick_actions(merge_request) super - # Ignore /merge if /rebase is used to avoid an unexpected race - params.delete(:merge) if params[:rebase] && params[:merge] - # Rebase is handled in MergeRequestActions to provide user feedback - params.delete(:rebase) + # Ensure this parameter gets used as an attribute + rebase = params.delete(:rebase) + + if rebase + rebase_from_quick_action(merge_request) + # Ignore /merge if /rebase is used to avoid an unexpected race + params.delete(:merge) + end merge_from_quick_action(merge_request) if params[:merge] end + def rebase_from_quick_action(merge_request) + merge_request.rebase_async(current_user.id) + end + def merge_from_quick_action(merge_request) last_diff_sha = params.delete(:merge) diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb index d39a00d86f53c3..8fb92f50f6150b 100644 --- a/lib/gitlab/quick_actions/merge_request_actions.rb +++ b/lib/gitlab/quick_actions/merge_request_actions.rb @@ -59,17 +59,9 @@ module MergeRequestActions # This will be used to avoid simultaneous /merge and /rebase actions @updates[:rebase] = true - result = MergeRequests::RebaseService.new(quick_action_target.project, current_user) - .execute(quick_action_target) - branch = quick_action_target.source_branch - @execution_message[:rebase] = - if result[:status] == :success - _('Scheduled a rebase of branch %{branch}.') - else - _('Failed to schedule a rebase of %{branch}.') - end % { branch: branch } + @execution_message[:rebase] = _('Scheduled a rebase of branch %{branch}.') % { branch: branch } end desc 'Toggle the Draft status' diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8d648c5507d6c4..b41d3d6367a10b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11880,9 +11880,6 @@ msgstr "" msgid "Failed to save preferences." msgstr "" -msgid "Failed to schedule a rebase of %{branch}." -msgstr "" - msgid "Failed to set due date because the date format is invalid." msgstr "" -- GitLab From b15df829aa0d40087d5b538d3fe7c8a10f4941b8 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 27 Dec 2020 22:19:16 -0800 Subject: [PATCH 4/4] Add documentation for /rebase quick action This documents the use of /rebase for merge requests. --- app/services/merge_requests/update_service.rb | 4 ++-- changelogs/unreleased/sh-quick-action-rebase.yml | 5 +++++ doc/user/project/quick_actions.md | 1 + lib/gitlab/quick_actions/merge_request_actions.rb | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/sh-quick-action-rebase.yml diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 8e894773f5e876..1d0ab403f12b22 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -127,12 +127,12 @@ def create_branch_change_note(issuable, branch_type, old_branch, new_branch) def handle_quick_actions(merge_request) super - # Ensure this parameter gets used as an attribute + # Ensure this parameter does not get used as an attribute rebase = params.delete(:rebase) if rebase rebase_from_quick_action(merge_request) - # Ignore /merge if /rebase is used to avoid an unexpected race + # Ignore "/merge" if "/rebase" is used to avoid an unexpected race params.delete(:merge) end diff --git a/changelogs/unreleased/sh-quick-action-rebase.yml b/changelogs/unreleased/sh-quick-action-rebase.yml new file mode 100644 index 00000000000000..a790d15228a0b4 --- /dev/null +++ b/changelogs/unreleased/sh-quick-action-rebase.yml @@ -0,0 +1,5 @@ +--- +title: Add a quick action for /rebase +merge_request: 49800 +author: +type: added diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md index 9f849051f40d2a..c758b2775d2a6c 100644 --- a/doc/user/project/quick_actions.md +++ b/doc/user/project/quick_actions.md @@ -56,6 +56,7 @@ The following quick actions are applicable to descriptions, discussions and thre | `/promote` | ✓ | | | Promote issue to epic. **(PREMIUM)** | | `/publish` | ✓ | | | Publish issue to an associated [Status Page](../../operations/incident_management/status_page.md) ([Introduced in GitLab 13.0](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/30906)) **(ULTIMATE)** | | `/reassign @user1 @user2` | ✓ | ✓ | | Replace current assignees with those specified. **(STARTER)** | +| `/rebase` | | ✓ | | Rebase source branch. This will schedule a background task that attempt to rebase the changes in the source branch on the latest commit of the target branch. If `/rebase` is used, `/merge` will be ignored to avoid a race condition where the source branch is merged or deleted before it is rebased. | | `/relabel ~label1 ~label2` | ✓ | ✓ | ✓ | Replace current labels with those specified. | | `/relate #issue1 #issue2` | ✓ | | | Mark issues as related. **(STARTER)** | | `/remove_child_epic ` | | | ✓ | Remove child epic from ``. The `` value should be in the format of `&epic`, `group&epic`, or a URL to an epic ([introduced in GitLab 12.0](https://gitlab.com/gitlab-org/gitlab/-/issues/7330)). **(ULTIMATE)** | diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb index 8fb92f50f6150b..9ac8b98f595dc0 100644 --- a/lib/gitlab/quick_actions/merge_request_actions.rb +++ b/lib/gitlab/quick_actions/merge_request_actions.rb @@ -56,7 +56,7 @@ module MergeRequestActions access_check.can_push_to_branch?(merge_request.source_branch) end command :rebase do - # This will be used to avoid simultaneous /merge and /rebase actions + # This will be used to avoid simultaneous "/merge" and "/rebase" actions @updates[:rebase] = true branch = quick_action_target.source_branch -- GitLab