WPB-27553: add tzid to meetings by blackheaven · Pull Request #5391 · wireapp/wire-server

blackheaven · 2026-07-29T17:09:24Z

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

Checklist

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

@v16

…e) shapes V17 meetings carry `duration` (serialized as whole-microsecond "<n>us") and `tzid` (IANA id, backed by the `tz` package's TZLabel) instead of `end_time`. V15/V16 keep the legacy `end_time` shape via duplicated MeetingLegacy / NewMeetingLegacy / UpdateMeetingLegacy types, backed by one store that persists duration+tzid. Legacy is a pure adapter layer: `toLegacy`/`fromLegacy`/`fromLegacyNewMeeting`, with the legacy time zone injected from the new `galley.config.settings.meetings.legacyTimeZone` operator setting (default Europe/Berlin). - Duration gains ToJSON/ToSchema/Arbitrary (lossless microsecond rendering). - Postgres migration: add duration+tzid (backfilled from end_time), keep end_time nullable/discontinued, add duration_positive CHECK and two start_time+duration expression indexes. - Store/Subsystem: StoredMeeting + Hasql statements use DiffTime interval; legacy subsystem ops delegate to the newest implementations. - Routes/handlers/wiring: legacy @v15/@v16 branches use *Legacy types; cleanup worker threads the default TimeZone. - Tests: toLegacy/fromLegacy property laws; legacy subsystem unit interop (incl. UpdateMeetingLegacy moving both start+end); V17/legacy integration interop cases; regenerated meeting golden files.

The V17 meeting `Duration` normalized on parse and re-serialized as whole microseconds, so a client-sent "1h" round-tripped as "3600000000us". Introduce `MeetingDuration` { parsed :: Duration, original :: Text } whose Eq/Ord compare on `parsed` and whose Show/ToJSON/ToSchema render `original`, so the exact client literal is echoed back on every create/get/update/list. The original text is persisted in a new nullable `meetings.duration_original` column; backfilled NULL rows still render the canonical microsecond form (unchanged behavior). `mkMeetingDuration` validates untrusted input (rejects non-positive), and the data constructor is hidden behind an explicit export list (`parsed`/`original`/`mkMeetingDuration`/`unsafeMeetingDuration`/ `canonicalMeetingDuration` exposed instead).

…27553) The testMeetingInteropNewToLegacy assertion compared the legacy-read end_time against a client-computed addUTCTime 3600 startTime2, where startTime2 derives from getCurrentTime at picosecond precision. Postgres timestamptz truncates to microseconds, so the two values mismatched in the last two digits (73964508 vs 739645). Replace the absolute-time comparison with a server-relative one: parse both start_time and end_time from the legacy V16 response and assert end == start + 3600s. Both server-returned values share microsecond precision, so their difference is exactly 3600s and independent of the client clock.

@maybe

…-27553) Collapse the bespoke case/error-plumbing into the suite's standard helpers (assertJust for parse failure, shouldMatch for the comparison), mirroring the existing pattern in Test/Notifications.hs. Add @maybe @utctime type apps to pin iso8601ParseM's result, since assertJust/shouldMatch are too polymorphic to infer it.

akshaymankar

Partial review.

akshaymankar · 2026-07-30T08:05:57Z

+        {{- if .legacyTimeZone }}
+        legacyTimeZone: {{ .legacyTimeZone }}
+        {{- end }}


Suggested change

{{- if .legacyTimeZone }}

legacyTimeZone: {{ .legacyTimeZone }}

{{- end }}

legacyTimeZone: {{ .legacyTimeZone }}

There is a default in the helm chart, so no need for this code.

akshaymankar · 2026-07-30T09:19:10Z

  let startTime = addUTCTime 3600 now
-      endTime = addUTCTime 7200 now
-      newMeeting = defaultMeetingJson "MLS Meeting" startTime endTime []
+      newMeeting = defaultMeetingJson "MLS Meeting" startTime "3600000000us" []


Why not use "1h"?

akshaymankar · 2026-07-30T09:25:06Z

+durationToText :: Duration -> Text
+durationToText (Duration d) = Text.pack (show (diffTimeToPicoseconds d `div` 1000000)) <> "us"


I think we should do better, most of the time this is going to be something like 1h, 30m, '5s' etc. So we should try to find the biggest unit that can represent the duration in a whole number.

akshaymankar · 2026-07-30T09:34:52Z

+canonicalMeetingDuration :: Duration -> MeetingDuration
+canonicalMeetingDuration d = unsafeMeetingDuration d (durationToText d)


Either we should unsafe to the name or add the check for positivity here.

akshaymankar · 2026-07-30T09:37:24Z

+-- | A meeting duration that preserves the client-supplied literal alongside its
+-- parsed value. 'parsed' drives equality/ordering/arithmetic; 'original' drives
+-- serialization so a value read as @"1h"@ is shown/sent back as @"1h"@.
+data MeetingDuration = MeetingDuration
+  { parsed :: Duration,
+    original :: Text
+  }
+  deriving stock (Generic)
+  deriving (ToJSON, FromJSON, S.ToSchema) via (Schema MeetingDuration)


Perhaps we can use DurationLiteral instead of this?

akshaymankar · 2026-07-30T09:39:55Z

-  schema = Versioned <$> unVersioned .= meetingWithConversationSchema (Just V15)
+-- | Legacy meeting shape (V15/V16). Keeps the deprecated @end_time@ field and
+-- the always-false @trial@ field.
+data MeetingLegacy = MeetingLegacy


This would ideally get named MeetingV16 so we know when this can be deleted.

akshaymankar · 2026-07-30T09:41:51Z

+-- This file is part of the Wire Server implementation.
+--
+-- Copyright (C) 2026 Wire Swiss GmbH <opensource@wire.com>
+--
+-- This program is free software: you can redistribute it and/or modify it under
+-- the terms of the GNU Affero General Public License as published by the Free
+-- Software Foundation, either version 3 of the License, or (at your option) any
+-- later version.
+--
+-- This program is distributed in the hope that it will be useful, but WITHOUT
+-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+-- details.
+--
+-- You should have received a copy of the GNU Affero General Public License along
+-- with this program. If not, see <https://www.gnu.org/licenses/>.


The other files don't have tihs, maybe we can keep this notice out of the sql files?

akshaymankar · 2026-07-30T13:01:16Z

+ALTER TABLE meetings ADD COLUMN eff_end timestamptz;
+
+-- Backfill the new columns from the discontinued end_time.
+UPDATE meetings SET duration = end_time - start_time WHERE duration IS NULL;


I think its better to keep the end_time around and always infer the duration in haskell part. This way we don't have to do this migration and we can get rid of the eff_end field.

akshaymankar · 2026-07-30T13:03:10Z

+
+-- Backfill the new columns from the discontinued end_time.
+UPDATE meetings SET duration = end_time - start_time WHERE duration IS NULL;
+UPDATE meetings SET tzid = 'Europe/Berlin' WHERE tzid IS NULL;


Updating timezone for old meetings to make it Europe/Berlin but still giving an option to operator to specify the default timezone in V16 APi is not very consistent. IMO we shouldn't backfill this data and always make up the meeting id on the fly, i.e. read this as a Maybe and replace it with the default timezone from the config.

akshaymankar · 2026-07-30T13:03:46Z

+ALTER TABLE meetings ALTER COLUMN tzid SET DEFAULT 'Europe/Berlin';
+ALTER TABLE meetings ALTER COLUMN tzid SET NOT NULL;


We don't need to do this if we infer meaning of null timezone at runtime.

…se, V16 rename (WPB-27553) - Drop eff_end; keep end_time (NOT NULL, indexed) and infer duration/tzid at read time. Migrations reduce to nullable duration/tzid + CHECK; reuse the pre-existing end_time indexes. - Reuse Data.Misc.DurationLiteral (delete bespoke MeetingDuration); durationToText renders the biggest whole unit (3600s -> "1h"). - Rename the *Legacy meeting family to *V16 (route URL names unchanged). - Remove dead helm legacyTimeZone guard; update integration test literals to "1h" where the V17 read infers the duration.

) Merge the V17/V16 Meeting pairs into a parameterized Meeting' tz family. end_time is the DB source of truth; the V17 wire duration is derived (end_time - start_time). Drop the duration and duration_original columns; make tzid NOT NULL (backfilled to Europe/Berlin). legacyTimeZone is now used only on the V16 create path. - API (Wire.API.Meeting): Meeting' tz + aliases, distinct per-era ToSchema, derived-duration wire field; toLegacy/fromLegacy conversions; microsecond-aligned Arbitrary for the V17 shapes. - Store/interpreter: endTime params; reads use the NOT NULL tzid directly; validation compares endTime vs startTime. - Migrations: tzid-only ADD COLUMN; delete duration-original; new tzid-notnull backfill.

end_time is the persisted source of truth, so the V17 wire no longer carries a derived duration field -- both V17 and V16 serialize end_time directly. Collapse UpdateMeeting and UpdateMeetingV16 into one type (optional end_time), and revert the Data.Misc additions that the derived field required (durationToText and the ToJSON/ToSchema/Arbitrary Duration instances); FromJSON Duration and the Duration/DurationLiteral newtypes are kept. The V16 update path now delegates straight through (the redundant Store read and convertUpdateV16 are gone). Regenerates the V17 Meeting goldens (end_time, no duration).

…type Change the phantom parameter of Meeting', MeetingWithConversation' and NewMeeting' from the tzid field type (tz) to an explicit API-era index (era :: MeetingEra), with a closed TzidOf type family mapping era to the tzid field type: 'V17Era -> TimeZone, 'V16Era -> (). Pure type-level refactor: wire/JSON behaviour is unchanged (goldens are byte-identical, the deprecated V16-only trial field is preserved). The type aliases keep their names, so every consumer compiles unchanged. Eq/Show move to standalone derivings carrying a TzidOf era (resp. Meeting' era) context, because an attached stock deriving cannot synthesise them for a type-family field on a free type variable. Arbitrary becomes explicit per-era instances for the same reason.

blackheaven requested review from a team as code owners July 29, 2026 17:09

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

blackheaven force-pushed the gdifolco/WPB-27553-meeting-tzid branch from e605d50 to ae662ab Compare July 29, 2026 18:05

blackheaven force-pushed the gdifolco/WPB-27553-meeting-tzid branch from ae662ab to 1f859e9 Compare July 29, 2026 21:08

blackheaven added 5 commits July 30, 2026 00:13

fix: formatting

f72a405

Hello CI

fd0135e

akshaymankar requested changes Jul 30, 2026

View reviewed changes

blackheaven changed the title ~~WPB-27553: split Meeting into new (duration+tzid) and legacy (end_time) shapes~~ WPB-27553: unify Meeting around end_time; add duration+tzid (V17); persist tzid NOT NULL Jul 31, 2026

blackheaven added 2 commits July 31, 2026 14:18

blackheaven changed the title ~~WPB-27553: unify Meeting around end_time; add duration+tzid (V17); persist tzid NOT NULL~~ WPB-27553: add tzid to meetings Jul 31, 2026

blackheaven added 2 commits July 31, 2026 20:20

chore: drop unrelated changes

25f90e5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

WPB-27553: add tzid to meetings - #5391

WPB-27553: add tzid to meetings#5391
blackheaven wants to merge 11 commits into
developfrom
gdifolco/WPB-27553-meeting-tzid

blackheaven commented Jul 29, 2026

Uh oh!

akshaymankar left a comment

Uh oh!

akshaymankar Jul 30, 2026

Uh oh!

akshaymankar Jul 30, 2026

Uh oh!

akshaymankar Jul 30, 2026

Uh oh!

akshaymankar Jul 30, 2026

Uh oh!

akshaymankar Jul 30, 2026

Uh oh!

akshaymankar Jul 30, 2026

Uh oh!

akshaymankar Jul 30, 2026

Uh oh!

akshaymankar Jul 30, 2026

Uh oh!

akshaymankar Jul 30, 2026

Uh oh!

akshaymankar Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		durationToText :: Duration -> Text
		durationToText (Duration d) = Text.pack (show (diffTimeToPicoseconds d `div` 1000000)) <> "us"

		canonicalMeetingDuration :: Duration -> MeetingDuration
		canonicalMeetingDuration d = unsafeMeetingDuration d (durationToText d)

		ALTER TABLE meetings ALTER COLUMN tzid SET DEFAULT 'Europe/Berlin';
		ALTER TABLE meetings ALTER COLUMN tzid SET NOT NULL;

Uh oh!

Conversation

blackheaven commented Jul 29, 2026

Checklist

Uh oh!

akshaymankar 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants