Surface team member removal failures in Teams settings UI - #14964
Surface team member removal failures in Teams settings UI#14964warp-agent-staging[bot] wants to merge 1 commit into
Conversation
Previously, remove_user_from_team spawned with on_workspaces_updated as its completion callback, so a failed removal only logged "Failed to load user workspaces" and never told the Teams settings UI anything went wrong. The member just stayed in the list with no visible feedback. Add a dedicated on_remove_user_from_team handler (mirroring the pattern used by every other team mutation) that emits RemoveUserFromTeamRejected/RemoveUserFromTeamSuccess events, and wire it into remove_user_from_team instead of the shared on_workspaces_updated error arm. Surface the rejected event as an error toast in TeamsPageView that includes the server's error message, and refresh the member list on success as before. Fixes APP-5317. Co-Authored-By: Warp Agent <agent@warp.dev>
There was a problem hiding this comment.
Overview
Adds a dedicated on_remove_user_from_team handler with RemoveUserFromTeamSuccess/RemoveUserFromTeamRejected events and surfaces the failure as a toast on the Teams page, so a rejected removal is no longer silent. Reviewed and found no correctness defects; two items below need your judgment rather than a code change.
Concerns
- Toast copy for the failure you actually hit. For the reported error, the toast renders as
Failed to remove team member: missing response data for RemoveUserFromTeam: Not found: no rows in result set— intact and under the truncation threshold, but "missing response data" is client-internal vocabulary and "no rows in result set" is a database-layer leak, so you learn that the removal failed and still not why. There are two error shapes here: a serverUserFacingErroryields a clean server-authored sentence, while a top-level GraphQL error yields this wrapper, and your case is the second. Your call between keeping it as-is, using static copy with the error appended only when it is user-facing, or appending always with a generic tail such as "please contact support"; see the inline comment. - Visual proof is missing, and unblocking it needs a credential. The Teams page has no view-event harness, so the five lines that actually put the message on screen are covered by no test, and an attempt to capture the real toast failed: the client authenticated as an agent service account and never got past the logged-out screen (
Unauthorized: Expected a user account). A user-scoped staging or dogfood credential for an account on a team with at least one other member is enough to finish this, and the capture would also settle the copy question above at a glance. - Successful removals gained a confirmation toast ("Removed team member") that the report did not ask for. It is consistent with the sibling flows on this page, and the member list still refreshes exactly as before; flagging it only so the addition is deliberate rather than discovered.
Verdict
Checks: build pass, tests pass, CI green, visual proof missing
Found: 0 critical, 2 important, 1 suggestion, 0 nits
| } | ||
| UserWorkspacesEvent::RemoveUserFromTeamRejected(err) => { | ||
| self.show_error( | ||
| format!("Failed to remove team member: {err}"), |
There was a problem hiding this comment.
The error is passed through without a .context() wrapper, which is deliberate and correct — anyhow's Display shows only the outermost message, so wrapping would have hidden the server's text. The open question is what that text reads like for the top-level GraphQL error shape you hit: Failed to remove team member: missing response data for RemoveUserFromTeam: Not found: no rows in result set.
Note this is the only toast on this page that embeds the error text; every sibling uses static copy ("Failed to update team member role", "Error leaving team"). The divergence is what satisfies your acceptance criterion, so it is defensible — but if you would rather not surface internal plumbing to operators, the alternative is a static headline plus the server message only when it is a UserFacingError. Tell me which you want and I will turn it around.
Description
When removing a team member from Settings > Teams fails on the server, the client showed no feedback at all — the member just stayed in the list, and the only trace was in the log file.
Root cause:
UserWorkspaces::remove_user_from_teamspawned its GraphQL request with the sharedon_workspaces_updatedcompletion callback. That callback's error arm only callsreport_error!(e.context("Failed to load user workspaces"))— it never emits an event, so the Teams settings UI had nothing to react to, and the logged context string was misleading (it's a member-removal failure, not a workspaces-load failure).Fix: follow the existing pattern used by every other team mutation in
user_workspaces.rs(e.g.on_add_invite_link_domain_restrictions,on_team_member_role_set). Added a dedicatedon_remove_user_from_teamhandler that emitsRemoveUserFromTeamRejected(err)/RemoveUserFromTeamSuccessonUserWorkspacesEvent, and wired it in as the spawn callback forremove_user_from_teaminstead of the sharedon_workspaces_updated.TeamsPageView::handle_model_eventnow surfaces the rejected event as an error toast that includes the server's actual error message, and refreshes the member list on success exactly as before.This only changes the removal path —
on_workspaces_updated's shared error arm (used by every other flow viaon_workspaces_updated(Ok(...))from their own success arms) is untouched.Linked Issue
Fixes APP-5317.
Testing
test_remove_user_from_team_rejected_emits_error_event_without_updating_workspacesandtest_remove_user_from_team_success_emits_success_event_and_refreshes_membersinapp/src/workspaces/user_workspaces_tests.rs, covering both the rejected path (event emitted with the server's error message, local state untouched) and the success path (event emitted, member list refreshed).cargo nextest run -p warp -E 'test(user_workspaces) or test(teams_page)'— 54 passed.cargo fmt -- --check— clean.cargo clippy -p warp --tests -- -D warnings— clean. (Note:cargo clippy -p warp --all-targets --all-featurescurrently fails to compile due to pre-existing clippy errors inwarp_completer'sv2feature, unrelated to this change, reproducible onmasterbefore this PR, and consistent with presubmit's own exclusion of that crate from--all-features.)Warp CImatrix: Linux/macOS/Windows tests, formatting + clippy for Linux/macOS/Windows/wasm, release-flag compilation variants, misc checks) — all green.RemoveUserFromTeamto fail with the exact error string from the original bug report and to inject a removable team member, then launched the real Warp GUI under computer use. The app authenticated as an Oz agent service account rather than a real Warp user (Unauthorized: Expected a user account), so it landed on the logged-out onboarding screen and never reached an authenticated terminal, let alone Settings > Teams. No user-scoped credential (STAGING_USER_WARP_API_KEY) was available in that sandbox either. So the toast render path is exercised only by code review + the fact that the message string is built and passed through unchanged (format!("Failed to remove team member: {err}")) to the pre-existingshow_toast/ToastFlavor::Errorcomponent already used by sibling rejection flows (e.g.SetTeamMemberRoleRejected) elsewhere in this same file. A screenshot from an environment with a real user-scoped Warp credential and an existing multi-member team would close this gap; happy to produce one if such credentials/account can be provided.Agent Mode
CHANGELOG-BUG-FIX: Fixed an issue where removing a team member from Settings > Teams would fail silently with no error shown if the removal was rejected by the server.
Conversation: https://staging.warp.dev/conversation/00f772cb-d726-478e-a64f-a77a139df241
Run: https://oz.staging.warp.dev/runs/019ff2ae-45b1-78f2-9900-e778550deff4
This PR was generated with Oz.