Skip to content

Commit 1456e15

Browse files
authored
Fix oneOf nested fields rendering incorrectly in webhook docs (#56270)
1 parent 3f1644e commit 1456e15

File tree

3 files changed

+738
-22
lines changed

3 files changed

+738
-22
lines changed

src/rest/scripts/utils/get-body-params.ts

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,19 @@ async function getTopLevelOneOfProperty(
7474
}
7575

7676
// Gets the body parameters for a given schema recursively.
77+
// Helper function to handle oneOf fields where all items are objects
78+
async function handleObjectOnlyOneOf(
79+
param: Schema,
80+
paramType: string[],
81+
): Promise<TransformedParam[]> {
82+
if (param.oneOf && param.oneOf.every((object: TransformedParam) => object.type === 'object')) {
83+
paramType.push('object')
84+
param.oneOfObject = true
85+
return await getOneOfChildParams(param)
86+
}
87+
return []
88+
}
89+
7790
export async function getBodyParams(schema: Schema, topLevel = false): Promise<TransformedParam[]> {
7891
const bodyParametersParsed: TransformedParam[] = []
7992
const schemaObject = schema.oneOf && topLevel ? await getTopLevelOneOfProperty(schema) : schema
@@ -161,38 +174,45 @@ export async function getBodyParams(schema: Schema, topLevel = false): Promise<T
161174
}
162175
} else if (paramType.includes('object')) {
163176
if (param.oneOf) {
164-
if (param.oneOf.every((object: TransformedParam) => object.type === 'object')) {
165-
param.oneOfObject = true
166-
childParamsGroups.push(...(await getOneOfChildParams(param)))
177+
const oneOfChildren = await handleObjectOnlyOneOf(param, paramType)
178+
if (oneOfChildren.length > 0) {
179+
childParamsGroups.push(...oneOfChildren)
167180
}
168181
} else {
169182
childParamsGroups.push(...(await getBodyParams(param, false)))
170183
}
171184
} else if (param.oneOf) {
172-
const descriptions: { type: string; description: string }[] = []
173-
for (const childParam of param.oneOf) {
174-
paramType.push(childParam.type)
175-
if (!param.description) {
176-
if (childParam.type === 'array') {
177-
if (childParam.items.description) {
178-
descriptions.push({
179-
type: childParam.type,
180-
description: childParam.items.description,
181-
})
185+
// Check if all oneOf items are objects - if so, treat this as a oneOfObject case
186+
const oneOfChildren = await handleObjectOnlyOneOf(param, paramType)
187+
if (oneOfChildren.length > 0) {
188+
childParamsGroups.push(...oneOfChildren)
189+
} else {
190+
// Handle mixed types or non-object oneOf cases
191+
const descriptions: { type: string; description: string }[] = []
192+
for (const childParam of param.oneOf) {
193+
paramType.push(childParam.type)
194+
if (!param.description) {
195+
if (childParam.type === 'array') {
196+
if (childParam.items.description) {
197+
descriptions.push({
198+
type: childParam.type,
199+
description: childParam.items.description,
200+
})
201+
}
202+
} else {
203+
if (childParam.description) {
204+
descriptions.push({ type: childParam.type, description: childParam.description })
205+
}
182206
}
183207
} else {
184-
if (childParam.description) {
185-
descriptions.push({ type: childParam.type, description: childParam.description })
186-
}
208+
descriptions.push({ type: param.type, description: param.description })
187209
}
188-
} else {
189-
descriptions.push({ type: param.type, description: param.description })
190210
}
211+
// Occasionally, there is no parent description and the description
212+
// is in the first child parameter.
213+
const oneOfDescriptions = descriptions.length ? descriptions[0].description : ''
214+
if (!param.description) param.description = oneOfDescriptions
191215
}
192-
// Occasionally, there is no parent description and the description
193-
// is in the first child parameter.
194-
const oneOfDescriptions = descriptions.length ? descriptions[0].description : ''
195-
if (!param.description) param.description = oneOfDescriptions
196216

197217
// This is a workaround for an operation that incorrectly defines anyOf
198218
// for a body parameter. We use the first object in the list of the anyOf array.

src/webhooks/tests/oneof-handling.js

Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,268 @@
1+
import { describe, expect, test } from 'vitest'
2+
import { getBodyParams } from '../../rest/scripts/utils/get-body-params'
3+
4+
describe('oneOf handling in webhook parameters', () => {
5+
test('should handle oneOf fields correctly for secret_scanning_alert_location details', async () => {
6+
// Mock schema representing the secret_scanning_alert_location details field
7+
// This simulates the structure found in the actual OpenAPI schema
8+
const mockSchema = {
9+
type: 'object',
10+
properties: {
11+
location: {
12+
type: 'object',
13+
properties: {
14+
type: {
15+
type: 'string',
16+
description:
17+
'The location type. Because secrets may be found in different types of resources (ie. code, comments, issues, pull requests, discussions), this field identifies the type of resource where the secret was found.',
18+
enum: [
19+
'commit',
20+
'wiki_commit',
21+
'issue_title',
22+
'issue_body',
23+
'issue_comment',
24+
'discussion_title',
25+
'discussion_body',
26+
'discussion_comment',
27+
'pull_request_title',
28+
'pull_request_body',
29+
'pull_request_comment',
30+
'pull_request_review',
31+
'pull_request_review_comment',
32+
],
33+
},
34+
details: {
35+
oneOf: [
36+
{
37+
type: 'object',
38+
title: 'commit',
39+
description:
40+
"Represents a 'commit' secret scanning location type. This location type shows that a secret was detected inside a commit to a repository.",
41+
properties: {
42+
path: {
43+
type: 'string',
44+
description: 'The file path in the repository',
45+
},
46+
start_line: {
47+
type: 'integer',
48+
description: 'Line number at which the secret starts in the file',
49+
},
50+
end_line: {
51+
type: 'integer',
52+
description: 'Line number at which the secret ends in the file',
53+
},
54+
start_column: {
55+
type: 'integer',
56+
description:
57+
'The column at which the secret starts within the start line when the file is interpreted as 8BIT ASCII',
58+
},
59+
end_column: {
60+
type: 'integer',
61+
description:
62+
'The column at which the secret ends within the end line when the file is interpreted as 8BIT ASCII',
63+
},
64+
blob_sha: {
65+
type: 'string',
66+
description: 'SHA-1 hash ID of the associated blob',
67+
},
68+
blob_url: {
69+
type: 'string',
70+
description: 'The API URL to get the associated blob resource',
71+
},
72+
commit_sha: {
73+
type: 'string',
74+
description: 'SHA-1 hash ID of the associated commit',
75+
},
76+
commit_url: {
77+
type: 'string',
78+
description: 'The API URL to get the associated commit resource',
79+
},
80+
},
81+
},
82+
{
83+
type: 'object',
84+
title: 'issue_title',
85+
description:
86+
"Represents an 'issue_title' secret scanning location type. This location type shows that a secret was detected in the title of an issue.",
87+
properties: {
88+
issue_title_url: {
89+
type: 'string',
90+
description: 'The API URL to get the associated issue resource',
91+
},
92+
},
93+
},
94+
{
95+
type: 'object',
96+
title: 'issue_body',
97+
description:
98+
"Represents an 'issue_body' secret scanning location type. This location type shows that a secret was detected in the body of an issue.",
99+
properties: {
100+
issue_body_url: {
101+
type: 'string',
102+
description: 'The API URL to get the associated issue resource',
103+
},
104+
},
105+
},
106+
{
107+
type: 'object',
108+
title: 'issue_comment',
109+
description:
110+
"Represents an 'issue_comment' secret scanning location type. This location type shows that a secret was detected in a comment on an issue.",
111+
properties: {
112+
issue_comment_url: {
113+
type: 'string',
114+
description: 'The API URL to get the associated issue comment resource',
115+
},
116+
},
117+
},
118+
],
119+
},
120+
},
121+
},
122+
},
123+
}
124+
125+
const result = await getBodyParams(mockSchema, true)
126+
127+
// Find the location parameter
128+
const locationParam = result.find((param) => param.name === 'location')
129+
expect(locationParam).toBeDefined()
130+
expect(locationParam?.childParamsGroups).toBeDefined()
131+
132+
// Find the details parameter within location
133+
const detailsParam = locationParam?.childParamsGroups?.find((param) => param.name === 'details')
134+
expect(detailsParam).toBeDefined()
135+
expect(detailsParam?.type).toBe('object')
136+
137+
// Verify that oneOf handling created multiple child param groups
138+
expect(detailsParam?.childParamsGroups).toBeDefined()
139+
expect(detailsParam?.childParamsGroups?.length).toBeGreaterThan(1)
140+
141+
// Check that we have the expected oneOf objects
142+
const childParams = detailsParam?.childParamsGroups || []
143+
const commitParam = childParams.find((param) => param.name === 'commit')
144+
const issueTitleParam = childParams.find((param) => param.name === 'issue_title')
145+
const issueBodyParam = childParams.find((param) => param.name === 'issue_body')
146+
const issueCommentParam = childParams.find((param) => param.name === 'issue_comment')
147+
148+
expect(commitParam).toBeDefined()
149+
expect(commitParam?.description).toContain("commit' secret scanning location type")
150+
expect(commitParam?.childParamsGroups?.length).toBeGreaterThan(0)
151+
152+
expect(issueTitleParam).toBeDefined()
153+
expect(issueTitleParam?.description).toContain("issue_title' secret scanning location type")
154+
155+
expect(issueBodyParam).toBeDefined()
156+
expect(issueBodyParam?.description).toContain("issue_body' secret scanning location type")
157+
158+
expect(issueCommentParam).toBeDefined()
159+
expect(issueCommentParam?.description).toContain("issue_comment' secret scanning location type")
160+
161+
// Verify that the oneOfObject flag is set
162+
expect(detailsParam?.oneOfObject).toBe(true)
163+
})
164+
165+
test('should handle oneOf fields with missing titles gracefully', async () => {
166+
const mockSchemaWithoutTitles = {
167+
type: 'object',
168+
properties: {
169+
details: {
170+
oneOf: [
171+
{
172+
type: 'object',
173+
description: 'First option without title',
174+
properties: {
175+
field1: {
176+
type: 'string',
177+
description: 'Field 1',
178+
},
179+
},
180+
},
181+
{
182+
type: 'object',
183+
description: 'Second option without title',
184+
properties: {
185+
field2: {
186+
type: 'string',
187+
description: 'Field 2',
188+
},
189+
},
190+
},
191+
],
192+
},
193+
},
194+
}
195+
196+
const result = await getBodyParams(mockSchemaWithoutTitles, true)
197+
198+
const detailsParam = result.find((param) => param.name === 'details')
199+
expect(detailsParam).toBeDefined()
200+
expect(detailsParam?.childParamsGroups?.length).toBe(2)
201+
202+
// When titles are missing, the name should be undefined or handled gracefully
203+
detailsParam?.childParamsGroups?.forEach((param) => {
204+
expect(param.type).toBe('object')
205+
expect(param.description).toBeDefined()
206+
})
207+
})
208+
209+
test('should handle nested oneOf correctly', async () => {
210+
const mockNestedOneOfSchema = {
211+
type: 'object',
212+
properties: {
213+
wrapper: {
214+
type: 'object',
215+
properties: {
216+
inner: {
217+
oneOf: [
218+
{
219+
type: 'object',
220+
title: 'option_a',
221+
description: 'Option A description',
222+
properties: {
223+
field_a: {
224+
type: 'string',
225+
description: 'Field A',
226+
},
227+
},
228+
},
229+
{
230+
type: 'object',
231+
title: 'option_b',
232+
description: 'Option B description',
233+
properties: {
234+
field_b: {
235+
type: 'string',
236+
description: 'Field B',
237+
},
238+
},
239+
},
240+
],
241+
},
242+
},
243+
},
244+
},
245+
}
246+
247+
const result = await getBodyParams(mockNestedOneOfSchema, true)
248+
249+
const wrapperParam = result.find((param) => param.name === 'wrapper')
250+
expect(wrapperParam).toBeDefined()
251+
252+
const innerParam = wrapperParam?.childParamsGroups?.find((param) => param.name === 'inner')
253+
expect(innerParam).toBeDefined()
254+
expect(innerParam?.oneOfObject).toBe(true)
255+
expect(innerParam?.childParamsGroups?.length).toBe(2)
256+
257+
const optionA = innerParam?.childParamsGroups?.find((param) => param.name === 'option_a')
258+
const optionB = innerParam?.childParamsGroups?.find((param) => param.name === 'option_b')
259+
260+
expect(optionA).toBeDefined()
261+
expect(optionA?.description).toContain('Option A description')
262+
expect(optionA?.childParamsGroups?.length).toBe(1)
263+
264+
expect(optionB).toBeDefined()
265+
expect(optionB?.description).toContain('Option B description')
266+
expect(optionB?.childParamsGroups?.length).toBe(1)
267+
})
268+
})

0 commit comments

Comments
 (0)