[CXH-2366] - Add Zoom ownership transfer custom action - #33
Conversation
Adds a resource-scoped connector action that reassigns a user's meetings,
webinars, and cloud recordings to another Zoom user, then disassociates or
permanently deletes the user, via Zoom's DELETE /v2/users/{userId} transfer
query parameters.
| if err != nil { | ||
| if isUserNotFound(err) { | ||
| return actions.NewReturnValues( | ||
| true, | ||
| actions.NewStringReturnField("message", fmt.Sprintf("user %s was already removed from the account", userID)), | ||
| ), nil, nil | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: any 404 from DELETE /users/{userId} is reported as success: true with "already removed", but the target user is not the only thing this endpoint can fail to find — Zoom also 404s (code 1001, "User not exist") when transfer_email names a user that isn't in the account. In that case the user is not removed and nothing is transferred, yet the automation records success and moves on. Consider narrowing the short-circuit: inspect the Zoom error code/message in APIError.Body and only treat it as already-gone when it refers to userID, or only apply the short-circuit when no transfer fields were supplied.
| // another user (TransferEmail) as part of removing them from the account. | ||
| type DeleteUserOptions struct { | ||
| // Action is Disassociate (unlink the user from the account) or Delete | ||
| // (permanently remove the user). Empty defers to Zoom's default (delete). |
There was a problem hiding this comment.
🟡 Suggestion: Zoom's documented default for the action query param on DELETE /v2/users/{userId} is disassociate, not delete. That makes this comment misleading for the DeleteUser path (CAPABILITY_RESOURCE_DELETE), which passes a zero DeleteUserOptions and therefore disassociates rather than permanently deletes.
| // (permanently remove the user). Empty defers to Zoom's default (delete). | |
| // (permanently remove the user). Empty defers to Zoom's default | |
| // (disassociate). |
| actions.NewStringReturnField("message", fmt.Sprintf("user %s was already removed from the account", userID)), | ||
| ), nil, nil | ||
| } | ||
| return nil, nil, fmt.Errorf("baton-zoom: transfer_and_delete_user: %s: %w", userID, err) |
There was a problem hiding this comment.
🟡 Suggestion: this path returns a bare wrapped error with no gRPC status code. pkg/zoom uses a raw *http.Client rather than uhttp.BaseHttpClient, so nothing upstream maps the Zoom status onto a code — a 403 or 429 reaches the SDK as Unknown. Since APIError now carries StatusCode, map it here (401→Unauthenticated, 403→PermissionDenied, 429→ResourceExhausted, 5xx→Internal) via uhttp.WrapErrors or status.Error so retry vs. surface is decided correctly.
| return actions.NewReturnValues( | ||
| true, | ||
| actions.NewStringReturnField("message", fmt.Sprintf("user %s data transferred and %sd from the account", userID, deleteAction)), | ||
| ), nil, nil |
There was a problem hiding this comment.
🟡 Suggestion: all three transfer flags are optional, so the common "just remove the user" invocation returns "user X data transferred and deleted from the account" when nothing was transferred. Consider building the message conditionally on whether any transfer flag was set, so the automation record reflects what actually happened.
Connector PR Review: [CXH-2366] - Add Zoom ownership transfer custom actionBlocking Issues: 0 | Suggestions: 5 | Threads Resolved: 0 Review SummaryScanned the full PR diff for security and correctness: a new resource-scoped Security IssuesNone found. Correctness IssuesNone found. Suggestions
Prompt for AI agents |
INTENT
CXH-2366 has no acceptance-criteria section; the criteria below are derived from its description and the originating Slack request (ShopMy, via #C087N95G127), which asks for the same custom-action pattern across five connectors (Zoom, Notion, Figma, Loom, Miro):
baton-zoomthat transfers ownership of a user's calls/meetings/webinars/recordings to another user, invocable on demand (e.g. via C1's "Perform connector action" automation step) prior to deleting the account.Fixes CXH-2366
What / Why / How
What: Adds a resource-scoped
transfer_and_delete_useraction on the Zoom connector's user resource type. It calls Zoom'sDELETE /v2/users/{userId}with theaction,transfer_email,transfer_meeting,transfer_webinar, andtransfer_recordingquery params in a single call, then removes the user (disassociate or permanent delete, operator's choice).Why: Customer (ShopMy) wants an explicit, on-demand step to hand off a departing user's Zoom data to e.g. their manager before removing them, rather than losing recordings/meetings/webinars on deletion.
How tested:
go build ./...,go vet ./..., andgo test ./...all pass. Self-reviewed againstreview-actions-layer's 20 rules (all pass/N/A — see below). Manual end-to-end testing against a real Zoom sandbox tenant/user is still needed before merge (see Follow-ups) since this is a permanent, destructive operation not suited to running unattended in CI.Review notes
ResourceActionProvider(notGlobalActionProvider) sinceACTION_TYPE_RESOURCE_DELETEisn't a lifecycle-FSM type — mirrorsbaton-okta'sdelete_useraction.zoom.APIError(previously all client errors were untyped strings) so the handler can treat a 404 ("already deleted") as success on retry, per the idempotency rule.baton_capabilities.json/config_schema.jsonregenerated via./connector capabilities/./connector config;docs/connector.mdxupdated with the action table.Follow-ups (not in scope for this PR)
baton-zoomis not currently container-ready (fails theRunConnector/session-store/V2-interface audit). Out of scope here; should be tracked as its own ticket before this connector can run in the container/Lambda deployment path.Delete()deprovisioning flow (auto-derives the manager, transfers recordings/meetings/whiteboards automatically, no operator input). That's a different customer ask (automatic vs. this PR's on-demand, operator-controlled action) but touches the samepkg/zoom/client.goDeleteUsercode path, so there's a real chance of a merge conflict or duplicated logic depending on merge order. Worth a quick sync with whoever owns CXH-2024 (Jacob Aguon) on whether the two should be reconciled (e.g. this action's client method could eventually replace/absorb feat: add transfer parameters to user deletion #31's, or feat: add transfer parameters to user deletion #31 could addtransfer_whiteboardsupport to this action's schema instead of a separate code path).