From 665bf1634731d60d6ef625a7ff5ee9ffe812b88e Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Thu, 14 May 2026 14:08:57 +0930 Subject: [PATCH 01/10] fix(visitor_mailer): not all signals have all fields, so make them nilable --- drivers/place/visitor_mailer.cr | 21 ++++++++++++++------- drivers/place/visitor_models.cr | 6 +++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/drivers/place/visitor_mailer.cr b/drivers/place/visitor_mailer.cr index 601c7c5a0a9..54c6da6593e 100644 --- a/drivers/place/visitor_mailer.cr +++ b/drivers/place/visitor_mailer.cr @@ -641,6 +641,13 @@ class Place::VisitorMailer < PlaceOS::Driver # only respond to updates, not creates or cancellations return unless details.action == "update" + # These fields may be missing from some payloads (e.g. cancelled events, + # metadata-only updates) so the model marks them nilable. + host = details.host + event_start = details.event_start + event_end = details.event_end + return unless host && event_start && event_end + # ensure the event is for this building if zones = details.zones check = [building_zone.id] + @parent_zone_ids @@ -653,13 +660,13 @@ class Place::VisitorMailer < PlaceOS::Driver # --- Host change notification if prev_host = details.previous_host_email - if prev_host.downcase != details.host.downcase + if prev_host.downcase != host.downcase send_original_host_email( @notify_original_host_template, prev_host, - details.host, + host, details.title, - details.event_start, + event_start, ) end end @@ -669,10 +676,10 @@ class Place::VisitorMailer < PlaceOS::Driver # Date or time changed if prev_start = details.previous_event_start - fields_changed = true if prev_start != details.event_start + fields_changed = true if prev_start != event_start end if prev_end = details.previous_event_end - fields_changed = true if prev_end != details.event_end + fields_changed = true if prev_end != event_end end # Location changed (system_id represents the room) @@ -716,8 +723,8 @@ class Place::VisitorMailer < PlaceOS::Driver guests = staff_api.event_guests(details.event_id, details.system_id).get.as_a send_booking_changed_emails( guests, - details.host, - details.event_start, + host, + event_start, details.title, details.previous_event_start, previous_building_name, diff --git a/drivers/place/visitor_models.cr b/drivers/place/visitor_models.cr index fb9cff831c0..6f60e2f01e3 100644 --- a/drivers/place/visitor_models.cr +++ b/drivers/place/visitor_models.cr @@ -136,11 +136,11 @@ module Place property system_id : String property event_id : String property event_ical_uid : String? - property host : String + property host : String? property resource : String? property title : String? - property event_start : Int64 - property event_end : Int64 + property event_start : Int64? + property event_end : Int64? property zones : Array(String)? # Previous values — only present when action is "update" and the meta was persisted. From b5c5e40c1aac704936c49f28717a17c26c3cc385 Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Thu, 14 May 2026 17:46:03 +0930 Subject: [PATCH 02/10] fix(staff_api): support #guest_list ical_uid lookup (PPT-2375) --- drivers/place/staff_api.cr | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/place/staff_api.cr b/drivers/place/staff_api.cr index 5f2cf29f19d..381f1274312 100644 --- a/drivers/place/staff_api.cr +++ b/drivers/place/staff_api.cr @@ -993,9 +993,13 @@ class Place::StaffAPI < PlaceOS::Driver JSON.parse(response.body) end - def event_guests(event_id : String, system_id : String) + def event_guests(event_id : String, system_id : String, ical_uid : String? = nil) logger.debug { "getting guests for event #{event_id} in system #{system_id}" } - response = get("/api/staff/v1/events/#{event_id}/guests?system_id=#{system_id}", headers: authentication) + params = URI::Params.build do |form| + form.add "system_id", system_id + form.add "ical_uid", ical_uid if ical_uid + end + response = get("/api/staff/v1/events/#{event_id}/guests?#{params}", headers: authentication) raise "issue getting guests for event #{event_id}: #{response.status_code}" unless response.success? JSON.parse(response.body) end From d02b95a586f3e5ae36b55fb589a8fe98d9f31ad8 Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Thu, 14 May 2026 17:46:25 +0930 Subject: [PATCH 03/10] fix(visitor_mailer): support #guest_list ical_uid lookup (PPT-2375) --- drivers/place/visitor_mailer.cr | 2 +- drivers/place/visitor_mailer_spec.cr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/place/visitor_mailer.cr b/drivers/place/visitor_mailer.cr index 54c6da6593e..02fec19102f 100644 --- a/drivers/place/visitor_mailer.cr +++ b/drivers/place/visitor_mailer.cr @@ -720,7 +720,7 @@ class Place::VisitorMailer < PlaceOS::Driver end end - guests = staff_api.event_guests(details.event_id, details.system_id).get.as_a + guests = staff_api.event_guests(details.event_id, details.system_id, details.event_ical_uid).get.as_a send_booking_changed_emails( guests, host, diff --git a/drivers/place/visitor_mailer_spec.cr b/drivers/place/visitor_mailer_spec.cr index 257da8c3cf1..33d34fa7ee8 100644 --- a/drivers/place/visitor_mailer_spec.cr +++ b/drivers/place/visitor_mailer_spec.cr @@ -131,7 +131,7 @@ class StaffAPIMock < DriverSpecs::MockDriver ] end - def event_guests(event_id : String, system_id : String) + def event_guests(event_id : String, system_id : String, ical_uid : String? = nil) [ { email: "visitor@external.com", From 6f73b270dfeaba2ccc96d04a1e6eb4f39a71b99d Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Thu, 14 May 2026 17:52:12 +0930 Subject: [PATCH 04/10] refactor(staff_api): match style of other ical_uid lookups --- drivers/place/staff_api.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/place/staff_api.cr b/drivers/place/staff_api.cr index 381f1274312..f7700616ac8 100644 --- a/drivers/place/staff_api.cr +++ b/drivers/place/staff_api.cr @@ -997,7 +997,7 @@ class Place::StaffAPI < PlaceOS::Driver logger.debug { "getting guests for event #{event_id} in system #{system_id}" } params = URI::Params.build do |form| form.add "system_id", system_id - form.add "ical_uid", ical_uid if ical_uid + form.add "ical_uid", ical_uid.to_s if ical_uid.presence end response = get("/api/staff/v1/events/#{event_id}/guests?#{params}", headers: authentication) raise "issue getting guests for event #{event_id}: #{response.status_code}" unless response.success? From cfd604af1a231cc5e78218d2dc2e0d6fc23dd13d Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Fri, 15 May 2026 13:44:29 +0930 Subject: [PATCH 05/10] feat(staff_api): add include_linked guests (PPT-2375) --- drivers/place/staff_api.cr | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/place/staff_api.cr b/drivers/place/staff_api.cr index f7700616ac8..18c50893461 100644 --- a/drivers/place/staff_api.cr +++ b/drivers/place/staff_api.cr @@ -986,9 +986,10 @@ class Place::StaffAPI < PlaceOS::Driver JSON.parse(response.body) end - def booking_guests(booking_id : String | Int64) + def booking_guests(booking_id : String | Int64, include_linked : Bool = false) logger.debug { "getting guests for booking #{booking_id}" } - response = get("/api/staff/v1/bookings/#{booking_id}/guests", headers: authentication) + params = include_linked ? "?include_linked=true" : "" + response = get("/api/staff/v1/bookings/#{booking_id}/guests#{params}", headers: authentication) raise "issue getting guests for booking #{booking_id}: #{response.status_code}" unless response.success? JSON.parse(response.body) end From 497ad538b3fefc313164c4a95275424d97f882f4 Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Fri, 15 May 2026 13:44:59 +0930 Subject: [PATCH 06/10] fix(visitor_mailer): support group bookings (PPT-2375) --- drivers/place/visitor_mailer.cr | 8 +++- drivers/place/visitor_mailer_spec.cr | 70 ++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/drivers/place/visitor_mailer.cr b/drivers/place/visitor_mailer.cr index 02fec19102f..861f08048f5 100644 --- a/drivers/place/visitor_mailer.cr +++ b/drivers/place/visitor_mailer.cr @@ -614,7 +614,13 @@ class Place::VisitorMailer < PlaceOS::Driver end end - guests = staff_api.booking_guests(details.id).get.as_a + # include_linked: true ensures guests from child bookings (e.g. per-visitor + # bookings under a group parent) are returned in a single request. + guests = staff_api.booking_guests(details.id, include_linked: true).get.as_a + + # Deduplicate by email in case a guest appears on multiple child bookings + guests.uniq! { |guest| guest["email"].as_s.downcase } + send_booking_changed_emails( guests, details.user_email, diff --git a/drivers/place/visitor_mailer_spec.cr b/drivers/place/visitor_mailer_spec.cr index 33d34fa7ee8..8be11b3a66a 100644 --- a/drivers/place/visitor_mailer_spec.cr +++ b/drivers/place/visitor_mailer_spec.cr @@ -120,15 +120,23 @@ class StaffAPIMock < DriverSpecs::MockDriver end end - def booking_guests(booking_id : Int64) - [ - { - email: "visitor@external.com", - name: "Visitor One", - checked_in: false, - visit_expected: true, - }, - ] + # When include_linked is true, parent group bookings (e.g. id 300) return + # guests from all child bookings in a single response — just like the real + # staff-api endpoint. + def booking_guests(booking_id : Int64, include_linked : Bool = false) + case booking_id + when 300 + if include_linked + [ + {email: "visitor-a@external.com", name: "Visitor A", checked_in: false, visit_expected: true}, + {email: "visitor-b@external.com", name: "Visitor B", checked_in: false, visit_expected: true}, + ] + else + [] of NamedTuple(email: String, name: String, checked_in: Bool, visit_expected: Bool) + end + else + [{email: "visitor@external.com", name: "Visitor One", checked_in: false, visit_expected: true}] + end end def event_guests(event_id : String, system_id : String, ical_uid : String? = nil) @@ -874,4 +882,48 @@ DriverSpecs.mock_driver "Place::VisitorMailer" do # "approved" is not a visitor-notification action — no email should be sent system(:Mailer)[:send_count].should eq count_before_approved + + # ================================================================== + # Group booking child-guest fallback tests + # ================================================================== + + # ------------------------------------------------------------------ + # Test 20: booking_changed for a parent "group" booking whose guests + # live on child "visitor" bookings. The driver should fall + # back to fetching guests from linked_bookings when + # booking_guests returns empty for the parent. + # ------------------------------------------------------------------ + + count_before_group = system(:Mailer)[:send_count].as_i + + group_changed_payload = { + action: "changed", + id: 300_i64, + booking_type: "group", + booking_start: now + 7200, + booking_end: now + 10800, + timezone: "GMT", + resource_id: "host@example.com[2026-05-15]", + resource_ids: ["host@example.com[2026-05-15]"], + user_email: "host@example.com", + title: "Group Visit", + zones: ["zone-building", "zone-room"], + previous_booking_start: now + 3600, + previous_booking_end: now + 7200, + }.to_json + + publish("staff/booking/changed", group_changed_payload) + sleep 1.5 + + # Two child bookings (301, 302) each have one unique guest, so two + # emails should be sent. + system(:Mailer)[:send_count].should eq count_before_group + 2 + + # Last email should be to visitor-b (second child processed) + system(:Mailer)[:last_to].should eq "visitor-b@external.com" + system(:Mailer)[:last_template].should eq ["visitor_invited", "booking_changed"] + + args20 = system(:Mailer)[:last_args] + args20["event_title"].should eq "Group Visit" + args20["host_email"].should eq "host@example.com" end From 4d18baeba79dd88386415690bf365cde2ca6df5c Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Fri, 15 May 2026 14:13:35 +0930 Subject: [PATCH 07/10] refactor(visitor_mailer): (PPT-2375) --- drivers/place/visitor_mailer_spec.cr | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/place/visitor_mailer_spec.cr b/drivers/place/visitor_mailer_spec.cr index 8be11b3a66a..c5dbb780cd2 100644 --- a/drivers/place/visitor_mailer_spec.cr +++ b/drivers/place/visitor_mailer_spec.cr @@ -884,14 +884,15 @@ DriverSpecs.mock_driver "Place::VisitorMailer" do system(:Mailer)[:send_count].should eq count_before_approved # ================================================================== - # Group booking child-guest fallback tests + # Group booking linked-guest tests # ================================================================== # ------------------------------------------------------------------ - # Test 20: booking_changed for a parent "group" booking whose guests - # live on child "visitor" bookings. The driver should fall - # back to fetching guests from linked_bookings when - # booking_guests returns empty for the parent. + # Test 20: booking_changed for a parent "group" booking. The driver + # passes include_linked: true so the API aggregates guests + # from child bookings into a single response. The mock + # returns 2 unique guests for booking 300 when the flag is + # set, simulating this aggregation. # ------------------------------------------------------------------ count_before_group = system(:Mailer)[:send_count].as_i @@ -915,8 +916,8 @@ DriverSpecs.mock_driver "Place::VisitorMailer" do publish("staff/booking/changed", group_changed_payload) sleep 1.5 - # Two child bookings (301, 302) each have one unique guest, so two - # emails should be sent. + # The mock returns 2 unique guests for booking 300 with + # include_linked: true, so 2 emails should be sent. system(:Mailer)[:send_count].should eq count_before_group + 2 # Last email should be to visitor-b (second child processed) From 483392d7afc20cc24583322fadf640e8515ae693 Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Fri, 15 May 2026 15:22:59 +0930 Subject: [PATCH 08/10] refactor(visitor_mailer): (PPT-2375) --- drivers/place/visitor_mailer.cr | 2 +- drivers/place/visitor_mailer_spec.cr | 76 ++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/drivers/place/visitor_mailer.cr b/drivers/place/visitor_mailer.cr index 861f08048f5..ceb9fd10282 100644 --- a/drivers/place/visitor_mailer.cr +++ b/drivers/place/visitor_mailer.cr @@ -616,7 +616,7 @@ class Place::VisitorMailer < PlaceOS::Driver # include_linked: true ensures guests from child bookings (e.g. per-visitor # bookings under a group parent) are returned in a single request. - guests = staff_api.booking_guests(details.id, include_linked: true).get.as_a + guests = staff_api.booking_guests(details.id, include_linked: details.booking_type == "group").get.as_a # Deduplicate by email in case a guest appears on multiple child bookings guests.uniq! { |guest| guest["email"].as_s.downcase } diff --git a/drivers/place/visitor_mailer_spec.cr b/drivers/place/visitor_mailer_spec.cr index c5dbb780cd2..4136acecf56 100644 --- a/drivers/place/visitor_mailer_spec.cr +++ b/drivers/place/visitor_mailer_spec.cr @@ -134,6 +134,15 @@ class StaffAPIMock < DriverSpecs::MockDriver else [] of NamedTuple(email: String, name: String, checked_in: Bool, visit_expected: Bool) end + when 301 + if include_linked + [ + {email: "visitor-dup@external.com", name: "Visitor Dup A", checked_in: false, visit_expected: true}, + {email: "VISITOR-DUP@external.com", name: "Visitor Dup B", checked_in: false, visit_expected: true}, + ] + else + [] of NamedTuple(email: String, name: String, checked_in: Bool, visit_expected: Bool) + end else [{email: "visitor@external.com", name: "Visitor One", checked_in: false, visit_expected: true}] end @@ -927,4 +936,71 @@ DriverSpecs.mock_driver "Place::VisitorMailer" do args20 = system(:Mailer)[:last_args] args20["event_title"].should eq "Group Visit" args20["host_email"].should eq "host@example.com" + + # ------------------------------------------------------------------ + # Test 21: deduplication — the mock for booking 301 returns two guests + # with the same email address (different cases) when + # include_linked is true. The driver's uniq! should collapse + # them into a single email. + # ------------------------------------------------------------------ + + count_before_dedup = system(:Mailer)[:send_count].as_i + + dedup_payload = { + action: "changed", + id: 301_i64, + booking_type: "group", + booking_start: now + 7200, + booking_end: now + 10800, + timezone: "GMT", + resource_id: "host@example.com[2026-05-15]", + resource_ids: ["host@example.com[2026-05-15]"], + user_email: "host@example.com", + title: "Dedup Visit", + zones: ["zone-building", "zone-room"], + previous_booking_start: now + 3600, + previous_booking_end: now + 7200, + }.to_json + + publish("staff/booking/changed", dedup_payload) + sleep 1.5 + + # Two guests with the same email (case-insensitive) should be + # deduplicated to a single notification. + system(:Mailer)[:send_count].should eq count_before_dedup + 1 + system(:Mailer)[:last_to].should eq "visitor-dup@external.com" + + # ------------------------------------------------------------------ + # Test 22: non-group booking should NOT pass include_linked. The + # mock for booking 300 returns an empty guest list when + # include_linked is false, so if the driver incorrectly + # passes include_linked: true for a non-group type the + # assertion below would fail (2 emails would be sent). + # ------------------------------------------------------------------ + + count_before_non_group = system(:Mailer)[:send_count].as_i + + non_group_payload = { + action: "changed", + id: 300_i64, + booking_type: "desk", + booking_start: now + 7200, + booking_end: now + 10800, + timezone: "GMT", + resource_id: "desk-1", + resource_ids: ["desk-1"], + user_email: "host@example.com", + title: "Desk Booking", + zones: ["zone-building", "zone-room"], + previous_booking_start: now + 3600, + previous_booking_end: now + 7200, + }.to_json + + publish("staff/booking/changed", non_group_payload) + sleep 0.5 + + # Booking 300 with include_linked: false returns no guests, so no + # emails should be sent. This proves the driver does not pass + # include_linked: true for non-group booking types. + system(:Mailer)[:send_count].should eq count_before_non_group end From eac234772164b9f0de947ea5c3f2ceeadacf43ca Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Fri, 15 May 2026 15:44:02 +0930 Subject: [PATCH 09/10] refactor(visitor_mailer): (PPT-2375) --- drivers/place/staff_api.cr | 2 +- drivers/place/visitor_mailer_spec.cr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/place/staff_api.cr b/drivers/place/staff_api.cr index 18c50893461..0fab97f3043 100644 --- a/drivers/place/staff_api.cr +++ b/drivers/place/staff_api.cr @@ -986,7 +986,7 @@ class Place::StaffAPI < PlaceOS::Driver JSON.parse(response.body) end - def booking_guests(booking_id : String | Int64, include_linked : Bool = false) + def booking_guests(booking_id : String | Int64, include_linked : Bool? = nil) logger.debug { "getting guests for booking #{booking_id}" } params = include_linked ? "?include_linked=true" : "" response = get("/api/staff/v1/bookings/#{booking_id}/guests#{params}", headers: authentication) diff --git a/drivers/place/visitor_mailer_spec.cr b/drivers/place/visitor_mailer_spec.cr index 4136acecf56..b99432716fa 100644 --- a/drivers/place/visitor_mailer_spec.cr +++ b/drivers/place/visitor_mailer_spec.cr @@ -123,7 +123,7 @@ class StaffAPIMock < DriverSpecs::MockDriver # When include_linked is true, parent group bookings (e.g. id 300) return # guests from all child bookings in a single response — just like the real # staff-api endpoint. - def booking_guests(booking_id : Int64, include_linked : Bool = false) + def booking_guests(booking_id : Int64, include_linked : Bool? = nil) case booking_id when 300 if include_linked From 349f3c69ec7fbcf7d3fcec23fe2833e329b2a18c Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Fri, 15 May 2026 15:50:13 +0930 Subject: [PATCH 10/10] refactor(visitor_mailer): (PPT-2375) --- drivers/place/visitor_mailer.cr | 3 -- drivers/place/visitor_mailer_spec.cr | 44 +--------------------------- 2 files changed, 1 insertion(+), 46 deletions(-) diff --git a/drivers/place/visitor_mailer.cr b/drivers/place/visitor_mailer.cr index ceb9fd10282..d4839a16502 100644 --- a/drivers/place/visitor_mailer.cr +++ b/drivers/place/visitor_mailer.cr @@ -618,9 +618,6 @@ class Place::VisitorMailer < PlaceOS::Driver # bookings under a group parent) are returned in a single request. guests = staff_api.booking_guests(details.id, include_linked: details.booking_type == "group").get.as_a - # Deduplicate by email in case a guest appears on multiple child bookings - guests.uniq! { |guest| guest["email"].as_s.downcase } - send_booking_changed_emails( guests, details.user_email, diff --git a/drivers/place/visitor_mailer_spec.cr b/drivers/place/visitor_mailer_spec.cr index b99432716fa..c1e74f6986f 100644 --- a/drivers/place/visitor_mailer_spec.cr +++ b/drivers/place/visitor_mailer_spec.cr @@ -134,15 +134,6 @@ class StaffAPIMock < DriverSpecs::MockDriver else [] of NamedTuple(email: String, name: String, checked_in: Bool, visit_expected: Bool) end - when 301 - if include_linked - [ - {email: "visitor-dup@external.com", name: "Visitor Dup A", checked_in: false, visit_expected: true}, - {email: "VISITOR-DUP@external.com", name: "Visitor Dup B", checked_in: false, visit_expected: true}, - ] - else - [] of NamedTuple(email: String, name: String, checked_in: Bool, visit_expected: Bool) - end else [{email: "visitor@external.com", name: "Visitor One", checked_in: false, visit_expected: true}] end @@ -938,40 +929,7 @@ DriverSpecs.mock_driver "Place::VisitorMailer" do args20["host_email"].should eq "host@example.com" # ------------------------------------------------------------------ - # Test 21: deduplication — the mock for booking 301 returns two guests - # with the same email address (different cases) when - # include_linked is true. The driver's uniq! should collapse - # them into a single email. - # ------------------------------------------------------------------ - - count_before_dedup = system(:Mailer)[:send_count].as_i - - dedup_payload = { - action: "changed", - id: 301_i64, - booking_type: "group", - booking_start: now + 7200, - booking_end: now + 10800, - timezone: "GMT", - resource_id: "host@example.com[2026-05-15]", - resource_ids: ["host@example.com[2026-05-15]"], - user_email: "host@example.com", - title: "Dedup Visit", - zones: ["zone-building", "zone-room"], - previous_booking_start: now + 3600, - previous_booking_end: now + 7200, - }.to_json - - publish("staff/booking/changed", dedup_payload) - sleep 1.5 - - # Two guests with the same email (case-insensitive) should be - # deduplicated to a single notification. - system(:Mailer)[:send_count].should eq count_before_dedup + 1 - system(:Mailer)[:last_to].should eq "visitor-dup@external.com" - - # ------------------------------------------------------------------ - # Test 22: non-group booking should NOT pass include_linked. The + # Test 21: non-group booking should NOT pass include_linked. The # mock for booking 300 returns an empty guest list when # include_linked is false, so if the driver incorrectly # passes include_linked: true for a non-group type the