Skip to content

PROD | filter out deleted clients from company member list - #1361

Merged
arpandhakal merged 3 commits into
productionfrom
main
Jun 30, 2026
Merged

PROD | filter out deleted clients from company member list#1361
arpandhakal merged 3 commits into
productionfrom
main

Conversation

@arpandhakal

Copy link
Copy Markdown
Collaborator

No description provided.

arpandhakal and others added 2 commits June 30, 2026 11:14
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
OUT-3930 | Filter out deleted /inactive users while creating grouped email notifications when task is assigned or any such thing.
@arpandhakal arpandhakal self-assigned this Jun 30, 2026
@vercel

vercel Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tasks-app Ready Ready Preview, Comment Jun 30, 2026 7:00am

Request Review

@vercel

vercel Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Deploying Serverless Functions to multiple regions is restricted to the Pro and Enterprise plans.

Learn More: https://vercel.link/multiple-function-regions

@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown

Greptile Summary

This PR filters out deleted clients from the result of _getCompanyClients, preventing them from appearing in the company member list. The change is a single-line client-side filter applied after the Copilot API response is fetched.

  • _getCompanyClients now excludes any ClientResponse where status === 'deleted' before returning the list to callers such as the notification service, task-sharing service, and reminder job.
  • The status field is guaranteed to be a non-null string by the Zod schema (ClientResponseSchema), so the comparison is safe.

Confidence Score: 4/5

The change is a minimal, targeted client-side filter with no side effects on other callers; the status field is always present per the Zod schema, so the comparison cannot panic at runtime.

The fix is correct and well-scoped — deleted clients will no longer surface in the company member list, notification recipients, or task-sharing queries. The only open question is whether the hardcoded lowercase string 'deleted' matches every possible value the upstream Copilot API can return, since the schema accepts any string.

src/utils/CopilotAPI.ts — specifically the status field definition in ClientResponseSchema in src/types/common.ts, which remains an unconstrained string type.

Important Files Changed

Filename Overview
src/utils/CopilotAPI.ts Adds a client-side filter in _getCompanyClients to exclude clients whose status equals the hard-coded string 'deleted'; the fix is correct and the field is always present per the Zod schema, but the status value is an untyped plain string rather than an enum.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant CopilotAPI
    participant CopilotSDK

    Caller->>CopilotAPI: getCompanyClients(companyId)
    CopilotAPI->>CopilotSDK: "listClients({ limit: 10000, companyId })"
    CopilotSDK-->>CopilotAPI: ClientsResponse (includes deleted clients)
    Note over CopilotAPI: filter(c => c.status !== 'deleted')
    CopilotAPI-->>Caller: ClientResponse[] (deleted clients excluded)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant CopilotAPI
    participant CopilotSDK

    Caller->>CopilotAPI: getCompanyClients(companyId)
    CopilotAPI->>CopilotSDK: "listClients({ limit: 10000, companyId })"
    CopilotSDK-->>CopilotAPI: ClientsResponse (includes deleted clients)
    Note over CopilotAPI: filter(c => c.status !== 'deleted')
    CopilotAPI-->>Caller: ClientResponse[] (deleted clients excluded)
Loading

Reviews (1): Last reviewed commit: "Merge branch 'production' into main" | Re-trigger Greptile

Comment thread src/utils/CopilotAPI.ts
console.info('CopilotAPI#_getCompanyClients', this.token)
return (await this.getClients({ limit: 10000, companyId })).data || []
const clients = (await this.getClients({ limit: 10000, companyId })).data || []
return clients.filter((c) => c.status !== 'deleted')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The status field in ClientResponseSchema is typed as z.string() — a plain, unconstrained string. If the Copilot API ever returns a differently-cased value (e.g. 'Deleted') or a related variant (e.g. 'deactivated'), the filter will silently miss those clients. Consider narrowing the schema to a z.enum to make valid statuses explicit and catch unexpected values at parse time.

Suggested change
return clients.filter((c) => c.status !== 'deleted')
return clients.filter((c) => c.status?.toLowerCase() !== 'deleted')

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@priosshrsth priosshrsth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@arpandhakal
arpandhakal merged commit 4607f08 into production Jun 30, 2026
2 checks passed
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