Update vacation status when someone is on vacation#392
Conversation
…n-status-when-someone-is-on-vacation
arodriguezvaz
changed the title
There was a problem hiding this comment.
Pull request overview
Adds a new scheduled Lambda that runs daily to transition vacation requests from APPROVED → ONGOING (when the vacation starts) and APPROVED/ONGOING → COMPLETED (after the vacation ends), and wires it into the Serverless deployment.
Changes:
- Introduces
update-vacation-status-by-datescheduled function configuration and handler implementation. - Registers the new function in
serverless.tsso it’s deployed and runnable. - Updates
package-lock.json(dependency tree cleanup).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/functions/vacation-request/update-vacation-status-by-date/index.ts | Adds scheduled event configuration for the new daily vacation status updater Lambda. |
| src/functions/vacation-request/update-vacation-status-by-date/handler.ts | Implements the daily scan + status transition logic and writes updates to DynamoDB. |
| serverless.ts | Registers the new function so it’s included in the deployed Serverless functions list. |
| package-lock.json | Removes some nested dependency entries as part of lockfile regeneration/cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await vacationRequestService.updateVacationRequest({ | ||
| ...vacation, | ||
| status: [ | ||
| { | ||
| status: nextStatus, | ||
| createdBy: "system", | ||
| updatedAt: new Date().toISOString() as unknown as Date | ||
| } | ||
| ], | ||
| updatedAt: new Date().toISOString() |
| const startDate = DateTime.fromISO(vacation.startDate).toISODate(); | ||
| const endDate = DateTime.fromISO(vacation.endDate).toISODate(); | ||
|
|
| @@ -0,0 +1,14 @@ | |||
| import { handlerPath } from "src/libs/handler-resolver"; | |||
|
| vacations.map(async (vacation) => { | ||
| const latestStatus = vacation.status?.at(-1)?.status; | ||
|
|
||
| if (!latestStatus) { | ||
| throw new Error( | ||
| `Vacation request ${vacation.id} has no status, cannot update status by date.` | ||
| ); | ||
| } | ||
|
|
||
| let nextStatus: VacationRequestStatuses = latestStatus; | ||
| const parsedStartDate = DateTime.fromISO(vacation.startDate); | ||
| const parsedEndDate = DateTime.fromISO(vacation.endDate); | ||
| if (!parsedStartDate.isValid || !parsedEndDate.isValid) { | ||
| throw new Error( | ||
| `Vacation request ${vacation.id} has invalid date values: startDate="${vacation.startDate}", endDate="${vacation.endDate}".` | ||
| ); | ||
| } | ||
| const startDate = parsedStartDate.toISODate(); | ||
| const endDate = parsedEndDate.toISODate(); | ||
| if (!startDate || !endDate) { | ||
| throw new Error( | ||
| `Vacation request ${vacation.id} could not be converted to ISO dates: startDate="${vacation.startDate}", endDate="${vacation.endDate}".` | ||
| ); | ||
| } | ||
|
|
||
| if ( | ||
| latestStatus === VacationRequestStatuses.Approved && | ||
| startDate <= today && | ||
| endDate >= today | ||
| ) { | ||
| nextStatus = VacationRequestStatuses.Ongoing; | ||
| } | ||
|
|
||
| if ( | ||
| (latestStatus === VacationRequestStatuses.Approved || | ||
| latestStatus === VacationRequestStatuses.Ongoing) && | ||
| endDate < today | ||
| ) { | ||
| nextStatus = VacationRequestStatuses.Completed; | ||
| } | ||
|
|
||
| if (latestStatus === nextStatus) return; | ||
| const updatedAt = new Date(); | ||
| await vacationRequestService.updateVacationRequest({ | ||
| ...vacation, | ||
| status: [ | ||
| { | ||
| status: nextStatus, | ||
| createdBy: "system", | ||
| updatedAt | ||
| } | ||
| ], | ||
| updatedAt: updatedAt.toISOString() | ||
| }); | ||
|
|
||
| return nextStatus; | ||
| }) | ||
| ); |
There was a problem hiding this comment.
It's a reasonable chunk of logic in this map.
I'd suggest either adding just a couple of comments to make clear what is happening in the various sections e.g. Checking dates to see if status should be updated.
OR you could refactor into a couple of smaller functions e.g.
checkNextStatus might be a reasonable candidate for a function
OR leave as is is also ok.
| if (result.status === "rejected") { | ||
| errors.push(result.reason instanceof Error ? result.reason.message : String(result.reason)); | ||
| } else if (result.value === VacationRequestStatuses.Ongoing) { | ||
| ongoingUpdated += 1; | ||
| } else if (result.value === VacationRequestStatuses.Completed) { | ||
| completedUpdated += 1; | ||
| } |
There was a problem hiding this comment.
Another candidate for a named function perhaps. Or as is is fine
| @@ -0,0 +1,14 @@ | |||
| import { handlerPath } from "@libs/handler-resolver"; | |||
|
|
|||
| const { DAILY_SCHEDULE_TIMER } = process.env; | |||
There was a problem hiding this comment.
Does this env already exists, and daily is the correct interval for this to run?
| throw new Error("Could not resolve current date"); | ||
| } | ||
|
|
||
| const vacations = await vacationRequestService.listVacationRequests(); |
There was a problem hiding this comment.
Is this listing all vacation requests, for all time? Is there some way this could be a little more targeted, otherwise after some years we might end up with quite a lot of results unnecessarily.
Every day, checks if there are vacation requests that should be active or ended, and updates their status to ONGOING or COMPLETED accordingly.
Regarding updatedAt: new Date().toISOString() as unknown as Date — there is likely a better solution than this cast, but I couldn't find one