diff --git a/app/graphql/types/todo_action_enum.rb b/app/graphql/types/todo_action_enum.rb index fda96796c0f2a6cb9dbb21917ba8ce798a91d0a3..45b83ea1d64657d736aa78a4d3de4a05ff9bb8a9 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 9b0810f3d17c6f47234a11431c8528f5b4a9f99a..4f17634f3e435b930fafbd77e3042ba4f9a222b0 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 f202e1a266dd6560dc4f131d13c5af02d11bf0c7..2f2e731fc7ef771dde091ff340a9f28abcb2f501 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 cbf39cf343ff41b2608cba32bcbb3589e64e9186..820f59895127803eca0e72d8d1786bc94cdea8da 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 6d1febdff64c640c77e4800262db5dfe8fa7dd9b..cff159424a94d71c31250bd5edb84a45f7a6bdd5 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 e4179412281f6dad16a631f05e68304269cdbaec..ea77b413ee1f0620424631744861a4210c6f537b 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 e7920c8e762a8243c0676ffa7ded9ad110c5d099..5f27e03299e7bcc6fe8800a1268b07319e4a2a9a 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 760367539fc5ff4d13b744ee452d813e1f16e2ec..12d77abe5c901932b428117e38a603480e227419 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 9cbcca69dc8933dbe7b4d1418815f83e23029669..dfb5cb995bc75e2d359d3563dca7041547755c81 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