fix(bookings): PPT-2436 list a cancelled series when include_deleted is set

shard.lock

@@ -151,7 +151,7 @@ shards:
 
   placeos-models:
     git: https://github.com/placeos/models.git
-    version: 9.117.0
+    version: 9.117.1
 
   promise:
     git: https://github.com/spider-gazelle/promise.git

spec/controllers/recurring_bookings_spec.cr

@@ -125,7 +125,11 @@ describe Bookings do
         route = "#{BOOKINGS_BASE}/?period_start=#{starting}&period_end=#{ending}&type=desk&zones=#{zone}&include_deleted=true&limit=2"
         seen = follow_booking_pages(client, route, headers)
 
-        expected = deleted.map { |booking| {booking.id.not_nil!, nil.as(Int64?)} }
+        # each cancelled series comes back as its cancelled occurrences
+        expected = deleted.flat_map do |booking|
+          booking.calculate_daily(Time.unix(starting), Time.unix(ending)).instances.map { |time| {booking.id.not_nil!, time.to_unix.as(Int64?)} }
+        end
+        expected.size.should eq 15
         seen.size.should eq expected.size
         seen.to_set.should eq expected.to_set
       end
@@ -148,7 +152,7 @@ describe Bookings do
         seen.to_set.should eq expected.to_set
       end
 
-      it "should return every instance once when active and deleted recurring bookings share a page" do
+      it "should return every instance once when active, deleted and rejected recurring bookings share a page" do
         tenant_id = get_tenant.id.not_nil!
 
         # the active booking is created first so it sorts before the deleted one on `created`
@@ -159,20 +163,28 @@ describe Bookings do
         deleted.deleted_at = Time.utc.to_unix
         deleted.save!
 
+        # a rejected series is still returned as a single row, so the pages mix
+        # unexpanded rows with expanded ones
+        rejected = make_recurring.call(tenant_id)
+        rejected.rejected = true
+        rejected.rejected_at = Time.utc.to_unix
+        rejected.save!
+
         route = "#{BOOKINGS_BASE}/?period_start=#{starting}&period_end=#{ending}&type=desk&zones=#{zone}&include_deleted=true&limit=2"
         seen = follow_booking_pages(client, route, headers)
 
         instances = active.calculate_daily(Time.unix(starting), Time.unix(ending)).instances
         instances.size.should eq 5
 
         expected = instances.map { |time| {active.id.not_nil!, time.to_unix.as(Int64?)} }
-        expected << {deleted.id.not_nil!, nil.as(Int64?)}
+        expected.concat instances.map { |time| {deleted.id.not_nil!, time.to_unix.as(Int64?)} }
+        expected << {rejected.id.not_nil!, nil.as(Int64?)}
         seen.size.should eq expected.size
         seen.to_set.should eq expected.to_set
       end
     end
 
-    describe "listing a cancelled occurrence of a recurring booking" do
+    describe "listing cancelled occurrences of a recurring booking" do
       zone = "zone-recurring-cancelled"
       day_start = Time.local.at_beginning_of_day
       starting = day_start.to_unix
@@ -222,6 +234,36 @@ describe Bookings do
 
         bookings.map(&.["instance"].as_i64).sort!.should eq(instances - [cancelled])
       end
+
+      it "should return every occurrence of a cancelled series, marked deleted, when include_deleted is set" do
+        booking = BookingsHelper.create_booking(get_tenant.id.not_nil!)
+        booking.zones = [zone]
+        booking.recurrence_type = :daily
+        booking.recurrence_days = 0b1111111
+        booking.timezone = "Europe/Berlin"
+        booking.booking_start = (day_start + 4.hours).to_unix
+        booking.booking_end = (day_start + 6.hours).to_unix
+        booking.save!
+        # a period after the first occurrence, so the series is only found as a recurrence
+        later = (day_start + 1.day).to_unix
+        instances = booking.calculate_daily(Time.unix(later), Time.unix(ending)).instances.map(&.to_unix)
+        instances.size.should eq 4
+
+        client.delete("#{BOOKINGS_BASE}/#{booking.id}", headers: headers).success?.should be_true
+
+        route = "#{BOOKINGS_BASE}/?period_start=#{later}&period_end=#{ending}&type=desk&zones=#{zone}&include_deleted=true"
+        result = client.get(route, headers: headers)
+        result.success?.should be_true
+        bookings = JSON.parse(result.body).as_a
+
+        bookings.map(&.["instance"].as_i64).sort!.should eq instances
+        bookings.all? { |item| item["id"].as_i64 == booking.id && item["deleted"].as_bool }.should be_true
+
+        route = "#{BOOKINGS_BASE}/?period_start=#{later}&period_end=#{ending}&type=desk&zones=#{zone}"
+        result = client.get(route, headers: headers)
+        result.success?.should be_true
+        JSON.parse(result.body).as_a.should be_empty
+      end
     end
 
     it "should delete a booking instance" do

src/controllers/bookings.cr

@@ -211,8 +211,9 @@ class Bookings < Application
     if event_ids.empty?
       raise AC::Route::Param::MissingError.new("missing required parameter 'type'", "type", "String") unless booking_type.presence
 
+      series_not_deleted = include_deleted ? "" : " AND deleted_at IS NULL"
       query = query.where(
-        %{(((recurrence_end > ? OR recurrence_end IS NULL) AND recurrence_type <> 'NONE' AND "booking_start" < ? AND rejected_at IS NULL AND deleted_at IS NULL) OR ("booking_start" < ? AND "booking_end" > ?))},
+        %{(((recurrence_end > ? OR recurrence_end IS NULL) AND recurrence_type <> 'NONE' AND "booking_start" < ? AND rejected_at IS NULL#{series_not_deleted}) OR ("booking_start" < ? AND "booking_end" > ?))},
         starting, ending, ending, starting
       )
 
@@ -262,16 +263,18 @@ class Bookings < Application
     total = query.count
     query = query.join(:left, Attendee, :booking_id).join(:left, Guest, "guests.id = attendees.guest_id") if auth_token_present?
 
-    # rows that are returned as-is (standard bookings and deleted / rejected recurring
-    # parents, see `Booking#recurring_booking?`) must sort before the recurring bookings
-    # that get expanded into instances: the next page offset is `offset + rows consumed`
-    # which is only correct when the consumed rows are a prefix of this page
-    query = query.order("(bookings.recurrence_type <> 'NONE' AND NOT bookings.deleted AND NOT bookings.rejected)", :created)
+    # rows that are returned as-is (standard bookings and rejected recurring parents,
+    # plus deleted ones unless `include_deleted`, see `Booking#recurring_booking?`) must
+    # sort before the recurring bookings that get expanded into instances: the next page
+    # offset is `offset + rows consumed` which is only correct when the consumed rows are
+    # a prefix of this page. This expression must match `recurring_booking?(include_deleted)`.
+    expanded_series = include_deleted ? "bookings.recurrence_type <> 'NONE' AND NOT bookings.rejected" : "bookings.recurrence_type <> 'NONE' AND NOT bookings.deleted AND NOT bookings.rejected"
+    query = query.order("(#{expanded_series})", :created)
       .offset(offset)
       .limit(limit)
 
     result = query.to_a
-    num_unexpanded = result.count { |booking| !booking.recurring_booking? }
+    num_unexpanded = result.count { |booking| !booking.recurring_booking?(include_deleted) }
 
     result = Booking.hydrate_parents(result) if include_parent_bookings && !result.empty?