From 50e371f30f011d93ed28be6cfc3e52764e1976ab Mon Sep 17 00:00:00 2001 From: Dasa122 Date: Sun, 21 Jun 2026 19:18:16 +0000 Subject: [PATCH 1/5] feat(substitution): add conflict check for teacher substitutions --- .../src/routes/timetable/substitution.ts | 249 +++++++++++++----- 1 file changed, 189 insertions(+), 60 deletions(-) diff --git a/apps/chronos/src/routes/timetable/substitution.ts b/apps/chronos/src/routes/timetable/substitution.ts index abe9b2c..1bcdc78 100644 --- a/apps/chronos/src/routes/timetable/substitution.ts +++ b/apps/chronos/src/routes/timetable/substitution.ts @@ -1,6 +1,6 @@ import { zValidator } from '@hono/zod-validator'; import { getLogger } from '@logtape/logtape'; -import { and, eq, gte, inArray, sql } from 'drizzle-orm'; +import { and, eq, gte, inArray, ne, sql } from 'drizzle-orm'; import { HTTPException } from 'hono/http-exception'; import { describeRoute, resolver } from 'hono-openapi'; import { StatusCodes } from 'http-status-codes'; @@ -212,6 +212,121 @@ async function enrichLessons(lessonIds: string[]) { }); } +// Type for both database and transaction instances used by helpers +type TxOrDb = typeof db | Parameters[0]>[0]; + +// Check if a teacher already has a substitution in any of the same periods +// on the same date. Throws 409 CONFLICT if an overlap is detected. +async function checkTeacherSubstitutionConflict( + dbOrTx: TxOrDb, + date: Date, + substituter: string | null | undefined, + lessonIds: string[], + excludeSubstitutionId?: string +): Promise { + // Early return if substituter is null or undefined (no conflict possible) + if (substituter == null) { + return; + } + + // Get period IDs for the incoming lessons + const incomingLessons = await dbOrTx + .select({ periodId: lesson.periodId }) + .from(lesson) + .where(inArray(lesson.id, lessonIds)); + + const incomingPeriodIds = new Set( + incomingLessons + .map((l) => l.periodId) + .filter((id): id is string => id != null) + ); + + // Find existing substitutions for the same date and substituter + const conditions = [ + eq(substitution.date, date), + eq(substitution.substituter, substituter), + ]; + if (excludeSubstitutionId) { + conditions.push(ne(substitution.id, excludeSubstitutionId)); + } + + const existingSubstitutions = await dbOrTx + .select({ id: substitution.id }) + .from(substitution) + .where(and(...conditions)); + + if (existingSubstitutions.length === 0) { + return; + } + + const existingSubIds = existingSubstitutions.map((s) => s.id); + + // Get lesson IDs linked to those existing substitutions + const existingLessons = await dbOrTx + .select({ lessonId: substitutionLessonMTM.lessonId }) + .from(substitutionLessonMTM) + .where(inArray(substitutionLessonMTM.substitutionId, existingSubIds)); + + const existingLessonIds = existingLessons.map((l) => l.lessonId); + + if (existingLessonIds.length === 0) { + return; + } + + // Get period IDs for those linked lessons + const existingLessonPeriods = await dbOrTx + .select({ periodId: lesson.periodId }) + .from(lesson) + .where(inArray(lesson.id, existingLessonIds)); + + const existingPeriodIds = new Set( + existingLessonPeriods + .map((l) => l.periodId) + .filter((id): id is string => id != null) + ); + + // Check for overlap + for (const periodId of incomingPeriodIds) { + if (existingPeriodIds.has(periodId)) { + throw new HTTPException(StatusCodes.CONFLICT, { + message: + 'Teacher already has a substitution in the same period on this date', + }); + } + } +} + +// Resolve effective update values and delegate to checkTeacherSubstitutionConflict. +async function validateUpdateTeacherConflict( + dbOrTx: TxOrDb, + id: string, + body: { + date?: Date | undefined; + substituter?: string | null | undefined; + lessonIds?: string[] | null | undefined; + }, + existing: { date: Date; substituter: string | null } +): Promise { + const effectiveSubstituter = body.substituter ?? existing.substituter; + if (effectiveSubstituter == null) { + return; + } + + const existingLessonRecords = await dbOrTx + .select({ lessonId: substitutionLessonMTM.lessonId }) + .from(substitutionLessonMTM) + .where(eq(substitutionLessonMTM.substitutionId, id)); + const existingLessonIds = existingLessonRecords.map((r) => r.lessonId); + + await checkTeacherSubstitutionConflict( + dbOrTx, + body.date ?? existing.date, + effectiveSubstituter, + body.lessonIds ?? existingLessonIds, + id + ); +} + const substitutionWithRelationsType = '@listof SubstitutionWithRelations @field(.substitution, Substitution) @field(.teacher, Teacher) @field(.lessons, List)'; @@ -492,37 +607,47 @@ export const createSubstitution = timetableFactory.createHandlers( }); } - const result = await db.transaction(async (tx) => { - const [insertedSubstitution] = await tx - .insert(substitution) - .values({ - comment, + const result = await db.transaction( + async (tx) => { + await checkTeacherSubstitutionConflict( + tx, date, - id: crypto.randomUUID(), substituter, - }) - .returning(); + lessonIds + ); - if (!insertedSubstitution) { - throw new HTTPException(StatusCodes.INTERNAL_SERVER_ERROR, { - cause: - env.mode === 'development' - ? 'No substitution returned from insert query' - : undefined, - message: 'Failed to create substitution.', - }); - } + const [insertedSubstitution] = await tx + .insert(substitution) + .values({ + comment, + date, + id: crypto.randomUUID(), + substituter, + }) + .returning(); - // Insert the many-to-many relationships - const mtmValues = lessonIds.map((lessonId) => ({ - lessonId, - substitutionId: insertedSubstitution.id, - })); + if (!insertedSubstitution) { + throw new HTTPException(StatusCodes.INTERNAL_SERVER_ERROR, { + cause: + env.mode === 'development' + ? 'No substitution returned from insert query' + : undefined, + message: 'Failed to create substitution.', + }); + } - await tx.insert(substitutionLessonMTM).values(mtmValues); + // Insert the many-to-many relationships + const mtmValues = lessonIds.map((lessonId) => ({ + lessonId, + substitutionId: insertedSubstitution.id, + })); - return insertedSubstitution; - }); + await tx.insert(substitutionLessonMTM).values(mtmValues); + + return insertedSubstitution; + }, + { isolationLevel: 'serializable' } + ); dispatchPendingNotification(result.id, 'substitution', { date: result.date, @@ -586,13 +711,13 @@ export const updateSubstitution = timetableFactory.createHandlers( const { id } = c.req.valid('param'); const body = c.req.valid('json'); - const existingSubstitution = await db + const [existing] = await db .select() .from(substitution) .where(eq(substitution.id, id)) .limit(1); - if (existingSubstitution.length === 0) { + if (!existing) { throw new HTTPException(StatusCodes.NOT_FOUND, { message: 'Substitution not found', }); @@ -610,43 +735,47 @@ export const updateSubstitution = timetableFactory.createHandlers( } } - // Use a transaction to update the substitution and the many-to-many relationships cancelPendingNotification(id, 'substitution'); - const updatedSubstitution = await db.transaction(async (tx) => { - const [updated] = await tx - .update(substitution) - .set({ - comment: body.comment, - date: body.date ?? undefined, - substituter: body.substituter, - }) - .where(eq(substitution.id, id)) - .returning(); - - // If lessonIds were provided, update the many-to-many relationships - if (body.lessonIds) { - // Delete existing relationships - await tx - .delete(substitutionLessonMTM) - .where(eq(substitutionLessonMTM.substitutionId, id)); - - // Insert new relationships - const mtmValues = body.lessonIds.map((lessonId) => ({ - lessonId, - substitutionId: id, - })); - - await tx.insert(substitutionLessonMTM).values(mtmValues); - } - - return updated; - }); + const updatedSubstitution = await db.transaction( + async (tx) => { + await validateUpdateTeacherConflict(tx, id, body, existing); + + const [updated] = await tx + .update(substitution) + .set({ + comment: body.comment, + date: body.date ?? undefined, + substituter: body.substituter, + }) + .where(eq(substitution.id, id)) + .returning(); + + // If lessonIds were provided, update the many-to-many relationships + if (body.lessonIds) { + // Delete existing relationships + await tx + .delete(substitutionLessonMTM) + .where(eq(substitutionLessonMTM.substitutionId, id)); + + // Insert new relationships + const mtmValues = body.lessonIds.map((lessonId) => ({ + lessonId, + substitutionId: id, + })); + + await tx.insert(substitutionLessonMTM).values(mtmValues); + } + + return updated; + }, + { isolationLevel: 'serializable' } + ); if (updatedSubstitution) { dispatchPendingNotification(id, 'substitution', { - date: body.date ?? existingSubstitution[0]?.date, + date: body.date ?? existing.date, lessonIds: body.lessonIds ?? [], - substituter: body.substituter ?? existingSubstitution[0]?.substituter, + substituter: body.substituter ?? existing.substituter, }); } From 4322b604d642e74f1cbc7e4c5dc7f1af66a45e91 Mon Sep 17 00:00:00 2001 From: Dasa122 Date: Sun, 21 Jun 2026 20:19:07 +0000 Subject: [PATCH 2/5] fix(substitution): handle lessonIds check and notification fallback --- apps/chronos/src/routes/timetable/substitution.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/apps/chronos/src/routes/timetable/substitution.ts b/apps/chronos/src/routes/timetable/substitution.ts index 1bcdc78..4230866 100644 --- a/apps/chronos/src/routes/timetable/substitution.ts +++ b/apps/chronos/src/routes/timetable/substitution.ts @@ -723,7 +723,7 @@ export const updateSubstitution = timetableFactory.createHandlers( }); } - if (body.lessonIds) { + if (body.lessonIds?.length) { const lessonCount = await db.$count( lesson, inArray(lesson.id, body.lessonIds) @@ -735,7 +735,6 @@ export const updateSubstitution = timetableFactory.createHandlers( } } - cancelPendingNotification(id, 'substitution'); const updatedSubstitution = await db.transaction( async (tx) => { await validateUpdateTeacherConflict(tx, id, body, existing); @@ -751,7 +750,7 @@ export const updateSubstitution = timetableFactory.createHandlers( .returning(); // If lessonIds were provided, update the many-to-many relationships - if (body.lessonIds) { + if (body.lessonIds?.length) { // Delete existing relationships await tx .delete(substitutionLessonMTM) @@ -771,10 +770,18 @@ export const updateSubstitution = timetableFactory.createHandlers( { isolationLevel: 'serializable' } ); + cancelPendingNotification(id, 'substitution'); if (updatedSubstitution) { + // Fetch existing lesson IDs for notification fallback + const existingLessonRecords = await db + .select({ lessonId: substitutionLessonMTM.lessonId }) + .from(substitutionLessonMTM) + .where(eq(substitutionLessonMTM.substitutionId, id)); + const existingLessonIds = existingLessonRecords.map((r) => r.lessonId); + dispatchPendingNotification(id, 'substitution', { date: body.date ?? existing.date, - lessonIds: body.lessonIds ?? [], + lessonIds: body.lessonIds ?? existingLessonIds, substituter: body.substituter ?? existing.substituter, }); } From d48709c1bb41adf7a06371520db16c3ae7e63654 Mon Sep 17 00:00:00 2001 From: Dasa122 Date: Mon, 22 Jun 2026 21:03:46 +0000 Subject: [PATCH 3/5] fix(substitution): handle not found case in updateSubstitution --- .../src/routes/timetable/substitution.ts | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/apps/chronos/src/routes/timetable/substitution.ts b/apps/chronos/src/routes/timetable/substitution.ts index 4230866..e079408 100644 --- a/apps/chronos/src/routes/timetable/substitution.ts +++ b/apps/chronos/src/routes/timetable/substitution.ts @@ -770,22 +770,27 @@ export const updateSubstitution = timetableFactory.createHandlers( { isolationLevel: 'serializable' } ); - cancelPendingNotification(id, 'substitution'); - if (updatedSubstitution) { - // Fetch existing lesson IDs for notification fallback - const existingLessonRecords = await db - .select({ lessonId: substitutionLessonMTM.lessonId }) - .from(substitutionLessonMTM) - .where(eq(substitutionLessonMTM.substitutionId, id)); - const existingLessonIds = existingLessonRecords.map((r) => r.lessonId); - - dispatchPendingNotification(id, 'substitution', { - date: body.date ?? existing.date, - lessonIds: body.lessonIds ?? existingLessonIds, - substituter: body.substituter ?? existing.substituter, + if (!updatedSubstitution) { + throw new HTTPException(StatusCodes.NOT_FOUND, { + message: 'Substitution not found', }); } + cancelPendingNotification(id, 'substitution'); + + // Fetch existing lesson IDs for notification fallback + const existingLessonRecords = await db + .select({ lessonId: substitutionLessonMTM.lessonId }) + .from(substitutionLessonMTM) + .where(eq(substitutionLessonMTM.substitutionId, id)); + const existingLessonIds = existingLessonRecords.map((r) => r.lessonId); + + dispatchPendingNotification(id, 'substitution', { + date: body.date ?? existing.date, + lessonIds: body.lessonIds?.length ? body.lessonIds : existingLessonIds, + substituter: body.substituter ?? existing.substituter, + }); + return c.json>({ data: updatedSubstitution, success: true, From e5761a6c13dd7f051c428b268bcf63c774b824ea Mon Sep 17 00:00:00 2001 From: Dasa122 Date: Mon, 22 Jun 2026 21:20:27 +0000 Subject: [PATCH 4/5] fix(substitution): enforce minimum lessonIds requirement --- apps/chronos/src/routes/timetable/substitution.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/chronos/src/routes/timetable/substitution.ts b/apps/chronos/src/routes/timetable/substitution.ts index e079408..c62b958 100644 --- a/apps/chronos/src/routes/timetable/substitution.ts +++ b/apps/chronos/src/routes/timetable/substitution.ts @@ -563,7 +563,7 @@ const createSchema = createInsertSchema(substitution) .omit({ id: true }) .extend({ date: z.coerce.date(), - lessonIds: z.string().array(), + lessonIds: z.string().array().min(1), }); const createResponseSchema = z.object({ @@ -788,7 +788,8 @@ export const updateSubstitution = timetableFactory.createHandlers( dispatchPendingNotification(id, 'substitution', { date: body.date ?? existing.date, lessonIds: body.lessonIds?.length ? body.lessonIds : existingLessonIds, - substituter: body.substituter ?? existing.substituter, + substituter: + 'substituter' in body ? body.substituter : existing.substituter, }); return c.json>({ From 6826e8475b47e92b01fdc37ee3c8dcfcde8be54c Mon Sep 17 00:00:00 2001 From: Dasa122 Date: Mon, 22 Jun 2026 21:34:01 +0000 Subject: [PATCH 5/5] fix(substitution): handle empty lessonIds in updates --- .../src/routes/timetable/substitution.ts | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/apps/chronos/src/routes/timetable/substitution.ts b/apps/chronos/src/routes/timetable/substitution.ts index c62b958..412c18d 100644 --- a/apps/chronos/src/routes/timetable/substitution.ts +++ b/apps/chronos/src/routes/timetable/substitution.ts @@ -229,6 +229,11 @@ async function checkTeacherSubstitutionConflict( return; } + // Early return if no lessons to check (empty array would produce invalid SQL) + if (lessonIds.length === 0) { + return; + } + // Get period IDs for the incoming lessons const incomingLessons = await dbOrTx .select({ periodId: lesson.periodId }) @@ -318,11 +323,14 @@ async function validateUpdateTeacherConflict( .where(eq(substitutionLessonMTM.substitutionId, id)); const existingLessonIds = existingLessonRecords.map((r) => r.lessonId); + const effectiveLessonIds = + body.lessonIds == null ? existingLessonIds : body.lessonIds; + await checkTeacherSubstitutionConflict( dbOrTx, body.date ?? existing.date, effectiveSubstituter, - body.lessonIds ?? existingLessonIds, + effectiveLessonIds, id ); } @@ -749,20 +757,22 @@ export const updateSubstitution = timetableFactory.createHandlers( .where(eq(substitution.id, id)) .returning(); - // If lessonIds were provided, update the many-to-many relationships - if (body.lessonIds?.length) { + // If lessonIds were explicitly provided, replace MTM relationships + if (body.lessonIds != null) { // Delete existing relationships await tx .delete(substitutionLessonMTM) .where(eq(substitutionLessonMTM.substitutionId, id)); - // Insert new relationships - const mtmValues = body.lessonIds.map((lessonId) => ({ - lessonId, - substitutionId: id, - })); + // Insert new relationships (if any — empty array clears all) + if (body.lessonIds.length > 0) { + const mtmValues = body.lessonIds.map((lessonId) => ({ + lessonId, + substitutionId: id, + })); - await tx.insert(substitutionLessonMTM).values(mtmValues); + await tx.insert(substitutionLessonMTM).values(mtmValues); + } } return updated; @@ -787,7 +797,7 @@ export const updateSubstitution = timetableFactory.createHandlers( dispatchPendingNotification(id, 'substitution', { date: body.date ?? existing.date, - lessonIds: body.lessonIds?.length ? body.lessonIds : existingLessonIds, + lessonIds: body.lessonIds == null ? existingLessonIds : body.lessonIds, substituter: 'substituter' in body ? body.substituter : existing.substituter, });