Commit da3c00f7 authored by Marc Shaw's avatar Marc Shaw

Stop excess logs when group member no longer exists

Issue: gitlab.com/gitlab-org/gitlab/-/issues/28291
parent 1eee0c41
...@@ -13,6 +13,8 @@ module Emails ...@@ -13,6 +13,8 @@ module Emails
@member_source_type = member_source_type @member_source_type = member_source_type
@member_id = member_id @member_id = member_id
return unless member_exists?
user = User.find(recipient_id) user = User.find(recipient_id)
member_email_with_layout( member_email_with_layout(
...@@ -24,6 +26,8 @@ module Emails ...@@ -24,6 +26,8 @@ module Emails
@member_source_type = member_source_type @member_source_type = member_source_type
@member_id = member_id @member_id = member_id
return unless member_exists?
member_email_with_layout( member_email_with_layout(
to: member.user.notification_email_for(notification_group), to: member.user.notification_email_for(notification_group),
subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted")) subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted"))
...@@ -45,6 +49,8 @@ module Emails ...@@ -45,6 +49,8 @@ module Emails
@member_id = member_id @member_id = member_id
@token = token @token = token
return unless member_exists?
member_email_with_layout( member_email_with_layout(
to: member.invite_email, to: member.invite_email,
subject: subject("Invitation to join the #{member_source.human_name} #{member_source.model_name.singular}")) subject: subject("Invitation to join the #{member_source.human_name} #{member_source.model_name.singular}"))
...@@ -53,6 +59,8 @@ module Emails ...@@ -53,6 +59,8 @@ module Emails
def member_invite_accepted_email(member_source_type, member_id) def member_invite_accepted_email(member_source_type, member_id)
@member_source_type = member_source_type @member_source_type = member_source_type
@member_id = member_id @member_id = member_id
return unless member_exists?
return unless member.created_by return unless member.created_by
member_email_with_layout( member_email_with_layout(
...@@ -74,9 +82,11 @@ module Emails ...@@ -74,9 +82,11 @@ module Emails
subject: subject('Invitation declined')) subject: subject('Invitation declined'))
end end
# rubocop: disable CodeReuse/ActiveRecord
def member def member
@member ||= Member.find(@member_id) @member ||= Member.find_by(id: @member_id)
end end
# rubocop: enable CodeReuse/ActiveRecord
def member_source def member_source
@member_source ||= member.source @member_source ||= member.source
...@@ -88,6 +98,11 @@ module Emails ...@@ -88,6 +98,11 @@ module Emails
private private
def member_exists?
Gitlab::AppLogger.info("Tried to send an email invitation for a deleted group. Member id: #{@member_id}") if member.blank?
member.present?
end
def member_source_class def member_source_class
@member_source_type.classify.constantize @member_source_type.classify.constantize
end end
......
---
title: Stop excess logs from failure to send invite email when group no longer exists
merge_request:
author:
type: security
...@@ -45,6 +45,21 @@ RSpec.describe Notify do ...@@ -45,6 +45,21 @@ RSpec.describe Notify do
end end
end end
shared_examples 'it requires a group' do
context 'when given an deleted group' do
before do
# destroy group and group member
group_member.destroy!
group.destroy!
end
it 'returns NullMail type message' do
expect(Gitlab::AppLogger).to receive(:info)
expect(subject.message).to be_a(ActionMailer::Base::NullMail)
end
end
end
context 'for a project' do context 'for a project' do
shared_examples 'an assignee email' do shared_examples 'an assignee email' do
let(:recipient) { assignee } let(:recipient) { assignee }
...@@ -1318,6 +1333,7 @@ RSpec.describe Notify do ...@@ -1318,6 +1333,7 @@ RSpec.describe Notify do
it_behaves_like "a user cannot unsubscribe through footer link" it_behaves_like "a user cannot unsubscribe through footer link"
it_behaves_like 'appearance header and footer enabled' it_behaves_like 'appearance header and footer enabled'
it_behaves_like 'appearance header and footer not enabled' it_behaves_like 'appearance header and footer not enabled'
it_behaves_like 'it requires a group'
it 'contains all the useful information' do it 'contains all the useful information' do
is_expected.to have_subject "Access to the #{group.name} group was granted" is_expected.to have_subject "Access to the #{group.name} group was granted"
...@@ -1352,6 +1368,7 @@ RSpec.describe Notify do ...@@ -1352,6 +1368,7 @@ RSpec.describe Notify do
it_behaves_like "a user cannot unsubscribe through footer link" it_behaves_like "a user cannot unsubscribe through footer link"
it_behaves_like 'appearance header and footer enabled' it_behaves_like 'appearance header and footer enabled'
it_behaves_like 'appearance header and footer not enabled' it_behaves_like 'appearance header and footer not enabled'
it_behaves_like 'it requires a group'
it 'contains all the useful information' do it 'contains all the useful information' do
is_expected.to have_subject "Invitation to join the #{group.name} group" is_expected.to have_subject "Invitation to join the #{group.name} group"
...@@ -1378,6 +1395,7 @@ RSpec.describe Notify do ...@@ -1378,6 +1395,7 @@ RSpec.describe Notify do
it_behaves_like "a user cannot unsubscribe through footer link" it_behaves_like "a user cannot unsubscribe through footer link"
it_behaves_like 'appearance header and footer enabled' it_behaves_like 'appearance header and footer enabled'
it_behaves_like 'appearance header and footer not enabled' it_behaves_like 'appearance header and footer not enabled'
it_behaves_like 'it requires a group'
it 'contains all the useful information' do it 'contains all the useful information' do
is_expected.to have_subject 'Invitation accepted' is_expected.to have_subject 'Invitation accepted'
......
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