feat(pagination): update pagination for remaining integrations that support it#3233
feat(pagination): update pagination for remaining integrations that support it#3233waleedlatif1 merged 3 commits intostagingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR adds pagination support to several integrations that previously lacked it: Zendesk, Pipedrive, Supabase, Typeform, and Incident.io. The changes are broadly correct and well-structured — shared helpers ( Key observations:
Confidence Score: 3/5
Important Files Changed
Flowchartflowchart TD
subgraph Zendesk
ZT["get_tickets"] -->|"has filters (status/priority/type/assignee/org)"| ZSE["/search/export\ncursor: page[after]"]
ZT -->|"no filters"| ZTE["/tickets\ncursor: page[after]"]
ZS["search"] --> ZSE2["/search/export\nfilter[type]=ticket|user|org|group\ncursor: page[after]"]
ZU["get_users"] --> ZUE["/users\ncursor: page[after]"]
ZO["get_organizations"] --> ZOE["/organizations\ncursor: page[after]"]
ZSU["search_users"] --> ZSUE["/users/search\noffset: page param\nafter_cursor always null"]
ZAO["autocomplete_organizations"] --> ZAOE["/organizations/autocomplete\noffset: page param\nafter_cursor always null"]
end
subgraph Pipedrive
PD["get_all_deals\nget_projects"] -->|"cursor"| PDC["v2/deals or v1/projects\ncursor-based\nnext_cursor"]
PA["get_activities\nget_pipeline_deals\nget_pipelines\nget_leads\nget_mail_messages\nget_files"] -->|"start offset"| PAS["v1 endpoints\nnext_start from additional_data.pagination"]
end
subgraph Supabase
SQ["query\ntext_search"] -->|"offset param"| SQE["PostgREST REST API\n?offset=N&limit=M"]
end
subgraph Typeform
TF["responses"] -->|"before OR after\ncursor"| TFE["api.typeform.com\n/responses?before=X or ?after=Y"]
end
subgraph "Incident.io"
IU["users_list"] -->|"after cursor"| IUE["api.incident.io/v2/users\n?after=cursor&page_size=N"]
end
Last reviewed commit: d002c73 |
Additional Comments (4)
Add the missing declarations:
The
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
@cursor review |
|
@greptile |
|
@greptile |
|
@cursor review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (params.before) { | ||
| queryParams.push(`before=${encodeURIComponent(params.before)}`) | ||
| } | ||
|
|
||
| if (params.after) { | ||
| queryParams.push(`after=${encodeURIComponent(params.after)}`) | ||
| } |
There was a problem hiding this comment.
Mutually exclusive before/after params can both be sent
The Typeform Responses API treats before and after as mutually exclusive cursors — sending both in the same request produces an error. Since both params are independently optional with no guard, a caller that populates both will get an API error with no explanation at the tool level.
Add a runtime guard before building the URL:
| if (params.before) { | |
| queryParams.push(`before=${encodeURIComponent(params.before)}`) | |
| } | |
| if (params.after) { | |
| queryParams.push(`after=${encodeURIComponent(params.after)}`) | |
| } | |
| if (params.before && params.after) { | |
| throw new Error("Typeform 'before' and 'after' cursor params are mutually exclusive. Provide only one.") | |
| } | |
| if (params.before) { | |
| queryParams.push(`before=${encodeURIComponent(params.before)}`) | |
| } | |
| if (params.after) { | |
| queryParams.push(`after=${encodeURIComponent(params.after)}`) | |
| } |
| limit: { | ||
| type: 'string', | ||
| required: false, | ||
| visibility: 'user-or-llm', | ||
| description: 'Filter by deal status: open, won, lost', | ||
| description: 'Number of results to return (e.g., "50", default: 100, max: 500)', | ||
| }, | ||
| limit: { | ||
| start: { | ||
| type: 'string', | ||
| required: false, | ||
| visibility: 'user-or-llm', | ||
| description: 'Number of results to return (e.g., "50", default: 100, max: 500)', | ||
| description: 'Pagination start offset (0-based index of the first item to return)', | ||
| }, |
There was a problem hiding this comment.
Breaking removal of status filter
The status filter (open, won, lost) that previously existed on this tool has been removed and replaced with start (pagination offset). While adding pagination is the goal of this PR, removing status is a silent breaking change — existing users who relied on filtering pipeline deals by status will silently lose that capability with no migration path or deprecation notice.
The Pipedrive /v1/pipelines/{id}/deals endpoint does support a status query parameter, so adding pagination via start doesn't require removing it. Consider keeping the status param alongside start.
| user_id: { | ||
| type: 'string', | ||
| required: false, | ||
| visibility: 'user-or-llm', | ||
| description: 'Filter activities by deal ID (e.g., "123")', | ||
| }, | ||
| person_id: { | ||
| type: 'string', | ||
| required: false, | ||
| visibility: 'user-or-llm', | ||
| description: 'Filter activities by person ID (e.g., "456")', | ||
| }, | ||
| org_id: { | ||
| type: 'string', | ||
| required: false, | ||
| visibility: 'user-or-llm', | ||
| description: 'Filter activities by organization ID (e.g., "789")', | ||
| description: 'Filter activities by user ID (e.g., "123")', | ||
| }, |
There was a problem hiding this comment.
Breaking removal of deal_id, person_id, org_id filters
The three activity filter params (deal_id, person_id, org_id) have been removed and replaced with user_id. These are all valid filter params on the Pipedrive /v1/activities endpoint and are not mutually exclusive with user_id. Removing them silently breaks existing users who filter activities by deal or person.
The same applies to get_files where deal_id, person_id, and org_id filters were removed entirely (see route.ts and get_files.ts). The Pipedrive /v1/files endpoint does support these filters, so pagination via start doesn't require their removal.
Consider reinstating these filter params alongside the new pagination field.
| const operatorMap: Record<string, string> = { | ||
| plain: 'fts', | ||
| plain: 'plfts', |
There was a problem hiding this comment.
Silent breaking change: plain search operator changed from fts to plfts
The PostgREST operator for plain text search has changed from fts to plfts (plainto_tsquery). While plfts is likely the correct operator for plain text search, this is a silent breaking change for anyone using searchType: 'plain'. fts in PostgREST corresponds to to_tsquery (which accepts raw tsquery syntax), while plfts uses plainto_tsquery (which takes plain text). The results can differ significantly for queries with special characters or operators.
If this is intentional (fixing a pre-existing bug), it warrants a comment explaining the correction.
Additional Comments (2)
The If
Similar to The companion |
1 participant
Summary
Type of Change
Testing
Tested manually
Checklist