From 05bf47f96a22ecc0121d4a5a10f665b71f16c2e1 Mon Sep 17 00:00:00 2001
From: Alex Buijs <abuijs@gitlab.com>
Date: Thu, 16 May 2024 15:30:44 +0000
Subject: [PATCH] Display custom role title in invitation emails
When a custom role is assigned, display the
custom role's title in access granted notifications.
Changelog: changed
---
.rubocop_todo/style/format_string.yml | 1 -
app/helpers/invite_members_helper.rb | 14 +++++------
app/presenters/member_presenter.rb | 4 +++
.../member_access_granted_email.html.haml | 14 +++++------
.../member_access_granted_email.text.erb | 5 +++-
.../notify/member_invited_email.html.haml | 7 +++---
.../notify/member_invited_email.text.erb | 7 +++---
ee/app/presenters/ee/member_presenter.rb | 7 ++++++
.../identity_verification_spec.rb | 8 ++++--
.../saas/standard_flow_with_2fa_spec.rb | 3 ++-
ee/spec/presenters/member_presenter_spec.rb | 16 ++++++++++++
.../helpers/saas_registration_helpers.rb | 3 ++-
locale/gitlab.pot | 25 ++++++++-----------
spec/features/invites_spec.rb | 9 ++++---
.../registrations/oauth_registration_spec.rb | 3 ++-
spec/helpers/invite_members_helper_spec.rb | 22 ++++++++++++++++
spec/mailers/notify_spec.rb | 11 +++++---
spec/presenters/member_presenter_spec.rb | 6 +++++
18 files changed, 116 insertions(+), 49 deletions(-)
diff --git a/.rubocop_todo/style/format_string.yml b/.rubocop_todo/style/format_string.yml
index e7827f1d82c9..1126e08f8148 100644
--- a/.rubocop_todo/style/format_string.yml
+++ b/.rubocop_todo/style/format_string.yml
@@ -39,7 +39,6 @@ Style/FormatString:
- 'app/helpers/groups/group_members_helper.rb'
- 'app/helpers/groups_helper.rb'
- 'app/helpers/import_helper.rb'
- - 'app/helpers/invite_members_helper.rb'
- 'app/helpers/issuables_helper.rb'
- 'app/helpers/issues_helper.rb'
- 'app/helpers/members_helper.rb'
diff --git a/app/helpers/invite_members_helper.rb b/app/helpers/invite_members_helper.rb
index 0443861903b0..f381a54a208e 100644
--- a/app/helpers/invite_members_helper.rb
+++ b/app/helpers/invite_members_helper.rb
@@ -10,14 +10,12 @@ def can_invite_members_for_project?(project)
end
def invite_accepted_notice(member)
- case member.source
- when Project
- _("You have been granted %{member_human_access} access to project %{name}.") %
- { member_human_access: member.human_access, name: member.source.name }
- when Group
- _("You have been granted %{member_human_access} access to group %{name}.") %
- { member_human_access: member.human_access, name: member.source.name }
- end
+ format(
+ _('You have been granted access to the %{source_name} %{source_type} with the following role: %{role_name}.'),
+ source_name: member.source.name,
+ source_type: member.source.model_name.singular,
+ role_name: member.present.human_access
+ )
end
# Overridden in EE
diff --git a/app/presenters/member_presenter.rb b/app/presenters/member_presenter.rb
index 42b49a80b8a6..d0c075c5b7e5 100644
--- a/app/presenters/member_presenter.rb
+++ b/app/presenters/member_presenter.rb
@@ -19,6 +19,10 @@ def valid_member_roles
[]
end
+ def role_type
+ 'default'
+ end
+
def can_resend_invite?
invite? &&
can?(current_user, admin_member_permission, source)
diff --git a/app/views/notify/member_access_granted_email.html.haml b/app/views/notify/member_access_granted_email.html.haml
index 49a571f154eb..9835883e5809 100644
--- a/app/views/notify/member_access_granted_email.html.haml
+++ b/app/views/notify/member_access_granted_email.html.haml
@@ -1,13 +1,13 @@
-- link_end = '</a>'.html_safe
- source_type = member_source.model_name.singular
-- leave_link = polymorphic_url([member_source], leave: 1)
-- source_link = link_to(member_source.human_name, member_source.web_url, target: '_blank', rel: 'noopener noreferrer', class: :highlight)
-- access_level = content_tag(:span, member.human_access, class: :highlight)
+- source_name = link_to(member_source.human_name, member_source.web_url, target: '_blank', rel: 'noopener noreferrer', class: :highlight)
+- role_name = content_tag(:span, member.present.human_access, class: :highlight)
+- role_type = content_tag(:span, "#{member.present.role_type} role", class: :highlight)
%tr
%td.text-content
%p
- = _('You have been granted %{access_level} access to the %{source_link} %{source_type}.').html_safe % { access_level: access_level, source_link: source_link, source_type: source_type }
+ = safe_format(_('You have been granted access to the %{source_name} %{source_type} with the following role: %{role_name}.'), source_name: source_name, source_type: source_type, role_name: role_name)
+ = safe_format(_('This is a %{role_type}.'), role_type: role_type)
%p
- - leave_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: leave_link }
- = _('If this was a mistake you can %{leave_link_start}leave the %{source_type}%{link_end}.').html_safe % { source_type: source_type, leave_link_start: leave_link_start, link_end: link_end }
+ - leave_link = link_to('', polymorphic_url([member_source], leave: 1), target: '_blank', rel: 'noopener noreferrer')
+ = safe_format(_('If this was a mistake you can %{leave_link_start}leave the %{source_type}%{link_end}.'), tag_pair(leave_link, :leave_link_start, :link_end), source_type: source_type)
diff --git a/app/views/notify/member_access_granted_email.text.erb b/app/views/notify/member_access_granted_email.text.erb
index 445009bb413d..5bb879fea604 100644
--- a/app/views/notify/member_access_granted_email.text.erb
+++ b/app/views/notify/member_access_granted_email.text.erb
@@ -1,5 +1,8 @@
<% source_type = member_source.model_name.singular %>
-<%= _('You have been granted %{access_level} access to the %{source_name} %{source_type}.') % { access_level: member.human_access, source_name: member_source.human_name, source_type: source_type } %>
+<% role_name = member.present.human_access %>
+<% role_type = member.present.role_type %>
+<%= _('You have been granted access to the %{source_name} %{source_type} with the following role: %{role_name}.') % { source_name: member_source.human_name, source_type: source_type, role_name: role_name } %>
+<%= _('This is a %{role_type} role.') % { role_type: role_type } %>
<%= member_source.web_url %>
diff --git a/app/views/notify/member_invited_email.html.haml b/app/views/notify/member_invited_email.html.haml
index 21d0f8b9108f..06e1ee836b1b 100644
--- a/app/views/notify/member_invited_email.html.haml
+++ b/app/views/notify/member_invited_email.html.haml
@@ -3,7 +3,8 @@
project_or_group_name: member_source.human_name,
project_or_group: member_source.model_name.singular,
br_tag: '<br/>'.html_safe,
- role: member.human_access.downcase }
+ role: member.present.human_access,
+ role_type: member.present.role_type }
- join_text = s_('InviteEmail|Join now')
- inviter_name = member.created_by.name if member.created_by
- join_url = invite_url(@token, invite_type: Emails::Members::INITIAL_INVITE)
@@ -21,9 +22,9 @@
%img.mail-avatar{ height: "60", src: avatar_icon_for_user(member.created_by, 60, only_path: false), width: "60", alt: "" }
%p
- if member.created_by
- = html_escape(s_("InviteEmail|%{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} as a %{role}")) % placeholders.merge({ inviter: (link_to inviter_name, user_url(member.created_by)).html_safe })
+ = html_escape(s_("InviteEmail|%{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} with the following role: %{role}.%{br_tag}This is a %{role_type} role.")) % placeholders.merge({ inviter: (link_to inviter_name, user_url(member.created_by)).html_safe })
- else
- = html_escape(s_("InviteEmail|You are invited to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} as a %{role}")) % placeholders
+ = html_escape(s_("InviteEmail|You are invited to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} with the following role: %{role}.%{br_tag}This is a %{role_type} role.")) % placeholders
%p.invite-actions
= link_to join_text, join_url, class: 'invite-btn-join'
%tr.border-top
diff --git a/app/views/notify/member_invited_email.text.erb b/app/views/notify/member_invited_email.text.erb
index e58dfc10810a..b82c4c3faff2 100644
--- a/app/views/notify/member_invited_email.text.erb
+++ b/app/views/notify/member_invited_email.text.erb
@@ -1,9 +1,10 @@
-<% placeholders = { project_or_group_name: member_source.human_name, project_or_group: member_source.model_name.singular, role: member.human_access.downcase } %>
+<% placeholders = { project_or_group_name: member_source.human_name, project_or_group: member_source.model_name.singular, role: member.present.human_access} %>
<% if member.created_by %>
-<%= s_('InviteEmail|%{inviter} invited you to join the %{project_or_group_name} %{project_or_group} as a %{role}') % placeholders.merge({ inviter: sanitize_name(member.created_by.name) }) %>
+<%= s_('InviteEmail|%{inviter} invited you to join the %{project_or_group_name} %{project_or_group} with the following role: %{role}.') % placeholders.merge({ inviter: sanitize_name(member.created_by.name) }) %>
<% else %>
-<%= s_('InviteEmail|You have been invited to join the %{project_or_group_name} %{project_or_group} as a %{role}') % placeholders %>
+<%= s_('InviteEmail|You have been invited to join the %{project_or_group_name} %{project_or_group} with the following role: %{role}.') % placeholders %>
<% end %>
+<%= _('This is a %{role_type} role.') % { role_type: member.present.role_type } %>
<%= s_('InviteEmail|Join now') %>: <%= invite_url(@token) %>
diff --git a/ee/app/presenters/ee/member_presenter.rb b/ee/app/presenters/ee/member_presenter.rb
index 911f03c8e0ad..568a00a67d1f 100644
--- a/ee/app/presenters/ee/member_presenter.rb
+++ b/ee/app/presenters/ee/member_presenter.rb
@@ -21,6 +21,13 @@ def human_access
super
end
+ override :role_type
+ def role_type
+ return 'custom' if member_role
+
+ super
+ end
+
delegator_override :valid_member_roles
def valid_member_roles
source = member.source
diff --git a/ee/spec/features/registrations/identity_verification_spec.rb b/ee/spec/features/registrations/identity_verification_spec.rb
index 473515868c15..6219e8eef53a 100644
--- a/ee/spec/features/registrations/identity_verification_spec.rb
+++ b/ee/spec/features/registrations/identity_verification_spec.rb
@@ -323,7 +323,9 @@ def send_request(session, method, path, headers:)
it 'does not verify the user and lands on group page' do
expect(page).to have_current_path(group_path(invitation.group))
- expect(page).to have_content("You have been granted Developer access to group #{invitation.group.name}.")
+ expect(page).to have_content(
+ "You have been granted access to the #{invitation.group.name} group with the following role: Developer."
+ )
end
end
@@ -342,7 +344,9 @@ def send_request(session, method, path, headers:)
it 'allows the user to complete registration' do
expect(page).to have_current_path(group_path(invitation.group))
- expect(page).to have_content("You have been granted Developer access to group #{invitation.group.name}.")
+ expect(page).to have_content(
+ "You have been granted access to the #{invitation.group.name} group with the following role: Developer."
+ )
end
end
end
diff --git a/ee/spec/features/registrations/saas/standard_flow_with_2fa_spec.rb b/ee/spec/features/registrations/saas/standard_flow_with_2fa_spec.rb
index bd21ad2f8e11..73eb2173334f 100644
--- a/ee/spec/features/registrations/saas/standard_flow_with_2fa_spec.rb
+++ b/ee/spec/features/registrations/saas/standard_flow_with_2fa_spec.rb
@@ -101,7 +101,8 @@ def expect_to_be_on_2fa_verification(with_invite_notification: false)
return unless with_invite_notification # rubocop:disable RSpec/AvoidConditionalStatements
- expect(page).to have_content('You have been granted Developer access to group Test Group')
+ expect(page)
+ .to have_content('You have been granted access to the Test Group group with the following role: Developer.')
end
def expect_to_be_on_profile_account_page
diff --git a/ee/spec/presenters/member_presenter_spec.rb b/ee/spec/presenters/member_presenter_spec.rb
index c48a336872da..3cf9155b205c 100644
--- a/ee/spec/presenters/member_presenter_spec.rb
+++ b/ee/spec/presenters/member_presenter_spec.rb
@@ -46,6 +46,22 @@
end
end
+ describe '#role_type' do
+ context 'when a default role is assigned' do
+ it "returns 'default'" do
+ expect(presenter.role_type).to eq('default')
+ end
+ end
+
+ context 'when a custom role is assigned' do
+ it "returns 'custom'" do
+ member_root.member_role = create(:member_role, namespace: root_group)
+
+ expect(presenter.role_type).to eq('custom')
+ end
+ end
+ end
+
describe '#valid_member_roles' do
let_it_be(:member_role_guest) { create(:member_role, :guest, name: 'guest plus', namespace: root_group) }
let_it_be(:member_role_reporter) do
diff --git a/ee/spec/support/helpers/saas_registration_helpers.rb b/ee/spec/support/helpers/saas_registration_helpers.rb
index 3b41679a17d8..f5cdfd605fa7 100644
--- a/ee/spec/support/helpers/saas_registration_helpers.rb
+++ b/ee/spec/support/helpers/saas_registration_helpers.rb
@@ -41,7 +41,8 @@ def expect_to_see_welcome_form_for_invites
def expect_to_be_on_page_for(group)
expect(page).to have_current_path(group_path(group), ignore_query: true)
- expect(page).to have_content('You have been granted Developer access to group Test Group')
+ expect(page)
+ .to have_content('You have been granted access to the Test Group group with the following role: Developer')
end
def confirm_account
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 2605e33c0c79..b521bf8d01b5 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -27940,10 +27940,10 @@ msgstr ""
msgid "Invite members"
msgstr ""
-msgid "InviteEmail|%{inviter} invited you to join the %{project_or_group_name} %{project_or_group} as a %{role}"
+msgid "InviteEmail|%{inviter} invited you to join the %{project_or_group_name} %{project_or_group} with the following role: %{role}."
msgstr ""
-msgid "InviteEmail|%{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} as a %{role}"
+msgid "InviteEmail|%{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} with the following role: %{role}.%{br_tag}This is a %{role_type} role."
msgstr ""
msgid "InviteEmail|%{project_or_group} details"
@@ -27967,10 +27967,10 @@ msgstr ""
msgid "InviteEmail|What's it about?"
msgstr ""
-msgid "InviteEmail|You are invited to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} as a %{role}"
+msgid "InviteEmail|You are invited to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} with the following role: %{role}.%{br_tag}This is a %{role_type} role."
msgstr ""
-msgid "InviteEmail|You have been invited to join the %{project_or_group_name} %{project_or_group} as a %{role}"
+msgid "InviteEmail|You have been invited to join the %{project_or_group_name} %{project_or_group} with the following role: %{role}."
msgstr ""
msgid "InviteMembersBanner|Collaborate with your team"
@@ -53093,6 +53093,12 @@ msgstr ""
msgid "This is a \"Ghost User\", created to hold all issues authored by users that have since been deleted. This user cannot be removed."
msgstr ""
+msgid "This is a %{role_type} role."
+msgstr ""
+
+msgid "This is a %{role_type}."
+msgstr ""
+
msgid "This is a Jira user."
msgstr ""
@@ -59771,16 +59777,7 @@ msgstr ""
msgid "You have been added to ticket #%{issue_iid}."
msgstr ""
-msgid "You have been granted %{access_level} access to the %{source_link} %{source_type}."
-msgstr ""
-
-msgid "You have been granted %{access_level} access to the %{source_name} %{source_type}."
-msgstr ""
-
-msgid "You have been granted %{member_human_access} access to group %{name}."
-msgstr ""
-
-msgid "You have been granted %{member_human_access} access to project %{name}."
+msgid "You have been granted access to the %{source_name} %{source_type} with the following role: %{role_name}."
msgstr ""
msgid "You have been invited by %{link_to_inviter} to join %{source_name} %{strong_open}%{link_to_source}%{strong_close} as %{role}"
diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb
index 1ea3ad019d92..b63705affbf0 100644
--- a/spec/features/invites_spec.rb
+++ b/spec/features/invites_spec.rb
@@ -87,7 +87,8 @@
end
it 'accepts invite' do
- expect(page).to have_content('You have been granted Developer access to group Owned.')
+ expect(page)
+ .to have_content('You have been granted access to the Owned group with the following role: Developer.')
end
end
end
@@ -167,7 +168,8 @@ def expect_to_be_on_group_page(group)
fill_in_sign_up_form(new_user, invite: true)
expect(page).to have_current_path(group_path(group), ignore_query: true)
- expect(page).to have_content('You have been granted Owner access to group Owned.')
+ expect(page)
+ .to have_content('You have been granted access to the Owned group with the following role: Owner.')
end
end
end
@@ -230,7 +232,8 @@ def expect_to_be_on_group_page(group)
fill_in_sign_up_form(new_user, 'Register', invite: true)
expect(page).to have_current_path(group_path(group))
- expect(page).to have_content('You have been granted Owner access to group Owned.')
+ expect(page)
+ .to have_content('You have been granted access to the Owned group with the following role: Owner.')
end
end
diff --git a/spec/features/registrations/oauth_registration_spec.rb b/spec/features/registrations/oauth_registration_spec.rb
index 5265a05c66f6..ca335059bf75 100644
--- a/spec/features/registrations/oauth_registration_spec.rb
+++ b/spec/features/registrations/oauth_registration_spec.rb
@@ -106,7 +106,8 @@
visit invite_path(group_invite.raw_invite_token, extra_params)
click_link_or_button Gitlab::Auth::OAuth::Provider.label_for(provider)
- expect(page).to have_content('You have been granted Owner access to group Owned.')
+ expect(page)
+ .to have_content('You have been granted access to the Owned group with the following role: Owner.')
expect(page).to have_current_path(group_path(group), ignore_query: true)
end
end
diff --git a/spec/helpers/invite_members_helper_spec.rb b/spec/helpers/invite_members_helper_spec.rb
index 16e4bee47c07..8f4e288d83a0 100644
--- a/spec/helpers/invite_members_helper_spec.rb
+++ b/spec/helpers/invite_members_helper_spec.rb
@@ -94,4 +94,26 @@
end
end
end
+
+ describe '#invite_accepted_notice' do
+ context 'for group invites' do
+ let_it_be(:group) { create(:group, name: 'My group') }
+ let_it_be(:member) { build(:group_member, :guest, group: group) }
+
+ it 'returns the expected message' do
+ expect(helper.invite_accepted_notice(member))
+ .to eq('You have been granted access to the My group group with the following role: Guest.')
+ end
+ end
+
+ context 'for project invites' do
+ let_it_be(:project) { create(:project, name: 'My project') }
+ let_it_be(:member) { build(:project_member, :guest, project: project) }
+
+ it 'returns the expected message' do
+ expect(helper.invite_accepted_notice(member))
+ .to eq('You have been granted access to the My project project with the following role: Guest.')
+ end
+ end
+ end
end
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index bbb5a77e89c5..b285aaa0ee38 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -1085,6 +1085,7 @@ def id
is_expected.to have_body_text project.full_name
is_expected.to have_body_text project.web_url
is_expected.to have_body_text project_member.human_access
+ is_expected.to have_body_text 'default role'
is_expected.to have_body_text 'leave the project'
is_expected.to have_body_text project_url(project, leave: 1)
end
@@ -1122,7 +1123,8 @@ def invite_to_project(project, inviter:, user: nil)
it 'contains all the useful information' do
is_expected.to have_subject "#{inviter.name} invited you to join GitLab"
is_expected.to have_body_text project.full_name
- is_expected.to have_body_text project_member.human_access.downcase
+ is_expected.to have_body_text project_member.human_access
+ is_expected.to have_body_text 'default role'
is_expected.to have_body_text project_member.invite_token
is_expected.to have_link(
'Join now',
@@ -1140,7 +1142,8 @@ def invite_to_project(project, inviter:, user: nil)
it 'contains all the useful information' do
is_expected.to have_subject "Invitation to join the #{project.full_name} project"
is_expected.to have_body_text project.full_name
- is_expected.to have_body_text project_member.human_access.downcase
+ is_expected.to have_body_text project_member.human_access
+ is_expected.to have_body_text 'default role'
is_expected.to have_body_text project_member.invite_token
is_expected.to have_link(
'Join now',
@@ -1796,7 +1799,7 @@ def invite_to_group(group, inviter:, user: nil)
it 'contains all the useful information' do
is_expected.to have_subject "#{group_member.created_by.name} invited you to join GitLab"
is_expected.to have_body_text group.name
- is_expected.to have_body_text group_member.human_access.downcase
+ is_expected.to have_body_text group_member.human_access
is_expected.to have_body_text group_member.invite_token
end
end
@@ -1807,7 +1810,7 @@ def invite_to_group(group, inviter:, user: nil)
it 'contains all the useful information' do
is_expected.to have_subject "Invitation to join the #{group.name} group"
is_expected.to have_body_text group.name
- is_expected.to have_body_text group_member.human_access.downcase
+ is_expected.to have_body_text group_member.human_access
is_expected.to have_body_text group_member.invite_token
end
end
diff --git a/spec/presenters/member_presenter_spec.rb b/spec/presenters/member_presenter_spec.rb
index 7223c98d5f7a..898f4288936f 100644
--- a/spec/presenters/member_presenter_spec.rb
+++ b/spec/presenters/member_presenter_spec.rb
@@ -21,6 +21,12 @@
end
end
+ describe '#role_type' do
+ it "returns 'default'" do
+ expect(presenter.role_type).to eq('default')
+ end
+ end
+
describe '#valid_level_roles' do
it 'does not return levels lower than user highest membership in the hierarchy' do
expect(described_class.new(subgroup_member).valid_level_roles).to eq(
--
GitLab