From ee1c73ed2f32067c51be11a9c3714aee8bd59290 Mon Sep 17 00:00:00 2001
From: Raimund Hook <rhook@gitlab.com>
Date: Tue, 13 Sep 2022 12:06:10 +0000
Subject: [PATCH] Add feature flag for /draft toggle
Changelog: deprecated
Signed-off-by: Raimund Hook <raimund.hook@exfo.com>
---
.../draft_quick_action_non_toggle.yml | 8 +++
doc/user/project/merge_requests/drafts.md | 4 +-
doc/user/project/quick_actions.md | 2 +-
.../quick_actions/merge_request_actions.rb | 39 +++++++------
locale/gitlab.pot | 20 ++++---
spec/services/notes/create_service_spec.rb | 4 +-
.../quick_actions/interpret_service_spec.rb | 57 ++++++++++++++++++-
7 files changed, 105 insertions(+), 29 deletions(-)
create mode 100644 config/feature_flags/development/draft_quick_action_non_toggle.yml
diff --git a/config/feature_flags/development/draft_quick_action_non_toggle.yml b/config/feature_flags/development/draft_quick_action_non_toggle.yml
new file mode 100644
index 0000000000000..4d28b61f3bf71
--- /dev/null
+++ b/config/feature_flags/development/draft_quick_action_non_toggle.yml
@@ -0,0 +1,8 @@
+---
+name: draft_quick_action_non_toggle
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92654
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/368610
+milestone: '15.4'
+type: development
+group: group::code review
+default_enabled: false
diff --git a/doc/user/project/merge_requests/drafts.md b/doc/user/project/merge_requests/drafts.md
index 4bb6034c0bdf8..cdd8c6a80aaa5 100644
--- a/doc/user/project/merge_requests/drafts.md
+++ b/doc/user/project/merge_requests/drafts.md
@@ -20,6 +20,7 @@ the **Merge** button until you remove the **Draft** flag:
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/32692) in GitLab 13.2, Work-In-Progress (WIP) merge requests were renamed to **Draft**.
> - [Removed](https://gitlab.com/gitlab-org/gitlab/-/issues/228685) all support for using **WIP** in GitLab 14.8.
> - **Mark as draft** and **Mark as ready** buttons [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/227421) in GitLab 13.5.
+> `/draft` quick action as a toggle [deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92654) in GitLab 15.3.
There are several ways to flag a merge request as a draft:
@@ -29,8 +30,7 @@ There are several ways to flag a merge request as a draft:
below the **Title** field.
- **Commenting in an existing merge request**: Add the `/draft`
[quick action](../quick_actions.md#issues-merge-requests-and-epics)
- in a comment. This quick action is a toggle, and can be repeated to change the status
- back to Ready.
+ in a comment. GitLab 15.4 [deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92654) the toggle behavior of `/draft`. To mark a merge request as ready, use `/ready`.
- **Creating a commit**: Add `draft:`, `Draft:`, `fixup!`, or `Fixup!` to the
beginning of a commit message targeting the merge request's source branch. This
is not a toggle, and adding this text again in a later commit doesn't mark the
diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md
index af43d53679bbd..8b8eba62cced0 100644
--- a/doc/user/project/quick_actions.md
+++ b/doc/user/project/quick_actions.md
@@ -67,7 +67,7 @@ threads. Some quick actions might not be available to all subscription tiers.
| `/copy_metadata <#issue>` | **{check-circle}** Yes | **{check-circle}** Yes | **{dotted-circle}** No | Copy labels and milestone from another issue in the project. |
| `/create_merge_request <branch name>` | **{check-circle}** Yes | **{dotted-circle}** No | **{dotted-circle}** No | Create a new merge request starting from the current issue. |
| `/done` | **{check-circle}** Yes | **{check-circle}** Yes | **{check-circle}** Yes | Mark to do as done. |
-| `/draft` | **{dotted-circle}** No | **{check-circle}** Yes | **{dotted-circle}** No | Toggle the [draft status](merge_requests/drafts.md). |
+| `/draft` | **{dotted-circle}** No | **{check-circle}** Yes | **{dotted-circle}** No | Set the [draft status](merge_requests/drafts.md). Use for toggling the draft status ([deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92654) in GitLab 15.4.) |
| `/due <date>` | **{check-circle}** Yes | **{dotted-circle}** No | **{dotted-circle}** No | Set due date. Examples of valid `<date>` include `in 2 days`, `this Friday` and `December 31st`. |
| `/duplicate <#issue>` | **{check-circle}** Yes | **{dotted-circle}** No | **{dotted-circle}** No | Close this issue and mark as a duplicate of another issue. Also, mark both as related. |
| `/epic <epic>` | **{check-circle}** Yes | **{dotted-circle}** No | **{dotted-circle}** No | Add to epic `<epic>`. The `<epic>` value should be in the format of `&epic`, `group&epic`, or a URL to an epic. |
diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb
index 3cb01db1491c6..d38b81bff0b0b 100644
--- a/lib/gitlab/quick_actions/merge_request_actions.rb
+++ b/lib/gitlab/quick_actions/merge_request_actions.rb
@@ -88,33 +88,21 @@ module MergeRequestActions
@execution_message[:rebase] = _('Scheduled a rebase of branch %{branch}.') % { branch: branch }
end
- desc { _('Toggle the Draft status') }
+ desc { _('Set the Draft status') }
explanation do
- noun = quick_action_target.to_ability_name.humanize(capitalize: false)
- if quick_action_target.draft?
- _("Marks this %{noun} as ready.")
- else
- _("Marks this %{noun} as a draft.")
- end % { noun: noun }
+ draft_action_message(_("Marks"))
end
execution_message do
- noun = quick_action_target.to_ability_name.humanize(capitalize: false)
- if quick_action_target.draft?
- _("Marked this %{noun} as ready.")
- else
- _("Marked this %{noun} as a draft.")
- end % { noun: noun }
+ draft_action_message(_("Marked"))
end
types MergeRequest
condition do
quick_action_target.respond_to?(:draft?) &&
- # Allow it to mark as draft on MR creation page or through MR notes
- #
(quick_action_target.new_record? || current_user.can?(:"update_#{quick_action_target.to_ability_name}", quick_action_target))
end
command :draft do
- @updates[:wip_event] = quick_action_target.draft? ? 'ready' : 'draft'
+ @updates[:wip_event] = draft_action_command
end
desc { _('Set the Ready status') }
@@ -317,6 +305,25 @@ def reviewers_to_add(users)
end
end
+ def draft_action_message(verb)
+ noun = quick_action_target.to_ability_name.humanize(capitalize: false)
+ if !quick_action_target.draft?
+ _("%{verb} this %{noun} as a draft.")
+ elsif Feature.disabled?(:draft_quick_action_non_toggle, quick_action_target.project)
+ _("%{verb} this %{noun} as ready.")
+ else
+ _("No change to this %{noun}'s draft status.")
+ end % { verb: verb, noun: noun }
+ end
+
+ def draft_action_command
+ if Feature.disabled?(:draft_quick_action_non_toggle, quick_action_target.project)
+ quick_action_target.draft? ? 'ready' : 'draft'
+ else
+ 'draft'
+ end
+ end
+
def merge_orchestration_service
@merge_orchestration_service ||= ::MergeRequests::MergeOrchestrationService.new(project, current_user)
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 732436619690b..7a27209c079f8 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -1180,6 +1180,12 @@ msgstr ""
msgid "%{verb} %{time_spent_value} spent time."
msgstr ""
+msgid "%{verb} this %{noun} as a draft."
+msgstr ""
+
+msgid "%{verb} this %{noun} as ready."
+msgstr ""
+
msgid "%{webhooks_link_start}%{webhook_type}%{link_end} enable you to send notifications to web applications in response to events in a group or project."
msgstr ""
@@ -24260,6 +24266,9 @@ msgstr ""
msgid "MarkdownToolbar|Supports %{markdownDocsLinkStart}Markdown%{markdownDocsLinkEnd}"
msgstr ""
+msgid "Marked"
+msgstr ""
+
msgid "Marked For Deletion At - %{deletion_time}"
msgstr ""
@@ -24269,9 +24278,6 @@ msgstr ""
msgid "Marked as ready. Merging is now allowed."
msgstr ""
-msgid "Marked this %{noun} as a draft."
-msgstr ""
-
msgid "Marked this %{noun} as ready."
msgstr ""
@@ -24284,7 +24290,7 @@ msgstr ""
msgid "Marked to do as done."
msgstr ""
-msgid "Marks this %{noun} as a draft."
+msgid "Marks"
msgstr ""
msgid "Marks this %{noun} as ready."
@@ -36298,6 +36304,9 @@ msgstr ""
msgid "Set target branch to %{branch_name}."
msgstr ""
+msgid "Set the Draft status"
+msgstr ""
+
msgid "Set the Ready status"
msgstr ""
@@ -41381,9 +41390,6 @@ msgstr ""
msgid "Toggle sidebar"
msgstr ""
-msgid "Toggle the Draft status"
-msgstr ""
-
msgid "Toggle the Performance Bar"
msgstr ""
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 74684bc05cefa..4922e72b7a468 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -406,9 +406,9 @@
expect(issuable.draft?).to eq(can_use_quick_action)
}
),
- # Remove draft status
+ # Remove draft (set ready) status
QuickAction.new(
- action_text: "/draft",
+ action_text: "/ready",
before_action: -> {
issuable.reload.update!(title: "Draft: title")
},
diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb
index 2d38d968ce4a8..a43f3bc55bf58 100644
--- a/spec/services/quick_actions/interpret_service_spec.rb
+++ b/spec/services/quick_actions/interpret_service_spec.rb
@@ -1393,14 +1393,41 @@
let(:issuable) { issue }
end
+ # /draft is a toggle (ff disabled)
it_behaves_like 'draft command' do
let(:content) { '/draft' }
let(:issuable) { merge_request }
+
+ before do
+ stub_feature_flags(draft_quick_action_non_toggle: false)
+ end
end
+ # /draft is a toggle (ff disabled)
it_behaves_like 'ready command' do
let(:content) { '/draft' }
let(:issuable) { merge_request }
+
+ before do
+ stub_feature_flags(draft_quick_action_non_toggle: false)
+ issuable.update!(title: issuable.draft_title)
+ end
+ end
+
+ # /draft is one way (ff enabled)
+ it_behaves_like 'draft command' do
+ let(:content) { '/draft' }
+ let(:issuable) { merge_request }
+ end
+
+ # /draft is one way (ff enabled)
+ it_behaves_like 'draft/ready command no action' do
+ let(:content) { '/draft' }
+ let(:issuable) { merge_request }
+
+ before do
+ issuable.update!(title: issuable.draft_title)
+ end
end
it_behaves_like 'draft/ready command no action' do
@@ -2646,7 +2673,28 @@
end
end
- describe 'draft command' do
+ describe 'draft command toggle (deprecated)' do
+ let(:content) { '/draft' }
+
+ before do
+ stub_feature_flags(draft_quick_action_non_toggle: false)
+ end
+
+ it 'includes the new status' do
+ _, explanations = service.explain(content, merge_request)
+
+ expect(explanations).to match_array(['Marks this merge request as a draft.'])
+ end
+
+ it 'sets the ready status on a draft' do
+ merge_request.update!(title: merge_request.draft_title)
+ _, explanations = service.explain(content, merge_request)
+
+ expect(explanations).to match_array(["Marks this merge request as ready."])
+ end
+ end
+
+ describe 'draft command set' do
let(:content) { '/draft' }
it 'includes the new status' do
@@ -2654,6 +2702,13 @@
expect(explanations).to match_array(['Marks this merge request as a draft.'])
end
+
+ it 'includes the no change message when status unchanged' do
+ merge_request.update!(title: merge_request.draft_title)
+ _, explanations = service.explain(content, merge_request)
+
+ expect(explanations).to match_array(["No change to this merge request's draft status."])
+ end
end
describe 'ready command' do
--
GitLab