From 79b0b0481167a35c4ff97c38b76b5efdc78bceec Mon Sep 17 00:00:00 2001
From: Eugenia Grieff <egrieff@gitlab.com>
Date: Fri, 30 Jun 2023 07:07:41 +0000
Subject: [PATCH] Add todo quick actions to work items
Fix target_type todos column for work items
Update documentation
Changelog: added
---
app/graphql/types/current_user_todos.rb | 3 +-
app/models/todo.rb | 13 +++-
app/models/work_item.rb | 10 +++-
app/models/work_items/widgets/base.rb | 4 ++
.../work_items/widgets/current_user_todos.rb | 13 ++++
app/services/todo_service.rb | 2 +-
doc/user/project/quick_actions.md | 2 +
spec/models/work_item_spec.rb | 59 ++++++++++++++++--
spec/models/work_items/widgets/base_spec.rb | 6 ++
.../widgets/current_user_todos_spec.rb | 60 +++++++++++++++++++
.../quick_actions/interpret_service_spec.rb | 12 ++++
spec/services/todo_service_spec.rb | 51 ++++++++++++++++
.../work_items/update_service_spec.rb | 32 ++++++++++
.../update_service_spec.rb | 2 +-
14 files changed, 258 insertions(+), 11 deletions(-)
create mode 100644 spec/models/work_items/widgets/current_user_todos_spec.rb
diff --git a/app/graphql/types/current_user_todos.rb b/app/graphql/types/current_user_todos.rb
index 4c4cb51697988..d5441ea1d1562 100644
--- a/app/graphql/types/current_user_todos.rb
+++ b/app/graphql/types/current_user_todos.rb
@@ -17,7 +17,8 @@ module CurrentUserTodos
def current_user_todos(state: nil)
state ||= %i[done pending] # TodosFinder treats a `nil` state param as `pending`
- key = [state, unpresented.class.name]
+ target_type_name = unpresented.try(:todoable_target_type_name) || unpresented.class.name
+ key = [state, target_type_name]
BatchLoader::GraphQL.for(unpresented).batch(default_value: [], key: key) do |targets, loader, args|
state, klass_name = args[:key]
diff --git a/app/models/todo.rb b/app/models/todo.rb
index 724f97c4812f0..f202e1a266dd6 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -117,7 +117,18 @@ def for_group_ids_and_descendants(group_ids)
# target - The value of the `target_type` column, such as `Issue`.
# state - The value of the `state` column, such as `pending` or `done`.
def any_for_target?(target, state = nil)
- state.nil? ? exists?(target: target) : exists?(target: target, state: state)
+ conditions = {}
+
+ if target.respond_to?(:todoable_target_type_name)
+ conditions[:target_type] = target.todoable_target_type_name
+ conditions[:target_id] = target.id
+ else
+ conditions[:target] = target
+ end
+
+ conditions[:state] = state unless state.nil?
+
+ exists?(conditions)
end
# Updates attributes of a relation of todos to the new state.
diff --git a/app/models/work_item.rb b/app/models/work_item.rb
index 9f28ffbf7b6bd..adf424a1d94cb 100644
--- a/app/models/work_item.rb
+++ b/app/models/work_item.rb
@@ -65,6 +65,12 @@ def noteable_target_type_name
'issue'
end
+ # Todo: remove method after target_type cleanup
+ # See https://gitlab.com/gitlab-org/gitlab/-/issues/416009
+ def todoable_target_type_name
+ %w[Issue WorkItem]
+ end
+
def widgets
strong_memoize(:widgets) do
work_item_type.widgets.map do |widget_class|
@@ -114,7 +120,9 @@ def transform_quick_action_params(command_params)
.filter { |param_name| common_params.key?(param_name) }
.each do |param_name|
widget_params[widget.api_symbol] ||= {}
- widget_params[widget.api_symbol][param_name] = common_params.delete(param_name)
+ param_value = common_params.delete(param_name)
+
+ widget_params[widget.api_symbol].merge!(widget.process_quick_action_param(param_name, param_value))
end
end
diff --git a/app/models/work_items/widgets/base.rb b/app/models/work_items/widgets/base.rb
index a8b1b3f9a59af..c4e87decdbffc 100644
--- a/app/models/work_items/widgets/base.rb
+++ b/app/models/work_items/widgets/base.rb
@@ -15,6 +15,10 @@ def self.quick_action_commands
[]
end
+ def self.process_quick_action_param(param_name, value)
+ { param_name => value }
+ end
+
def self.callback_class
WorkItems::Callbacks.const_get(name.demodulize, false)
rescue NameError
diff --git a/app/models/work_items/widgets/current_user_todos.rb b/app/models/work_items/widgets/current_user_todos.rb
index 61c4fcb453bc0..64297b433dd8b 100644
--- a/app/models/work_items/widgets/current_user_todos.rb
+++ b/app/models/work_items/widgets/current_user_todos.rb
@@ -3,6 +3,19 @@
module WorkItems
module Widgets
class CurrentUserTodos < Base
+ def self.quick_action_commands
+ [:todo, :done]
+ end
+
+ def self.quick_action_params
+ [:todo_event]
+ end
+
+ def self.process_quick_action_param(param_name, value)
+ return super unless param_name == :todo_event
+
+ { action: value == 'done' ? 'mark_as_done' : 'add' }
+ end
end
end
end
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index c55e1680bfef2..1f6cf2c83c95f 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -377,7 +377,7 @@ def attributes_for_target(target)
attributes = {
project_id: target&.project&.id,
target_id: target.id,
- target_type: target.class.name,
+ target_type: target.class.try(:polymorphic_name) || target.class.name,
commit_id: nil
}
diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md
index 3e8ce00974088..219255993a34e 100644
--- a/doc/user/project/quick_actions.md
+++ b/doc/user/project/quick_actions.md
@@ -165,6 +165,8 @@ The following quick actions can be applied through the description field when ed
| `/clear_weight` | **{check-circle}** Yes | **{dotted-circle}** No | **{dotted-circle}** No | Clear weight.
| `/type` | **{check-circle}** Yes | **{dotted-circle}** Yes | **{dotted-circle}** Yes | Converts work item to specified type. Available options for `<type>` include `Issue`, `Task`, `Objective` and `Key Result`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/385227) in GitLab 16.0.
| `/promote_to <type>` | **{check-circle}** Yes | **{dotted-circle}** No | **{check-circle}** Yes | Promotes work item to specified type. Available options for `<type>`: `issue` (promote a task) and `objective` (promote a key result). [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/412534) in GitLab 16.1.
+| `/todo` | **{check-circle}** Yes | **{check-circle}** Yes | **{check-circle}** Yes | Add a to-do item. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/412277) in GitLab 16.2.
+| `/done` | **{check-circle}** Yes | **{check-circle}** Yes | **{check-circle}** Yes | Mark to-do item as done. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/412277) in GitLab 16.2.
## Commit messages
diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb
index bf43a2032e736..b2c0d757a5c91 100644
--- a/spec/models/work_item_spec.rb
+++ b/spec/models/work_item_spec.rb
@@ -87,6 +87,14 @@
end
end
+ describe '#todoable_target_type_name' do
+ it 'returns correct target name' do
+ work_item = build(:work_item)
+
+ expect(work_item.todoable_target_type_name).to contain_exactly('Issue', 'WorkItem')
+ end
+ end
+
describe '#widgets' do
subject { build(:work_item).widgets }
@@ -176,17 +184,32 @@
is_expected.not_to include(:due, :remove_due_date)
end
end
+
+ context 'when work item supports the current user todos widget' do
+ it 'returns todos related quick action commands' do
+ is_expected.to include(:todo, :done)
+ end
+ end
+
+ context 'when work item does not support current user todos widget' do
+ let(:work_item) { build(:work_item, :task) }
+
+ before do
+ WorkItems::Type.default_by_type(:task).widget_definitions
+ .find_by_widget_type(:current_user_todos).update!(disabled: true)
+ end
+
+ it 'omits todos related quick action commands' do
+ is_expected.not_to include(:todo, :done)
+ end
+ end
end
describe 'transform_quick_action_params' do
+ let(:command_params) { { title: 'bar', assignee_ids: ['foo'] } }
let(:work_item) { build(:work_item, :task) }
- subject(:transformed_params) do
- work_item.transform_quick_action_params({
- title: 'bar',
- assignee_ids: ['foo']
- })
- end
+ subject(:transformed_params) { work_item.transform_quick_action_params(command_params) }
it 'correctly separates widget params from regular params' do
expect(transformed_params).to eq({
@@ -200,6 +223,30 @@
}
})
end
+
+ context 'with current user todos widget' do
+ let(:command_params) { { title: 'bar', todo_event: param } }
+
+ where(:param, :expected) do
+ 'done' | 'mark_as_done'
+ 'add' | 'add'
+ end
+
+ with_them do
+ it 'correctly transform todo_event param' do
+ expect(transformed_params).to eq({
+ common: {
+ title: 'bar'
+ },
+ widgets: {
+ current_user_todos_widget: {
+ action: expected
+ }
+ }
+ })
+ end
+ end
+ end
end
describe 'callbacks' do
diff --git a/spec/models/work_items/widgets/base_spec.rb b/spec/models/work_items/widgets/base_spec.rb
index 9b4b4d9e98fae..29b54a706c2f8 100644
--- a/spec/models/work_items/widgets/base_spec.rb
+++ b/spec/models/work_items/widgets/base_spec.rb
@@ -16,4 +16,10 @@
it { is_expected.to eq(:base) }
end
+
+ describe '.process_quick_action_param' do
+ subject { described_class.process_quick_action_param(:label_ids, [1, 2]) }
+
+ it { is_expected.to eq({ label_ids: [1, 2] }) }
+ end
end
diff --git a/spec/models/work_items/widgets/current_user_todos_spec.rb b/spec/models/work_items/widgets/current_user_todos_spec.rb
new file mode 100644
index 0000000000000..9fdf28beada85
--- /dev/null
+++ b/spec/models/work_items/widgets/current_user_todos_spec.rb
@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe WorkItems::Widgets::CurrentUserTodos, feature_category: :team_planning do
+ let_it_be(:project) { create(:project) }
+ let_it_be(:milestone) { create(:milestone, project: project) }
+ let_it_be(:work_item) { create(:work_item, :issue, project: project, milestone: milestone) }
+
+ describe '.type' do
+ subject { described_class.type }
+
+ it { is_expected.to eq(:current_user_todos) }
+ end
+
+ describe '#type' do
+ subject { described_class.new(work_item).type }
+
+ it { is_expected.to eq(:current_user_todos) }
+ end
+
+ describe '.quick_action_params' do
+ subject { described_class.quick_action_params }
+
+ it { is_expected.to contain_exactly(:todo_event) }
+ end
+
+ describe '.quick_action_commands' do
+ subject { described_class.quick_action_commands }
+
+ it { is_expected.to contain_exactly(:todo, :done) }
+ end
+
+ describe '.process_quick_action_param' do
+ subject { described_class.process_quick_action_param(param_name, param_value) }
+
+ context 'when quick action param is todo_event' do
+ let(:param_name) { :todo_event }
+
+ context 'when param value is `done`' do
+ let(:param_value) { 'done' }
+
+ it { is_expected.to eq({ action: 'mark_as_done' }) }
+ end
+
+ context 'when param value is `add`' do
+ let(:param_value) { 'add' }
+
+ it { is_expected.to eq({ action: 'add' }) }
+ end
+ end
+
+ context 'when quick action param is not todo_event' do
+ let(:param_name) { :foo }
+ let(:param_value) { 'foo' }
+
+ it { is_expected.to eq({ foo: 'foo' }) }
+ end
+ end
+end
diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb
index bd09dae0a5a74..27a966f0521ba 100644
--- a/spec/services/quick_actions/interpret_service_spec.rb
+++ b/spec/services/quick_actions/interpret_service_spec.rb
@@ -35,6 +35,7 @@
end
describe '#execute' do
+ let_it_be(:work_item) { create(:work_item, :task, project: project) }
let(:merge_request) { create(:merge_request, source_project: project) }
shared_examples 'reopen command' do
@@ -1369,6 +1370,11 @@
let(:issuable) { merge_request }
end
+ it_behaves_like 'done command' do
+ let(:content) { '/done' }
+ let(:issuable) { work_item }
+ end
+
it_behaves_like 'subscribe command' do
let(:content) { '/subscribe' }
let(:issuable) { issue }
@@ -1590,6 +1596,12 @@
end
end
+ context 'if issuable is a work item' do
+ it_behaves_like 'todo command' do
+ let(:issuable) { work_item }
+ end
+ end
+
context 'if issuable is a MergeRequest' do
it_behaves_like 'todo command' do
let(:issuable) { merge_request }
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 1ec6a3250fcc6..32e17df4d693b 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -717,6 +717,57 @@
end
end
+ describe 'Work Items' do
+ let_it_be(:work_item) { create(:work_item, :task, project: project, author: author) }
+
+ describe '#mark_todo' do
+ it 'creates a todo from a work item' do
+ service.mark_todo(work_item, author)
+
+ should_create_todo(user: author, target: work_item, action: Todo::MARKED)
+ end
+ end
+
+ describe '#todo_exists?' do
+ it 'returns false when no todo exist for the given work_item' do
+ expect(service.todo_exist?(work_item, author)).to be_falsy
+ end
+
+ it 'returns true when a todo exist for the given work_item' do
+ service.mark_todo(work_item, author)
+
+ expect(service.todo_exist?(work_item, author)).to be_truthy
+ end
+ end
+
+ describe '#resolve_todos_for_target' do
+ it 'marks related pending todos to the target for the user as done' do
+ first_todo = create(:todo, :assigned, user: john_doe, project: project, target: work_item, author: author)
+ second_todo = create(:todo, :assigned, user: john_doe, project: project, target: work_item, author: author)
+
+ service.resolve_todos_for_target(work_item, john_doe)
+
+ expect(first_todo.reload).to be_done
+ expect(second_todo.reload).to be_done
+ end
+
+ describe 'cached counts' do
+ it 'updates when todos change' do
+ create(:todo, :assigned, user: john_doe, project: project, target: work_item, author: author)
+
+ expect(john_doe.todos_done_count).to eq(0)
+ expect(john_doe.todos_pending_count).to eq(1)
+ expect(john_doe).to receive(:update_todos_count_cache).and_call_original
+
+ service.resolve_todos_for_target(work_item, john_doe)
+
+ expect(john_doe.todos_done_count).to eq(1)
+ expect(john_doe.todos_pending_count).to eq(0)
+ end
+ end
+ end
+ end
+
describe '#reassigned_assignable' do
let(:described_method) { :reassigned_assignable }
diff --git a/spec/services/work_items/update_service_spec.rb b/spec/services/work_items/update_service_spec.rb
index 30c1645835369..40684dd293698 100644
--- a/spec/services/work_items/update_service_spec.rb
+++ b/spec/services/work_items/update_service_spec.rb
@@ -383,6 +383,38 @@
end
end
end
+
+ context 'for current user todos widget' do
+ let_it_be(:user_todo) { create(:todo, target: work_item, user: developer, project: project, state: :pending) }
+ let_it_be(:other_todo) { create(:todo, target: work_item, user: create(:user), project: project, state: :pending) }
+
+ context 'when action is mark_as_done' do
+ let(:widget_params) { { current_user_todos_widget: { action: 'mark_as_done' } } }
+
+ it 'marks current user todo as done' do
+ expect do
+ update_work_item
+ user_todo.reload
+ other_todo.reload
+ end.to change(user_todo, :state).from('pending').to('done').and not_change { other_todo.state }
+ end
+
+ it_behaves_like 'update service that triggers GraphQL work_item_updated subscription' do
+ subject(:execute_service) { update_work_item }
+ end
+ end
+
+ context 'when action is add' do
+ let(:widget_params) { { current_user_todos_widget: { action: 'add' } } }
+
+ it 'adds a ToDo for the work item' do
+ expect do
+ update_work_item
+ work_item.reload
+ end.to change(Todo, :count).by(1)
+ end
+ end
+ end
end
describe 'label updates' do
diff --git a/spec/services/work_items/widgets/current_user_todos_service/update_service_spec.rb b/spec/services/work_items/widgets/current_user_todos_service/update_service_spec.rb
index 85b7e7a70dfe9..aa7257e9e628d 100644
--- a/spec/services/work_items/widgets/current_user_todos_service/update_service_spec.rb
+++ b/spec/services/work_items/widgets/current_user_todos_service/update_service_spec.rb
@@ -56,7 +56,7 @@
todo = current_user.todos.last
- expect(todo.target).to eq(work_item)
+ expect(todo.target.id).to eq(work_item.id)
expect(todo).to be_pending
end
end
--
GitLab