Skip to content

Commit 06960db

Browse files
ericcrosson-bitgoericcrosson-bitgo
authored
Merge pull request #44 from BitGo/DX-998-fix-component-change-detection
fix: fix component change detection in endpoints with multiple methods
2 parents 83c7250 + c3e83ed commit 06960db

File tree

4 files changed

+289
-82
lines changed

4 files changed

+289
-82
lines changed

‎scripts/api-diff.js‎

Lines changed: 76 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const changes = {
1010
removed: {}, // Group by path
1111
modified: {}, // Group by path
1212
components: new Set(), // Track changed components
13-
affectedByComponents: {} // Track paths affected by component changes
13+
affectedByComponents: new Map() // Track path/method combinations affected by component changes
1414
};
1515

1616
// Helper function to track component references
@@ -61,22 +61,21 @@ function findAffectedPaths() {
6161
if (changes.components.size === 0) return;
6262

6363
Object.entries(currentSpec.paths || {}).forEach(([path, methods]) => {
64-
const affectedMethods = [];
6564
Object.entries(methods).forEach(([method, details]) => {
6665
const usedComponents = new Set();
6766
findComponentRefs(details, usedComponents);
6867

6968
for (const comp of usedComponents) {
7069
if (changes.components.has(comp)) {
71-
affectedMethods.push(method.toUpperCase());
72-
if (!changes.affectedByComponents[path]) {
73-
changes.affectedByComponents[path] = {
74-
methods: new Set(),
70+
const key = `${path}::${method.toUpperCase()}`;
71+
if (!changes.affectedByComponents.has(key)) {
72+
changes.affectedByComponents.set(key, {
73+
path,
74+
method: method.toUpperCase(),
7575
components: new Set()
76-
};
76+
});
7777
}
78-
changes.affectedByComponents[path].methods.add(method.toUpperCase());
79-
changes.affectedByComponents[path].components.add(comp);
78+
changes.affectedByComponents.get(key).components.add(comp);
8079
}
8180
}
8281
});
@@ -244,9 +243,6 @@ function findComponentUsage(details, componentName) {
244243

245244
// Generate markdown release notes
246245
function generateReleaseNotes() {
247-
let releaseDescription = '';
248-
249-
250246
const sections = [];
251247

252248
// Added endpoints
@@ -260,84 +256,85 @@ function generateReleaseNotes() {
260256
sections.push(section);
261257
}
262258

263-
// Helper function to generate route modification details
264-
function generateModifiedRouteDetails(path, changes) {
265-
let details = '';
266-
const methodsToProcess = new Set();
259+
// Modified endpoints
260+
if (Object.keys(changes.modified).length > 0 || changes.affectedByComponents.size > 0) {
261+
let section = '## Modified\n';
262+
263+
// First show all directly modified paths
264+
Object.entries(changes.modified)
265+
.sort(([a], [b]) => a.localeCompare(b))
266+
.forEach(([path, methodChanges]) => {
267+
methodChanges
268+
.sort((a, b) => a.method.localeCompare(b.method))
269+
.forEach(({method, changes: methodChanges}) => {
270+
section += `- [${method}] \`${path}\`\n`;
271+
methodChanges.sort().forEach(change => {
272+
section += ` - ${change}\n`;
273+
});
274+
});
275+
});
276+
277+
// Then handle component-affected paths
278+
const componentAffectedPaths = new Map();
267279

268-
// Collect all affected methods
269-
if (changes.modified[path]) {
270-
changes.modified[path].forEach(({method}) => methodsToProcess.add(method));
271-
}
272-
if (changes.affectedByComponents[path]) {
273-
changes.affectedByComponents[path].methods.forEach(method => methodsToProcess.add(method));
280+
for (const [_, value] of changes.affectedByComponents) {
281+
const { path, method, components } = value;
282+
// Skip if this path/method was already shown in direct modifications
283+
if (changes.modified[path]?.some(m => m.method === method)) continue;
284+
285+
if (!componentAffectedPaths.has(path)) {
286+
componentAffectedPaths.set(path, new Map());
287+
}
288+
componentAffectedPaths.get(path).set(method, Array.from(components));
274289
}
275290

276-
// Process each method
277-
Array.from(methodsToProcess)
278-
.sort()
279-
.forEach(method => {
280-
details += `- [${method}] \`${path}\`\n`;
281-
282-
// Add direct changes
283-
const directChanges = changes.modified[path]?.find(m => m.method === method);
284-
if (directChanges) {
285-
directChanges.changes.sort().forEach(change => {
286-
details += ` - ${change}\n`;
287-
});
288-
}
291+
// Show first 5 component-affected paths
292+
const sortedComponentPaths = Array.from(componentAffectedPaths.keys()).sort();
293+
const visibleComponentPaths = sortedComponentPaths.slice(0, 5);
294+
295+
// Add a blank line before component-affected paths if there were direct modifications
296+
if (Object.keys(changes.modified).length > 0 && visibleComponentPaths.length > 0) {
297+
section += '\n';
298+
}
289299

290-
// Add component changes
291-
if (changes.affectedByComponents[path]?.methods.has(method)) {
300+
visibleComponentPaths.forEach(path => {
301+
const methods = componentAffectedPaths.get(path);
302+
Array.from(methods.entries())
303+
.sort(([a], [b]) => a.localeCompare(b))
304+
.forEach(([method, components]) => {
305+
section += `- [${method}] \`${path}\`\n`;
292306
const methodDetails = currentSpec.paths[path][method.toLowerCase()];
293-
Array.from(changes.affectedByComponents[path].components)
307+
components
294308
.sort()
295309
.forEach(component => {
296310
const usageLocations = findComponentUsage(methodDetails, component).sort();
297-
details += ` - \`${component}\` modified in ${usageLocations.join(', ')}\n`;
311+
if (usageLocations.length > 0) {
312+
section += ` - \`${component}\` modified in ${usageLocations.join(', ')}\n`;
313+
}
298314
});
299-
}
300-
});
301-
return details;
302-
}
303-
304-
// Modified endpoints
305-
if (Object.keys(changes.modified).length > 0 || Object.keys(changes.affectedByComponents).length > 0) {
306-
let section = '## Modified\n';
307-
308-
// First show all directly modified paths
309-
const directlyModifiedPaths = Object.keys(changes.modified).sort();
310-
directlyModifiedPaths.forEach(path => {
311-
section += generateModifiedRouteDetails(path, changes);
312-
});
313-
314-
// Then show component-affected paths (but not ones that were directly modified)
315-
const componentAffectedEntries = Object.entries(changes.affectedByComponents)
316-
.filter(([path]) => !changes.modified[path]) // Only paths not already shown above
317-
.flatMap(([path, details]) =>
318-
Array.from(details.methods).map(method => ({path, method}))
319-
)
320-
.sort((a, b) => a.path.localeCompare(b.path) || a.method.localeCompare(b.method));
321-
322-
// Show first 5 component-affected method-path combinations
323-
const visibleEntries = componentAffectedEntries.slice(0, 5);
324-
const processedPaths = new Set();
325-
326-
visibleEntries.forEach(({path}) => {
327-
if (!processedPaths.has(path)) {
328-
section += generateModifiedRouteDetails(path, changes);
329-
processedPaths.add(path);
330-
}
315+
});
331316
});
332317

333-
// Collapse any remaining entries
334-
const remainingEntries = componentAffectedEntries.slice(5);
335-
if (remainingEntries.length > 0) {
318+
// Collapse remaining component-affected paths
319+
const remainingPaths = sortedComponentPaths.slice(5);
320+
if (remainingPaths.length > 0) {
336321
section += '\n<details><summary>Show more routes affected by component changes...</summary>\n\n';
337-
const remainingPaths = new Set();
338-
remainingEntries.forEach(({path}) => remainingPaths.add(path));
339-
Array.from(remainingPaths).sort().forEach(path => {
340-
section += generateModifiedRouteDetails(path, changes);
322+
remainingPaths.forEach(path => {
323+
const methods = componentAffectedPaths.get(path);
324+
Array.from(methods.entries())
325+
.sort(([a], [b]) => a.localeCompare(b))
326+
.forEach(([method, components]) => {
327+
section += `- [${method}] \`${path}\`\n`;
328+
const methodDetails = currentSpec.paths[path][method.toLowerCase()];
329+
components
330+
.sort()
331+
.forEach(component => {
332+
const usageLocations = findComponentUsage(methodDetails, component).sort();
333+
if (usageLocations.length > 0) {
334+
section += ` - \`${component}\` modified in ${usageLocations.join(', ')}\n`;
335+
}
336+
});
337+
});
341338
});
342339
section += '</details>\n';
343340
}
@@ -356,17 +353,14 @@ function generateReleaseNotes() {
356353
sections.push(section);
357354
}
358355

359-
360356
// Sort sections alphabetically and combine
361357
sections.sort((a, b) => {
362358
const titleA = a.split('\n')[0];
363359
const titleB = b.split('\n')[0];
364360
return titleA.localeCompare(titleB);
365361
});
366362

367-
releaseDescription += sections.join('\n');
368-
369-
return releaseDescription;
363+
return sections.join('\n');
370364
}
371365

372366
// Main execution

‎tests/fixtures/modified-components-in-multiple-methods/current.json‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
{
2+
"openapi": "3.0.0",
3+
"info": {
4+
"title": "Test API",
5+
"description": "A sample API for testing"
6+
},
7+
"servers": [
8+
{
9+
"url": "https://api.example.com/v1"
10+
}
11+
],
12+
"paths": {
13+
"/user": {
14+
"get": {
15+
"summary": "Get a list of users",
16+
"responses": {
17+
"200": {
18+
"description": "A list of users",
19+
"content": {
20+
"application/json": {
21+
"schema": {
22+
"type": "array",
23+
"items": {
24+
"$ref": "#/components/schemas/User"
25+
}
26+
}
27+
}
28+
}
29+
}
30+
}
31+
},
32+
"post": {
33+
"summary": "Create a new user profile",
34+
"requestBody": {
35+
"required": true,
36+
"content": {
37+
"application/json": {
38+
"schema": {
39+
"$ref": "#/components/schemas/NewUser"
40+
}
41+
}
42+
}
43+
},
44+
"responses": {
45+
"201": {
46+
"description": "User created successfully",
47+
"content": {
48+
"application/json": {
49+
"schema": {
50+
"$ref": "#/components/schemas/UserCreated"
51+
}
52+
}
53+
}
54+
}
55+
}
56+
}
57+
}
58+
},
59+
"components": {
60+
"schemas": {
61+
"User": {
62+
"type": "object",
63+
"properties": {
64+
"userId": {
65+
"type": "string"
66+
},
67+
"name": {
68+
"type": "string"
69+
},
70+
"email": {
71+
"$ref": "#/components/schemas/Email"
72+
}
73+
}
74+
},
75+
"UserCreated": {
76+
"type": "object",
77+
"properties": {
78+
"userId": {
79+
"type": "string"
80+
}
81+
}
82+
},
83+
"Email": {
84+
"type": "string",
85+
"format": "email"
86+
},
87+
"NewUser": {
88+
"type": "object",
89+
"required": [
90+
"name",
91+
"email"
92+
],
93+
"properties": {
94+
"name": {
95+
"type": "string"
96+
},
97+
"email": {
98+
"type": "string"
99+
}
100+
}
101+
}
102+
}
103+
}
104+
}

‎tests/fixtures/modified-components-in-multiple-methods/expected.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
## Modified
2+
- [GET] `/user`
3+
- `User` modified in responses
4+
- [POST] `/user`
5+
- `UserCreated` modified in responses

0 commit comments

Comments
 (0)