WPB-21964: Add Wire Meetings list by blackheaven · Pull Request #5109 · wireapp/wire-server

blackheaven · 2026-03-10T17:02:47Z

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

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

Checklist

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

supersven · 2026-03-10T17:18:50Z

services/galley/src/Galley/API/Internal.hs

              SelfConv -> Nothing
              ConnectConv -> Nothing
-      renderedPage <- mapMaybe mk <$> getConversations (fst $ partitionQualified luid convids)
+      renderedPage <- mapMaybe mk <$> E.getConversations (fst $ partitionQualified luid convids)


Should E.getConversations not be better e.g. ConversationStore.getConversations as in services/galley/src/Galley/API/Query.hs? (E. translates to Error. in my weird head 😄 )

libs/wire-subsystems/test/unit/Wire/MockInterpreters/MeetingsStore.hs

supersven · 2026-03-10T17:23:04Z

libs/wire-subsystems/src/Wire/ConversationSubsystem.hs

+    Local UserId ->
+    Range 1 1000 Int32 ->
+    Maybe (Qualified ConvId) ->
+    ConversationSubsystem r (ResultSet (Qualified ConvId))


This seems to leak a Cassandra specific type (ResultSet) to upper layers. Is this intended?

supersven · 2026-03-10T17:31:00Z

integration/test/Test/Meetings.hs


  putMeeting otherUser domain meetingId update >>= assertStatus 404
+
+testMeetingLists :: (HasCallStack) => App ()


Shouldn't there be more tests?

E.g.

There is no meeting for the user -> list none

There is a meeting for another user, but none for the user to test -> list none

There are multiple meetings for the user -> list them

These could probably be unit tests as well. However, I found none.

It would probably also be good to ensure that the pagination works as expected.

I've added some

supersven · 2026-03-10T17:43:56Z

libs/wire-subsystems/src/Wire/MeetingsSubsystem/Interpreter.hs

+        Member ConversationSubsystem r
+      ) =>
+      Maybe (Qualified ConvId) ->
+      Sem r [API.Meeting]


r has the same constraints here as the r from the containing function. Have you considered to use https://ghc.gitlab.haskell.org/ghc/doc/users_guide/exts/scoped_type_variables.html#extension-ScopedTypeVariables?

supersven · 2026-03-11T08:33:28Z

integration/test/Test/Meetings.hs

+  meetings <- resp.json & asList
+  length (meetings :: [Value]) `shouldMatchInt` 3
+
+testMeetingListPagination :: (HasCallStack) => App ()


Uhm, how does this test pagination? Are pages only 4 elements? 🤔

1001 is enough :)

supersven · 2026-03-11T15:08:34Z

integration/test/Test/Meetings.hs

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


BTW - Have you considered to validate the values themselves, not only the count?

I have validated ids, should I fully validate all fields?

blackheaven requested review from a team as code owners March 10, 2026 17:02

blackheaven force-pushed the gdifolco/WPB-21964-list-meetings branch from 062fc96 to 3478cfd Compare March 10, 2026 17:11

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

supersven reviewed Mar 10, 2026

View reviewed changes

libs/wire-subsystems/test/unit/Wire/MockInterpreters/MeetingsStore.hs Show resolved Hide resolved

supersven reviewed Mar 10, 2026

View reviewed changes

libs/wire-subsystems/test/unit/Wire/MockInterpreters/MeetingsStore.hs Show resolved Hide resolved

supersven reviewed Mar 10, 2026

View reviewed changes

supersven requested changes Mar 10, 2026

View reviewed changes

blackheaven force-pushed the gdifolco/WPB-21964-list-meetings branch 2 times, most recently from d25ac2c to c3a2a4f Compare March 10, 2026 21:56

blackheaven requested a review from supersven March 11, 2026 08:29

supersven reviewed Mar 11, 2026

View reviewed changes

blackheaven added 2 commits March 11, 2026 10:29

WPB-21964: Add Wire Meetings list

7580cae

fix: Sven feedbacks

e2e25ee

blackheaven force-pushed the gdifolco/WPB-21964-list-meetings branch from d455d62 to e2e25ee Compare March 11, 2026 09:34

supersven reviewed Mar 11, 2026

View reviewed changes

fix: improve tests (incl. pagination)

7626b7e

blackheaven requested a review from supersven March 13, 2026 11:57


		putMeeting otherUser domain meetingId update >>= assertStatus 404

		testMeetingLists :: (HasCallStack) => App ()

Conversation

blackheaven commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

blackheaven commented Mar 10, 2026 •

edited

Loading