Conversation
added 2 commits
April 6, 2026 16:02
… for skillGroup to follow api level api-specificaiton definetion
…s based on the new api-specification structure of skillGroup
C5rogers
requested review from
HabtamuTesafaye,
Copilot,
irumvanselme and
rav3n11
C5rogers
added
bug
There was a problem hiding this comment.
Pull request overview
Refactors the SkillGroup API specifications into an endpoint-oriented folder structure (GET/POST and nested detail routes), and updates backend validation, controllers, OpenAPI generation, and tests to align with the new spec layout.
Changes:
- Reorganized
api-specificationsfor SkillGroup intoGET/,POST/, and[id]/(withparents/+children/) modules, plus_shared/definitions. - Updated backend SkillGroup controller, AJV schema registration, and OpenAPI generator mappings to reference the new spec namespaces.
- Updated unit/integration tests and error-schema unions/snapshots to include the expanded SkillGroup endpoint error codes and schema refs.
Reviewed changes
Copilot reviewed 95 out of 96 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/src/validator.ts | Updates AJV schema registration for refactored SkillGroup specs. |
| backend/src/esco/skillGroup/index.ts | Refactors controller schema/error-code references to new SkillGroup spec modules. |
| backend/src/esco/skillGroup/index.test.ts | Updates handler unit tests to new SkillGroup error-code namespaces. |
| backend/src/esco/skillGroup/index.integration.test.ts | Updates integration test AJV schema registrations and $id lookups. |
| backend/openapi/generateOpenApiDoc.ts | Updates $id deletions and OpenAPI schema/error mappings for new SkillGroup structure. |
| api-specifications/src/esco/skillGroup/schema.GET.request.query.param.test.ts | Removes legacy SkillGroup GET query schema test (moved under GET/). |
| api-specifications/src/esco/skillGroup/POST/types.ts | Adds POST-specific exported types backed by shared SkillGroup types. |
| api-specifications/src/esco/skillGroup/POST/schema.response.ts | Adjusts POST response schema to use shared base schema location. |
| api-specifications/src/esco/skillGroup/POST/schema.request.ts | Adjusts POST request schema to use shared base properties location. |
| api-specifications/src/esco/skillGroup/POST/schema.request.test.ts | Updates POST request schema tests to POST module structure and shared utilities. |
| api-specifications/src/esco/skillGroup/POST/schema.request.param.ts | Adjusts POST path-param schema to shared base schema location. |
| api-specifications/src/esco/skillGroup/POST/schema.request.param.test.ts | Updates POST param schema test import/usage for new POST module. |
| api-specifications/src/esco/skillGroup/POST/index.ts | Introduces SkillGroup POST module index exporting Schemas/Types/Constants/Enums. |
| api-specifications/src/esco/skillGroup/POST/index.test.ts | Adds POST module index import + snapshot tests. |
| api-specifications/src/esco/skillGroup/POST/enums.ts | Adds POST-specific error-code enums. |
| api-specifications/src/esco/skillGroup/POST/constants.ts | Adds POST constants derived from shared SkillGroup constants. |
| api-specifications/src/esco/skillGroup/POST/snapshots/index.test.ts.snap | Adds POST module snapshot. |
| api-specifications/src/esco/skillGroup/index.ts | Replaces monolithic SkillGroup spec export with GET/POST/[id] module exports + shared. |
| api-specifications/src/esco/skillGroup/index.test.ts | Updates SkillGroup root module tests to new module exports. |
| api-specifications/src/esco/skillGroup/GET/types.ts | Adds GET list endpoint types using shared SkillGroup types. |
| api-specifications/src/esco/skillGroup/GET/schema.response.ts | Adjusts GET response schema to shared base schema/constants/regex imports. |
| api-specifications/src/esco/skillGroup/GET/schema.response.test.ts | Updates GET response schema tests for new module layout/imports. |
| api-specifications/src/esco/skillGroup/GET/schema.request.query.param.ts | Moves GET list query schema under GET/ and points at shared base schema. |
| api-specifications/src/esco/skillGroup/GET/schema.request.query.param.test.ts | Adds GET list query schema tests under GET/. |
| api-specifications/src/esco/skillGroup/GET/schema.request.param.ts | Moves GET list path-param schema under GET/ and points at shared base schema. |
| api-specifications/src/esco/skillGroup/GET/schema.request.param.test.ts | Adds GET list path-param schema tests under GET/. |
| api-specifications/src/esco/skillGroup/GET/index.ts | Introduces SkillGroup GET module index exporting Schemas/Types/Constants/Enums. |
| api-specifications/src/esco/skillGroup/GET/index.test.ts | Adds GET module index import + snapshot tests. |
| api-specifications/src/esco/skillGroup/GET/enums.ts | Adds GET list endpoint error-code enums. |
| api-specifications/src/esco/skillGroup/GET/constants.ts | Adds GET constants derived from shared SkillGroup constants. |
| api-specifications/src/esco/skillGroup/GET/snapshots/index.test.ts.snap | Adds GET module snapshot. |
| api-specifications/src/esco/skillGroup/enums.ts | Removes legacy monolithic SkillGroup enums (moved to _shared/ and per-endpoint enums). |
| api-specifications/src/esco/skillGroup/[id]/types.ts | Adds detail route param types (modelId + id) backed by shared types. |
| api-specifications/src/esco/skillGroup/[id]/schema.request.param.ts | Adds detail route param schema using shared base schema. |
| api-specifications/src/esco/skillGroup/[id]/schema.request.param.test.ts | Adds detail route param schema tests. |
| api-specifications/src/esco/skillGroup/[id]/parents/index.ts | Adds [id]/parents module index. |
| api-specifications/src/esco/skillGroup/[id]/parents/index.test.ts | Adds [id]/parents module index tests + snapshot. |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/types.ts | Adds GET parents operation types backed by shared types. |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/schema.parents.response.ts | Adds parents paginated response schema using shared base schema/constants/regex. |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/schema.parents.response.test.ts | Adds parents response schema tests. |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/schema.parents.request.query.param.ts | Adds parents query schema (limit/cursor) using shared constants/regex. |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/schema.parents.request.query.param.test.ts | Adds parents query schema tests. |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/schema.parent.response.ts | Adds single parent response schema using shared base response schema. |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/schema.parent.response.test.ts | Adds single parent response schema tests. |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/index.ts | Adds parents GET operation index exporting Schemas/Types/Constants/Enums. |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/index.test.ts | Adds parents GET operation index tests + snapshot. |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/enums.ts | Adds parents GET operation enums (objectTypes + error codes). |
| api-specifications/src/esco/skillGroup/[id]/parents/GET/constants.ts | Adds parents GET constants (derived from shared). |
| api-specifications/src/esco/skillGroup/[id]/index.ts | Adds [id] module index exporting detail GET + parents + children + param schema/types. |
| api-specifications/src/esco/skillGroup/[id]/GET/types.ts | Adds detail GET response type backed by shared SkillGroup response type. |
| api-specifications/src/esco/skillGroup/[id]/GET/schema.response.ts | Adds detail GET response schema using shared base response schema. |
| api-specifications/src/esco/skillGroup/[id]/GET/index.ts | Adds detail GET operation index exporting Schemas/Types/Enums. |
| api-specifications/src/esco/skillGroup/[id]/GET/index.test.ts | Adds detail GET operation index tests + snapshot. |
| api-specifications/src/esco/skillGroup/[id]/GET/enums.ts | Adds detail GET enums (objectTypes + error codes). |
| api-specifications/src/esco/skillGroup/[id]/GET/snapshots/index.test.ts.snap | Adds detail GET snapshot. |
| api-specifications/src/esco/skillGroup/[id]/children/index.ts | Adds [id]/children module index. |
| api-specifications/src/esco/skillGroup/[id]/children/index.test.ts | Adds [id]/children module index tests + snapshot. |
| api-specifications/src/esco/skillGroup/[id]/children/GET/types.ts | Adds GET children operation types backed by shared types. |
| api-specifications/src/esco/skillGroup/[id]/children/GET/schema.children.response.ts | Adds children paginated response schema using shared base schema/constants/regex. |
| api-specifications/src/esco/skillGroup/[id]/children/GET/schema.children.response.test.ts | Adds children response schema tests. |
| api-specifications/src/esco/skillGroup/[id]/children/GET/schema.children.request.query.param.ts | Adds children query schema (limit/cursor) using shared constants/regex. |
| api-specifications/src/esco/skillGroup/[id]/children/GET/schema.children.request.query.param.test.ts | Adds children query schema tests. |
| api-specifications/src/esco/skillGroup/[id]/children/GET/schema.child.response.ts | Adds single child response schema using shared base children schema. |
| api-specifications/src/esco/skillGroup/[id]/children/GET/schema.child.response.test.ts | Adds single child response schema tests. |
| api-specifications/src/esco/skillGroup/[id]/children/GET/index.ts | Adds children GET operation index exporting Schemas/Types/Constants/Enums. |
| api-specifications/src/esco/skillGroup/[id]/children/GET/index.test.ts | Adds children GET operation index tests + snapshot. |
| api-specifications/src/esco/skillGroup/[id]/children/GET/enums.ts | Adds children GET operation enums (objectTypes + error codes). |
| api-specifications/src/esco/skillGroup/[id]/children/GET/constants.ts | Adds children GET constants (derived from shared). |
| api-specifications/src/esco/skillGroup/[id]/children/GET/snapshots/index.test.ts.snap | Adds children GET operation snapshot. |
| api-specifications/src/esco/skillGroup/[id]/children/snapshots/index.test.ts.snap | Adds children module snapshot. |
| api-specifications/src/esco/skillGroup/_shared/types.ts | Exports shared SkillGroup interfaces/types for reuse across endpoint modules. |
| api-specifications/src/esco/skillGroup/_shared/schemas.base.ts | Moves base schema definitions to shared module and fixes regex import path. |
| api-specifications/src/esco/skillGroup/_shared/regex.ts | Moves regex definitions to shared module and fixes import paths. |
| api-specifications/src/esco/skillGroup/_shared/enums.ts | Introduces shared relationship enums (parents/children object types). |
| api-specifications/src/esco/skillGroup/_shared/constants.ts | Introduces shared SkillGroup constants used by all endpoint modules. |
| api-specifications/src/esco/skill/schemas.base.ts | Updates Skill schemas to reference SkillGroup shared constants/regex. |
| api-specifications/src/esco/skill/schema.POST.response.test.ts | Updates Skill POST response tests to reference SkillGroup shared constants/regex. |
| api-specifications/src/esco/skill/schema.GET.response.test.ts | Updates Skill GET response tests to reference SkillGroup shared constants/regex. |
| api-specifications/src/esco/skill/relations/parents/types.ts | Updates Skill parent relation types import to SkillGroup shared types. |
| api-specifications/src/esco/skill/relations/parents/schemaTestData.ts | Updates Skill parent relation test data to SkillGroup shared constants. |
| api-specifications/src/esco/skill/relations/parents/schema.GET.parents.response.ts | Updates Skill parents response schema to use SkillGroup shared base schema. |
| api-specifications/src/esco/skill/relations/children/schema.GET.children.response.ts | Updates Skill children response schema to use SkillGroup shared base schema. |
| api-specifications/src/error/types.ts | Expands SkillGroup-related error code unions to include new nested endpoints. |
| api-specifications/src/error/schema.ts | Updates error schema enum list to include new SkillGroup endpoint error codes. |
| api-specifications/src/error/schema.test.ts | Updates error schema tests for expanded SkillGroup error code set. |
| api-specifications/src/error/schema.POST.ts | Updates POST error schema to use SkillGroup POST module error codes. |
| api-specifications/src/error/schema.POST.test.ts | Updates POST error schema tests for SkillGroup POST module error codes. |
| api-specifications/src/error/schema.GET.ts | Updates GET error schema to include nested SkillGroup GET endpoint error codes. |
| api-specifications/src/error/schema.GET.test.ts | Updates GET error schema tests for nested SkillGroup GET endpoint error codes. |
| api-specifications/src/error/get.schema.test.ts | Updates GetErrorSchema tests for expanded SkillGroup error code coverage. |
| api-specifications/src/error/snapshots/index.test.ts.snap | Updates error module snapshot for new SkillGroup error codes. |
Comments suppressed due to low confidence (1)
api-specifications/src/esco/skillGroup/POST/schema.request.param.test.ts:16
- Typo in identifier: "SkillGroupPOStAPISpecs" uses inconsistent casing (POSt). Rename to "SkillGroupPOSTAPISpecs" (or similar consistent name) to match the module name and avoid confusion when grepping/importing across the spec tests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
136
to
+142
| ajvInstance.addSchema( | ||
| SkillGroupAPISpecs.Schemas.GET.Response.ById.Payload, | ||
| SkillGroupAPISpecs.Schemas.GET.Response.ById.Payload.$id | ||
| SkillGroupAPISpecs.SkillGroup.Parent.GET.Schemas.Response.Payload, | ||
| SkillGroupAPISpecs.SkillGroup.Parent.GET.Schemas.Response.Payload.$id | ||
| ); | ||
| ajvInstance.addSchema( | ||
| SkillGroupAPISpecs.Schemas.GET.Parents.Request.Query.Payload, | ||
| SkillGroupAPISpecs.Schemas.GET.Parents.Request.Query.Payload.$id | ||
| SkillGroupAPISpecs.SkillGroup.Parent.GET.Schemas.Response.Parents.Payload, | ||
| SkillGroupAPISpecs.SkillGroup.Parent.GET.Schemas.Response.Parents.Payload.$id |
There was a problem hiding this comment.
The AJV schema registration for SkillGroup parents/children endpoints appears incomplete: the parents endpoint request query schema (SkillGroupAPISpecs.SkillGroup.Parent.GET.Schemas.Request.Query.Payload) is not registered here, even though the API-specifications define a dedicated parents query schema ($id: /components/schemas/SkillGroupParentsRequestQuerySchemaGET). Without registering it, switching the controller to validate against the parents-specific query schema (or any consumer calling ajvInstance.getSchema with that $id) will return undefined at runtime. Register the parents request query schema (and keep response schemas registered separately).
Comment on lines
489
to
493
| }; | ||
|
|
||
| const validateQueryFunction = ajvInstance.getSchema( | ||
| SkillGroupAPISpecs.Schemas.GET.Parents.Request.Query.Payload.$id as string | ||
| SkillGroupAPISpecs.GET.Schemas.Request.Query.Payload.$id as string | ||
| ) as ValidateFunction<SkillGroupAPISpecs.Types.GET.Parents.Request.Query.Payload>; |
There was a problem hiding this comment.
This parents endpoint is validating query parameters against the generic SkillGroup GET query schema. The OpenAPI docs for this route reference a parents-specific query schema, and the api-specifications include SkillGroupAPISpecs.SkillGroup.Parent.GET.Schemas.Request.Query.Payload. Use the parents-specific query schema $id here so validation matches the endpoint contract.
Comment on lines
+671
to
675
| SkillGroupAPISpecs.SkillGroup.Children.GET.Schemas.Request.Query.Payload.$id as string | ||
| ) as ValidateFunction<SkillGroupAPISpecs.SkillGroup.Children.GET.Types.Request.Query.Payload>; | ||
| const isQueryValid = validateQueryFunction(queryParams); | ||
| console.log("we are here 2", { isQueryValid, errors: validateQueryFunction.errors }); | ||
| if (!isQueryValid) { |
There was a problem hiding this comment.
Remove this debug console.log before merge (it will add noise to logs and may leak validation details). If you need structured logging, route it through the existing logger/errorLoggerInstance with an appropriate log level and without dumping full AJV error objects in production logs.
| | SkillGroupAPI.SkillGroup.Parent.GET.Enums.Response.Status400.ErrorCodes | ||
| | SkillGroupAPI.SkillGroup.Parent.GET.Enums.Response.Status404.ErrorCodes | ||
| | SkillGroupAPI.SkillGroup.Parent.GET.Enums.Response.Status500.ErrorCodes | ||
| | SkillGroupAPI.SkillGroup.Parent.GET.Enums.Response.Status500.ErrorCodes |
There was a problem hiding this comment.
This errorCode union includes SkillGroupAPI.SkillGroup.Parent.GET.Enums.Response.Status500.ErrorCodes twice, which is redundant and makes the type harder to maintain. Remove the duplicate entry to avoid confusion and keep the union list canonical.
Suggested change
| | SkillGroupAPI.SkillGroup.Parent.GET.Enums.Response.Status500.ErrorCodes |
Comment on lines
+7
to
+23
| namespace OccupationGroupGETParentTypes { | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const _ = 0; | ||
| export namespace Response { | ||
| export type Payload = ISkillGroupResponse; | ||
| export namespace Parents { | ||
| export type Payload = PaginatedSkillGroupParentsResponse; | ||
| } | ||
| } | ||
| export namespace Request { | ||
| export namespace Query { | ||
| export type Payload = ISkillGroupParentsRequestQuery; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export default OccupationGroupGETParentTypes; |
There was a problem hiding this comment.
The namespace/export name "OccupationGroupGETParentTypes" appears to be a copy/paste artifact in the SkillGroup parents types module. Renaming it to something SkillGroup-specific will make the generated API types easier to understand and prevents confusion when importing types across operations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SkillGroupSkillGroup