Skip to content

fix(visitor_mailer): Booking Changed notification (PPT-2375) #618

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
chillfox merged 1 commit into from
Jul 14, 2026
+28 −28
Merged

fix(visitor_mailer): Booking Changed notification (PPT-2375) #618

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
19 changes: 8 additions & 11 deletions drivers/place/visitor_mailer.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,17 +611,14 @@ class Place::VisitorMailer < PlaceOS::Driver
# "checked_in") are ignored by default and don't trigger spurious emails.
return unless details.action.in?("changed", "metadata_changed")

# Bookings auto-created from a calendar event (extension_data.parent_id set)
# are already covered by the event_changed flow (staff/event/changed), which
# resolves the room from the system. Skip them here so a single edit doesn't
# produce two notifications. Opt out with skip_event_linked_booking_email.
if @skip_event_linked_booking_email
parent_id = details.extension_data.try(&.["parent_id"]?).try(&.as_s?)
if parent_id && !parent_id.empty?
logger.debug { "skipping booking_changed email for booking #{details.id} as it is linked to event #{parent_id}" }
return
end
end
# NOTE: event-linked bookings (extension_data.parent_id set — e.g. the
# visitor bookings attendee_scanner creates for external calendar guests)
# are intentionally NOT skipped here. Editing a booking only emits
# staff/booking/changed; the linked calendar event is left untouched, so no
# staff/event/changed fires to cover it. Skipping them dropped the visitor's
# only change notification (PPT-2375). Duplicate suppression for the initial
# invite still lives in guest_event, where a genuine event+booking
# double-invite occurs on creation.

# ensure the event is for this building
if zones = details.zones
Expand Down
37 changes: 20 additions & 17 deletions drivers/place/visitor_mailer_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1530,28 +1530,28 @@ DriverSpecs.mock_driver "Place::VisitorMailer" do
system(:Mailer)[:last_template].should eq ["visitor_invited", "custom_booking_changed"]

# ==================================================================
# Issue 2 — de-duplicate event-linked booking notifications
# Issue 2 — event-linked booking changes must still notify (PPT-2375)
# ==================================================================
#
# A calendar event with a room auto-creates a linked booking
# (extension_data.parent_id points to the event). Editing the event
# fires staff/event/changed (handled by event_changed_event) AND the
# linked booking fires staff/booking/changed — so without de-duplication
# the visitor receives TWO change notifications. booking_changed_event
# must skip event-linked bookings (same skip_event_linked_booking_email
# rule guest_event uses), letting the event_changed flow be the single
# source of truth.
# attendee_scanner creates visitor bookings for external calendar guests
# with extension_data.parent_id pointing at the source event. Editing
# such a booking only emits staff/booking/changed — the linked calendar
# event is untouched, so NO staff/event/changed fires to cover it.
# An earlier de-duplication attempt skipped these bookings entirely, which
# silently dropped the visitor's only change notification. They must be
# treated like any other booking change.

settings({
timezone: "GMT",
booking_space_name: "Client Floor",
invite_zone_tag: "building",
# skip_event_linked_booking_email defaults to true
# skip_event_linked_booking_email defaults to true (invite flow only)
})
sleep 1.0

# ------------------------------------------------------------------
# Test 34: booking_changed for an event-linked booking is suppressed
# Test 34: booking_changed for an event-linked booking STILL notifies
# (regression: it was previously suppressed, losing the email)
# ------------------------------------------------------------------

count_before_linked_change = system(:Mailer)[:send_count].as_i
Expand All @@ -1574,11 +1574,13 @@ DriverSpecs.mock_driver "Place::VisitorMailer" do
}.to_json

publish("staff/booking/changed", linked_booking_changed)
sleep 1.0
sleep 1.5

# Event-linked: the event_changed flow already notifies these visitors, so
# no booking_changed email should be sent.
system(:Mailer)[:send_count].should eq count_before_linked_change
# The booking edit is the only signal these visitors receive, so it must
# produce the booking_changed notification.
system(:Mailer)[:send_count].should eq count_before_linked_change + 1
system(:Mailer)[:last_to].should eq "visitor@external.com"
system(:Mailer)[:last_template].should eq ["visitor_invited", "booking_changed"]

# ------------------------------------------------------------------
# Test 35: standalone booking_changed (no parent_id) still notifies
Expand Down Expand Up @@ -1611,8 +1613,9 @@ DriverSpecs.mock_driver "Place::VisitorMailer" do
system(:Mailer)[:last_template].should eq ["visitor_invited", "booking_changed"]

# ------------------------------------------------------------------
# Test 36: opt-out (skip_event_linked_booking_email: false) restores
# the booking_changed email even for event-linked bookings.
# Test 36: skip_event_linked_booking_email governs only the invite
# (guest_event) flow — event-linked booking *changes* notify
# regardless of the setting value.
# ------------------------------------------------------------------

settings({
Expand Down
Loading