diff --git a/src/lib/openapi/util/openapi-tags.ts b/src/lib/openapi/util/openapi-tags.ts index 0647955969..e04a5ed4a7 100644 --- a/src/lib/openapi/util/openapi-tags.ts +++ b/src/lib/openapi/util/openapi-tags.ts @@ -1,3 +1,6 @@ +// The "official" OpenAPI tags that we use. These tags are added to the OpenAPI +// spec as the root-level "tags" list. Consider creating a new entry here when +// creating a new endpoint. const OPENAPI_TAGS = [ { name: 'Addons', @@ -90,4 +93,6 @@ export const openApiTags = [...OPENAPI_TAGS].sort((a, b) => a.name.localeCompare(b.name), ); -export type OpenApiTag = typeof OPENAPI_TAGS[number]['name']; +export type OpenApiTag = + // The official OpenAPI tags that we use. + typeof OPENAPI_TAGS[number]['name']; diff --git a/src/lib/routes/admin-api/user/pat.ts b/src/lib/routes/admin-api/user/pat.ts index bb3dde1999..c255fb63c8 100644 --- a/src/lib/routes/admin-api/user/pat.ts +++ b/src/lib/routes/admin-api/user/pat.ts @@ -39,7 +39,7 @@ export default class PatController extends Controller { permission: NONE, middleware: [ openApiService.validPath({ - tags: ['admin'], + tags: ['API tokens'], operationId: 'getPats', responses: { 200: createResponseSchema('patsSchema') }, }), @@ -52,7 +52,7 @@ export default class PatController extends Controller { permission: NONE, middleware: [ openApiService.validPath({ - tags: ['admin'], + tags: ['API tokens'], operationId: 'createPat', requestBody: createRequestSchema('patSchema'), responses: { 200: createResponseSchema('patSchema') }, @@ -68,7 +68,7 @@ export default class PatController extends Controller { permission: NONE, middleware: [ openApiService.validPath({ - tags: ['admin'], + tags: ['API tokens'], operationId: 'deletePat', responses: { 200: emptyResponse }, }), diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index 4acf2c079a..d47e435f29 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -7018,7 +7018,7 @@ If the provided project does not exist, the list of events will be empty.", }, }, "tags": [ - "admin", + "API tokens", ], }, "post": { @@ -7047,7 +7047,7 @@ If the provided project does not exist, the list of events will be empty.", }, }, "tags": [ - "admin", + "API tokens", ], }, }, @@ -7070,7 +7070,7 @@ If the provided project does not exist, the list of events will be empty.", }, }, "tags": [ - "admin", + "API tokens", ], }, }, diff --git a/src/test/e2e/api/openapi/openapi.e2e.test.ts b/src/test/e2e/api/openapi/openapi.e2e.test.ts index 9f4ee011a1..732d270156 100644 --- a/src/test/e2e/api/openapi/openapi.e2e.test.ts +++ b/src/test/e2e/api/openapi/openapi.e2e.test.ts @@ -4,6 +4,7 @@ import getLogger from '../../../fixtures/no-logger'; import SwaggerParser from '@apidevtools/swagger-parser'; import enforcer from 'openapi-enforcer'; import semver from 'semver'; +import { openApiTags } from '../../../../lib/openapi/util/openapi-tags'; let app; let db; @@ -100,3 +101,93 @@ test('the generated OpenAPI spec is valid', async () => { expect(enforcerWarning ?? enforcerError).toBe(undefined); }); + +test('all root-level tags are "approved tags"', async () => { + const { body: spec } = await app.request + .get('/docs/openapi.json') + .expect('Content-Type', /json/) + .expect(200); + + const specTags = spec.tags; + const approvedTags = openApiTags; + + // expect spec tags to be a subset of the approved tags + expect(approvedTags).toEqual(expect.arrayContaining(specTags)); +}); +// All tags that are used for OpenAPI path operations must also be listed in the +// OpenAPI root-level tags list. For us, there's two immediate things that make +// this important: +// +// 1. Swagger UI groups operations by tags. To make sure that endpoints are +// listed where users would expect to find them, they should be given an +// appropriate tag. +// +// 2. The OpenAPI/docusaurus integration we use does not generate documentation +// for paths whose tags are not listed in the root-level tags list. +// +// If none of the official tags seem appropriate for an endpoint, consider +// creating a new tag. +test('all tags are listed in the root "tags" list', async () => { + const { body: spec } = await app.request + .get('/docs/openapi.json') + .expect('Content-Type', /json/) + .expect(200); + + const rootLevelTagNames = new Set(spec.tags.map((tag) => tag.name)); + + // dictionary of all invalid tags found in the spec + let invalidTags = {}; + for (const [path, data] of Object.entries(spec.paths)) { + for (const [operation, opData] of Object.entries(data)) { + // ensure that the list of tags for every operation is a subset of + // the list of tags defined on the root level + + // check each tag for this operation + for (const tag of opData.tags) { + if (!rootLevelTagNames.has(tag)) { + // store other invalid tags that already exist on this + // operation + const preExistingTags = + (invalidTags[path] ?? {})[operation]?.invalidTags ?? []; + + // add information about the invalid tag to the invalid tags + // dict. + invalidTags = { + ...invalidTags, + [path]: { + ...invalidTags[path], + [operation]: { + operationId: opData.operationId, + invalidTags: [...preExistingTags, tag], + }, + }, + }; + } + } + } + } + + if (Object.keys(invalidTags).length) { + // create a human-readable list of invalid tags per operation + const msgs = Object.entries(invalidTags).flatMap(([path, data]) => + Object.entries(data).map( + ([operation, opData]) => + `${operation.toUpperCase()} ${path} (operation id: ${ + opData.operationId + }) has the following invalid tags: ${opData.invalidTags + .map((tag) => `"${tag}"`) + .join(', ')}`, + ), + ); + + // format message + const errorMessage = `The OpenAPI spec contains path-level tags that are not listed in the root-level tags object. The relevant paths, operation ids, and tags are as follows:\n\n${msgs.join( + '\n\n', + )}\n\nFor reference, the root-level tags are: ${spec.tags + .map((tag) => `"${tag.name}"`) + .join(', ')}`; + + console.error(errorMessage); + } + expect(invalidTags).toStrictEqual({}); +});