From d694710755553e628248912301e3927a07596e23 Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Wed, 9 Apr 2025 18:18:13 +0200 Subject: [PATCH 1/9] Rename notification for being added as approver From: You are added as an approver on a merge request To: Merge request you're eligible to approve is created This clarifies what triggers the notification. Changelog: other --- app/assets/javascripts/notifications/constants.js | 2 +- doc/user/profile/notifications.md | 2 +- doc/user/project/merge_requests/approvals/rules.md | 4 ++++ doc/user/todos.md | 2 +- ee/app/views/notify/added_as_approver_email.html.haml | 2 +- ee/app/views/notify/added_as_approver_email.text.erb | 2 +- ee/spec/mailers/emails/merge_requests_spec.rb | 4 +++- locale/gitlab.pot | 10 +++++----- 8 files changed, 17 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/notifications/constants.js b/app/assets/javascripts/notifications/constants.js index d32e0dcd0ad495..48f729738495dc 100644 --- a/app/assets/javascripts/notifications/constants.js +++ b/app/assets/javascripts/notifications/constants.js @@ -56,6 +56,6 @@ export const i18n = { reopen_issue: s__('NotificationEvent|Issue is reopened'), reopen_merge_request: s__('NotificationEvent|Merge request is reopened'), success_pipeline: s__('NotificationEvent|Pipeline is successful'), - approver: s__('NotificationEvent|You are added as an approver on a merge request'), + approver: s__("NotificationEvent|Merge request you're eligible to approve is created"), }, }; diff --git a/doc/user/profile/notifications.md b/doc/user/profile/notifications.md index d85a9616385bec..e2e2d5ed92f1ed 100644 --- a/doc/user/profile/notifications.md +++ b/doc/user/profile/notifications.md @@ -300,7 +300,7 @@ epics: | Merge Request | Review requested | Participants, Watchers, Subscribers, Custom notification level with this event selected, and the old reviewer. | | Merge Request | Reopened | Subscribers and participants. | | Merge Request | Title or description changed | Any new mentions by username. | -| Merge Request | Added as approver | Custom notification level with this event selected. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12855) in GitLab 16.7. | +| Merge Request | Merge request you're [eligible to approve](../project/merge_requests/approvals/rules.md#eligible-approvers) is created | Custom notification level with this event selected. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12855) in GitLab 16.7. [Renamed](https://gitlab.com/gitlab-org/gitlab/-/issues/465347) from "Added as approver" in GitLab 17.11. | | Pipeline | Failed | The author of the pipeline. | | Pipeline | Fixed | The author of the pipeline. Enabled by default. | | Pipeline | Successful | The author of the pipeline, with Custom notification level for successful pipelines. If the pipeline failed previously, a "Fixed pipeline" message is sent for the first successful pipeline after the failure, and then a "Successful pipeline" message for any further successful pipelines. | diff --git a/doc/user/project/merge_requests/approvals/rules.md b/doc/user/project/merge_requests/approvals/rules.md index 3c5998ffcaede7..3a959def46a210 100644 --- a/doc/user/project/merge_requests/approvals/rules.md +++ b/doc/user/project/merge_requests/approvals/rules.md @@ -158,6 +158,10 @@ approvals from other users with at least the Developer role in the project count toward meeting the required number of approvals, even if the users were not explicitly listed in the approval rules. +To get email notifications every time a merge request you're eligible to approve is created, +[set your notification level](../../../profile/notifications.md#edit-notification-settings) to Custom +and select this event. + ### Group approvers You can add a group of users as approvers. All **direct members** of this group diff --git a/doc/user/todos.md b/doc/user/todos.md index 736918aa4d14a3..0de560468ffe23 100644 --- a/doc/user/todos.md +++ b/doc/user/todos.md @@ -92,7 +92,7 @@ For other actions that create to-do items like assignments or review requests, you receive only one notification per action type, even if that action occurs multiple times in the same issue or merge request. To-do items aren't affected by [GitLab notification email settings](profile/notifications.md). -The only exception: If your notification setting is set to **Custom** and **Added as approver** is +The only exception: If your notification setting is set to **Custom** and **Merge request you're eligible to approve is created** is selected, you get a to-do item when you are eligible to approve a merge request. ## Create a to-do item diff --git a/ee/app/views/notify/added_as_approver_email.html.haml b/ee/app/views/notify/added_as_approver_email.html.haml index 0a7c07c7aa2139..fa6e9d35fafbcd 100644 --- a/ee/app/views/notify/added_as_approver_email.html.haml +++ b/ee/app/views/notify/added_as_approver_email.html.haml @@ -1,2 +1,2 @@ %p - = _('%{strong_start}%{author}%{strong_end} has added you as an approver.').html_safe % { author: @merge_request.author.name, strong_start: ''.html_safe, strong_end: ''.html_safe } + = _('%{strong_start}%{author}%{strong_end} has created a merge request that you can approve.').html_safe % { author: @merge_request.author.name, strong_start: ''.html_safe, strong_end: ''.html_safe } diff --git a/ee/app/views/notify/added_as_approver_email.text.erb b/ee/app/views/notify/added_as_approver_email.text.erb index e175b72ecfd92c..c1b16b15915614 100644 --- a/ee/app/views/notify/added_as_approver_email.text.erb +++ b/ee/app/views/notify/added_as_approver_email.text.erb @@ -1 +1 @@ -<%= _('%{author} has added you as an approver.') % { author: @merge_request.author.name } %> +<%= _('%{author} has created a merge request that you can approve.') % { author: @merge_request.author.name } %> diff --git a/ee/spec/mailers/emails/merge_requests_spec.rb b/ee/spec/mailers/emails/merge_requests_spec.rb index ee0ff7b4805d93..6a1cd805c3371e 100644 --- a/ee/spec/mailers/emails/merge_requests_spec.rb +++ b/ee/spec/mailers/emails/merge_requests_spec.rb @@ -25,7 +25,9 @@ path = project_merge_request_url(merge_request.target_project, merge_request) is_expected.to have_referable_subject(merge_request, reply: true) - is_expected.to have_body_text("#{current_user.name} has added you as an approver.") + is_expected.to have_body_text( + "#{current_user.name} has created a merge request that you can approve." + ) is_expected.to have_link("View it on GitLab", href: path) end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 373e22884e3934..f0bd8763592f7e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -631,7 +631,7 @@ msgstr "" msgid "%{author} commented %{time}" msgstr "" -msgid "%{author} has added you as an approver." +msgid "%{author} has created a merge request that you can approve." msgstr "" msgid "%{author} requested to merge %{source_branch} %{copy_button} into %{target_branch} %{target_copy_button} %{created_at}" @@ -1439,7 +1439,7 @@ msgstr "" msgid "%{strongStart}Tip:%{strongEnd} You can also %{linkStart}check out with merge request ID%{linkEnd}." msgstr "" -msgid "%{strong_start}%{author}%{strong_end} has added you as an approver." +msgid "%{strong_start}%{author}%{strong_end} has created a merge request that you can approve." msgstr "" msgid "%{strong_start}%{branch_count}%{strong_end} Branch" @@ -40720,6 +40720,9 @@ msgstr "" msgid "NotificationEvent|Merge request reviewers are changed" msgstr "" +msgid "NotificationEvent|Merge request you're eligible to approve is created" +msgstr "" + msgid "NotificationEvent|Merge when pipeline succeeds" msgstr "" @@ -40771,9 +40774,6 @@ msgstr "" msgid "NotificationEvent|Reopen merge request" msgstr "" -msgid "NotificationEvent|You are added as an approver on a merge request" -msgstr "" - msgid "NotificationLevel|Custom" msgstr "" -- GitLab From 4032df89036c2df88c04d03b045478201a7ba68e Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Wed, 9 Apr 2025 19:05:25 +0200 Subject: [PATCH 2/9] Clarify GraphQL and UI text for approver to-dos --- app/assets/javascripts/todos/components/todo_item_body.vue | 4 +++- app/graphql/types/todo_action_enum.rb | 4 ++-- doc/api/graphql/reference/_index.md | 4 ++-- locale/gitlab.pot | 6 +++--- spec/features/dashboard/todos/todos_spec.rb | 2 +- spec/frontend/todos/components/todo_item_body_spec.js | 4 ++-- 6 files changed, 13 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/todos/components/todo_item_body.vue b/app/assets/javascripts/todos/components/todo_item_body.vue index 9dfb438e5864c4..3ef15eeff557f6 100644 --- a/app/assets/javascripts/todos/components/todo_item_body.vue +++ b/app/assets/javascripts/todos/components/todo_item_body.vue @@ -126,7 +126,9 @@ export default { this.todo.action === TODO_ACTION_TYPE_APPROVAL_REQUIRED || this.todo.action === TODO_ACTION_TYPE_ADDED_APPROVER ) { - name = sprintf(s__('Todos|set %{who} as an approver'), { who: this.actionSubject }); + name = sprintf(s__('Todos|created a merge request %{who} can approve'), { + who: this.actionSubject, + }); } if (this.todo.action === TODO_ACTION_TYPE_UNMERGEABLE) { diff --git a/app/graphql/types/todo_action_enum.rb b/app/graphql/types/todo_action_enum.rb index 2fc1760582650e..9a7cba34979812 100644 --- a/app/graphql/types/todo_action_enum.rb +++ b/app/graphql/types/todo_action_enum.rb @@ -6,7 +6,7 @@ class TodoActionEnum < BaseEnum value 'mentioned', value: 2, description: 'User was mentioned.' value 'build_failed', value: 3, description: 'Build triggered by the user failed.' value 'marked', value: 4, description: 'User added a to-do item.' - value 'approval_required', value: 5, description: 'User was set as an approver.' + value 'approval_required', value: 5, description: 'User was set as a required approver.' value 'unmergeable', value: 6, description: 'Merge request authored by the user could not be merged.' value 'directly_addressed', value: 7, description: 'User was directly addressed.' value 'merge_train_removed', value: 8, description: 'Merge request authored by the user was removed from the merge train.' @@ -14,7 +14,7 @@ class TodoActionEnum < BaseEnum 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.' value 'okr_checkin_requested', value: 12, description: 'An OKR assigned to the user requires an update.' - value 'added_approver', value: 13, description: 'User was added as an approver.' + value 'added_approver', value: 13, description: 'User is eligible to approvea merge request.' value 'ssh_key_expired', value: 14, description: 'SSH key of the user has expired.' value 'ssh_key_expiring_soon', value: 15, description: 'SSH key of the user will expire soon.' value 'duo_pro_access_granted', value: 16, description: 'Access to Duo Pro has been granted to the user.' diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index b44e0789ce6fac..a3638e19ca034d 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -44903,8 +44903,8 @@ Values for sorting timelogs. | Value | Description | | ----- | ----------- | -| `added_approver` | User was added as an approver. | -| `approval_required` | User was set as an approver. | +| `added_approver` | User is eligible to approvea merge request. | +| `approval_required` | User was set as a required approver. | | `assigned` | User was assigned. | | `build_failed` | Build triggered by the user failed. | | `directly_addressed` | User was directly addressed. | diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f0bd8763592f7e..4d316247419e85 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -63003,6 +63003,9 @@ msgstr "" msgid "Todos|assigned you" msgstr "" +msgid "Todos|created a merge request %{who} can approve" +msgstr "" + msgid "Todos|has requested access to %{what} %{which}" msgstr "" @@ -63021,9 +63024,6 @@ msgstr "" msgid "Todos|reviewed your merge request" msgstr "" -msgid "Todos|set %{who} as an approver" -msgstr "" - msgid "Todos|you" msgstr "" diff --git a/spec/features/dashboard/todos/todos_spec.rb b/spec/features/dashboard/todos/todos_spec.rb index a60ec9f90f60d3..4cc003f5802f88 100644 --- a/spec/features/dashboard/todos/todos_spec.rb +++ b/spec/features/dashboard/todos/todos_spec.rb @@ -136,7 +136,7 @@ it 'shows you set yourself as an approver message' do within_testid('todo-item-list-container') do expect(page).to have_content("Fixes issue ยท #{project.namespace.owner_name} / #{project.name} #{merge_request.to_reference}") - expect(page).to have_content("You set yourself as an approver.") + expect(page).to have_content("You created a merge request you can approve.") end end end diff --git a/spec/frontend/todos/components/todo_item_body_spec.js b/spec/frontend/todos/components/todo_item_body_spec.js index e0a16043a2c533..0d5541162cc90f 100644 --- a/spec/frontend/todos/components/todo_item_body_spec.js +++ b/spec/frontend/todos/components/todo_item_body_spec.js @@ -64,8 +64,8 @@ describe('TodoItemBody', () => { describe('correct text for actionName', () => { it.each` actionName | text | showsAuthor - ${TODO_ACTION_TYPE_ADDED_APPROVER} | ${'set you as an approver.'} | ${true} - ${TODO_ACTION_TYPE_APPROVAL_REQUIRED} | ${'set you as an approver.'} | ${true} + ${TODO_ACTION_TYPE_ADDED_APPROVER} | ${'created a merge request you can approve.'} | ${true} + ${TODO_ACTION_TYPE_APPROVAL_REQUIRED} | ${'created a merge request you can approve.'} | ${true} ${TODO_ACTION_TYPE_ASSIGNED} | ${'assigned you.'} | ${true} ${TODO_ACTION_TYPE_BUILD_FAILED} | ${'The pipeline failed.'} | ${false} ${TODO_ACTION_TYPE_DIRECTLY_ADDRESSED} | ${'mentioned you.'} | ${true} -- GitLab From d3b05b4c080754233e408178528d5d0b99cba2bb Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Wed, 9 Apr 2025 19:11:14 +0200 Subject: [PATCH 3/9] Fix a typo --- app/graphql/types/todo_action_enum.rb | 2 +- doc/api/graphql/reference/_index.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/graphql/types/todo_action_enum.rb b/app/graphql/types/todo_action_enum.rb index 9a7cba34979812..508921ccb7aa3b 100644 --- a/app/graphql/types/todo_action_enum.rb +++ b/app/graphql/types/todo_action_enum.rb @@ -14,7 +14,7 @@ class TodoActionEnum < BaseEnum 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.' value 'okr_checkin_requested', value: 12, description: 'An OKR assigned to the user requires an update.' - value 'added_approver', value: 13, description: 'User is eligible to approvea merge request.' + value 'added_approver', value: 13, description: 'User is eligible to approve a merge request.' value 'ssh_key_expired', value: 14, description: 'SSH key of the user has expired.' value 'ssh_key_expiring_soon', value: 15, description: 'SSH key of the user will expire soon.' value 'duo_pro_access_granted', value: 16, description: 'Access to Duo Pro has been granted to the user.' diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index a3638e19ca034d..8ff4e708922f14 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -44903,7 +44903,7 @@ Values for sorting timelogs. | Value | Description | | ----- | ----------- | -| `added_approver` | User is eligible to approvea merge request. | +| `added_approver` | User is eligible to approve a merge request. | | `approval_required` | User was set as a required approver. | | `assigned` | User was assigned. | | `build_failed` | Build triggered by the user failed. | -- GitLab From a4e060c5f0a6cf163c7ddfd1dc4ec06b24db1380 Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Thu, 10 Apr 2025 11:34:11 +0200 Subject: [PATCH 4/9] Boldly go "Custom" --- doc/user/project/merge_requests/approvals/rules.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/project/merge_requests/approvals/rules.md b/doc/user/project/merge_requests/approvals/rules.md index 3a959def46a210..6139f4c3e38d2a 100644 --- a/doc/user/project/merge_requests/approvals/rules.md +++ b/doc/user/project/merge_requests/approvals/rules.md @@ -159,7 +159,7 @@ in the project count toward meeting the required number of approvals, even if th users were not explicitly listed in the approval rules. To get email notifications every time a merge request you're eligible to approve is created, -[set your notification level](../../../profile/notifications.md#edit-notification-settings) to Custom +[set your notification level](../../../profile/notifications.md#edit-notification-settings) to **Custom** and select this event. ### Group approvers -- GitLab From 859818da0e0372ccc6dba7cc755c1bc511b8abbb Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Thu, 10 Apr 2025 12:42:15 +0200 Subject: [PATCH 5/9] Change approver to-do text --- app/assets/javascripts/todos/components/todo_item_body.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/todos/components/todo_item_body.vue b/app/assets/javascripts/todos/components/todo_item_body.vue index 3ef15eeff557f6..6dcd6ca46379e0 100644 --- a/app/assets/javascripts/todos/components/todo_item_body.vue +++ b/app/assets/javascripts/todos/components/todo_item_body.vue @@ -126,7 +126,7 @@ export default { this.todo.action === TODO_ACTION_TYPE_APPROVAL_REQUIRED || this.todo.action === TODO_ACTION_TYPE_ADDED_APPROVER ) { - name = sprintf(s__('Todos|created a merge request %{who} can approve'), { + name = sprintf(s__('Todos|created a merge request you can approve'), { who: this.actionSubject, }); } -- GitLab From 4861c46a72a4d93f131c58ad4c8a597c54f0d1ef Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Thu, 10 Apr 2025 14:26:57 +0200 Subject: [PATCH 6/9] Regenerate gitlab.pot file --- locale/gitlab.pot | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4d316247419e85..29bbf9192a3f81 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -63003,7 +63003,7 @@ msgstr "" msgid "Todos|assigned you" msgstr "" -msgid "Todos|created a merge request %{who} can approve" +msgid "Todos|created a merge request you can approve" msgstr "" msgid "Todos|has requested access to %{what} %{which}" -- GitLab From 25e9c45ab1b25b4e0903b8e5fd7b37fc798d54c3 Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Thu, 10 Apr 2025 16:17:23 +0200 Subject: [PATCH 7/9] Delete sprintf from to-do message --- app/assets/javascripts/todos/components/todo_item_body.vue | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/assets/javascripts/todos/components/todo_item_body.vue b/app/assets/javascripts/todos/components/todo_item_body.vue index 6dcd6ca46379e0..93b50e238d4ed8 100644 --- a/app/assets/javascripts/todos/components/todo_item_body.vue +++ b/app/assets/javascripts/todos/components/todo_item_body.vue @@ -126,9 +126,7 @@ export default { this.todo.action === TODO_ACTION_TYPE_APPROVAL_REQUIRED || this.todo.action === TODO_ACTION_TYPE_ADDED_APPROVER ) { - name = sprintf(s__('Todos|created a merge request you can approve'), { - who: this.actionSubject, - }); + name = s__('Todos|created a merge request you can approve'); } if (this.todo.action === TODO_ACTION_TYPE_UNMERGEABLE) { -- GitLab From 3f44e32a9ef5854ad7b90c8c6f12a2359fefd77b Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Fri, 11 Apr 2025 10:54:42 +0200 Subject: [PATCH 8/9] Change GraphQL description per backend reviewer --- app/graphql/types/todo_action_enum.rb | 2 +- doc/api/graphql/reference/_index.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/graphql/types/todo_action_enum.rb b/app/graphql/types/todo_action_enum.rb index 508921ccb7aa3b..7d3a229069bcb6 100644 --- a/app/graphql/types/todo_action_enum.rb +++ b/app/graphql/types/todo_action_enum.rb @@ -6,7 +6,7 @@ class TodoActionEnum < BaseEnum value 'mentioned', value: 2, description: 'User was mentioned.' value 'build_failed', value: 3, description: 'Build triggered by the user failed.' value 'marked', value: 4, description: 'User added a to-do item.' - value 'approval_required', value: 5, description: 'User was set as a required approver.' + value 'approval_required', value: 5, description: 'User is eligible to approve a merge request.' value 'unmergeable', value: 6, description: 'Merge request authored by the user could not be merged.' value 'directly_addressed', value: 7, description: 'User was directly addressed.' value 'merge_train_removed', value: 8, description: 'Merge request authored by the user was removed from the merge train.' diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 8ff4e708922f14..b1280d96c35826 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -44904,7 +44904,7 @@ Values for sorting timelogs. | Value | Description | | ----- | ----------- | | `added_approver` | User is eligible to approve a merge request. | -| `approval_required` | User was set as a required approver. | +| `approval_required` | User is eligible to approve a merge request. | | `assigned` | User was assigned. | | `build_failed` | Build triggered by the user failed. | | `directly_addressed` | User was directly addressed. | -- GitLab From 5ce69ef1d7984769248bcab15861ce5d190d0c8c Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Fri, 11 Apr 2025 11:05:54 +0200 Subject: [PATCH 9/9] Add new doc task --- doc/user/project/merge_requests/approvals/rules.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/user/project/merge_requests/approvals/rules.md b/doc/user/project/merge_requests/approvals/rules.md index 6139f4c3e38d2a..8a71ca769c8adf 100644 --- a/doc/user/project/merge_requests/approvals/rules.md +++ b/doc/user/project/merge_requests/approvals/rules.md @@ -158,8 +158,11 @@ approvals from other users with at least the Developer role in the project count toward meeting the required number of approvals, even if the users were not explicitly listed in the approval rules. -To get email notifications every time a merge request you're eligible to approve is created, -[set your notification level](../../../profile/notifications.md#edit-notification-settings) to **Custom** +### Get notified about all merge requests you can approve + +To get email notifications every time a merge request you're eligible to approve is created: + +- [Set your notification level](../../../profile/notifications.md#edit-notification-settings) to **Custom** and select this event. ### Group approvers -- GitLab