From d4d761fc83660692d39b03c50b7ab2a18417503c Mon Sep 17 00:00:00 2001
From: Joseph Wambua <jjoshua@gitlab.com>
Date: Thu, 15 Feb 2024 19:47:24 +0000
Subject: [PATCH] Add remove_child quick action for work items
This quick action removes a work item as a child to the current work
item.
Changelog: added
---
app/models/work_items/widgets/hierarchy.rb | 6 ++-
.../widgets/hierarchy_service/base_service.rb | 25 ++++++---
...27_i_quickactions_remove_child_monthly.yml | 26 +++++++++
...528_i_quickactions_remove_child_weekly.yml | 26 +++++++++
doc/user/project/quick_actions.md | 1 +
lib/gitlab/quick_actions/work_item_actions.rb | 19 ++++++-
.../hll_redis_legacy_events.yml | 1 +
locale/gitlab.pot | 15 ++++--
.../notes/quick_actions_service_spec.rb | 42 +++++++++++++++
.../quick_actions/interpret_service_spec.rb | 53 +++++++++++++++++++
.../hierarchy_service/update_service_spec.rb | 24 +++++++--
11 files changed, 223 insertions(+), 15 deletions(-)
create mode 100644 config/metrics/counts_28d/20240208152527_i_quickactions_remove_child_monthly.yml
create mode 100644 config/metrics/counts_7d/20240208152528_i_quickactions_remove_child_weekly.yml
diff --git a/app/models/work_items/widgets/hierarchy.rb b/app/models/work_items/widgets/hierarchy.rb
index f184ceefef91e..c994d0964b209 100644
--- a/app/models/work_items/widgets/hierarchy.rb
+++ b/app/models/work_items/widgets/hierarchy.rb
@@ -16,11 +16,11 @@ def ancestors
end
def self.quick_action_commands
- [:set_parent, :add_child, :remove_parent]
+ [:set_parent, :add_child, :remove_parent, :remove_child]
end
def self.quick_action_params
- [:set_parent, :add_child, :remove_parent]
+ [:set_parent, :add_child, :remove_parent, :remove_child]
end
def self.process_quick_action_param(param_name, value)
@@ -28,6 +28,8 @@ def self.process_quick_action_param(param_name, value)
if [:set_parent, :remove_parent].include?(param_name)
{ parent: value.is_a?(WorkItem) ? value : nil }
+ elsif param_name == :remove_child
+ { remove_child: value }
else
{ children: value }
end
diff --git a/app/services/work_items/widgets/hierarchy_service/base_service.rb b/app/services/work_items/widgets/hierarchy_service/base_service.rb
index 45393eab58ccf..8f7cf38e8744a 100644
--- a/app/services/work_items/widgets/hierarchy_service/base_service.rb
+++ b/app/services/work_items/widgets/hierarchy_service/base_service.rb
@@ -7,12 +7,14 @@ class BaseService < WorkItems::Widgets::BaseService
private
def handle_hierarchy_changes(params)
- return incompatible_args_error if incompatible_args?(params)
+ return incompatible_args_error if params.slice(*mutually_exclusive_args).size > 1
if params.key?(:parent)
update_work_item_parent(params.delete(:parent))
elsif params.key?(:children)
update_work_item_children(params.delete(:children))
+ elsif params.key?(:remove_child)
+ remove_child(params.delete(:remove_child))
else
invalid_args_error(params)
end
@@ -33,26 +35,37 @@ def set_parent(parent)
end
# rubocop: disable CodeReuse/ActiveRecord
- def remove_parent
- link = ::WorkItems::ParentLink.find_by(work_item: work_item)
+ def remove_parent_link(child)
+ link = ::WorkItems::ParentLink.find_by(work_item: child)
return success unless link.present?
::WorkItems::ParentLinks::DestroyService.new(link, current_user).execute
end
# rubocop: enable CodeReuse/ActiveRecord
+ def remove_parent
+ remove_parent_link(work_item)
+ end
+
+ def remove_child(child)
+ remove_parent_link(child)
+ end
+
def update_work_item_children(children)
::WorkItems::ParentLinks::CreateService
.new(work_item, current_user, { issuable_references: children })
.execute
end
- def incompatible_args?(params)
- params[:children] && params[:parent]
+ def mutually_exclusive_args
+ [:children, :parent, :remove_child]
end
def incompatible_args_error
- error(_('A Work Item can be a parent or a child, but not both.'))
+ error(format(
+ _("One and only one of %{params} is required"),
+ params: mutually_exclusive_args.to_sentence(last_word_connector: ' or ')
+ ))
end
def invalid_args_error(params)
diff --git a/config/metrics/counts_28d/20240208152527_i_quickactions_remove_child_monthly.yml b/config/metrics/counts_28d/20240208152527_i_quickactions_remove_child_monthly.yml
new file mode 100644
index 0000000000000..d62c49435ea48
--- /dev/null
+++ b/config/metrics/counts_28d/20240208152527_i_quickactions_remove_child_monthly.yml
@@ -0,0 +1,26 @@
+---
+key_path: redis_hll_counters.quickactions.i_quickactions_remove_child_monthly
+name: quickactions_remove_child_monthly
+description: Count of MAU using the `/remove_child` quick action
+product_section: dev
+product_stage: plan
+product_group: product_planning
+value_type: number
+status: active
+milestone: "16.10"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141354
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+ events:
+ - i_quickactions_remove_child
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate
diff --git a/config/metrics/counts_7d/20240208152528_i_quickactions_remove_child_weekly.yml b/config/metrics/counts_7d/20240208152528_i_quickactions_remove_child_weekly.yml
new file mode 100644
index 0000000000000..b3c11a101d81c
--- /dev/null
+++ b/config/metrics/counts_7d/20240208152528_i_quickactions_remove_child_weekly.yml
@@ -0,0 +1,26 @@
+---
+key_path: redis_hll_counters.quickactions.i_quickactions_remove_child_weekly
+name: quickactions_remove_child_weekly
+description: Count of WAU using the `/remove_child` quick action
+product_section: dev
+product_stage: plan
+product_group: product_planning
+value_type: number
+status: active
+milestone: "16.10"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141354
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+ events:
+ - i_quickactions_remove_child
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate
diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md
index 7a55132e1fbd8..ba68a88e78f13 100644
--- a/doc/user/project/quick_actions.md
+++ b/doc/user/project/quick_actions.md
@@ -168,6 +168,7 @@ To auto-format this table, use the VS Code Markdown Table formatter: `https://do
| `/reassign @user1 @user2` | **{check-circle}** Yes | **{check-circle}** Yes | **{check-circle}** Yes | Replace current assignees with those specified. |
| `/relabel ~label1 ~label2` | **{check-circle}** Yes | **{check-circle}** Yes | **{check-circle}** Yes | Replace current labels with those specified. |
| `/remove_due_date` | **{check-circle}** Yes | **{dotted-circle}** No | **{check-circle}** Yes | Remove due date. |
+| `/remove_child <work_item>` | **{dotted-circle}** No | **{check-circle}** Yes | **{dotted-circle}** No | Remove the child `<work_item>`. The `<work_item>` value should be in the format of `#iid`, `group/project#iid`, or a URL to a work item. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/132761) in GitLab 16.10. |
| `/remove_parent` | **{check-circle}** Yes | **{dotted-circle}** No | **{check-circle}** Yes | Removes the parent work item. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/434344) in GitLab 16.9. |
| `/reopen` | **{check-circle}** Yes | **{check-circle}** Yes | **{check-circle}** Yes | Reopen. |
| `/set_parent <work_item>` | **{check-circle}** Yes | **{dotted-circle}** No | **{check-circle}** Yes | Set parent work item to `<work_item>`. The `<work_item>` value should be in the format of `#iid`, `group/project#iid`, or a URL to a work item. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/420798) in GitLab 16.5. |
diff --git a/lib/gitlab/quick_actions/work_item_actions.rb b/lib/gitlab/quick_actions/work_item_actions.rb
index adb85bd38c203..48f0634079ac6 100644
--- a/lib/gitlab/quick_actions/work_item_actions.rb
+++ b/lib/gitlab/quick_actions/work_item_actions.rb
@@ -65,6 +65,18 @@ module WorkItemActions
@updates[:add_child] = extract_work_items(child_param)
@execution_message[:add_child] = success_msg[:add_child]
end
+
+ desc { _('Remove child from work item') }
+ explanation do |child_param|
+ format(_("Remove the child %{child_ref} from this work item."), child_ref: child_param)
+ end
+ types WorkItem
+ params 'Child #iid, reference or URL'
+ condition { has_children? && can_admin_link? }
+ command :remove_child do |child_param|
+ @updates[:remove_child] = extract_work_items(child_param).first
+ @execution_message[:remove_child] = success_msg[:remove_child]
+ end
end
private
@@ -141,7 +153,8 @@ def success_msg
promote_to: _("Work item promoted successfully."),
set_parent: _('Work item parent set successfully'),
remove_parent: _('Work item parent removed successfully'),
- add_child: _('Child work item(s) added successfully')
+ add_child: _('Child work item(s) added successfully'),
+ remove_child: _('Child work item removed successfully')
}
end
@@ -157,6 +170,10 @@ def supports_children?
::WorkItems::HierarchyRestriction.find_by_parent_type_id(quick_action_target.work_item_type_id).present?
end
+ def has_children?
+ supports_children? && quick_action_target.work_item_children.present?
+ end
+
def can_admin_link?
current_user.can?(:admin_issue_link, quick_action_target)
end
diff --git a/lib/gitlab/usage_data_counters/hll_redis_legacy_events.yml b/lib/gitlab/usage_data_counters/hll_redis_legacy_events.yml
index 780d9d3092a17..7220e3c5ad620 100644
--- a/lib/gitlab/usage_data_counters/hll_redis_legacy_events.yml
+++ b/lib/gitlab/usage_data_counters/hll_redis_legacy_events.yml
@@ -283,6 +283,7 @@
- i_quickactions_remove_estimate
- i_quickactions_remove_iteration
- i_quickactions_remove_milestone
+- i_quickactions_remove_child
- i_quickactions_remove_parent
- i_quickactions_remove_parent_epic
- i_quickactions_remove_time_spent
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 8bdd11c1a8c47..9e2e87b9413e4 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -1775,9 +1775,6 @@ msgstr ""
msgid "A Let's Encrypt SSL certificate can not be obtained until your domain is verified."
msgstr ""
-msgid "A Work Item can be a parent or a child, but not both."
-msgstr ""
-
msgid "A basic folder structure of Astro Starter Kit, to help you get started."
msgstr ""
@@ -10431,6 +10428,9 @@ msgstr ""
msgid "Child issues and epics"
msgstr ""
+msgid "Child work item removed successfully"
+msgstr ""
+
msgid "Child work item(s) added successfully"
msgstr ""
@@ -34275,6 +34275,9 @@ msgstr ""
msgid "Once imported, repositories can be mirrored over SSH. Read more %{link_start}here%{link_end}."
msgstr ""
+msgid "One and only one of %{params} is required"
+msgstr ""
+
msgid "One more item"
msgid_plural "%d more items"
msgstr[0] ""
@@ -41090,6 +41093,9 @@ msgstr ""
msgid "Remove child epic from an epic"
msgstr ""
+msgid "Remove child from work item"
+msgstr ""
+
msgid "Remove customer relation contact(s)."
msgstr ""
@@ -41195,6 +41201,9 @@ msgstr ""
msgid "Remove summary"
msgstr ""
+msgid "Remove the child %{child_ref} from this work item."
+msgstr ""
+
msgid "Remove time estimate"
msgstr ""
diff --git a/spec/services/notes/quick_actions_service_spec.rb b/spec/services/notes/quick_actions_service_spec.rb
index 091c47aa793b2..8e2ae1e0a8443 100644
--- a/spec/services/notes/quick_actions_service_spec.rb
+++ b/spec/services/notes/quick_actions_service_spec.rb
@@ -387,6 +387,48 @@
end
end
+ describe '/remove_child' do
+ let_it_be_with_reload(:noteable) { create(:work_item, :objective, project: project) }
+ let_it_be_with_reload(:child) { create(:work_item, :objective, project: project) }
+ let_it_be(:note_text) { "/remove_child #{child.to_reference}" }
+ let_it_be(:note) { create(:note, noteable: noteable, project: project, note: note_text) }
+
+ before do
+ create(:parent_link, work_item_parent: noteable, work_item: child)
+ end
+
+ shared_examples 'removes child work item' do
+ it 'leaves the note empty' do
+ expect(execute(note)).to be_empty
+ end
+
+ it 'removes child work item' do
+ expect { execute(note) }.to change { WorkItems::ParentLink.count }.by(-1)
+
+ expect(noteable.valid?).to be_truthy
+ expect(noteable.work_item_children).to be_empty
+ end
+ end
+
+ context 'when using work item reference' do
+ let_it_be(:note_text) { "/remove_child #{child.to_reference(full: true)}" }
+
+ it_behaves_like 'removes child work item'
+ end
+
+ context 'when using work item iid' do
+ it_behaves_like 'removes child work item'
+ end
+
+ context 'when using work item URL' do
+ let_it_be(:project_path) { "#{Gitlab.config.gitlab.url}/#{project.full_path}" }
+ let_it_be(:url) { "#{project_path}/work_items/#{child.iid}" }
+ let_it_be(:note_text) { "/remove_child #{url}" }
+
+ it_behaves_like 'removes child work item'
+ end
+ end
+
describe '/set_parent' do
let_it_be_with_reload(:noteable) { create(:work_item, :objective, project: project) }
let_it_be_with_reload(:parent) { create(:work_item, :objective, project: project) }
diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb
index 4ea3467e33cba..8664f13ab167c 100644
--- a/spec/services/quick_actions/interpret_service_spec.rb
+++ b/spec/services/quick_actions/interpret_service_spec.rb
@@ -3531,6 +3531,59 @@
it_behaves_like 'command is not available'
end
end
+
+ describe '/remove child command' do
+ let_it_be(:child) { create(:work_item, :objective, project: project) }
+ let_it_be(:work_item) { create(:work_item, :objective, project: project) }
+ let_it_be(:child_ref) { child.to_reference(project) }
+
+ let(:command) { "/remove_child #{child_ref}" }
+
+ shared_examples 'command is available' do
+ before do
+ create(:parent_link, work_item_parent: work_item, work_item: child)
+ end
+
+ it 'explanation contains correct message' do
+ _, explanations = service.explain(command, work_item)
+
+ expect(explanations)
+ .to contain_exactly("Remove the child #{child_ref} from this work item.")
+ end
+
+ it 'contains command' do
+ expect(service.available_commands(work_item)).to include(a_hash_including(name: :remove_child))
+ end
+ end
+
+ shared_examples 'command is not available' do
+ it 'explanation is empty' do
+ _, explanations = service.explain(command, work_item)
+
+ expect(explanations).to eq([])
+ end
+
+ it 'does not contain command' do
+ expect(service.available_commands(work_item)).not_to include(a_hash_including(name: :remove_child))
+ end
+ end
+
+ context 'when user can admin link' do
+ it_behaves_like 'command is available'
+ end
+
+ context 'when user cannot admin link' do
+ subject(:service) { described_class.new(project, create(:user)) }
+
+ it_behaves_like 'command is not available'
+ end
+
+ context 'when work item does not support children' do
+ let_it_be(:work_item) { create(:work_item, :key_result, project: project) }
+
+ it_behaves_like 'command is not available'
+ end
+ end
end
describe '#available_commands' do
diff --git a/spec/services/work_items/widgets/hierarchy_service/update_service_spec.rb b/spec/services/work_items/widgets/hierarchy_service/update_service_spec.rb
index 229ba81d676de..30e20d425184f 100644
--- a/spec/services/work_items/widgets/hierarchy_service/update_service_spec.rb
+++ b/spec/services/work_items/widgets/hierarchy_service/update_service_spec.rb
@@ -27,10 +27,18 @@
describe '#update' do
subject { described_class.new(widget: widget, current_user: user).before_update_in_transaction(params: params) }
- context 'when parent and children params are present' do
- let(:params) { { parent: parent_work_item, children: [child_work_item] } }
+ context 'when multiple params are present' do
+ it_behaves_like 'raises a WidgetError', 'One and only one of children, parent or remove_child is required' do
+ let(:params) { { parent: parent_work_item, children: [child_work_item] } }
+ end
+
+ it_behaves_like 'raises a WidgetError', 'One and only one of children, parent or remove_child is required' do
+ let(:params) { { parent: parent_work_item, remove_child: child_work_item } }
+ end
- it_behaves_like 'raises a WidgetError', 'A Work Item can be a parent or a child, but not both.'
+ it_behaves_like 'raises a WidgetError', 'One and only one of children, parent or remove_child is required' do
+ let(:params) { { remove_child: child_work_item, children: [child_work_item] } }
+ end
end
context 'when invalid params are present' do
@@ -104,6 +112,16 @@
end
end
+ context 'with remove_child param' do
+ let(:params) { { remove_child: [child_work_item] } }
+
+ it 'correctly removes the work item child' do
+ expect { subject }.to change { WorkItems::ParentLink.count }.by(-1)
+
+ expect(work_item.reload.work_item_children).to be_empty
+ end
+ end
+
context 'when child is already assigned' do
let(:params) { { children: [child_work_item] } }
--
GitLab