Skip to content

Commit 7a1d73d

Browse files
committed
fix(jira): gate the project selector to operations that use it
`projectId`/`manualProjectId` had no `condition`, so Select Project rendered on all 30 operations, including the 9 whose Jira endpoint accepts no project and whose visible fields do not depend on one. Both members now share one positive condition composed from two hoisted lists: PROJECT_PARAM_OPERATIONS (write, read-bulk, get_project, whose tools declare a projectId param) and ISSUE_KEY_OPERATIONS (the 18 single-issue operations, where the project scopes the issue picker). 21 shown, 9 hidden. ISSUE_KEY_OPERATIONS also replaces four verbatim copies of the same 18-element array previously pasted across issueKey/manualIssueKey's condition and required, which is why those lists drifted apart in the first place. Relabels Read Issue to Get Issue and Read Bulk Issues to Get Bulk Issues to match the sibling Get X convention. Operation ids and tool ids are unchanged, so stored workflows are unaffected; integrations.json is regenerated.
1 parent ad83796 commit 7a1d73d

3 files changed

Lines changed: 312 additions & 100 deletions

File tree

Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Schema gating for the Jira block's resource selectors.
5+
*
6+
* The expectations are derived from the tool contracts rather than hand-copied
7+
* operation lists, so adding an operation that consumes a project fails here
8+
* until the block's `condition` is widened to match.
9+
*/
10+
import { describe, expect, it } from 'vitest'
11+
import { evaluateSubBlockCondition } from '@/lib/workflows/subblocks/visibility'
12+
import { JiraBlock } from '@/blocks/blocks/jira'
13+
import type { SubBlockConfig } from '@/blocks/types'
14+
import {
15+
jiraAddAttachmentTool,
16+
jiraAddCommentTool,
17+
jiraAddWatcherTool,
18+
jiraAddWorklogTool,
19+
jiraAssignIssueTool,
20+
jiraBulkRetrieveTool,
21+
jiraCreateIssueLinkTool,
22+
jiraDeleteAttachmentTool,
23+
jiraDeleteCommentTool,
24+
jiraDeleteIssueLinkTool,
25+
jiraDeleteIssueTool,
26+
jiraDeleteWorklogTool,
27+
jiraGetAttachmentsTool,
28+
jiraGetCommentsTool,
29+
jiraGetFieldsTool,
30+
jiraGetProjectTool,
31+
jiraGetTransitionsTool,
32+
jiraGetUsersTool,
33+
jiraGetWorklogsTool,
34+
jiraListIssueTypesTool,
35+
jiraListProjectsTool,
36+
jiraRemoveWatcherTool,
37+
jiraRetrieveTool,
38+
jiraSearchIssuesTool,
39+
jiraSearchUsersTool,
40+
jiraTransitionIssueTool,
41+
jiraUpdateCommentTool,
42+
jiraUpdateTool,
43+
jiraUpdateWorklogTool,
44+
jiraWriteTool,
45+
} from '@/tools/jira'
46+
import type { ToolConfig } from '@/tools/types'
47+
48+
/**
49+
* The block's tools, keyed by id. Imported directly rather than through
50+
* `@/tools/registry`, which `vitest.setup.ts` mocks to `{}` for import cost.
51+
* Going through the registry would make every contract assertion below pass
52+
* vacuously.
53+
*/
54+
const TOOLS_BY_ID = new Map<string, ToolConfig>(
55+
(
56+
[
57+
jiraAddAttachmentTool,
58+
jiraAddCommentTool,
59+
jiraAddWatcherTool,
60+
jiraAddWorklogTool,
61+
jiraAssignIssueTool,
62+
jiraBulkRetrieveTool,
63+
jiraCreateIssueLinkTool,
64+
jiraDeleteAttachmentTool,
65+
jiraDeleteCommentTool,
66+
jiraDeleteIssueLinkTool,
67+
jiraDeleteIssueTool,
68+
jiraDeleteWorklogTool,
69+
jiraGetAttachmentsTool,
70+
jiraGetCommentsTool,
71+
jiraGetFieldsTool,
72+
jiraGetProjectTool,
73+
jiraGetTransitionsTool,
74+
jiraGetUsersTool,
75+
jiraGetWorklogsTool,
76+
jiraListIssueTypesTool,
77+
jiraListProjectsTool,
78+
jiraRemoveWatcherTool,
79+
jiraRetrieveTool,
80+
jiraSearchIssuesTool,
81+
jiraSearchUsersTool,
82+
jiraTransitionIssueTool,
83+
jiraUpdateCommentTool,
84+
jiraUpdateTool,
85+
jiraUpdateWorklogTool,
86+
jiraWriteTool,
87+
] as ToolConfig[]
88+
).map((tool) => [tool.id, tool])
89+
)
90+
91+
/**
92+
* Operations where the project selector has no role: the endpoint takes no
93+
* project (issue/user search, site-wide lookups, issue links, attachment
94+
* delete) and no visible subblock depends on it.
95+
*/
96+
const PROJECT_HIDDEN_OPERATIONS = [
97+
'search',
98+
'search_users',
99+
'get_users',
100+
'list_projects',
101+
'get_fields',
102+
'create_link',
103+
'delete_link',
104+
'delete_attachment',
105+
'list_issue_types',
106+
] as const
107+
108+
const operationSubBlock = JiraBlock.subBlocks.find((sb) => sb.id === 'operation')
109+
const operationIds = (operationSubBlock?.options as Array<{ id: string }>).map((o) => o.id)
110+
111+
const subBlockById = (id: string): SubBlockConfig => {
112+
const found = JiraBlock.subBlocks.find((sb) => sb.id === id)
113+
if (!found) throw new Error(`Jira block has no subblock "${id}"`)
114+
return found
115+
}
116+
117+
/** Whether a subblock renders for `operation`, using the app's own evaluator. */
118+
const isVisibleFor = (sb: SubBlockConfig, operation: string): boolean =>
119+
evaluateSubBlockCondition(sb.condition, { operation })
120+
121+
/** Whether a subblock is marked required for `operation`. */
122+
const isRequiredFor = (sb: SubBlockConfig, operation: string): boolean => {
123+
if (typeof sb.required === 'boolean') return sb.required
124+
if (!sb.required) return false
125+
return evaluateSubBlockCondition(sb.required, { operation })
126+
}
127+
128+
/** The tool an operation dispatches to, via the block's own switch. */
129+
const toolIdFor = (operation: string): string =>
130+
(JiraBlock.tools.config?.tool as (params: Record<string, string>) => string)({ operation })
131+
132+
/** Whether that tool declares `paramId` as a required input. */
133+
const toolRequiresParam = (operation: string, paramId: string): boolean => {
134+
const toolId = toolIdFor(operation)
135+
const tool = TOOLS_BY_ID.get(toolId)
136+
if (!tool) throw new Error(`No imported tool for "${toolId}" (operation "${operation}")`)
137+
return Boolean(tool.params?.[paramId]?.required)
138+
}
139+
140+
const PROJECT_MEMBERS = ['projectId', 'manualProjectId'] as const
141+
const ISSUE_MEMBERS = ['issueKey', 'manualIssueKey'] as const
142+
143+
describe('Jira block operation gating', () => {
144+
it('exposes every operation exactly once', () => {
145+
expect(new Set(operationIds).size).toBe(operationIds.length)
146+
expect(operationIds).toContain('read')
147+
expect(operationIds).toContain('read-bulk')
148+
})
149+
150+
it('resolves every operation to an imported tool', () => {
151+
// Guards the contract assertions below from passing vacuously: if an
152+
// operation dispatches to a tool this file does not import, fail loudly
153+
// rather than treat its params as absent.
154+
const unresolved = operationIds.filter((operation) => !TOOLS_BY_ID.has(toolIdFor(operation)))
155+
expect(unresolved).toEqual([])
156+
})
157+
158+
describe('project selector matches the tool contract', () => {
159+
it.each(operationIds)(
160+
'shows the project selector for %s whenever its tool requires projectId',
161+
(operation) => {
162+
if (!toolRequiresParam(operation, 'projectId')) return
163+
for (const id of PROJECT_MEMBERS) {
164+
expect(isVisibleFor(subBlockById(id), operation)).toBe(true)
165+
}
166+
}
167+
)
168+
169+
it.each(PROJECT_HIDDEN_OPERATIONS)('hides the project selector for %s', (operation) => {
170+
expect(toolRequiresParam(operation, 'projectId')).toBe(false)
171+
for (const id of PROJECT_MEMBERS) {
172+
expect(isVisibleFor(subBlockById(id), operation)).toBe(false)
173+
}
174+
})
175+
176+
it('keeps the project selector on read-bulk, whose tool requires it', () => {
177+
expect(toolRequiresParam('read-bulk', 'projectId')).toBe(true)
178+
for (const id of PROJECT_MEMBERS) {
179+
expect(isVisibleFor(subBlockById(id), 'read-bulk')).toBe(true)
180+
expect(isRequiredFor(subBlockById(id), 'read-bulk')).toBe(true)
181+
}
182+
})
183+
184+
it('keeps the project selector wherever the issue picker depends on it', () => {
185+
for (const operation of operationIds) {
186+
const issuePickerVisible = isVisibleFor(subBlockById('issueKey'), operation)
187+
if (!issuePickerVisible) continue
188+
expect(subBlockById('issueKey').dependsOn).toContain('projectId')
189+
expect(isVisibleFor(subBlockById('projectId'), operation)).toBe(true)
190+
}
191+
})
192+
})
193+
194+
describe('condition is a superset of required', () => {
195+
it.each([...PROJECT_MEMBERS, ...ISSUE_MEMBERS])(
196+
'%s is visible on every operation where it is required',
197+
(id) => {
198+
const sb = subBlockById(id)
199+
for (const operation of operationIds) {
200+
if (!isRequiredFor(sb, operation)) continue
201+
expect(isVisibleFor(sb, operation)).toBe(true)
202+
}
203+
}
204+
)
205+
})
206+
207+
describe('basic and advanced twins agree', () => {
208+
it.each([PROJECT_MEMBERS, ISSUE_MEMBERS])('%s render identically', (basicId, advancedId) => {
209+
const basic = subBlockById(basicId)
210+
const advanced = subBlockById(advancedId)
211+
expect(basic.canonicalParamId).toBe(advanced.canonicalParamId)
212+
expect(advanced.dependsOn).toEqual(basic.dependsOn)
213+
for (const operation of operationIds) {
214+
expect(isVisibleFor(advanced, operation)).toBe(isVisibleFor(basic, operation))
215+
expect(isRequiredFor(advanced, operation)).toBe(isRequiredFor(basic, operation))
216+
}
217+
})
218+
})
219+
220+
describe('no visible subblock depends on a hidden one', () => {
221+
it.each(operationIds)('%s leaves every rendered dependency satisfiable', (operation) => {
222+
const visible = JiraBlock.subBlocks.filter((sb) => isVisibleFor(sb, operation))
223+
const visibleIds = new Set(visible.map((sb) => sb.id))
224+
for (const sb of visible) {
225+
const deps = Array.isArray(sb.dependsOn) ? sb.dependsOn : []
226+
for (const dep of deps) {
227+
// A dependency satisfied by a canonical twin counts as present.
228+
const depConfigs = JiraBlock.subBlocks.filter(
229+
(candidate) => candidate.id === dep || candidate.canonicalParamId === dep
230+
)
231+
const anyVisible = depConfigs.some((candidate) => visibleIds.has(candidate.id))
232+
expect(
233+
anyVisible,
234+
`"${sb.id}" renders for "${operation}" but its dependency "${dep}" does not`
235+
).toBe(true)
236+
}
237+
}
238+
})
239+
})
240+
241+
describe('operation labels', () => {
242+
const labelFor = (id: string) =>
243+
(operationSubBlock?.options as Array<{ id: string; label: string }>).find((o) => o.id === id)
244+
?.label
245+
246+
it('uses the Get convention for retrieval operations', () => {
247+
expect(labelFor('read')).toBe('Get Issue')
248+
expect(labelFor('read-bulk')).toBe('Get Bulk Issues')
249+
})
250+
251+
it('leaves the persisted operation ids untouched', () => {
252+
expect(operationIds).toContain('read')
253+
expect(operationIds).toContain('read-bulk')
254+
expect(toolIdFor('read')).toBe('jira_retrieve')
255+
expect(toolIdFor('read-bulk')).toBe('jira_bulk_read')
256+
})
257+
})
258+
})

0 commit comments

Comments
 (0)