amadan
range query: an override volunteers its finished series, and UNTIL is measured to the occurrence's end
Two calendar bugs the booking page's busy set would have inherited (booking spec §4.1). An override dragged past the end of its series vanished: the master was excluded by until_utc, so nothing named the series, so the override was skipped in pass 1 and never loaded for pass 2. Override rows in the window now contribute their series id to the set overridesFor loads. A master was excluded when until_utc < from, but UNTIL bounds the last occurrence's START. A final occurrence four days long was gone from the range on its second day. The exclusion now adds the master's own duration. Both pinned in occurrence_test.go through the real router.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 files changed,
+82
−3
internal/calendar/store_range.go+26 −3internal/calendartest/occurrence_test.go+56 −0
diff --git a/internal/calendar/store_range.go b/internal/calendar/store_range.go| index a94bac8..ba330bf 100644 |
| --- a/internal/calendar/store_range.go |
| +++ b/internal/calendar/store_range.go |
| @@ -43,10 +43,27 @@ func (s *Store) EventsInRange(ctx context.Context, personID, from, to int64) ([] |
| // Overrides for every series touched, keyed by series and original |
| // occurrence. Fetched in one query rather than per occurrence: a |
| // query inside an expansion loop is a query per day per series. |
| + // |
| + // An override row in the window volunteers its own series too. Its |
| + // master may have FINISHED before the window — excluded by |
| + // until_utc — in which case nothing else names that series, and |
| + // without this the occurrence dragged past the series' end would be |
| + // skipped in pass 1 and never loaded for pass 2: a moved meeting |
| + // that vanishes. |
| var masterIDs []int64 |
| + seen := map[int64]bool{} |
| for _, r := range rows { |
| - if r.RRule != "" && r.SeriesID == nil { |
| - masterIDs = append(masterIDs, r.ID) |
| + switch { |
| + case r.RRule != "" && r.SeriesID == nil: |
| + if !seen[r.ID] { |
| + seen[r.ID] = true |
| + masterIDs = append(masterIDs, r.ID) |
| + } |
| + case r.SeriesID != nil: |
| + if !seen[*r.SeriesID] { |
| + seen[*r.SeriesID] = true |
| + masterIDs = append(masterIDs, *r.SeriesID) |
| + } |
| } |
| } |
| overrides, err := s.overridesFor(ctx, personID, masterIDs) |
| @@ -155,7 +172,13 @@ func (s *Store) rangeRows(ctx context.Context, personID, from, to int64) ([]even |
| -- series and for one bounded by COUNT, whose end cannot be |
| -- known without expanding it — so those are always |
| -- candidates, and the expander bounds them. |
| - OR (events.rrule <> '' AND events.start_utc < ? AND (events.until_utc IS NULL OR events.until_utc >= ?)) |
| + -- |
| + -- UNTIL bounds the last occurrence's START, not its end, so |
| + -- "finished" is until_utc plus one occurrence's length: a |
| + -- final occurrence that runs for four days is still in |
| + -- progress two days after the last start the rule allows. |
| + OR (events.rrule <> '' AND events.start_utc < ? |
| + AND (events.until_utc IS NULL OR events.until_utc + (events.end_utc - events.start_utc) >= ?)) |
| `, from, to, to, from). |
| Scan(&rows).Error |
| if err != nil { |
diff --git a/internal/calendartest/occurrence_test.go b/internal/calendartest/occurrence_test.go| index ee61a0f..eefd71f 100644 |
| --- a/internal/calendartest/occurrence_test.go |
| +++ b/internal/calendartest/occurrence_test.go |
| @@ -207,3 +207,59 @@ func calendarIDOf(t *testing.T, h http.Handler, session *http.Cookie) string { |
| t.Fatal("no selected calendar on the create form") |
| return "" |
| } |
| + |
| +// An override of a series that has already FINISHED is the one pass 2 |
| +// cannot reach by itself: the master is excluded by until_utc, so the |
| +// override's series is never loaded and the moved occurrence vanishes. |
| +// The override row itself is in the range rows — it must volunteer its |
| +// series (booking spec §4.1, r2 B3). |
| +func TestAMovedOccurrenceOfAFinishedSeriesStillShows(t *testing.T) { |
| + app, session := signedIn(t, "someone@example.com") |
| + h := app.Handler() |
| + |
| + loc := createEvent(t, h, session, url.Values{ |
| + "title": {"Sprint review"}, "start": {testAt(1, "09:30")}, "end": {testAt(1, "09:45")}, |
| + "kind": {"meeting"}, "status": {"confirmed"}, "repeat": {"daily"}, "repeat_end": {testDay(3)}, |
| + }) |
| + id := strings.TrimPrefix(loc, "/events/") |
| + |
| + // Move the LAST occurrence ten days past the series' end. |
| + rec := postAs(t, h, session, "/events/"+id, url.Values{ |
| + "scope": {"this"}, "occurrence": {testAt(3, "09:30")}, |
| + "title": {"Sprint review"}, "calendar_id": {calendarIDOf(t, h, session)}, |
| + "start": {testAt(13, "14:00")}, "end": {testAt(13, "14:15")}, |
| + "kind": {"meeting"}, "status": {"confirmed"}, "repeat": {"none"}, |
| + }) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("moving the last occurrence: status %d\n%s", rec.Code, rec.Body.String()) |
| + } |
| + |
| + moved := getAs(t, h, session, "/day/"+testDay(13)).Body.String() |
| + if !hasChipAt(moved, "14:00") { |
| + t.Errorf("the moved occurrence of a finished series does not show on its new day") |
| + } |
| + if n := strings.Count(moved, "Sprint review"); n != 1 { |
| + t.Errorf("the moved day shows the event %d times, want exactly 1", n) |
| + } |
| + if orig := getAs(t, h, session, "/day/"+testDay(3)).Body.String(); hasChipAt(orig, "09:30") { |
| + t.Error("the moved occurrence still appears at its ORIGINAL time") |
| + } |
| +} |
| + |
| +// UNTIL bounds the last occurrence's START, not its end. A master whose |
| +// final occurrence runs for four days must still be a candidate for a |
| +// window that opens two days after that start (booking spec §4.1, r3 f7). |
| +func TestALongFinalOccurrenceShowsOnItsLaterDays(t *testing.T) { |
| + app, session := signedIn(t, "someone@example.com") |
| + h := app.Handler() |
| + |
| + createEvent(t, h, session, url.Values{ |
| + "title": {"Offsite"}, "start": {testAt(1, "09:00")}, "end": {testAt(5, "09:00")}, |
| + "kind": {"commitment"}, "status": {"confirmed"}, "repeat": {"weekly"}, "repeat_end": {testDay(1)}, |
| + }) |
| + |
| + if body := getAs(t, h, session, "/day/"+testDay(3)).Body.String(); !strings.Contains(body, "Offsite") { |
| + t.Error("the third day of a four-day final occurrence shows nothing — " + |
| + "the master was excluded on until_utc alone, without its duration") |
| + } |
| +} |