feat: set reply-to to the host user (PPT-2520)

drivers/place/auto_release.cr

@@ -52,6 +52,19 @@ class Place::AutoRelease < PlaceOS::Driver
     system.implementing(Interface::Mailer)[0]
   end
 
+  # The current time. In non-release (spec/dev) builds it can be pinned via the
+  # `time_now_override` setting so tests mixing relative booking times with
+  # time-of-day logic are deterministic; `--release` builds compile the override
+  # out and always return `Time.utc`.
+  private def time_now : Time
+    {% unless flag?(:release) %}
+      if override = @time_now_override
+        return override
+      end
+    {% end %}
+    Time.utc
+  end
+
   @date_time_format : String = "%c"
   @time_format : String = "%l:%M%p"
   @date_format : String = "%A, %-d %B"
@@ -70,12 +83,26 @@ class Place::AutoRelease < PlaceOS::Driver
   @skip_same_day : Bool = true
   @skip_all_day : Bool = false
 
+  {% unless flag?(:release) %}
+    # Test-only seam (compiled out of `--release` builds): lets specs pin "now"
+    # via the `time_now_override` setting for deterministic time-of-day tests.
+    @time_now_override : Time? = nil
+  {% end %}
+
   def on_update
     @building_zone = nil
     @building_id = nil
     @levels = nil
     @timezone = nil
 
+    {% unless flag?(:release) %}
+      # Test-only seam (see `time_now`): pin "now" for deterministic specs.
+      # Persists once set so specs only need to provide it in their first settings.
+      if override_unix = setting?(Int64, :time_now_override)
+        @time_now_override = Time.unix(override_unix)
+      end
+    {% end %}
+
     @email_schedule = setting?(String, :email_schedule).presence
     @email_template = setting?(String, :email_template) || "auto_release"
     @unique_templates = setting?(Bool, :unique_templates) || false
@@ -176,8 +203,8 @@ class Place::AutoRelease < PlaceOS::Driver
     @auto_release.resources.each do |type|
       bookings = Array(Booking).from_json staff_api.query_bookings(
         type: type,
-        period_start: Time.utc.to_unix,
-        period_end: (Time.utc + @time_window_hours.hours).to_unix,
+        period_start: time_now.to_unix,
+        period_end: (time_now + @time_window_hours.hours).to_unix,
         zones: [building_zone.id],
       ).get_json
       results += bookings.select { |booking| !booking.checked_in }
@@ -339,7 +366,7 @@ class Place::AutoRelease < PlaceOS::Driver
       booking_start = booking.all_day ? all_day_start_time(booking_start).to_unix : booking.booking_start
 
       # convert minutes (time_after) to seconds for comparison with unix timestamps (booking_start)
-      if Time.utc.to_unix - booking_start > @auto_release.time_after(booking.booking_type) * 60
+      if time_now.to_unix - booking_start > @auto_release.time_after(booking.booking_type) * 60
         # skip if there's been changes to the cached bookings checked_in status or booking_start time
         if skip_release?(booking)
           explain[booking.id] = "skip release due to changes to checked_in status or booking_start time"
@@ -388,8 +415,8 @@ class Place::AutoRelease < PlaceOS::Driver
 
       # convert minutes (time_after) to seconds for comparison with unix timestamps (booking_start)
       if enabled? &&
-         (booking.booking_start - Time.utc.to_unix <= @auto_release.time_before(booking.booking_type) * 60) &&
-         (Time.utc.to_unix - booking.booking_start <= @auto_release.time_after(booking.booking_type) * 60)
+         (booking.booking_start - time_now.to_unix <= @auto_release.time_before(booking.booking_type) * 60) &&
+         (time_now.to_unix - booking.booking_start <= @auto_release.time_after(booking.booking_type) * 60)
         logger.debug { "sending release email to #{booking.user_email} for booking #{booking.id} as it is within the time_before window" }
 
         location = Time::Location.load(booking.timezone.presence || timezone.name)
@@ -426,7 +453,8 @@ class Place::AutoRelease < PlaceOS::Driver
           mailer.send_template(
             to: booking.user_email,
             template: {@email_template, "auto_release#{template_suffix(booking.booking_type)}"},
-            args: args)
+            args: args,
+            reply_to: booking.booked_by_email.presence)
           emailed_booking_ids << booking.id
         rescue error
           logger.warn(exception: error) { "failed to send release email to #{booking.user_email}" }

drivers/place/auto_release_readme.md

@@ -177,6 +177,18 @@ Times are specified in 24-hour format as decimal numbers:
 **Important**: If `release_outside_hours: true` is set, configuring `default_work_preferences` may be unnecessary. When `release_outside_hours` is enabled, any booking that doesn't match the user's configured work preferences (or default preferences) will automatically be flagged for release anyway. This makes `default_work_preferences` primarily useful when you want more granular control over release timing rather than blanket 24/7 release behavior.
 
 
+## Reply-To
+
+Auto-release notification emails set a `Reply-To` header so replies reach a useful
+person rather than the no-reply sender address. By default the reply-to is the
+**booking creator** (`booked_by_email`). This requires no configuration.
+
+This default can be overridden per-template (a `reply_to` field on the template
+metadata), tenant-wide (the `reply_to` setting on the Template Mailer), or for all
+mail (the `reply_to` setting on the SMTP Mailer). See the Template Mailer readme
+for the full precedence cascade.
+
+
 ## Template Configuration on Mailer
 
 The driver expects an email template for notifying users about pending releases:

drivers/place/auto_release_spec.cr

@@ -10,8 +10,15 @@ class StaffAPI < DriverSpecs::MockDriver
     self[:rejected] = self[:rejected].as_i + 1
   end
 
-  TIMEZONE          = "Australia/Sydney"
-  TIME_LOCAL        = Time.local(location: Time::Location.load(TIMEZONE))
+  TIMEZONE = "Australia/Sydney"
+  # Pinned to a fixed evening time-of-day so the whole suite is deterministic
+  # (the driver's "now" is pinned to the same instant via `time_now_override`).
+  # Evening is required because several bookings sit a few hours after "now" and
+  # are expected to land on the *next* day / outside work hours.
+  TIME_LOCAL = begin
+    n = Time.local(location: Time::Location.load(TIMEZONE))
+    Time.local(n.year, n.month, n.day, 19, 30, 0, location: Time::Location.load(TIMEZONE))
+  end
   TIME_YESTERDAY    = TIME_LOCAL - 1.day
   TIME_START_OF_DAY = TIME_LOCAL - TIME_LOCAL.hour.hours - TIME_LOCAL.minute.minutes - TIME_LOCAL.second.seconds
   TIME_END_OF_DAY   = TIME_START_OF_DAY + 1.day - 1.seconds
@@ -791,6 +798,7 @@ class Mailer < DriverSpecs::MockDriver
     reply_to : String | Array(String) | Nil = nil,
   )
     self[:sent] = self[:sent].as_i + 1
+    self[:reply_to] = reply_to
   end
 
   def send_mail(
@@ -816,7 +824,9 @@ DriverSpecs.mock_driver "Place::AutoRelease" do
   })
 
   timezone = "Australia/Sydney"
-  time_local = Time.local(location: Time::Location.load(timezone))
+  # use the same pinned instant the driver's clock is fixed to, so all_day_start
+  # offsets are evaluated consistently
+  time_local = StaffAPI::TIME_LOCAL
 
   settings({
     auto_release: {
@@ -825,6 +835,8 @@ DriverSpecs.mock_driver "Place::AutoRelease" do
       resources:   ["desk"],
     },
     release_locations: ["wfh", "aol"],
+    # pin the driver's clock to the same instant the bookings are anchored to
+    time_now_override: StaffAPI::TIME_LOCAL.to_unix,
   })
 
   resp = exec(:get_building_zone?).get
@@ -1223,6 +1235,8 @@ DriverSpecs.mock_driver "Place::AutoRelease" do
   resp = exec(:send_release_emails).get
   resp.should eq [16]
   system(:Mailer_1)[:sent].should eq 3
+  # replies go to the person who created the booking
+  system(:Mailer_1)[:reply_to].should eq "user_one@example.com"
 
   ########################################
   # End of tests for: #send_release_emails

drivers/place/booking_approval_workflows.cr

@@ -306,7 +306,8 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
         mailer.send_template(
           to: booking_details.booked_by_email,
           template: {"bookings", "group_booking_sent#{@template_suffix}"},
-          args: args
+          args: args,
+          reply_to: booking_details.booked_by_email.presence,
         )
       end
 
@@ -325,7 +326,8 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
         to: booking_details.user_email,
         template: {"bookings", third_party ? "approved_by#{@template_suffix}" : "approved#{@template_suffix}"},
         args: args,
-        attachments: attachments
+        attachments: attachments,
+        reply_to: booking_details.booked_by_email.presence,
       ).get
 
       staff_api.booking_state(booking_details.id, "approval_sent", booking_details.instance).get
@@ -335,7 +337,8 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
       mailer.send_template(
         to: user_email,
         template: {"bookings", "#{booking_details.action}#{@template_suffix}"},
-        args: args
+        args: args,
+        reply_to: booking_details.booked_by_email.presence,
       )
     when "cancelled"
       third_party = booking_details.approver_email && booking_details.approver_email != booking_details.user_email.downcase
@@ -345,14 +348,16 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
       mailer.send_template(
         to: user_email,
         template: {"bookings", third_party ? "cancelled_by#{@template_suffix}" : "cancelled#{@template_suffix}"},
-        args: args
+        args: args,
+        reply_to: booking_details.booked_by_email.presence,
       )
 
       if @notify_managers && (manager_email = get_manager(user_email).try(&.at(0)))
         mailer.send_template(
           to: manager_email,
           template: {"bookings", "manager_notify_cancelled#{@template_suffix}"},
-          args: args
+          args: args,
+          reply_to: booking_details.booked_by_email.presence,
         )
       end
     end
@@ -408,7 +413,8 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
           mailer.send_template(
             to: manager_email,
             template: {"bookings", "manager_approval#{@template_suffix}"},
-            args: args
+            args: args,
+            reply_to: booking_details.booked_by_email.presence,
           ).get
 
           # set the booking state
@@ -420,7 +426,8 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
             args: args.merge({
               manager_email: manager_email,
               manager_name:  manager_name,
-            })
+            }),
+            reply_to: booking_details.booked_by_email.presence,
           )
         else
           logger.debug { "manager not found, approving booking!" }
@@ -436,7 +443,8 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
             mailer.send_template(
               to: manager_email,
               template: {"bookings", "manager_approval#{@template_suffix}"},
-              args: args
+              args: args,
+              reply_to: booking_details.booked_by_email.presence,
             ).get
 
             # set the booking state
@@ -457,7 +465,8 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
               mailer.send_template(
                 to: manager_email,
                 template: {"bookings", "manager_approval#{@template_suffix}"},
-                args: args
+                args: args,
+                reply_to: booking_details.booked_by_email.presence,
               ).get
 
               # set the booking state
@@ -491,7 +500,8 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
         mailer.send_template(
           to: manager_email,
           template: {"bookings", "notify_manager#{@template_suffix}"},
-          args: args
+          args: args,
+          reply_to: booking_details.booked_by_email.presence,
         )
       end
     else
@@ -637,7 +647,8 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
               to: booking_details.user_email,
               template: {"bookings", third_party ? "approved_by#{@template_suffix}" : "approved#{@template_suffix}"},
               args: args,
-              attachments: attachments
+              attachments: attachments,
+              reply_to: booking_details.booked_by_email.presence,
             )
             staff_api.booking_state(booking_details.id, "approval_sent", booking_details.instance).get
           end
@@ -800,7 +811,8 @@ class Place::BookingApprovalWorkflows < PlaceOS::Driver
         to: booking_details.user_email,
         template: {"bookings", "checkin_reminder#{@template_suffix}"},
         args: args,
-        attachments: attachments
+        attachments: attachments,
+        reply_to: booking_details.booked_by_email.presence,
       )
     end
   end

drivers/place/booking_approval_workflows_readme.md

@@ -0,0 +1,44 @@
+# Booking Approval Workflows Readme
+
+Drives manager approval workflows for asset bookings (defaults to desks) and sends
+the associated notification emails: approvals, rejections, cancellations, manager
+approval requests/reminders/escalations, and check-in reminders.
+
+## Requirements
+
+Requires the following drivers in the system:
+
+* StaffAPI - for querying bookings and updating booking state
+* Mailer - for sending emails (and where the templates are configured)
+* Calendar - for looking up a user's manager
+
+## Configuration
+
+```yaml
+  booking_type:    "desk"
+  notify_managers: false
+  remind_after:    24   # hours before reminding a manager to approve
+  escalate_after:  48   # hours before escalating to the manager's manager
+
+  # zone_id => approval configuration
+  approval_type: {
+    zone_id1: {
+      approval:      "manager_approval",
+      name:          "Sydney Building 1",
+      support_email: "support@place.com",
+      attachments:   null,
+    },
+  }
+```
+
+## Reply-To
+
+Approval workflow emails set a `Reply-To` header so replies reach a useful person
+rather than the no-reply sender address. By default the reply-to is the **booking
+creator** (`booked_by_email`) -- including emails sent to managers. This requires
+no configuration.
+
+This default can be overridden per-template (a `reply_to` field on the template
+metadata), tenant-wide (the `reply_to` setting on the Template Mailer), or for all
+mail (the `reply_to` setting on the SMTP Mailer). See the Template Mailer readme
+for the full precedence cascade.