WPB-26782: keep recurring meetings alive past original lot - #5309

blackheaven · 2026-07-02T18:02:14Z

https://wearezeta.atlassian.net/browse/WPB-26782

Checklist

Add a new entry in an appropriate subdirectory of changelog.d
Read and follow the PR guidelines

Copilot

Pull request overview

This PR updates meeting expiry/cleanup semantics so recurring meetings remain accessible after their original time slot has passed, aligning API visibility and background cleanup behavior with the recurrence window (recurrence.until), including “never expire” behavior for open-ended recurrences.

Changes:

Introduces effectiveEndTime / isAlive to treat bounded recurrences as alive until max(endTime, recurrence.until) and open-ended recurrences as non-expiring.
Updates Postgres queries and in-memory test interpreter filtering/cleanup to use the effective end time instead of endTime.
Adds unit + integration coverage and documents the behavior change in the changelog.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
libs/wire-subsystems/test/unit/Wire/MockInterpreters/MeetingsStore.hs	Updates mock store list/cleanup filtering to use `effectiveEndTime`.
libs/wire-subsystems/test/unit/Wire/MeetingsSubsystem/InterpreterSpec.hs	Adds unit tests covering recurrence-vs-expiry behavior across core operations and cleanup.
libs/wire-subsystems/src/Wire/MeetingsSubsystem/Interpreter.hs	Centralizes expiry checks via `isAlive` (based on `Store.effectiveEndTime`) for multiple operations.
libs/wire-subsystems/src/Wire/MeetingsStore/Postgres.hs	Updates list and cleanup SQL to apply “effective end time” semantics for recurring meetings.
libs/wire-subsystems/src/Wire/MeetingsStore.hs	Adds `effectiveEndTime` helper defining the new expiry/cleanup rules.
integration/test/Test/Meetings.hs	Adds integration test ensuring recurring meetings remain accessible/listed past validity period.
changelog.d/3-bug-fixes/WPB-26782	Documents the behavioral fix for recurring meeting expiry and cleanup.

battermann

my findings are only nit-picks

but please consider the copilot comment on the index, it might be relevant

battermann · 2026-07-03T09:25:37Z

+  resp <- getMeetingsList owner
+  assertSuccess resp
+  meetings <- resp.json & asList
+  length (meetings :: [Value]) `shouldMatchInt` 1


type annotation not needed here, I think

battermann · 2026-07-03T09:27:56Z

+    let now = UTCTime (fromGregorian 2026 1 1) 0
+        gen = mkStdGen 42
+        uid = Id $ read "00000000-0000-0000-0000-000000000001"
+        zUser = toLocalUnsafe (Domain "wire.com") uid


I like to make this a property test and have this generated as input. But no need to change, this is just a comment.

- getOldMeetingsImpl: replace eff_end subselect with a sargable predicate so the non-recurring branch can use idx_meetings_end_time, avoiding a full-table sequential scan + sort in the cleanup worker. - integration (Meetings): fix an inaccurate "endTime = now: past" comment (it only becomes past after the threadDelay) and drop a redundant type annotation. - add a property test asserting meeting aliveness follows effectiveEndTime across getMeeting/listMeetings/cleanupOldMeetings.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

battermann · 2026-07-06T06:59:30Z

+            AND (
+              (m.recurrence_frequency IS NULL AND m.end_time >= ($2 :: timestamptz))
+              OR (m.recurrence_frequency IS NOT NULL AND m.recurrence_until IS NULL)
+              OR (m.recurrence_frequency IS NOT NULL AND m.recurrence_until IS NOT NULL
+                  AND GREATEST(m.end_time, m.recurrence_until) >= ($2 :: timestamptz))
+            )


Maybe the chance is low that we will have such malformed rows in the DB but I would still protect against it, because it's cheap.

battermann · 2026-07-06T07:00:09Z

+            AND (
+              (m.recurrence_frequency IS NULL AND m.end_time >= ($2 :: timestamptz))
+              OR (m.recurrence_frequency IS NOT NULL AND m.recurrence_until IS NULL)
+              OR (m.recurrence_frequency IS NOT NULL AND m.recurrence_until IS NOT NULL
+                  AND GREATEST(m.end_time, m.recurrence_until) >= ($2 :: timestamptz))
+            )


Same as above

also changed

battermann · 2026-07-06T07:00:53Z

+           WHERE (recurrence_frequency IS NULL AND end_time < ($1 :: timestamptz))
+              OR (recurrence_frequency IS NOT NULL AND recurrence_until IS NOT NULL
+                  AND GREATEST(end_time, recurrence_until) < ($1 :: timestamptz))


battermann · 2026-07-06T07:01:52Z

+    it "updateMeeting succeeds on a recurring meeting whose slot passed" $ do
+      result <-
+        runTestStack now gen Map.empty teamConfig $ do
+          (meeting, _conv) <- createMeeting zUser (expiredNewMeeting boundedRecurrence)
+          updateMeeting zUser meeting.id (API.UpdateMeeting Nothing Nothing (Just (unsafeRange "Updated")) Nothing)
+      fmap isJust result `shouldBe` Right True
+
+    it "addInvitedEmails succeeds on a recurring meeting whose slot passed" $ do
+      result <-
+        runTestStack now gen Map.empty teamConfig $ do
+          (meeting, _conv) <- createMeeting zUser (expiredNewMeeting boundedRecurrence)
+          addInvitedEmails zUser meeting.id [unsafeEmailAddress "user" "example.com"]
+      result `shouldBe` Right True
+


Maybe worthwhile, you decide.

battermann

Copilot's comments seem reasonable. I would suggest to fix them.

- Replace derived-table eff_end subquery in listMeetingsByUserImpl and listMeetingsByConversationImpl with explicit OR-branches over the base meetings table, keeping the non-recurring path index-friendly. - Add recurrence_interval IS NOT NULL guard to the recurring branches of listMeetingsByUserImpl, listMeetingsByConversationImpl and getOldMeetingsImpl so list/cleanup endpoints do not crash on rows with recurrence_frequency set but recurrence_interval NULL (which fail to unmarshal). - Add unit tests covering deleteMeeting, removeInvitedEmails and replaceInvitedEmails on a recurring meeting whose slot has passed.

battermann

LGTM

Add two partial indexes and rewrite the meetings cleanup query so the background worker stays fully index-driven as the meetings table grows: - idx_meetings_recurrence_eff_end on GREATEST(end_time, recurrence_until) for bounded recurring meetings (recurring branches of list/cleanup queries). - idx_meetings_end_time_nonrecurring on end_time for non-recurring meetings, so cleanup skips not-yet-expired recurring rows whose original slot is long past. Rewrite getOldMeetingsImpl to issue one bounded, index-backed query per meeting kind (non-recurring vs bounded recurring) and merge the batches in Haskell, mirroring the in-memory store. (hasql-th rejects UNION ALL in a FROM derived table, so the two statements are issued separately and merged.) Both indexes are built CONCURRENTLY and registered in nonTransactionMigrations. EXPLAIN ANALYZE on an adversarial data set (75k not-yet-expired recurring rows with the oldest end_time, few expired rows) drops from 1661 buffers / 11.6ms / 75086 rows filtered to 9 buffers / 0.2ms / 0 filtered. Follow-up to #5309 (discussion r3518905163).

blackheaven marked this pull request as ready for review July 2, 2026 18:02

blackheaven requested review from a team as code owners July 2, 2026 18:02

WPB-26782: keep recurring meetings alive past original slot

242faad

blackheaven force-pushed the gdifolco/WPB-26782-meeting-fix-end branch from 027131c to 242faad Compare July 2, 2026 18:03

zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jul 2, 2026

battermann requested a review from Copilot July 3, 2026 09:23

Copilot started reviewing on behalf of battermann July 3, 2026 09:24 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

Comment thread libs/wire-subsystems/src/Wire/MeetingsStore/Postgres.hs Outdated

Comment thread integration/test/Test/Meetings.hs Outdated

battermann requested changes Jul 3, 2026

View reviewed changes

blackheaven requested a review from battermann July 3, 2026 10:32

battermann requested a review from Copilot July 3, 2026 11:05

Copilot started reviewing on behalf of battermann July 3, 2026 11:05 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

Comment thread libs/wire-subsystems/src/Wire/MeetingsStore/Postgres.hs Outdated

Comment thread libs/wire-subsystems/src/Wire/MeetingsStore/Postgres.hs Outdated

refactor(copilot): SQL query performance

0d4377d

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

battermann requested a review from Copilot July 6, 2026 06:35

Copilot started reviewing on behalf of battermann July 6, 2026 06:36 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

battermann reviewed Jul 6, 2026

View reviewed changes

blackheaven requested a review from battermann July 6, 2026 07:39

battermann approved these changes Jul 6, 2026

View reviewed changes

blackheaven merged commit 805bd04 into develop Jul 6, 2026
10 checks passed

blackheaven deleted the gdifolco/WPB-26782-meeting-fix-end branch July 6, 2026 09:35

zebot mentioned this pull request Jul 7, 2026

Release 2026-07-07 - (expected chart version 5.34.0) #5321

Merged

Uh oh!

Conversation

blackheaven commented Jul 2, 2026

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

battermann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

battermann left a comment

Choose a reason for hiding this comment

Uh oh!

battermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants