fix: stop using broken addTeamMemberInternal in testCreateOne2OneWith - #5420
Open
blackheaven wants to merge 4 commits into
Open
fix: stop using broken addTeamMemberInternal in testCreateOne2OneWith#5420blackheaven wants to merge 4 commits into
blackheaven wants to merge 4 commits into
Conversation
…Members The test intermittently failed with a 500 from galley's internal POST i/teams/:tid/members endpoint (uncheckedAddTeamMember), reached via the addTeamMemberInternal helper. That helper is documented as broken (SQSERVICES-471): it force-adds an existing user through a path that fans out synchronous brig RPCs (getSize/getUser/ getAppIdsForTeam) plus Cassandra writes and SQS journaling, any of which can transiently exhaust its retries and surface as an unhandled 500 server-error. Switch to addUserToTeamWithRole, the proper brig invitation flow that updates both brig and galley and is used by every sibling test in this file. The member-join event is now asserted on the owner's WebSocket (the invitee is created inside the call and cannot have a pre-established connection), matching the testAddTeamConvWithRole precedent (Teams.hs:679-680). Also corrects the misleading doc-comment on the role parameter.
zebot
added
the
ok-to-test
battermann
reviewed
Aug 5, 2026
battermann
left a comment
battermann
left a comment
Contributor
There was a problem hiding this comment.
If addTeamMemberInternal is broken, can we remove it from Util, too?
Also could this test be migrated to the integration package, and be removed here all together?
Addresses @battermann's review on #5420. 1. Remove the broken `Util.addTeamMemberInternal` helper (SQSERVICES-471). All 14 call sites migrate to the proper invitation flow: - standard-role sites -> `addUserToTeamWithRole` - custom-permission sites -> `addUserToTeamWithRole` + new `updateTeamMemberPermissions` (public PUT /teams/:tid/members) - `testAddTeamMemberInternal` keeps inlined coverage of the internal add-member endpoint (its purpose; the invite flow cannot reproduce the member-websocket event assertion) - `testAddTeamMemberToConv` and `testCannotCreateLegalHoldDeviceOldAPI` restructured to mint members via the invitation flow. In `testAddTeamMemberToConv` the same-team members are no longer passed to `connectUsers` (same-binding-team connection requests are rejected with 403; those assertions are satisfied by the same-team condition). 2. Port `testCreateOne2OneWithMembers` and `testCreateOne2OneFailForNonTeamMembers` to the Testlib /integration package (`Test.One2OneTeamConv`) and remove them from galley. The galley-specific `assertTeamUpdate` (SQS queue) is replaced by a member-count check plus the real-time member-join websocket event. Cleanup: collapse `makeOwner` onto `updateTeamMemberPermissions`; delete the now-dead `createOne2OneTeamConv` helper. Compile-verified (exe:galley-integration + integration); runtime deferred to CI. `testAddTeamMemberToConv` was restructured and is the most behaviour-sensitive change.
Contributor
Author
|
I have migrated everything |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
changelog.d