WPB-19712: Allow team admin to update the channels to user-group association#4783

blackheaven · 2025-09-18T17:25:58Z

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

Checklist

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

WIP

Tests

eyeinsky

Just my comments, not all of them might be correct. Otherwise looks good and easy to read and understand.

eyeinsky · 2025-09-24T14:03:25Z

libs/wire-api/src/Wire/API/Error/Brig.hs


 type instance MapError 'UserGroupNotATeamAdmin = 'StaticError 403 "user-group-write-forbidden" "Only team admins can create, update, or delete user groups."

+type instance MapError 'UserGroupChannelNotFound = 'StaticError 400 "user-group-channel-not-found" "Specified Channel does not exists or belongs to the team"


I guess this could be a 404.

changed, thanks.

eyeinsky · 2025-09-24T14:34:53Z

libs/wire-subsystems/src/Wire/UserGroupStore/Postgres.hs

+      lmap
+        (bimap toUUID (fmap toUUID))
+        $ [resultlessStatement|
+          delete from user_group_channel where user_group_id = ($1 :: uuid) and conv_id <> any($2 :: uuid[])


Hmm, testing on postgres console SELECT 1 <> ANY(ARRAY[1, 2, 3]); gives me true, so if an existing conversation is in the new set (part of the array) it would be deleted, but it's the other way around we want, don't we? (Unless I'm mistaken on what the logic should be.) Perhaps add this to the test to make sure.

Also, to my brain ... conv_id NOT IN (..new_conv_ids..) would be easier to read, rather than ... <> any(...) (but that may be just habit).

That's also how my brain works... but not how hasql encoders work.

I guess it works because there is also an upsert operation following it.

I've tried something else.

eyeinsky · 2025-09-24T15:04:28Z

postgres-schema.sql

+);
+

-ALTER TABLE public.user_group_member OWNER TO "wire-server";


Does this row need to be deleted?

Dropped by mistake, thanks.

eyeinsky · 2025-09-24T16:10:22Z

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

    AddUser gid uid -> addUserImpl gid uid
    UpdateUsers gid uids -> updateUsersImpl gid uids
    RemoveUser gid uid -> removeUserImpl gid uid
+    UpdateUserGroupChannels _ gid convIds -> updateUserGroupChannelsImpl gid convIds


Maybe remove the TeamId field as it doesn't seem to be used (?).

it's used in the Mock interpreter (it's not a good idea though)

I think that is not a good reason to have it here.

dropped, I was thinking of the subsystem.

battermann · 2025-09-30T10:02:07Z

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

    AddUser gid uid -> addUserImpl gid uid
    UpdateUsers gid uids -> updateUsersImpl gid uids
    RemoveUser gid uid -> removeUserImpl gid uid
+    UpdateUserGroupChannels _ gid convIds -> updateUserGroupChannelsImpl gid convIds


I think that is not a good reason to have it here.

libs/wire-subsystems/src/Wire/UserGroupStore/Postgres.hs

libs/wire-api/src/Wire/API/Error/Brig.hs

libs/wire-api/src/Wire/API/Routes/Public/Brig.hs

integration/test/Test/UserGroup.hs

libs/wire-api/src/Wire/API/Routes/Public/Brig.hs

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

…ciation

battermann

LGTM

blackheaven · 2025-10-08T13:30:35Z

@copilot review this glorious PR please

Copilot

Pull Request Overview

This PR implements the ability for team admins to update channel associations for user groups, removing the previous stub implementation. The feature allows team admins to associate specific team channels with user groups via a PUT endpoint.

Replaces the stub implementation of updateUserGroupChannels with a full working implementation
Adds database schema and storage layer support for user group to channel associations
Implements validation to ensure only team channels can be associated with user groups

Reviewed Changes

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

Show a summary per file

File	Description
services/brig/src/Brig/API/Public.hs	Removes stub implementation and adds real handler for updating user group channels
postgres-schema.sql	Creates new `user_group_channel` table for storing channel associations
libs/wire-subsystems/src/Wire/UserGroupSubsystem/Interpreter.hs	Implements business logic for channel validation and updates
libs/wire-subsystems/src/Wire/UserGroupStore/Postgres.hs	Adds database operations for managing channel associations
libs/wire-api/src/Wire/API/Routes/Public/Brig.hs	Updates API documentation to remove stub notation
integration/test/Test/UserGroup.hs	Adds comprehensive test coverage for the new functionality

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

libs/wire-subsystems/src/Wire/UserGroupStore/Postgres.hs

zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Sep 18, 2025

blackheaven marked this pull request as ready for review September 19, 2025 16:49

blackheaven requested a review from a team as a code owner September 19, 2025 16:49

eyeinsky reviewed Sep 24, 2025

View reviewed changes

battermann requested changes Sep 30, 2025

View reviewed changes

blackheaven requested a review from battermann October 1, 2025 18:34

blackheaven force-pushed the gdifolco/WPB-19712_user-groups-update-endpoint branch from 4ed864c to 909912b Compare October 2, 2025 15:09

battermann requested changes Oct 7, 2025

View reviewed changes

libs/wire-subsystems/src/Wire/UserGroupSubsystem/Interpreter.hs Outdated Show resolved Hide resolved

WPB-19712: Allow team admin to update the channels to user-group asso…

2dea4d2

…ciation

blackheaven force-pushed the gdifolco/WPB-19712_user-groups-update-endpoint branch from db1b267 to 2dea4d2 Compare October 8, 2025 08:51

blackheaven added 4 commits October 8, 2025 12:45

feat: check that conversations are channels

756198b

fix: implement minBackend

82ed46c

fix: tests and add one

d1a293e

fix: test notification

d495acb

battermann approved these changes Oct 8, 2025

View reviewed changes

fix: docs

ff99a34

blackheaven requested review from a team as code owners October 8, 2025 13:26

battermann requested a review from Copilot October 8, 2025 13:32

Copilot AI reviewed Oct 8, 2025

View reviewed changes

libs/wire-subsystems/src/Wire/UserGroupStore/Postgres.hs Show resolved Hide resolved

libs/wire-subsystems/src/Wire/UserGroupStore/Postgres.hs Show resolved Hide resolved

blackheaven merged commit d88aebb into develop Oct 8, 2025
8 checks passed

blackheaven deleted the gdifolco/WPB-19712_user-groups-update-endpoint branch October 8, 2025 14:37

zebot mentioned this pull request Oct 20, 2025

Release 2025-10-20 - (expected chart version 5.23.0) #4818

Closed

blackheaven mentioned this pull request Oct 20, 2025

[WPB-21259] Finalize API Version V12 #4817

Merged

2 tasks

zebot mentioned this pull request Oct 21, 2025

[WPB-21261] Release 2025-10-21 - (expected chart version 5.23.0) #4820

Merged


		type instance MapError 'UserGroupNotATeamAdmin = 'StaticError 403 "user-group-write-forbidden" "Only team admins can create, update, or delete user groups."

		type instance MapError 'UserGroupChannelNotFound = 'StaticError 400 "user-group-channel-not-found" "Specified Channel does not exists or belongs to the team"

		);


		ALTER TABLE public.user_group_member OWNER TO "wire-server";

Conversation

blackheaven commented Sep 18, 2025 • edited by battermann Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

WIP

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

battermann left a comment

Choose a reason for hiding this comment

Uh oh!

blackheaven commented Oct 8, 2025

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

blackheaven commented Sep 18, 2025 •

edited by battermann

Loading