Skip to content

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

Open
blackheaven wants to merge 3 commits into
developfrom
gdifolco/WPB-26782-meeting-fix-end
Open

WPB-26782: keep recurring meetings alive past original lot#5309
blackheaven wants to merge 3 commits into
developfrom
gdifolco/WPB-26782-meeting-fix-end

Conversation

@blackheaven

Copy link
Copy Markdown
Contributor

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

@blackheaven blackheaven marked this pull request as ready for review July 2, 2026 18:02
@blackheaven blackheaven requested review from a team as code owners July 2, 2026 18:02
@blackheaven blackheaven force-pushed the gdifolco/WPB-26782-meeting-fix-end branch from 027131c to 242faad Compare July 2, 2026 18:03
@zebot 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 battermann requested a review from Copilot July 3, 2026 09:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread libs/wire-subsystems/src/Wire/MeetingsStore/Postgres.hs Outdated
Comment thread integration/test/Test/Meetings.hs Outdated

@battermann battermann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my findings are only nit-picks

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

Comment thread integration/test/Test/Meetings.hs Outdated
resp <- getMeetingsList owner
assertSuccess resp
meetings <- resp.json & asList
length (meetings :: [Value]) `shouldMatchInt` 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type annotation not needed here, I think

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines +911 to +914
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

- 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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread libs/wire-subsystems/src/Wire/MeetingsStore/Postgres.hs Outdated
Comment thread libs/wire-subsystems/src/Wire/MeetingsStore/Postgres.hs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants