From 49ece41320d2e1e6b8124ade140d1b12fe489fe5 Mon Sep 17 00:00:00 2001
From: Patrick Bajao <ebajao@gitlab.com>
Date: Thu, 20 Jul 2023 13:18:07 +0800
Subject: [PATCH] Add new REVIEW_SUBMITTED Todo action
The REVIEW_SUBMITTED todo gets created when a review is submitted.
This links to the MR that was reviewed.
---
app/graphql/types/todo_action_enum.rb | 1 +
app/helpers/todos_helper.rb | 1 +
app/models/todo.rb | 4 +++-
doc/api/graphql/reference/index.md | 1 +
ee/app/services/ee/todo_service.rb | 12 ++++++++++++
ee/spec/services/todo_service_spec.rb | 20 ++++++++++++++++++++
locale/gitlab.pot | 3 +++
spec/factories/todos.rb | 4 ++++
spec/helpers/todos_helper_spec.rb | 1 +
9 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/app/graphql/types/todo_action_enum.rb b/app/graphql/types/todo_action_enum.rb
index fda96796c0f2a..45b83ea1d6465 100644
--- a/app/graphql/types/todo_action_enum.rb
+++ b/app/graphql/types/todo_action_enum.rb
@@ -12,5 +12,6 @@ class TodoActionEnum < BaseEnum
value 'merge_train_removed', value: 8, description: 'Merge request authored by the user was removed from the merge train.'
value 'review_requested', value: 9, description: 'Review was requested from the user.'
value 'member_access_requested', value: 10, description: 'Group or project access requested from the user.'
+ value 'review_submitted', value: 11, description: 'Merge request authored by the user received a review.'
end
end
diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb
index 9b0810f3d17c6..4f17634f3e435 100644
--- a/app/helpers/todos_helper.rb
+++ b/app/helpers/todos_helper.rb
@@ -30,6 +30,7 @@ def todo_action_name(todo)
when Todo::MEMBER_ACCESS_REQUESTED then format(
s_("Todos|has requested access to %{what} %{which}"), what: _(todo.member_access_type), which: _(todo.target.name)
)
+ when Todo::REVIEW_SUBMITTED then s_('Todos|reviewed your merge request')
end
end
diff --git a/app/models/todo.rb b/app/models/todo.rb
index f202e1a266dd6..2f2e731fc7ef7 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -24,6 +24,7 @@ class Todo < ApplicationRecord
MERGE_TRAIN_REMOVED = 8 # This is an EE-only feature
REVIEW_REQUESTED = 9
MEMBER_ACCESS_REQUESTED = 10
+ REVIEW_SUBMITTED = 11 # This is an EE-only feature
ACTION_NAMES = {
ASSIGNED => :assigned,
@@ -35,7 +36,8 @@ class Todo < ApplicationRecord
UNMERGEABLE => :unmergeable,
DIRECTLY_ADDRESSED => :directly_addressed,
MERGE_TRAIN_REMOVED => :merge_train_removed,
- MEMBER_ACCESS_REQUESTED => :member_access_requested
+ MEMBER_ACCESS_REQUESTED => :member_access_requested,
+ REVIEW_SUBMITTED => :review_submitted
}.freeze
ACTIONS_MULTIPLE_ALLOWED = [Todo::MENTIONED, Todo::DIRECTLY_ADDRESSED, Todo::MEMBER_ACCESS_REQUESTED].freeze
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index cbf39cf343ff4..820f598951278 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -26956,6 +26956,7 @@ Values for sorting timelogs.
| <a id="todoactionenummentioned"></a>`mentioned` | User was mentioned. |
| <a id="todoactionenummerge_train_removed"></a>`merge_train_removed` | Merge request authored by the user was removed from the merge train. |
| <a id="todoactionenumreview_requested"></a>`review_requested` | Review was requested from the user. |
+| <a id="todoactionenumreview_submitted"></a>`review_submitted` | Merge request authored by the user received a review. |
| <a id="todoactionenumunmergeable"></a>`unmergeable` | Merge request authored by the user could not be merged. |
### `TodoStateEnum`
diff --git a/ee/app/services/ee/todo_service.rb b/ee/app/services/ee/todo_service.rb
index 6d1febdff64c6..cff159424a94d 100644
--- a/ee/app/services/ee/todo_service.rb
+++ b/ee/app/services/ee/todo_service.rb
@@ -40,6 +40,18 @@ def merge_train_removed(merge_request)
end
end
+ def review_submitted(review)
+ merge_request = review.merge_request
+
+ # We don't need to create a To-Do for the review author if they added a
+ # review for their own merge request.
+ return if merge_request.author == review.author
+
+ project = merge_request.project
+ attributes = attributes_for_todo(project, merge_request, review.author, ::Todo::REVIEW_SUBMITTED)
+ create_todos(merge_request.author, attributes, project.namespace, project)
+ end
+
private
override :attributes_for_target
diff --git a/ee/spec/services/todo_service_spec.rb b/ee/spec/services/todo_service_spec.rb
index e4179412281f6..ea77b413ee1f0 100644
--- a/ee/spec/services/todo_service_spec.rb
+++ b/ee/spec/services/todo_service_spec.rb
@@ -389,6 +389,26 @@
end
end
end
+
+ describe '#review_submitted' do
+ let(:review) { create(:review, merge_request: merge_request) }
+
+ before do
+ service.review_submitted(review)
+ end
+
+ it 'creates a pending todo for reviewed merge request author' do
+ should_create_todo(user: merge_request.author, author: review.author, target: merge_request, action: Todo::REVIEW_SUBMITTED)
+ end
+
+ context 'when merge request author is the review author' do
+ let(:review) { create(:review, merge_request: merge_request, author: merge_request.author) }
+
+ it 'does not create a pending todo for reviewed merge request author' do
+ should_not_create_todo(user: merge_request.author, author: review.author, target: merge_request, action: Todo::REVIEW_SUBMITTED)
+ end
+ end
+ end
end
def should_create_todo(attributes = {})
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index e7920c8e762a8..5f27e03299e7b 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -48428,6 +48428,9 @@ msgstr ""
msgid "Todos|requested a review"
msgstr ""
+msgid "Todos|reviewed your merge request"
+msgstr ""
+
msgid "Todos|set %{who} as an approver"
msgstr ""
diff --git a/spec/factories/todos.rb b/spec/factories/todos.rb
index 760367539fc5f..12d77abe5c901 100644
--- a/spec/factories/todos.rb
+++ b/spec/factories/todos.rb
@@ -45,6 +45,10 @@
action { Todo::MEMBER_ACCESS_REQUESTED }
end
+ trait :review_submitted do
+ action { Todo::REVIEW_SUBMITTED }
+ end
+
trait :pending do
state { :pending }
end
diff --git a/spec/helpers/todos_helper_spec.rb b/spec/helpers/todos_helper_spec.rb
index 9cbcca69dc893..dfb5cb995bc75 100644
--- a/spec/helpers/todos_helper_spec.rb
+++ b/spec/helpers/todos_helper_spec.rb
@@ -370,6 +370,7 @@
Todo::APPROVAL_REQUIRED | false | format(s_("Todos|set %{who} as an approver"), who: _('you'))
Todo::UNMERGEABLE | true | s_('Todos|Could not merge')
Todo::MERGE_TRAIN_REMOVED | true | s_("Todos|Removed from Merge Train")
+ Todo::REVIEW_SUBMITTED | false | s_('Todos|reviewed your merge request')
end
with_them do
--
GitLab