Commit 10a42a8a authored by Mario de la Ossa's avatar Mario de la Ossa

Respect alt email when sending group access notifications

When sending access granted/rejected emails we should also respect
custom emails set for groups/sub-groups
parent a1d1b3aa
......@@ -9,11 +9,11 @@ module Emails
helper_method :member_source, :member
end
def member_access_requested_email(member_source_type, member_id, recipient_notification_email)
def member_access_requested_email(member_source_type, member_id, recipient_id)
@member_source_type = member_source_type
@member_id = member_id
mail(to: recipient_notification_email,
mail(to: recipient(recipient_id, notification_group),
subject: subject("Request to join the #{member_source.human_name} #{member_source.model_name.singular}"))
end
......@@ -21,16 +21,15 @@ module Emails
@member_source_type = member_source_type
@member_id = member_id
mail(to: member.user.notification_email,
mail(to: recipient(member.user, notification_group),
subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted"))
end
def member_access_denied_email(member_source_type, source_id, user_id)
@member_source_type = member_source_type
@member_source = member_source_class.find(source_id)
requester = User.find(user_id)
mail(to: requester.notification_email,
mail(to: recipient(user_id, notification_group),
subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was denied"))
end
......@@ -48,7 +47,7 @@ module Emails
@member_id = member_id
return unless member.created_by
mail(to: member.created_by.notification_email,
mail(to: recipient(member.created_by, notification_group),
subject: subject('Invitation accepted'))
end
......@@ -59,7 +58,7 @@ module Emails
@member_source = member_source_class.find(source_id)
@invite_email = invite_email
mail(to: recipient(created_by_id, member_source_type == 'Project' ? @member_source.group : @member_source),
mail(to: recipient(created_by_id, notification_group),
subject: subject('Invitation declined'))
end
......@@ -71,6 +70,10 @@ module Emails
@member_source ||= member.source
end
def notification_group
@member_source_type.casecmp?('project') ? member_source.group : member_source
end
private
def member_source_class
......
......@@ -71,14 +71,18 @@ class Notify < BaseMailer
address.format
end
# Look up a User by their ID and return their email address
# Look up a User's notification email for a particular context.
# Can look up by their ID or can accept a User object.
#
# recipient_id - User ID
# recipient - User object OR a User ID
# notification_group - The parent group of the notification
#
# Returns a String containing the User's email address.
def recipient(recipient_id, notification_group = nil)
User.find(recipient_id).notification_email_for(notification_group)
def recipient(recipient, notification_group = nil)
user = recipient if recipient.is_a?(User)
user ||= User.find(recipient)
user.notification_email_for(notification_group)
end
# Formats arguments into a String suitable for use as an email subject
......
......@@ -105,11 +105,11 @@ class NotifyPreview < ActionMailer::Preview
end
def member_access_granted_email
Notify.member_access_granted_email('project', user.id).message
Notify.member_access_granted_email(member.source_type, member.id).message
end
def member_access_requested_email
Notify.member_access_requested_email('group', user.id, 'some@example.com').message
Notify.member_access_requested_email('group', user.id, user.id).message
end
def member_invite_accepted_email
......@@ -183,6 +183,10 @@ class NotifyPreview < ActionMailer::Preview
@user ||= User.last
end
def member
@member ||= Member.last
end
def create_note(params)
Notes::CreateService.new(project, user, params).execute
end
......
......@@ -595,7 +595,7 @@ class NotificationService
end
def deliver_access_request_email(recipient, member)
mailer.member_access_requested_email(member.real_source_type, member.id, recipient.user.notification_email).deliver_later
mailer.member_access_requested_email(member.real_source_type, member.id, recipient.user.id).deliver_later
end
def fallback_to_group_owners_maintainers?(recipients, member)
......
---
title: Respect group notification email when sending group access notifications
merge_request: 31089
author:
type: fixed
......@@ -640,7 +640,7 @@ describe Notify do
project.request_access(user)
project.requesters.find_by(user_id: user.id)
end
subject { described_class.member_access_requested_email('project', project_member.id, recipient.notification_email) }
subject { described_class.member_access_requested_email('project', project_member.id, recipient.id) }
it_behaves_like 'an email sent from GitLab'
it_behaves_like 'it should not have Gmail Actions links'
......@@ -737,9 +737,9 @@ describe Notify do
describe 'project invitation accepted' do
let(:invited_user) { create(:user, name: 'invited user') }
let(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } }
let(:recipient) { create(:user).tap { |u| project.add_maintainer(u) } }
let(:project_member) do
invitee = invite_to_project(project, inviter: maintainer)
invitee = invite_to_project(project, inviter: recipient)
invitee.accept_invite!(invited_user)
invitee
end
......@@ -747,6 +747,7 @@ describe Notify do
subject { described_class.member_invite_accepted_email('project', project_member.id) }
it_behaves_like 'an email sent from GitLab'
it_behaves_like 'an email sent to a user'
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
it_behaves_like 'appearance header and footer enabled'
......@@ -762,16 +763,17 @@ describe Notify do
end
describe 'project invitation declined' do
let(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } }
let(:recipient) { create(:user).tap { |u| project.add_maintainer(u) } }
let(:project_member) do
invitee = invite_to_project(project, inviter: maintainer)
invitee = invite_to_project(project, inviter: recipient)
invitee.decline_invite!
invitee
end
subject { described_class.member_invite_declined_email('Project', project.id, project_member.invite_email, maintainer.id) }
subject { described_class.member_invite_declined_email('Project', project.id, project_member.invite_email, recipient.id) }
it_behaves_like 'an email sent from GitLab'
it_behaves_like 'an email sent to a user'
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
it_behaves_like 'appearance header and footer enabled'
......@@ -1087,9 +1089,10 @@ describe Notify do
group.request_access(user)
group.requesters.find_by(user_id: user.id)
end
subject { described_class.member_access_requested_email('group', group_member.id, recipient.notification_email) }
subject { described_class.member_access_requested_email('group', group_member.id, recipient.id) }
it_behaves_like 'an email sent from GitLab'
it_behaves_like 'an email sent to a user'
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
it_behaves_like 'appearance header and footer enabled'
......@@ -1111,9 +1114,11 @@ describe Notify do
group.request_access(user)
group.requesters.find_by(user_id: user.id)
end
let(:recipient) { user }
subject { described_class.member_access_denied_email('group', group.id, user.id) }
it_behaves_like 'an email sent from GitLab'
it_behaves_like 'an email sent to a user'
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
it_behaves_like 'appearance header and footer enabled'
......@@ -1128,10 +1133,12 @@ describe Notify do
describe 'group access changed' do
let(:group_member) { create(:group_member, group: group, user: user) }
let(:recipient) { user }
subject { described_class.member_access_granted_email('group', group_member.id) }
it_behaves_like 'an email sent from GitLab'
it_behaves_like 'an email sent to a user'
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
it_behaves_like 'appearance header and footer enabled'
......
......@@ -50,7 +50,7 @@ shared_examples 'an email sent to a user' do
it 'is sent to user\'s group notification email' do
group_notification_email = 'user+group@example.com'
create(:notification_setting, user: recipient, source: project.group, notification_email: group_notification_email)
create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email)
expect(subject).to deliver_to(group_notification_email)
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment