From ee6aa1c738dedc618d32b6782053ee4c6798fc49 Mon Sep 17 00:00:00 2001 From: Matthew Rider Date: Tue, 23 Jul 2024 17:36:24 +0200 Subject: [PATCH] Avoid infinite loop in postmark (#1944) if we try to send a message to an admin notifying them that a contributor is inactive, but that admin has the same email as the contributor who has unsubscribed, it gets stuck in an infinite loop Fixes #1941 --- app/adapters/postmark_adapter/outbound.rb | 2 +- .../postmark_adapter/outbound_spec.rb | 57 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/app/adapters/postmark_adapter/outbound.rb b/app/adapters/postmark_adapter/outbound.rb index bd7f289d4..000796e78 100644 --- a/app/adapters/postmark_adapter/outbound.rb +++ b/app/adapters/postmark_adapter/outbound.rb @@ -51,7 +51,7 @@ def send_user_count_exceeds_plan_limit_message!(admin, organization) end def contributor_marked_as_inactive!(admin, contributor) - return unless admin&.email && admin&.admin? && contributor&.id + return unless admin&.email && admin&.admin? && contributor&.id && admin.email != contributor.email with(admin: admin, contributor: contributor).contributor_marked_as_inactive_email.deliver_later end diff --git a/spec/adapters/postmark_adapter/outbound_spec.rb b/spec/adapters/postmark_adapter/outbound_spec.rb index 853128e92..645bd7a8c 100644 --- a/spec/adapters/postmark_adapter/outbound_spec.rb +++ b/spec/adapters/postmark_adapter/outbound_spec.rb @@ -288,5 +288,62 @@ end end end + + describe '::contributor_marked_as_inactive!' do + subject { described_class.contributor_marked_as_inactive!(admin, contributor) } + + context 'no admin' do + let(:admin) { nil } + let(:contributor) { create(:contributor) } + + it 'does not enqueue a Mailer' do + expect { subject }.not_to have_enqueued_job + end + end + + context 'no contributor' do + let(:admin) { build(:user, admin: true) } + let(:contributor) { nil } + + it 'does not enqueue a Mailer' do + expect { subject }.not_to have_enqueued_job + end + end + + context 'user without an admin role' do + let(:admin) { build(:user) } + let(:contributor) { create(:contributor) } + + it 'does not enqueue a Mailer' do + expect { subject }.not_to have_enqueued_job + end + end + + context 'admin email equals contributor email' do + let(:admin) { create(:user, admin: true, email: 'my-email@example.org') } + let(:contributor) { create(:contributor, email: 'my-email@example.org') } + + it 'does not enqueue a Mailer' do + expect { subject }.not_to have_enqueued_job + end + end + + context 'with an admin and contributor without the same email address' do + let(:admin) { create(:user, admin: true, email: 'admin@example.org') } + let(:contributor) { create(:contributor, email: 'contributor@example.org') } + + it 'enqueues a Mailer' do + expect { subject }.to have_enqueued_job.on_queue('default').with( + 'PostmarkAdapter::Outbound', + 'contributor_marked_as_inactive_email', + 'deliver_now', # How ActionMailer works in test environment, even though in production we call deliver_later + { + params: { admin: admin, contributor: contributor }, + args: [] + } + ) + end + end + end end end