From 794cb37951b0ad8f040b4b8681f50ea71ff604f3 Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Tue, 14 Jul 2026 14:16:44 +0930 Subject: [PATCH] fix(visitor_mailer): Booking Changed notification (PPT-2375) --- drivers/place/visitor_mailer.cr | 19 ++++++-------- drivers/place/visitor_mailer_spec.cr | 37 +++++++++++++++------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/drivers/place/visitor_mailer.cr b/drivers/place/visitor_mailer.cr index 6dbb3f8563..5bf3610659 100644 --- a/drivers/place/visitor_mailer.cr +++ b/drivers/place/visitor_mailer.cr @@ -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 diff --git a/drivers/place/visitor_mailer_spec.cr b/drivers/place/visitor_mailer_spec.cr index 7e3ea8e7a9..f4b210f241 100644 --- a/drivers/place/visitor_mailer_spec.cr +++ b/drivers/place/visitor_mailer_spec.cr @@ -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 @@ -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 @@ -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({