improvement(zoho-desk): pick the data center from a dropdown and trim service-account help text - #6271
Merged
Conversation
… service-account help text The Zoho Desk Self Client modal rendered a paragraph of setup steps as the hint under Client secret, duplicating both the setup guide and two of its own field hints. Cut it to the one caveat that isn't derivable from the form, and moved it to the org-identifier field the caveats actually qualify. Data center is now a dropdown sourced from ZOHO_DESK_DATA_CENTERS. Same editorial pass across the other service accounts: Zoom, Salesforce, Shopify, Webflow, Trello and Cal.com dropped setup steps in favor of caveats. Also adds the documented Zoho Desk params that were missing (list_tickets assignee/channel/receivedInDays, list_comments and list_threads sortBy, get_contact and get_thread include), each gated per operation so a stale subBlock value can't leak into an endpoint that reads the same param name.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryCursor Bugbot is generating a summary for commit fa86518. Configure here. |
Contributor
Greptile SummaryThe PR improves Zoho Desk configuration, documentation, and API parameter handling while refining service-account guidance across several providers.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported silent-filter omission is fixed because all non-empty values reach the tool’s explicit validation, while only the empty “Any time” value is omitted.
|
| Filename | Overview |
|---|---|
| apps/sim/blocks/blocks/zoho-desk.ts | Adds operation-scoped parameter mapping and correctly forwards non-empty receivedInDays values to tool validation. |
| apps/sim/tools/zoho_desk/list_tickets.ts | Adds documented ticket filters and rejects unsupported response-window values rather than silently issuing an unfiltered request. |
| apps/sim/tools/zoho_desk/list_tickets.test.ts | Covers the complete block-to-tool merge seam, including malformed, unsupported, supported, and empty receivedInDays inputs. |
| apps/sim/tools/zoho_desk/utils.ts | Centralizes comma-list normalization and enriches Zoho API errors with field-level details. |
| apps/sim/app/workspace/[workspaceId]/integrations/components/connect-service-account-modal/client-credential-account-modal.tsx | Supports descriptor-defined dropdown fields and renders provider help text alongside the relevant identifier field. |
| apps/sim/lib/credentials/client-credential-accounts/descriptors.ts | Defines Zoho Desk data-center options and updates provider-specific field guidance. |
| apps/docs/content/docs/en/integrations/zoho-desk-service-account.mdx | Documents region-specific Self Client registration and the new data-center picker. |
Reviews (4): Last reviewed commit: "fix(zoho-desk): keep the legacy include ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…lidation The block mapper filtered on shape before forwarding, so a fractional or non-numeric value was dropped and List Tickets then ran with no window at all — returning the whole queue as though the requested filter had applied. The tool owns that validation, so the mapper now passes the value straight through. Adds a block-to-tool seam test: neither side's own tests could catch a value lost between them.
Collaborator
Author
Collaborator
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 11c5bd6. Configure here.
… them
The block mapper scoped params by destructuring them out of the spread, on the
assumption that a key left out of the return value never reaches the tool. It
does: both call sites merge the mapper's output on top of the original inputs
(`{ ...inputs, ...transformedParams }`), so an omitted key is restored.
The serializer is what actually held this together, and it has a gap — an
advanced subBlock with a retained value is emitted for every operation while the
block's advanced toggle is off, because that branch returns on isNonEmptyValue
without evaluating the subBlock's condition. So a Sort By set on List Tickets
reached List Comments, and a ticket Include reached Get Contact, each rejected
by Zoho. Out-of-range from/limit reached the wire for the same reason.
Every scoped param is now assigned unconditionally, undefined included, so the
merge cannot resurrect a stale value.
Also fixes a crash this branch introduced: clearing the Departments multi-select
stores [], which reached the comma-list normalizer and threw on .split. The
helper now takes arrays, which is what that subBlock actually stores.
The block-to-tool tests now model the real merge rather than the mapper's return
value alone — the previous version passed while production threw on the same
input. Corrects two comments that misstated where Zoho documents customFields
and errorMessage, and splits the shared include subBlock, since Get Ticket
accepts contract and skills and List Tickets does not.
Collaborator
Author
Collaborator
Author
|
@cursor review |
The previous commit made the mapper assign every operation-scoped param unconditionally, so the merge could not resurrect a stale value. That is right on the canvas path and wrong on the agent-tool path, where `operation` is a sibling of the tool call rather than a member of params: the mapper saw `operation === undefined`, every gate resolved to undefined, and the merge then overwrote the model's own arguments with it. A Zoho Desk tool called by an agent lost every parameter the model supplied. That path needs no scoping — the tool is already chosen, and the model addresses tool params by their real names — so it now returns early. Custom fields are still coerced there, since parsing JSON is a type fix rather than an operation gate, and that parsing is now shared by both paths.
Splitting the shared `include` subBlock into `include` and `ticketInclude` left workflows saved before the split reading an empty field, so their Get Ticket calls silently stopped embedding what they asked for. Get Ticket now reads `ticketInclude ?? include`. The fallback only goes that direction: Get Ticket accepts every value List Tickets does plus `contract` and `skills`, so a legacy value is always valid there, while List Tickets still reads only `include` and can never receive the two extra tokens it does not document.
Collaborator
Author
Collaborator
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 98a37b5. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ZOHO_DESK_DATA_CENTERSso a new region can't leave the picker behind)helpTextoff the Client secret field for all four client-credential providers. None of the four caveats is about the secret (Box = Admin Console authorization, Zoom = Account ID, Salesforce = Run As user, Zoho = triggers) — it now renders on the org-identifier fieldlist_ticketsassignee/channel/receivedInDays,list_commentsandlist_threadssortBy,get_contactandget_threadinclude. Each is gated per operation — a subBlock keeps its value across an operation change, so an ungated spread would send List Tickets'createdTimeto List CommentsgetZohoDeskErrorMessagenow appends Zoho's per-fielderrors[], soINVALID_DATAnames the failing field instead of reading "The data does not comply to the validation restrictions defined."On Holdstill matches)Corrections found while auditing
receivedInDaysfilters oncustomerResponseTime, not receipt — Zoho: "Time period (in days) for fetching tickets based on customerResponseTime". Labelled "Customer Responded Within"; calling it "received" would silently drop every ticket the customer hadn't replied toreceivedInDayswas silently dropped. The param is LLM-writable, so an agent asked for "the last week" sends7and gets the entire unfiltered queue presented as filtered. Now throwsclassificationwas a closed dropdown, but Zoho marks the fieldx-dynamic-enumand states "Custom values are also supported" — portals can rename or replace the picklist. Now free textapi-console.zoho.comwhile offering EU/IN/AU. Zoho pins a client to its registration region ("the accounts-server-url is specific to the location (i.e., datacenter) where the client is registered") and excludes Self Clients from multi-DC, so a non-US admin got a dead credential. Each region now routes to its own consolefrom, and the assertion that Zoho documents""as the clear-a-field idiom (it appears in a sample payload, not in prose)Type of Change
Testing
Every factual claim about Zoho traces to a verbatim quote from
desk.zoho.com/DeskAPIDocument. Tests added for thereceivedInDaysthrow and coercion, the comma-list normalizer, and theerrors[]append — each verified to fail with its fix reverted.Not covered: no live Zoho org has exercised this. The Self Client flow, the data-center dropdown and the new filters are untested against real credentials, and the
soidinference (ZohoDesk.<orgId>) still needs live verification. The block's per-operation gating has no regression test.Checklist