diff --git a/drivers/place/visitor_mailer.cr b/drivers/place/visitor_mailer.cr index d4839a1650..94f3938b60 100644 --- a/drivers/place/visitor_mailer.cr +++ b/drivers/place/visitor_mailer.cr @@ -54,6 +54,11 @@ class Place::VisitorMailer < PlaceOS::Driver invite_zone_tag: "building", is_campus: false, + # Suppresses the `booking` template when the booking has an + # extension_data.parent_id (i.e. auto-created from a calendar event that + # already triggers the `event` template). + skip_event_linked_booking_email: true, + domain_uri: "https://example.com/", jwt_private_key: PlaceOS::Model::JWTBase.private_key, }) @@ -139,6 +144,7 @@ class Place::VisitorMailer < PlaceOS::Driver @network_password_minimum_symbols : Int32 = DEFAULT_PASSWORD_MINIMUM_SYMBOLS @network_group_ids = [] of String @disable_event_visitors : Bool = true + @skip_event_linked_booking_email : Bool = true @uri : URI = URI.new @jwt_private_key : String = PlaceOS::Model::JWTBase.private_key @@ -170,6 +176,8 @@ class Place::VisitorMailer < PlaceOS::Driver @network_group_ids = setting?(Array(String), :network_group_ids) || [] of String @host_domain_filter = setting?(Array(String), :host_domain_filter) || [] of String @disable_event_visitors = setting?(Bool, :disable_event_visitors) || false + skip_event_linked = setting?(Bool, :skip_event_linked_booking_email) + @skip_event_linked_booking_email = skip_event_linked.nil? ? true : skip_event_linked @invite_zone_tag = setting?(String, :invite_zone_tag) || "building" @is_parent_zone = setting?(Bool, :is_campus) || false @@ -306,7 +314,17 @@ class Place::VisitorMailer < PlaceOS::Driver in BookingGuest area_name = @booking_space_name template = @booking_template - booking_type = staff_api.get_booking(guest_details.booking_id).get["booking_type"].as_s + booking = staff_api.get_booking(guest_details.booking_id).get + + if @skip_event_linked_booking_email + parent_id = booking.dig?("extension_data", "parent_id").try(&.as_s?) + if parent_id && !parent_id.empty? + logger.debug { "skipping booking_created email for booking #{guest_details.booking_id} as it is linked to event #{parent_id}" } + return + end + end + + booking_type = booking["booking_type"].as_s template = @group_event_template if booking_type == "group-event" in GuestNotification # should never get here @@ -692,37 +710,20 @@ class Place::VisitorMailer < PlaceOS::Driver return unless fields_changed - # Resolve previous location names from previous_system_id when room changed. - # Default previous_room_name to "unknown" when we know there was a different - # previous system — if the lookup succeeds it will be overwritten with the - # real name; if it fails (rescue) the "unknown" default is preserved and the - # recipient can see that the original location could not be determined. previous_building_name = building_zone.display_name.presence || building_zone.name previous_room_name = @booking_space_name if (prev_sys_id = details.previous_system_id) && prev_sys_id != details.system_id + # Use "unknown" as the fallback so a failed lookup surfaces in the email + # rather than silently showing the current room name. previous_room_name = "unknown" - begin - prev_sys = get_room_details(prev_sys_id) - previous_room_name = prev_sys.display_name.presence || prev_sys.name - if prev_zones = prev_sys.zones - prev_zones.each do |zone_id| - begin - zone = fetch_zone(zone_id) - if zone.tags.includes?(@invite_zone_tag) - previous_building_name = zone.display_name.presence || zone.name - break - end - rescue error - logger.warn(exception: error) { "error looking up previous zone #{zone_id}" } - end - end - end - rescue error - logger.warn(exception: error) { "error looking up previous system #{prev_sys_id}" } - end + previous_room_name, previous_building_name = resolve_system_location_names(prev_sys_id, previous_room_name, previous_building_name) end + current_building_name = building_zone.display_name.presence || building_zone.name + current_room_name = @booking_space_name + current_room_name, current_building_name = resolve_system_location_names(details.system_id, current_room_name, current_building_name) + guests = staff_api.event_guests(details.event_id, details.system_id, details.event_ical_uid).get.as_a send_booking_changed_emails( guests, @@ -732,6 +733,8 @@ class Place::VisitorMailer < PlaceOS::Driver details.previous_event_start, previous_building_name, previous_room_name, + current_building_name, + current_room_name, ) rescue error logger.error { error.inspect_with_backtrace } @@ -743,7 +746,9 @@ class Place::VisitorMailer < PlaceOS::Driver } end - # Sends booking-changed notification emails to each visitor in the guest list. + # `building_name` / `room_name` override the current location names; when + # omitted they fall back to `building_zone` / `@booking_space_name` (used by + # the booking flow, which has no system_id to resolve from). private def send_booking_changed_emails( guests : Array(JSON::Any), host_email : String, @@ -752,7 +757,12 @@ class Place::VisitorMailer < PlaceOS::Driver previous_start : Int64?, previous_building_name : String, previous_room_name : String, + building_name : String? = nil, + room_name : String? = nil, ) + resolved_building_name = building_name || (building_zone.display_name.presence || building_zone.name) + resolved_room_name = room_name || @booking_space_name + guests.each do |guest| visitor_email = guest["email"].as_s visitor_name = guest["name"].as_s? @@ -773,8 +783,8 @@ class Place::VisitorMailer < PlaceOS::Driver visitor_name: visitor_name, host_name: get_host_name(host_email), host_email: host_email, - room_name: @booking_space_name, - building_name: building_zone.display_name.presence || building_zone.name, + room_name: resolved_room_name, + building_name: resolved_building_name, event_title: event_title, event_start: local_start_time.to_s(@time_format), event_date: local_start_time.to_s(@date_format), @@ -790,6 +800,35 @@ class Place::VisitorMailer < PlaceOS::Driver end end + # Returns `{room_name, building_name}` for `system_id`, falling back to the + # supplied values (and logging a warning) if any lookup fails. + private def resolve_system_location_names(system_id : String, fallback_room : String, fallback_building : String) : {String, String} + room_name = fallback_room + building_name = fallback_building + + begin + sys = get_room_details(system_id) + room_name = sys.display_name.presence || sys.name + if zones = sys.zones + zones.each do |zone_id| + begin + zone = fetch_zone(zone_id) + if zone.tags.includes?(@invite_zone_tag) + building_name = zone.display_name.presence || zone.name + break + end + rescue error + logger.warn(exception: error) { "error looking up zone #{zone_id} for system #{system_id}" } + end + end + end + rescue error + logger.warn(exception: error) { "error looking up system #{system_id}" } + end + + {room_name, building_name} + end + @[Security(Level::Support)] def send_visitor_qr_email( template : String, diff --git a/drivers/place/visitor_mailer_spec.cr b/drivers/place/visitor_mailer_spec.cr index c1e74f6986..29f5b31681 100644 --- a/drivers/place/visitor_mailer_spec.cr +++ b/drivers/place/visitor_mailer_spec.cr @@ -41,6 +41,11 @@ class MailerMock < DriverSpecs::MockDriver ) : Bool true end + + # Used by send_visitor_qr_email when @disable_qr_code is false (the default). + def generate_png_qrcode(text : String, size : Int32 = 256) + "PNG-#{text}-#{size}" + end end # :nodoc: @@ -162,6 +167,47 @@ class StaffAPIMock < DriverSpecs::MockDriver {id: id, name: "Unknown Room", display_name: nil, map_id: nil, zones: [] of String} end end + + # 600 — standalone visitor booking + # 601 — event-linked visitor booking (parent_id set) + # 602 — group-event booking + def get_booking(booking_id : Int64, instance : Int64? = nil) + case booking_id + when 601_i64 + { + id: 601, + booking_type: "visitor", + booking_start: 0, + booking_end: 0, + resource_id: "visitor@external.com", + user_email: "host@example.com", + title: "Linked Visit", + extension_data: {parent_id: "event-evt-200"}, + } + when 602_i64 + { + id: 602, + booking_type: "group-event", + booking_start: 0, + booking_end: 0, + resource_id: "visitor@external.com", + user_email: "host@example.com", + title: "Group Event Visit", + extension_data: {} of String => String, + } + else + { + id: booking_id, + booking_type: "visitor", + booking_start: 0, + booking_end: 0, + resource_id: "visitor@external.com", + user_email: "host@example.com", + title: "Standalone Visit", + extension_data: {} of String => String, + } + end + end end DriverSpecs.mock_driver "Place::VisitorMailer" do @@ -818,8 +864,10 @@ DriverSpecs.mock_driver "Place::VisitorMailer" do # previous location should come from the previous system (sys-old-room), NOT the current args18["previous_room_name"].should eq "Old Conference Room 202" args18["previous_building_name"].should eq "Previous Building" - # current location should remain correct - args18["room_name"].should eq "Client Floor" + # current location should be resolved from the signal's system_id (sys-room1) + # rather than the static @booking_space_name setting, so visitors can see + # which room the meeting moved to. + args18["room_name"].should eq "Conference Room 1" args18["building_name"].should eq "Main Building" # ------------------------------------------------------------------ @@ -961,4 +1009,69 @@ DriverSpecs.mock_driver "Place::VisitorMailer" do # 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 + + # Test 22: event-linked booking_created is skipped by default + + count_before_linked = system(:Mailer)[:send_count].as_i + + linked_booking_created_payload = { + action: "booking_created", + booking_id: 601_i64, + resource_id: "visitor@external.com", + event_title: "Linked Visit", + event_summary: "Linked Visit", + event_starting: now + 3600, + attendee_name: "Visitor One", + attendee_email: "visitor@external.com", + host: "host@example.com", + zones: ["zone-building", "zone-room"], + }.to_json + + publish("staff/guest/attending", linked_booking_created_payload) + sleep 0.5 + + system(:Mailer)[:send_count].should eq count_before_linked + + # Test 23: standalone booking_created still emits the booking template + + count_before_standalone = system(:Mailer)[:send_count].as_i + + standalone_booking_created_payload = { + action: "booking_created", + booking_id: 600_i64, + resource_id: "visitor@external.com", + event_title: "Standalone Visit", + event_summary: "Standalone Visit", + event_starting: now + 3600, + attendee_name: "Visitor One", + attendee_email: "visitor@external.com", + host: "host@example.com", + zones: ["zone-building", "zone-room"], + }.to_json + + publish("staff/guest/attending", standalone_booking_created_payload) + sleep 0.5 + + system(:Mailer)[:send_count].should eq count_before_standalone + 1 + system(:Mailer)[:last_template].should eq ["visitor_invited", "booking"] + system(:Mailer)[:last_to].should eq "visitor@external.com" + + # Test 24: opt-out restores the legacy dual-email behaviour + + settings({ + timezone: "GMT", + booking_space_name: "Client Floor", + invite_zone_tag: "building", + skip_event_linked_booking_email: false, + }) + sleep 1.0 + + count_before_opt_out = system(:Mailer)[:send_count].as_i + + publish("staff/guest/attending", linked_booking_created_payload) + sleep 0.5 + + system(:Mailer)[:send_count].should eq count_before_opt_out + 1 + system(:Mailer)[:last_template].should eq ["visitor_invited", "booking"] + system(:Mailer)[:last_to].should eq "visitor@external.com" end