Skip to content

fix(template_mailer): never hand templated email to itself (PPT-2816) #635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stakach merged 1 commit into from
Sep 30, 2026
Merged

fix(template_mailer): never hand templated email to itself (PPT-2816) #635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion drivers/place/template_mailer.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,16 @@ class Place::TemplateMailer < PlaceOS::Driver
getter org_zone_id : String { get_local_zone_id(org_zone_ids).not_nil! }
getter building_zone_id : String { get_local_zone_id(building_zone_ids).not_nil! }

# The mailer templated email is handed to: the next Mailer in the system,
# never this module itself
def mailer
system.implementing(Interface::Mailer)[1]
mailers = system.implementing(Interface::Mailer)
mine = mailers.find { |mod| mod.module_id == module_id }
if mine
following = mailers.find { |mod| mod.module_name == mine.module_name && mod.index == mine.index + 1 }
return following if following
end
mailers.find { |mod| mod.module_id != module_id } || raise "no other mailer found in the system to send through"
end

SEPERATOR = "."
Expand Down
30 changes: 15 additions & 15 deletions drivers/place/template_mailer_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
require "placeos-driver/spec"
require "placeos-driver/interface/mailer"

# The TemplateMailer under test has `generic_name :Mailer`, so it occupies the
# Mailer_1 slot. The mock declared below becomes Mailer_2 -- the module that
# `system.implementing(Interface::Mailer)[1]` forwards to.
# The TemplateMailer under test hands templated email to the next Mailer in the
# system that is not itself. The spec runner keeps the driver under test out of
# the module map, so the first mock below (Mailer_1) is the one it forwards to.
class StaffAPI < DriverSpecs::MockDriver
ZONES = [
{
Expand Down Expand Up @@ -129,9 +129,9 @@ class Mailer < DriverSpecs::MockDriver
end

DriverSpecs.mock_driver "Place::TemplateMailer" do
# The TemplateMailer under test forwards to `system.implementing(Mailer)[1]`.
# In production that index 1 is the next mailer in the chain (e.g. SMTP); here
# we declare two mock mailers so index 1 is the recording mock (Mailer_2).
# The TemplateMailer under test forwards to the first Mailer that is not
# itself. Two mocks are declared so the choice between them is visible: the
# recording mock is Mailer_1 and Mailer_2 must stay untouched.
system({
StaffAPI: {StaffAPI},
Mailer: {Mailer, Mailer},
Expand All @@ -146,7 +146,7 @@ DriverSpecs.mock_driver "Place::TemplateMailer" do
args: {name: "Bob"},
reply_to: "host@org.com",
).get
system(:Mailer_2)[:reply_to].should eq "template-reply@org.com"
system(:Mailer_1)[:reply_to].should eq "template-reply@org.com"

# 2. With no template match and no configured reply_to, the host reply_to is
# forwarded through to the downstream mailer.
Expand All @@ -157,7 +157,7 @@ DriverSpecs.mock_driver "Place::TemplateMailer" do
args: {name: "Bob"},
reply_to: "host@org.com",
).get
system(:Mailer_2)[:reply_to].should eq "host@org.com"
system(:Mailer_1)[:reply_to].should eq "host@org.com"

# 3. A reply_to configured on the TemplateMailer overrides BOTH the
# per-template reply_to and the host reply_to passed in by the caller.
Expand All @@ -172,7 +172,7 @@ DriverSpecs.mock_driver "Place::TemplateMailer" do
args: {name: "Bob"},
reply_to: "host@org.com",
).get
system(:Mailer_2)[:reply_to].should eq "tenant@org.com"
system(:Mailer_1)[:reply_to].should eq "tenant@org.com"

# 4. A template with no recipients of its own leaves the caller's To, CC and
# BCC untouched.
Expand All @@ -183,9 +183,9 @@ DriverSpecs.mock_driver "Place::TemplateMailer" do
args: {name: "Bob"},
cc: ["assistant@org.com"],
).get
system(:Mailer_2)[:to].should eq "steve@org.com"
system(:Mailer_2)[:cc].should eq ["assistant@org.com"]
system(:Mailer_2)[:bcc].should eq [] of String
system(:Mailer_1)[:to].should eq "steve@org.com"
system(:Mailer_1)[:cc].should eq ["assistant@org.com"]
system(:Mailer_1)[:bcc].should eq [] of String

# 5. A template's `to` replaces the recipient the driver chose, and its `cc`
# and `bcc` are added to the caller's, without duplicates.
Expand All @@ -197,7 +197,7 @@ DriverSpecs.mock_driver "Place::TemplateMailer" do
cc: ["assistant@org.com"],
bcc: "reception@org.com",
).get
system(:Mailer_2)[:to].should eq ["reception@org.com", "security@org.com"]
system(:Mailer_2)[:cc].should eq ["assistant@org.com", "manager@org.com"]
system(:Mailer_2)[:bcc].should eq ["reception@org.com", "audit@org.com"]
system(:Mailer_1)[:to].should eq ["reception@org.com", "security@org.com"]
system(:Mailer_1)[:cc].should eq ["assistant@org.com", "manager@org.com"]
system(:Mailer_1)[:bcc].should eq ["reception@org.com", "audit@org.com"]
end
Loading