Update user job description handler#343
Conversation
|
There was a problem hiding this comment.
Pull request overview
Adds a new Keycloak-backed HTTP endpoint to update a user’s self-selected job description roles.
Changes:
- Added a new Serverless function definition for
PUT /users/{userId}/job-descriptions. - Implemented the Lambda handler to validate input and update Keycloak user attributes.
- Registered the function in the functions index and
serverless.ts, and bumped the API spec submodule.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/functions/keycloak/update-user-job-description/index.ts | Defines the new HTTP API route and authorizer config for the Lambda. |
| src/functions/keycloak/update-user-job-description/handler.ts | Implements request parsing/validation and Keycloak attribute update. |
| src/functions/index.ts | Exposes the new handler from the functions barrel export. |
| serverless.ts | Registers the new function in the Serverless configuration. |
| home-lambdas-API-spec | Updates the API spec submodule commit pointer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const requestBody = | ||
| typeof event.body === "string" ? JSON.parse(event.body) : event.body; | ||
|
|
||
| const { jobDescriptions } = requestBody as { jobDescriptions: JobDescriptionRole[] }; |
There was a problem hiding this comment.
event.body can be null (and JSON.parse can throw on invalid JSON), which currently falls into the catch and returns a 500. This makes malformed/empty request bodies look like server errors. Consider explicitly handling null/empty bodies and JSON parse failures by returning a 400 with a clear message (e.g., "Request body must be valid JSON").
| @@ -0,0 +1,77 @@ | |||
| import type { APIGatewayProxyEvent, APIGatewayProxyHandler } from "aws-lambda"; | |||
There was a problem hiding this comment.
This function is configured as an httpApi event (API Gateway v2), but the handler is typed as the v1 REST API (APIGatewayProxyHandler/APIGatewayProxyEvent). Even if it “works” at runtime, it’s easy to accidentally rely on the wrong event/response shape. Prefer APIGatewayProxyHandlerV2 / APIGatewayProxyEventV2 for httpApi functions (or change the Serverless event type to match v1 if that’s the intent).
| const updateUserJobDescriptionHandler: APIGatewayProxyHandler = async ( | ||
| event: APIGatewayProxyEvent | ||
| ) => { |
There was a problem hiding this comment.
This function is configured as an httpApi event (API Gateway v2), but the handler is typed as the v1 REST API (APIGatewayProxyHandler/APIGatewayProxyEvent). Even if it “works” at runtime, it’s easy to accidentally rely on the wrong event/response shape. Prefer APIGatewayProxyHandlerV2 / APIGatewayProxyEventV2 for httpApi functions (or change the Serverless event type to match v1 if that’s the intent).
| const requestBody = | ||
| typeof event.body === "string" ? JSON.parse(event.body) : event.body; | ||
|
|
||
| const { jobDescriptions } = requestBody as { jobDescriptions: JobDescriptionRole[] }; |
There was a problem hiding this comment.
The type assertion forces jobDescriptions to be JobDescriptionRole[] before validation, which undermines type-safety and can hide invalid shapes until runtime (e.g., jobDescriptions being non-strings). Consider treating the parsed body as unknown (or { jobDescriptions?: unknown }) and validating that jobDescriptions is an array of strings before checking membership in VALID_JOB_DESCRIPTIONS.
| "TRAINEE" | ||
| ] as const; | ||
|
|
||
| type JobDescriptionRole = (typeof VALID_JOB_DESCRIPTIONS)[number]; |
There was a problem hiding this comment.
JobDescriptionRole reads like an authorization role, but the values represent job descriptions. Renaming to something like JobDescription (or JobDescriptionValue) would better reflect the domain meaning and reduce confusion with Keycloak “roles”.
No description provided.