Skip to content

feat(booking): [PPT-2241] add ignore_assets parameter to clash checking #299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chillfox merged 2 commits into from
Nov 4, 2025
Merged

feat(booking): [PPT-2241] add ignore_assets parameter to clash checking #299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da7aa64
test(booking_spec): [PPT-2241] add clashing_bookings test with ignore…
chillfox Nov 4, 2025
32bc601
feat(booking): [PPT-2241] add ignore_assets parameter to clash checki…
chillfox Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions spec/booking_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,70 @@ module PlaceOS::Model
saved.should eq 1
end

it "tests clashing_bookings with ignore_assets parameter" do
user_email = "[email protected]"
tenant_id = Generator.tenant.id

# Create first booking with desk1
booking1 = Booking.new(
booking_type: "desk",
asset_ids: ["desk1"],
booking_start: 1.hour.from_now.to_unix,
booking_end: 2.hours.from_now.to_unix,
user_email: PlaceOS::Model::Email.new(user_email),
user_name: "Steve",
booked_by_email: PlaceOS::Model::Email.new(user_email),
booked_by_name: "Steve",
tenant_id: tenant_id,
booked_by_id: "user-1234",
history: [] of Booking::History
).save!

# Create second booking with same asset and overlapping time - should clash
booking2 = Booking.new(
booking_type: "desk",
asset_ids: ["desk1"],
booking_start: 1.5.hours.from_now.to_unix,
booking_end: 2.5.hours.from_now.to_unix,
user_email: PlaceOS::Model::Email.new(user_email),
user_name: "Steve",
booked_by_email: PlaceOS::Model::Email.new(user_email),
booked_by_name: "Steve",
tenant_id: tenant_id,
booked_by_id: "user-1234",
history: [] of Booking::History
)

# Should find clash with same asset
clashes = booking2.clashing_bookings
clashes.size.should eq 1
clashes.first.id.should eq booking1.id

# Create third booking with different asset but overlapping time
booking3 = Booking.new(
booking_type: "desk",
asset_ids: ["desk2"],
booking_start: 1.5.hours.from_now.to_unix,
booking_end: 2.5.hours.from_now.to_unix,
user_email: PlaceOS::Model::Email.new(user_email),
user_name: "Steve",
booked_by_email: PlaceOS::Model::Email.new(user_email),
booked_by_name: "Steve",
tenant_id: tenant_id,
booked_by_id: "user-1234",
history: [] of Booking::History
)

# Should not find clash with different asset (default behavior)
clashes_without_ignore = booking3.clashing_bookings
clashes_without_ignore.size.should eq 0

# Should find clash when ignoring assets
clashes_with_ignore = booking3.clashing_bookings(ignore_assets: true)
clashes_with_ignore.size.should eq 1
clashes_with_ignore.first.id.should eq booking1.id
end

it "rejects a booking with multiple same asset ids" do
user_email = "[email protected]"
tenant_id = Generator.tenant.id
Expand Down
22 changes: 13 additions & 9 deletions src/placeos-models/booking.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ module PlaceOS::Model
clashing_bookings.size > 0
end

protected def recurring_clash_check : Array(Booking)
protected def recurring_clash_check(ignore_assets : Bool = false) : Array(Booking)
# we need to check for clashes against each recurrence
starting = self.booking_start
ending = self.booking_end
Expand Down Expand Up @@ -538,6 +538,7 @@ module PlaceOS::Model
rec_ending = max_period.from_now.to_unix
end

asset_check_sql = ignore_assets ? "" : "AND b.asset_ids && #{Associations.format_list_for_postgres(asset_ids)}"
overrides = Booking.find_all_by_sql(<<-SQL, tenant_id, rec_ending, starting, end_time, start_time, booking_type)
SELECT b.* FROM "bookings" b
JOIN booking_instances i ON b.id = i.id
Expand All @@ -548,7 +549,7 @@ module PlaceOS::Model
AND i.ending_time > $5
AND i.checked_out_at IS NULL
AND b.booking_type = $6
AND b.asset_ids && #{Associations.format_list_for_postgres(asset_ids)}
#{asset_check_sql}
AND b.rejected <> TRUE
AND i.deleted <> TRUE
SQL
Expand All @@ -561,10 +562,11 @@ module PlaceOS::Model
expanded = Booking.expand_bookings!(starting_tz, rec_ending_tz, [self]).bookings

# starting - booking_length ensures we capture overlaps
asset_check_where = ignore_assets ? "" : "AND asset_ids && #{Associations.format_list_for_postgres(asset_ids)} "
query = Booking
.by_tenant(tenant_id)
.where(
"(((recurrence_end > ? OR recurrence_end IS NULL) AND recurrence_type <> 'NONE') OR (booking_end > ? AND booking_start < ?)) AND checked_out_at IS NULL AND starting_time < ? AND ending_time > ? AND booking_type = ? AND asset_ids && #{Associations.format_list_for_postgres(asset_ids)} AND rejected <> TRUE AND deleted <> TRUE",
"(((recurrence_end > ? OR recurrence_end IS NULL) AND recurrence_type <> 'NONE') OR (booking_end > ? AND booking_start < ?)) AND checked_out_at IS NULL AND starting_time < ? AND ending_time > ? AND booking_type = ? #{asset_check_where}AND rejected <> TRUE AND deleted <> TRUE",
starting, starting, rec_ending, end_time, start_time, booking_type
)
query = query.where("id != ?", id) unless id.nil?
Expand All @@ -573,10 +575,11 @@ module PlaceOS::Model
end
end

protected def regular_clash_check : Array(Booking)
protected def regular_clash_check(ignore_assets : Bool = false) : Array(Booking)
starting = self.booking_start
ending = self.booking_end

asset_check_sql = ignore_assets ? "" : "AND b.asset_ids && #{Associations.format_list_for_postgres(asset_ids)}"
clashing = BookingInstance.find_one_by_sql?(<<-SQL, tenant_id, ending, starting, booking_type)
SELECT i.* FROM "booking_instances" i
JOIN bookings b ON i.id = b.id
Expand All @@ -585,25 +588,26 @@ module PlaceOS::Model
AND i.booking_end > $3
AND i.checked_out_at IS NULL
AND b.booking_type = $4
AND b.asset_ids && #{Associations.format_list_for_postgres(asset_ids)}
#{asset_check_sql}
AND b.rejected <> TRUE
AND i.deleted <> TRUE
LIMIT 1
SQL
return [clashing.hydrate_booking] if clashing

# find any valid recurring bookings in the time period
asset_check_where = ignore_assets ? "" : "AND asset_ids && #{Associations.format_list_for_postgres(asset_ids)} "
query = Booking
.by_tenant(tenant_id)
.where(
"(((recurrence_end > ? OR recurrence_end IS NULL) AND recurrence_type <> 'NONE' AND booking_start < ?) OR (booking_start < ? AND booking_end > ?)) AND checked_out_at IS NULL AND booking_type = ? AND asset_ids && #{Associations.format_list_for_postgres(asset_ids)} AND rejected <> TRUE AND deleted <> TRUE",
"(((recurrence_end > ? OR recurrence_end IS NULL) AND recurrence_type <> 'NONE' AND booking_start < ?) OR (booking_start < ? AND booking_end > ?)) AND checked_out_at IS NULL AND booking_type = ? #{asset_check_where}AND rejected <> TRUE AND deleted <> TRUE",
starting, ending, ending, starting, booking_type
)
query = query.where("id != ?", id) unless id.nil?
Booking.expand_bookings!(starting_tz, ending_tz, query.to_a).bookings
end

def clashing_bookings : Array(Booking)
def clashing_bookings(ignore_assets : Bool = false) : Array(Booking)
return [] of Booking if self.booking_type.downcase == "visitor"

update_assets
Expand All @@ -614,9 +618,9 @@ module PlaceOS::Model

# find any overrides that might clash with the bookings
candidates = if recurring_booking?
recurring_clash_check
recurring_clash_check(ignore_assets)
else
regular_clash_check
regular_clash_check(ignore_assets)
end

# we need to do this as booking instances may only set checked out / deleted flags
Expand Down
Loading