Skip to content

Commit 9ac6b7e

Browse files
waleedlatif1claude
andcommitted
fix(manageengine): address review findings
- display_id on problems, changes and solutions reused the timestamp property map, so generated metadata described a record number like "140" as epoch milliseconds. Give it its own map. - parseSdpResponse turned a non-empty, non-JSON 2xx body into {} and reported success — a proxy or login page answering 200 with HTML would make a read look empty and a delete look like it worked. Throw instead; an empty body is still tolerated. - Scope the docs setup step: connect a Zoho account, with client registration called out as self-hosted only. - The block described notes on "any ITSM record"; only requests, problems and changes have note endpoints here. - Assert all ten data-center hosts, not four, and guard the count so a new code fails the test until asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HbUGaP3XyrSm3Lk3k959j2
1 parent 9c2fc99 commit 9ac6b7e

7 files changed

Lines changed: 93 additions & 9 deletions

File tree

‎apps/docs/content/docs/integrations/manageengine_sdp.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ This integration covers 31 operations across five modules — full create, read,
3333

3434
**Before you start**
3535

36-
Register a client in the [Zoho API console](https://api-console.zoho.com/) and configure it as Sim's Zoho OAuth client. The same client serves Zoho Desk and ServiceDesk Plus — Zoho scopes are chosen per authorization request, not per registered client, so no second app is needed.
36+
Connect your Zoho account from the block's credential field. Self-hosted deployments additionally need Sim's Zoho OAuth client configured in the [Zoho API console](https://api-console.zoho.com/). The same client serves Zoho Desk and ServiceDesk Plus — Zoho scopes are chosen per authorization request, not per registered client, so no second app is needed.
3737

3838
Three things shape what will work:
3939

@@ -54,7 +54,7 @@ Three things shape what will work:
5454

5555
## Usage Instructions
5656

57-
Full read and write access to ManageEngine ServiceDesk Plus Cloud: create, search, update and delete requests, problems, changes, assets and knowledge base solutions, and add notes to any ITSM record. Supports multi-portal accounts. Connecting requires a Zoho account in the US data center.
57+
Full read and write access to ManageEngine ServiceDesk Plus Cloud: create, search, update and delete requests, problems, changes, assets and knowledge base solutions, and add notes to requests, problems and changes. Supports multi-portal accounts. Connecting requires a Zoho account in the US data center.
5858

5959

6060

‎apps/sim/blocks/blocks/manageengine-sdp.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ export const ManageEngineSdpBlock: BlockConfig<SdpResponse> = {
106106
'Manage ServiceDesk Plus Cloud requests, problems, changes, assets, and knowledge base solutions',
107107
authMode: AuthMode.OAuth,
108108
longDescription:
109-
'Full read and write access to ManageEngine ServiceDesk Plus Cloud: create, search, update and delete requests, problems, changes, assets and knowledge base solutions, and add notes to any ITSM record. Supports multi-portal accounts. Connecting requires a Zoho account in the US data center.',
109+
'Full read and write access to ManageEngine ServiceDesk Plus Cloud: create, search, update and delete requests, problems, changes, assets and knowledge base solutions, and add notes to requests, problems and changes. Supports multi-portal accounts. Connecting requires a Zoho account in the US data center.',
110110
docsLink: 'https://docs.sim.ai/integrations/manageengine_sdp',
111111
category: 'tools',
112112
integrationType: IntegrationType.Support,

‎apps/sim/tools/generated/tool-outputs.ts‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

‎apps/sim/tools/manageengine_sdp/types.ts‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,25 @@ const TIME_PROPERTIES: Record<string, ToolOutputProperty> = {
255255
value: { type: 'string', description: 'Epoch milliseconds as a string', optional: true },
256256
}
257257

258+
/**
259+
* Problems, changes and solutions carry `display_id` as a `{ display_value,
260+
* value }` pair — the same *shape* as a timestamp but not the same meaning:
261+
* `display_value` is the prefixed record number shown in the UI ("PB-140",
262+
* "CH 44", "SOL-1") and `value` is its bare sequence number ("140"). Declared
263+
* separately from {@link TIME_PROPERTIES} so the generated tool metadata never
264+
* tells a consumer that `140` is epoch milliseconds.
265+
*
266+
* Requests differ again: their `display_id` is a plain string.
267+
*/
268+
const DISPLAY_ID_PROPERTIES: Record<string, ToolOutputProperty> = {
269+
display_value: {
270+
type: 'string',
271+
description: 'Record number as shown in the UI, e.g. PB-140',
272+
optional: true,
273+
},
274+
value: { type: 'string', description: 'Bare sequence number, e.g. 140', optional: true },
275+
}
276+
258277
const USER_PROPERTIES: Record<string, ToolOutputProperty> = {
259278
id: { type: 'string', description: 'User ID', optional: true },
260279
name: { type: 'string', description: 'User name', optional: true },
@@ -494,7 +513,7 @@ export const SDP_PROBLEM_PROPERTIES: Record<string, ToolOutputProperty> = {
494513
description: 'Problem number shown in the SDP UI',
495514
optional: true,
496515
nullable: true,
497-
properties: TIME_PROPERTIES,
516+
properties: DISPLAY_ID_PROPERTIES,
498517
},
499518
title: { type: 'string', description: 'Problem title', optional: true },
500519
description: {
@@ -632,7 +651,7 @@ export const SDP_CHANGE_PROPERTIES: Record<string, ToolOutputProperty> = {
632651
description: 'Change number shown in the SDP UI',
633652
optional: true,
634653
nullable: true,
635-
properties: TIME_PROPERTIES,
654+
properties: DISPLAY_ID_PROPERTIES,
636655
},
637656
title: { type: 'string', description: 'Change title', optional: true },
638657
description: {
@@ -901,7 +920,7 @@ export const SDP_SOLUTION_PROPERTIES: Record<string, ToolOutputProperty> = {
901920
description: 'Solution number shown in the SDP UI',
902921
optional: true,
903922
nullable: true,
904-
properties: TIME_PROPERTIES,
923+
properties: DISPLAY_ID_PROPERTIES,
905924
},
906925
title: { type: 'string', description: 'Solution title', optional: true },
907926
description: {

‎apps/sim/tools/manageengine_sdp/utils.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,27 @@ function jsonResponse(body: unknown, status = 200): Response {
2121
}
2222

2323
describe('resolveSdpBase', () => {
24+
/**
25+
* All ten hosts are asserted literally, not derived from the map under test —
26+
* these are transcribed from the docs' "API endpoints by data center" table,
27+
* and the apex changes with the region (US/EU/IN sit under
28+
* `sdpondemand.manageengine.*`, the rest under their own `servicedeskplus.*`),
29+
* so a typo in any one of them would otherwise ship silently.
30+
*/
2431
it('maps every data center code to its documented host', () => {
2532
expect(resolveSdpBase('US')).toBe('https://sdpondemand.manageengine.com')
2633
expect(resolveSdpBase('EU')).toBe('https://sdpondemand.manageengine.eu')
34+
expect(resolveSdpBase('IN')).toBe('https://sdpondemand.manageengine.in')
2735
expect(resolveSdpBase('AU')).toBe('https://servicedeskplus.net.au')
2836
expect(resolveSdpBase('JP')).toBe('https://servicedeskplus.jp')
37+
expect(resolveSdpBase('CA')).toBe('https://servicedeskplus.ca')
38+
expect(resolveSdpBase('SA')).toBe('https://servicedeskplus.sa')
39+
expect(resolveSdpBase('UK')).toBe('https://servicedeskplus.uk')
40+
expect(resolveSdpBase('CN')).toBe('https://servicedeskplus.cn')
41+
expect(resolveSdpBase('AE')).toBe('https://servicedeskplus.ae')
42+
// Guards the test itself: if a code is added to the map, this fails until
43+
// an assertion for it is added above.
44+
expect(Object.keys(SDP_DATA_CENTER_BASES)).toHaveLength(10)
2945
})
3046

3147
it('accepts a lower-case code, since the value round-trips through workflow JSON', () => {
@@ -225,6 +241,32 @@ describe('parseSdpResponse', () => {
225241
)
226242
})
227243

244+
it('throws on a 2xx whose body is not JSON, rather than reporting success', async () => {
245+
// A proxy or captive login page answering 200 with HTML. Swallowing this
246+
// would report a read as empty and a delete as having succeeded.
247+
const html = new Response('<html><body>Sign in</body></html>', {
248+
status: 200,
249+
headers: { 'Content-Type': 'text/html' },
250+
})
251+
await expect(parseSdpResponse(html, 'Failed to delete request')).rejects.toThrow(/non-JSON/)
252+
})
253+
254+
it('throws on a 2xx whose body is valid JSON but not an object', async () => {
255+
const scalar = new Response('"ok"', {
256+
status: 200,
257+
headers: { 'Content-Type': 'application/json' },
258+
})
259+
await expect(parseSdpResponse(scalar, 'Failed to get request')).rejects.toThrow(/non-JSON/)
260+
})
261+
262+
it('tolerates an empty 2xx body, which carries nothing this layer needs', async () => {
263+
// `null` rather than `''`: the Response constructor rejects a 204 with a body.
264+
const noContent = new Response(null, { status: 204 })
265+
await expect(parseSdpResponse(noContent, 'Failed to delete request')).resolves.toEqual({})
266+
const blank = new Response(' ', { status: 200 })
267+
await expect(parseSdpResponse(blank, 'Failed to delete request')).resolves.toEqual({})
268+
})
269+
228270
it('throws on a 200 whose response_status reports a failure', async () => {
229271
await expect(
230272
parseSdpResponse(

‎apps/sim/tools/manageengine_sdp/utils.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,33 @@ export async function parseSdpResponse(
205205
response: Response,
206206
fallbackMessage: string
207207
): Promise<Record<string, unknown>> {
208-
const data = (await response.json().catch(() => ({}))) as Record<string, unknown>
208+
const text = await response.text().catch(() => '')
209+
let data: Record<string, unknown> = {}
210+
let malformed = false
211+
if (text.trim()) {
212+
try {
213+
const parsed = JSON.parse(text)
214+
data = parsed && typeof parsed === 'object' ? (parsed as Record<string, unknown>) : {}
215+
malformed = data !== parsed
216+
} catch {
217+
malformed = true
218+
}
219+
}
220+
209221
if (!response.ok) {
210222
throw new Error(getSdpErrorMessage(data, `${fallbackMessage} (HTTP ${response.status})`))
211223
}
224+
225+
// A 2xx whose body is present but is not a JSON object did not come from the
226+
// v3 API — a proxy or a captive login page answering 200 with HTML is the
227+
// realistic case. Swallowing it would report a read as empty, and worse,
228+
// report a delete as having succeeded. An empty body is tolerated, since a
229+
// success carries no field this layer requires.
230+
if (malformed) {
231+
throw new Error(
232+
`${fallbackMessage}: ServiceDesk Plus returned a non-JSON response (HTTP ${response.status}). Check the data center and portal settings.`
233+
)
234+
}
212235
const failed = toResponseStatusEntries(data).some((entry) => entry.status === 'failed')
213236
if (failed) {
214237
throw new Error(getSdpErrorMessage(data, fallbackMessage))

‎packages/deployment-config/src/integrations.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14585,7 +14585,7 @@
1458514585
"slug": "manageengine-servicedesk-plus",
1458614586
"name": "ManageEngine ServiceDesk Plus",
1458714587
"description": "Manage ServiceDesk Plus Cloud requests, problems, changes, assets, and knowledge base solutions",
14588-
"longDescription": "Full read and write access to ManageEngine ServiceDesk Plus Cloud: create, search, update and delete requests, problems, changes, assets and knowledge base solutions, and add notes to any ITSM record. Supports multi-portal accounts. Connecting requires a Zoho account in the US data center.",
14588+
"longDescription": "Full read and write access to ManageEngine ServiceDesk Plus Cloud: create, search, update and delete requests, problems, changes, assets and knowledge base solutions, and add notes to requests, problems and changes. Supports multi-portal accounts. Connecting requires a Zoho account in the US data center.",
1458914589
"bgColor": "#FFFFFF",
1459014590
"iconName": "ManageEngineIcon",
1459114591
"docsUrl": "https://docs.sim.ai/integrations/manageengine_sdp",

0 commit comments

Comments
 (0)