From 5bc663d459459af7b64ab0bc3b7bebc279ec0671 Mon Sep 17 00:00:00 2001
From: William Le <will@place.tech>
Date: Fri, 23 Oct 2026 18:31:10 +0800
Subject: [PATCH] feat(place/booking_approver): optionally reject bookings for
 undefined assets

Adds a `reject_undefined_assets` mode. When enabled, the driver caches the
asset ids defined in the level zone metadata of every building (refreshed
on `bookable_assets_cache_cron`, default midnight) for each type in
`approve_booking_types`. New bookings whose assets aren't in the cache are
rejected; the rest are approved as before.

- booking_type => metadata key is configurable via `asset_metadata_keys`
- a failed refresh keeps the existing cache; bookings are never rejected
  before the cache has loaded
- already rejected bookings are skipped and excluded from the missed sweep
- adds a spec

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 drivers/place/booking_approver.cr      | 111 ++++++++++--
 drivers/place/booking_approver_spec.cr | 229 +++++++++++++++++++++++++
 2 files changed, 328 insertions(+), 12 deletions(-)
 create mode 100644 drivers/place/booking_approver_spec.cr

diff --git a/drivers/place/booking_approver.cr b/drivers/place/booking_approver.cr
index 2943510cdd..403db72b91 100644
--- a/drivers/place/booking_approver.cr
+++ b/drivers/place/booking_approver.cr
@@ -4,13 +4,24 @@ require "./booking_model"
 class Place::BookingApprover < PlaceOS::Driver
   descriptive_name "Booking Auto Approver"
   generic_name :BookingApprover
-  description %(Automatically approves all PlaceOS bookings)
+  description %(Automatically approves all PlaceOS bookings, optionally rejecting bookings for assets not defined in zone metadata)
 
   accessor staff_api : StaffAPI_1
 
   default_settings({
     approve_booking_types: ["desk"],
     approve_zones:         ["zone-12345"],
+
+    # reject bookings for assets that are not defined in the building level zone metadata
+    reject_undefined_assets:    false,
+    bookable_assets_cache_cron: "0 0 * * *",
+    # time_zone: "Australia/Sydney",
+
+    # booking_type => zone metadata key
+    asset_metadata_keys: {
+      "desk"    => "desks",
+      "parking" => "parking-spaces",
+    },
   })
 
   def on_load
@@ -21,16 +32,40 @@ class Place::BookingApprover < PlaceOS::Driver
     on_update
   end
 
-  @bookings_approved : Int32 = 0u32
+  @bookings_approved : Int32 = 0
+  @bookings_rejected : Int32 = 0
   @approve_zones : Array(String) = [] of String
   @approve_booking_types : Array(String) = [] of String
 
+  @reject_undefined_assets : Bool = false
+  @asset_metadata_keys : Hash(String, String) = {} of String => String
+
+  # booking_type => bookable asset ids
+  @bookable_assets : Hash(String, Set(String)) = {} of String => Set(String)
+
   def on_update
     @approve_zones = setting?(Array(String), :approve_zones) || [] of String
     @approve_booking_types = setting?(Array(String), :approve_booking_types) || [] of String
 
+    @reject_undefined_assets = setting?(Bool, :reject_undefined_assets) || false
+    @asset_metadata_keys = setting?(Hash(String, String), :asset_metadata_keys) || {
+      "desk"    => "desks",
+      "parking" => "parking-spaces",
+    }
+
     schedule.clear
     schedule.every(10.minutes) { approve_missed }
+
+    if @reject_undefined_assets
+      cron = setting?(String, :bookable_assets_cache_cron).presence || "0 0 * * *"
+      time_zone = setting?(String, :time_zone).presence
+      location = time_zone ? Time::Location.load(time_zone) : Time::Location.local
+      schedule.cron(cron, location) { cache_bookable_assets }
+      schedule.in(1.second) { cache_bookable_assets }
+    else
+      @bookable_assets = {} of String => Set(String)
+      self[:bookable_assets] = nil
+    end
   end
 
   # Finds the building ID for the current location services object
@@ -42,6 +77,37 @@ class Place::BookingApprover < PlaceOS::Driver
     nil
   end
 
+  # Caches the asset ids defined in the metadata of the levels of every building
+  def cache_bookable_assets
+    building_ids = staff_api.zones(tags: "building").get.as_a.map(&.[]("id").as_s)
+    assets = {} of String => Set(String)
+
+    @approve_booking_types.each do |booking_type|
+      metadata_key = @asset_metadata_keys[booking_type]? || "#{booking_type}s"
+      ids = Set(String).new
+
+      building_ids.each do |building_id|
+        staff_api.metadata_children(building_id, metadata_key).get.as_a.each do |level|
+          next unless details = level.dig?("metadata", metadata_key, "details").try(&.as_a?)
+          details.each do |asset|
+            if asset_id = asset["id"]?.try(&.as_s?).presence
+              ids << asset_id
+            end
+          end
+        end
+      end
+
+      assets[booking_type] = ids
+    end
+
+    @bookable_assets = assets
+    self[:bookable_assets] = assets.transform_values(&.size)
+    "cached #{assets.sum(&.[1].size)} bookable assets"
+  rescue error
+    logger.warn(exception: error) { "failed to cache bookable assets, keeping existing cache" }
+    raise error
+  end
+
   private def approve_booking(booking : Booking)
     return if booking.action != "create"
 
@@ -59,17 +125,33 @@ class Place::BookingApprover < PlaceOS::Driver
       end
     end
 
-    if !booking.approved
-      # approving booking instances isn't a thing
-      staff_api.approve(booking.id).get
-      logger.debug { "Approved Booking #{booking.id}" }
-      @bookings_approved += 1
-      true
-    else
-      false
+    return false if booking.approved || booking.rejected
+
+    if @reject_undefined_assets
+      bookable = @bookable_assets[booking.booking_type]?
+      if bookable.nil?
+        # don't reject bookings because the cache failed to load, approve_missed will retry
+        logger.warn { "bookable assets for #{booking.booking_type} not cached, ignoring booking #{booking.id}" }
+        return false
+      end
+
+      asset_ids = booking.asset_ids.empty? ? [booking.asset_id] : booking.asset_ids
+      undefined = asset_ids.reject { |asset_id| bookable.includes?(asset_id) }
+      if !undefined.empty?
+        staff_api.reject(booking.id, "booking_approval_driver").get
+        logger.debug { "Rejected Booking #{booking.id} as assets #{undefined} are not defined in zone metadata" }
+        @bookings_rejected += 1
+        return false
+      end
     end
+
+    # approving booking instances isn't a thing
+    staff_api.approve(booking.id).get
+    logger.debug { "Approved Booking #{booking.id}" }
+    @bookings_approved += 1
+    true
   rescue error
-    logger.warn(exception: error) { "failed to approve booking #{booking.id}" }
+    logger.warn(exception: error) { "failed to process booking #{booking.id}" }
   end
 
   def approve_missed
@@ -79,6 +161,7 @@ class Place::BookingApprover < PlaceOS::Driver
       type: booking_type,
       zones: [get_building_id],
       approved: false,
+      rejected: false,
       period_end: 8.weeks.from_now.to_unix
     ).get_json
 
@@ -91,6 +174,10 @@ class Place::BookingApprover < PlaceOS::Driver
   end
 
   def status
-    {bookings_approved: @bookings_approved}
+    {
+      bookings_approved: @bookings_approved,
+      bookings_rejected: @bookings_rejected,
+      bookable_assets:   @bookable_assets.transform_values(&.size),
+    }
   end
 end
diff --git a/drivers/place/booking_approver_spec.cr b/drivers/place/booking_approver_spec.cr
new file mode 100644
index 0000000000..d3fea5ec9d
--- /dev/null
+++ b/drivers/place/booking_approver_spec.cr
@@ -0,0 +1,229 @@
+require "placeos-driver/spec"
+
+# publishes a booking created event, assigning the next booking id to `id_var`
+macro create_booking(id_var, asset_ids, booking_type, approved)
+  begin
+    {{id_var}} += 1
+    now = Time.utc.to_unix
+    publish("staff/booking/changed", {
+      action:          "create",
+      id:              {{id_var}},
+      booking_type:    {{booking_type}},
+      booking_start:   now + 3600,
+      booking_end:     now + 7200,
+      asset_id:        {{asset_ids}}.first,
+      asset_ids:       {{asset_ids}},
+      user_id:         "user-1",
+      user_email:      "user@org.com",
+      user_name:       "User One",
+      zones:           ["zone-building", "zone-level-1"],
+      booked_by_name:  "User One",
+      booked_by_email: "user@org.com",
+      approved:        {{approved}},
+      rejected:        false,
+    }.to_json)
+    sleep 200.milliseconds
+    {{id_var}}.to_i64
+  end
+end
+
+DriverSpecs.mock_driver "Place::BookingApprover" do
+  system({
+    StaffAPI: {StaffAPIMock},
+  })
+
+  settings({
+    approve_booking_types: ["desk"],
+    approve_zones:         ["zone-building"],
+  })
+
+  staff_api = system(:StaffAPI).as(StaffAPIMock)
+  booking_id = 0
+
+  # ==============
+  # mode disabled
+  # ==============
+  id = create_booking(booking_id, ["desk-undefined"], "desk", false)
+  staff_api.approved.should eq [id]
+  staff_api.rejected.should be_empty
+  exec(:status).get.not_nil!["bookable_assets"].should eq({} of String => Int32)
+
+  # ==============
+  # mode enabled
+  # ==============
+  staff_api.reset
+  settings({
+    approve_booking_types:   ["desk", "parking"],
+    approve_zones:           ["zone-building"],
+    reject_undefined_assets: true,
+  })
+  sleep 2.seconds
+
+  # assets from every level of every building are cached, keyed by booking type
+  exec(:status).get.not_nil!["bookable_assets"].should eq({"desk" => 3, "parking" => 1})
+  staff_api.metadata_requests.sort.should eq [
+    {"zone-building", "desks"},
+    {"zone-building", "parking-spaces"},
+    {"zone-building-2", "desks"},
+    {"zone-building-2", "parking-spaces"},
+  ]
+
+  defined_desk = create_booking(booking_id, ["desk-1"], "desk", false)
+  other_building_desk = create_booking(booking_id, ["desk-3"], "desk", false)
+  undefined_desk = create_booking(booking_id, ["desk-undefined"], "desk", false)
+  partially_defined = create_booking(booking_id, ["desk-1", "desk-undefined"], "desk", false)
+  defined_parking = create_booking(booking_id, ["park-1"], "parking", false)
+  desk_as_parking = create_booking(booking_id, ["desk-1"], "parking", false)
+  already_approved = create_booking(booking_id, ["desk-undefined"], "desk", true)
+  not_monitored = create_booking(booking_id, ["locker-undefined"], "locker", false)
+
+  staff_api.approved.should eq [defined_desk, other_building_desk, defined_parking]
+  staff_api.rejected.should eq [undefined_desk, partially_defined, desk_as_parking]
+  staff_api.reject_sources.uniq.should eq ["booking_approval_driver"]
+  staff_api.approved.should_not contain(already_approved)
+  staff_api.rejected.should_not contain(not_monitored)
+
+  exec(:status).get.not_nil!["bookings_rejected"].should eq 3
+
+  # ===========================================
+  # a failed refresh keeps the existing cache
+  # ===========================================
+  staff_api.reset
+  staff_api.fail_metadata(true)
+  expect_raises(Exception) { exec(:cache_bookable_assets).get }
+  exec(:status).get.not_nil!["bookable_assets"].should eq({"desk" => 3, "parking" => 1})
+
+  create_booking(booking_id, ["desk-2"], "desk", false)
+  create_booking(booking_id, ["desk-undefined"], "desk", false)
+  staff_api.approved.size.should eq 1
+  staff_api.rejected.size.should eq 1
+
+  # ==================================================================
+  # bookings are never rejected when the cache has not loaded yet
+  # ==================================================================
+  staff_api.reset
+  settings({
+    approve_booking_types:   ["desk"],
+    approve_zones:           ["zone-building"],
+    reject_undefined_assets: false,
+  })
+  sleep 200.milliseconds
+  staff_api.fail_metadata(true)
+  settings({
+    approve_booking_types:   ["desk"],
+    approve_zones:           ["zone-building"],
+    reject_undefined_assets: true,
+  })
+  sleep 2.seconds
+  exec(:status).get.not_nil!["bookable_assets"].should eq({} of String => Int32)
+
+  create_booking(booking_id, ["desk-undefined"], "desk", false)
+  create_booking(booking_id, ["desk-1"], "desk", false)
+  staff_api.approved.should be_empty
+  staff_api.rejected.should be_empty
+
+  # the next refresh succeeds and bookings are processed again
+  staff_api.fail_metadata(false)
+  exec(:cache_bookable_assets).get
+  exec(:status).get.not_nil!["bookable_assets"].should eq({"desk" => 3})
+
+  # ==========================================
+  # empty metadata rejects all bookings of type
+  # ==========================================
+  staff_api.reset
+  settings({
+    approve_booking_types:   ["desk"],
+    approve_zones:           ["zone-building"],
+    reject_undefined_assets: true,
+    asset_metadata_keys:     {"desk" => "missing-key"},
+  })
+  sleep 2.seconds
+  exec(:status).get.not_nil!["bookable_assets"].should eq({"desk" => 0})
+  id = create_booking(booking_id, ["desk-1"], "desk", false)
+  staff_api.rejected.should eq [id]
+end
+
+# :nodoc:
+class StaffAPIMock < DriverSpecs::MockDriver
+  LEVEL_METADATA = JSON.parse(%({
+    "zone-building": [
+      {
+        "zone": {"id": "zone-level-1"},
+        "metadata": {
+          "desks": {
+            "name": "desks",
+            "details": [{"id": "desk-1", "name": "Desk 1"}, {"id": "desk-2", "name": "Desk 2"}]
+          },
+          "parking-spaces": {
+            "name": "parking-spaces",
+            "details": [{"id": "park-1", "name": "Park 1"}]
+          }
+        }
+      },
+      {"zone": {"id": "zone-level-2"}, "metadata": {}}
+    ],
+    "zone-building-2": [
+      {
+        "zone": {"id": "zone-level-3"},
+        "metadata": {
+          "desks": {
+            "name": "desks",
+            "details": [{"id": "desk-3", "name": "Desk 3"}, {"name": "Desk missing ID"}]
+          }
+        }
+      }
+    ]
+  }))
+
+  getter approved = [] of Int64
+  getter rejected = [] of Int64
+  getter reject_sources = [] of String?
+  getter metadata_requests = [] of Tuple(String, String)
+  @fail_metadata : Bool = false
+
+  def fail_metadata(state : Bool)
+    @fail_metadata = state
+  end
+
+  def reset
+    @approved.clear
+    @rejected.clear
+    @reject_sources.clear
+    @metadata_requests.clear
+    @fail_metadata = false
+  end
+
+  def zones(tags : String)
+    [{id: "zone-building"}, {id: "zone-building-2"}]
+  end
+
+  def metadata_children(id : String, key : String? = nil)
+    raise "metadata unavailable" if @fail_metadata
+    @metadata_requests << {id, key.to_s}
+    LEVEL_METADATA[id].as_a.map do |level|
+      metadata = level["metadata"].as_h.select { |name, _| name == key }
+      {zone: level["zone"], metadata: metadata}
+    end
+  end
+
+  def approve(booking_id : Int64, instance : Int64? = nil)
+    @approved << booking_id
+    true
+  end
+
+  def reject(booking_id : Int64, utm_source : String? = nil, instance : Int64? = nil)
+    @rejected << booking_id
+    @reject_sources << utm_source
+    true
+  end
+
+  def query_bookings(
+    type : String? = nil,
+    zones : Array(String?) = [] of String?,
+    approved : Bool? = nil,
+    rejected : Bool? = nil,
+    period_end : Int64? = nil,
+  )
+    [] of Nil
+  end
+end