Skip to content

feat(workspace-users): exclude removed users by default#13

Open
scottlovegrove wants to merge 2 commits into
mainfrom
feat/exclude-removed-workspace-users
Open

feat(workspace-users): exclude removed users by default#13
scottlovegrove wants to merge 2 commits into
mainfrom
feat/exclude-removed-workspace-users

Conversation

@scottlovegrove
Copy link
Copy Markdown
Collaborator

Summary

Port of Doist/twist-sdk-typescript#138 to comms-sdk.

  • getWorkspaceUsers now excludes users with removed: true by default; pass includeRemoved: true to include them.
  • The Comms API's workspace_users/get always returns removed users and exposes no override param, so the filtering happens client-side in the SDK.
  • GetWorkspaceUsersArgs gains an optional includeRemoved?: boolean (defaults to false).

Difference from the twist PR

twist-sdk applies the filter to both an awaited path and a batch-descriptor path (via a new transform hook on BatchRequestDescriptor). comms-sdk has no batch pattern, so the filter applies only to the awaited call — no batch plumbing needed.

Test plan

  • excludes removed users by default, and sends no include_removed / with_removed query param (proves client-side filtering)
  • includeRemoved: true returns all users
  • npm run type-check clean
  • npm run check (oxlint + oxfmt) clean
  • npm test — 128 pass (+2 new)

🤖 Generated with Claude Code

`getWorkspaceUsers` now excludes users with `removed: true` by default;
pass `includeRemoved: true` to include them. The Comms API's
`workspace_users/get` always returns removed users with no override
param, so the filtering happens client-side in the SDK.

Ported from Doist/twist-sdk-typescript#138. Unlike the twist SDK, comms
has no batch path, so the filter applies only to the awaited call.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove self-assigned this May 22, 2026
Copy link
Copy Markdown
Member

@doistbot doistbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR updates getWorkspaceUsers to filter out removed users client-side by default, smoothly porting the desired behavior from the Twist SDK. The implementation handles the API's lack of server-side filtering well while maintaining a clean interface with the new includeRemoved argument. A couple of minor adjustments were noted, specifically filtering the raw response data prior to Zod validation to reduce overhead, and adding query-parameter assertions to the tests to secure the intended wire contract.

Share FeedbackReview Logs

Comment thread src/clients/workspace-users-client.ts Outdated
Comment thread src/clients/workspace-users-client.test.ts Outdated
- Filter removed users out of the raw response before Zod parsing, so
  discarded users don't incur validation overhead.
- Assert no server-side filter param on the `includeRemoved: true` path
  too, locking in the client-side-only wire contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants