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

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.

akshaymankar

For types which change with verison numbers we either:

copy-paste the type to create the older version.
or define the ToSchema instance for Versioned v a and deal with the differences there.

The Era thing is interesting, but I think it adds more complexity than we need. And more importantly a third way of doing the same thing. Do we have a good reason for not going with already established patterns here?

Another thing I noticed is that you use RespondVersioned with an already versioned type, think we don't really need this, if we create a new type we can just use Respond. The RespondVersioned thing is to make servant use the ToSchema from Versioned v a.

(I'm on vacation starting this evening so please feel free to merge if other team-mates approve)

Introduce an IANA `tzid` (TimeZone) field on the V17 meetings API: - NewMeeting/Meeting/MeetingWithConversation carry `tzid`; the V15/V16 shapes keep the deprecated `trial:false` field and have no `tzid`, for backward compatibility. - `end_time` is the single source of truth (no `duration` field). - `tzid` is persisted NOT NULL and backfilled to the configured `meetings.legacyTimeZone` (Europe/Berlin by default). - The V17 and V15/V16 shapes are concrete, copy-pasted records (Meeting/MeetingV16, MeetingWithConversation/V16, NewMeeting/V16) with plain `Respond` routes -- one established pattern, per the PR #5391 review (reviewer's option 1). - toLegacy/fromLegacy bridge the V17 and V16 shapes in the interpreter. The V15/V16 wire shape (trial:false, no tzid) and the V17 wire shape (tzid, no trial) are unchanged.

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 2 times, most recently from ae662ab to 1f859e9 Compare July 29, 2026 21:08

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

akshaymankar requested changes Aug 6, 2026

View reviewed changes

blackheaven force-pushed the gdifolco/WPB-27553-meeting-tzid branch from 356db39 to 0fa66d2 Compare August 6, 2026 12:32

Navigation Menu

Uh oh!

WPB-27553: add tzid to meetings - #5391

WPB-27553: add tzid to meetings#5391
blackheaven wants to merge 1 commit 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!

akshaymankar left a comment

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!

akshaymankar left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants