From 8b8cf4c999075b649cc8c87724082e582a27d6ec Mon Sep 17 00:00:00 2001 From: razyd Date: Sun, 11 Oct 2026 12:10:47 +0200 Subject: [PATCH 1/7] fix(scheduling): keep fixed-day and approved-request cells when generating - generated-rows.ts: builds the rows to insert from the solver matrix and the locked cells, skipping only those stored with source_constraint_id. - schedule-generate.controller.ts: uses it; the locked cells that live only in lockedCells (fixed days, approved requests) are inserted with their locked value. - generated-rows.test.ts: covers both kinds of locked cell and the virtual days of the previous month. Why: generation skipped every locked cell when inserting, while the delete before it only spares the stored ones, so fixed-day and approved-request cells ended empty. Checked against the local database: the two fixed-days employees had 0 cells in the four solver-generated months; regenerating October 2026 gives each 31 (P on weekdays, L at weekends), also in the grid. --- .../schedule-generate.controller.ts | 33 +++---------- backend/services/scheduling/generated-rows.ts | 43 +++++++++++++++++ .../tests/scheduling/generated-rows.test.ts | 48 +++++++++++++++++++ 3 files changed, 98 insertions(+), 26 deletions(-) create mode 100644 backend/services/scheduling/generated-rows.ts create mode 100644 backend/tests/scheduling/generated-rows.test.ts diff --git a/backend/controllers/scheduling/schedule-generate.controller.ts b/backend/controllers/scheduling/schedule-generate.controller.ts index c3b3f213..ebd8e747 100644 --- a/backend/controllers/scheduling/schedule-generate.controller.ts +++ b/backend/controllers/scheduling/schedule-generate.controller.ts @@ -9,6 +9,7 @@ import { buildSolverInput } from '../../services/scheduling/build-solver-input.j import { runSolver } from '../../services/scheduling/solver-client.js' import type { SolverSuccess, SolverOutput } from '../../services/scheduling/types/solver.js' import { solverErrorMessage } from '../../services/scheduling/solver-errors.js' +import { generatedRows } from '../../services/scheduling/generated-rows.js' /** Persiste el registro del run; nunca lanza — error de log no debe romper la respuesta. */ async function recordSolverRun(data: SolverRunRecord): Promise { @@ -164,32 +165,12 @@ export async function generateSchedule(req: Request, res: Response): Promise [d.day_number, d.id])) - const lockedSet = new Set() - for (const [empId, dayDayMap] of Object.entries(solverInput.lockedCells)) { - for (const dayNum of Object.keys(dayDayMap)) { - lockedSet.add(`${empId}:${dayNum}`) - } - } - - const toInsert: { - day_id: number - employee_id: string - shift_code: string - source_constraint_id: null - }[] = [] - for (const [empId, dayShifts] of Object.entries(success.matrix)) { - for (const [dayNumStr, shiftCode] of Object.entries(dayShifts)) { - if (lockedSet.has(`${empId}:${dayNumStr}`)) continue - const dayId = dayMap.get(Number(dayNumStr)) - if (!dayId) continue - toInsert.push({ - day_id: dayId, - employee_id: empId, - shift_code: shiftCode, - source_constraint_id: null, - }) - } - } + const persisted = new Set( + (await repo.getAssignmentsByMonth(monthId)) + .filter((a) => a.source_constraint_id !== null) + .map((a) => `${a.employee_id}:${a.day_id}`) + ) + const toInsert = generatedRows(success.matrix, solverInput.lockedCells, dayMap, persisted) const { deleted, inserted } = await repo.applyGeneratedSchedule(monthId, toInsert) logger.info({ monthId, deleted, inserted }, '[generate] transacción completada') diff --git a/backend/services/scheduling/generated-rows.ts b/backend/services/scheduling/generated-rows.ts new file mode 100644 index 00000000..59deb43e --- /dev/null +++ b/backend/services/scheduling/generated-rows.ts @@ -0,0 +1,43 @@ +// services/scheduling/generated-rows.ts +// Rows to insert after a generation. applyGeneratedSchedule deletes every +// assignment of the month without source_constraint_id, so only the cells +// stored with one survive. The other locked cells (approved requests and +// fixed days) live only in lockedCells and are inserted with their locked +// value; skipping them, as before 2026-10-11, left those cells empty. + +export interface GeneratedRow { + day_id: number + employee_id: string + shift_code: string + source_constraint_id: null +} + +export function generatedRows( + matrix: Record>, + lockedCells: Record>, + dayIdByNumber: ReadonlyMap, + persisted: ReadonlySet // `${employee_id}:${day_id}` stored with source_constraint_id +): GeneratedRow[] { + const cells: Record> = {} + for (const source of [matrix, lockedCells]) { + for (const [empId, dayShifts] of Object.entries(source)) { + cells[empId] = { ...cells[empId], ...dayShifts } + } + } + + const rows: GeneratedRow[] = [] + for (const [empId, dayShifts] of Object.entries(cells)) { + for (const [dayNum, shiftCode] of Object.entries(dayShifts)) { + // Negative keys are the solver's virtual days of the previous month + const dayId = dayIdByNumber.get(Number(dayNum)) + if (!dayId || persisted.has(`${empId}:${dayId}`)) continue + rows.push({ + day_id: dayId, + employee_id: empId, + shift_code: shiftCode, + source_constraint_id: null, + }) + } + } + return rows +} diff --git a/backend/tests/scheduling/generated-rows.test.ts b/backend/tests/scheduling/generated-rows.test.ts new file mode 100644 index 00000000..26dcfb29 --- /dev/null +++ b/backend/tests/scheduling/generated-rows.test.ts @@ -0,0 +1,48 @@ +// tests/scheduling/generated-rows.test.ts +// Regression tests: generating a month skipped every locked cell when +// inserting, while the delete before it only spares cells stored with +// source_constraint_id. Fixed days and approved requests are locked but not +// stored that way, so their cells ended empty (seen in the solver-generated +// months: the fixed-days employees had 0 cells). + +import { describe, it, expect } from 'vitest' +import { generatedRows } from '../../services/scheduling/generated-rows.js' + +const dayIdByNumber = new Map([ + [1, 101], + [2, 102], + [3, 103], +]) + +describe('generatedRows', () => { + it('inserts the locked cells that are not stored, with their locked value', () => { + const rows = generatedRows( + { fixed: { '1': 'P', '2': 'P', '3': 'L' }, other: { '1': 'M', '2': 'T', '3': 'N' } }, + { fixed: { '1': 'P', '2': 'P', '3': 'L' }, other: { '2': 'V' } }, + dayIdByNumber, + new Set() + ) + const cell = (emp: string, day: number) => + rows.find((r) => r.employee_id === emp && r.day_id === day)?.shift_code + expect(rows).toHaveLength(6) + expect([cell('fixed', 101), cell('fixed', 102), cell('fixed', 103)]).toEqual(['P', 'P', 'L']) + expect(cell('other', 102)).toBe('V') // the approved request wins over the matrix + }) + + it('skips the cells stored with source_constraint_id, which the delete keeps', () => { + const rows = generatedRows( + { e: { '1': 'V', '2': 'M' } }, + { e: { '1': 'V' } }, + dayIdByNumber, + new Set(['e:101']) + ) + expect(rows).toEqual([ + { day_id: 102, employee_id: 'e', shift_code: 'M', source_constraint_id: null }, + ]) + }) + + it('ignores the virtual days of the previous month', () => { + const rows = generatedRows({ e: { '-1': 'N', '1': 'L' } }, {}, dayIdByNumber, new Set()) + expect(rows.map((r) => r.day_id)).toEqual([101]) + }) +}) From 1c4899ea8c2c66eeb9be317ddc7e1b18f19b431d Mon Sep 17 00:00:00 2001 From: razyd Date: Sun, 11 Oct 2026 12:12:43 +0200 Subject: [PATCH 2/7] fix(scheduling): make a published month read-only - scheduling-controller.ts: refusePublished answers 409 when the month is published; editing a cell or a day (one or in bulk) and creating, editing, approving or deleting a constraint use it. PUT /months/:id no longer goes back to draft: that is what unpublish is for. Notes can still be edited. - published-month.test.ts: the nine paths answer 409 and write nothing. Why: none of these paths looked at the month status, so a published roster could change under the staff reading it. Checked with curl against the local database: on a published month a cell edit and PUT back to draft answer 409; after unpublish the same edit answers 200. --- .../scheduling/scheduling-controller.ts | 22 ++++ .../tests/scheduling/published-month.test.ts | 120 ++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 backend/tests/scheduling/published-month.test.ts diff --git a/backend/controllers/scheduling/scheduling-controller.ts b/backend/controllers/scheduling/scheduling-controller.ts index 7a733520..0addb5cf 100644 --- a/backend/controllers/scheduling/scheduling-controller.ts +++ b/backend/controllers/scheduling/scheduling-controller.ts @@ -165,6 +165,15 @@ function handleZodError(err: unknown, res: Response): boolean { // CONFIG // ============================================ +// A published month is read-only: its cells, days and constraints change only +// after POST /months/:id/unpublish. Answers 409 and returns true when it is +async function refusePublished(monthId: number, res: Response): Promise { + const month = await repo.getMonthById(monthId) + if (month?.status !== 'published') return false + res.status(409).json({ error: 'El mes está publicado: despublícalo para editarlo' }) + return true +} + export async function getAllConfig(_req: Request, res: Response): Promise { try { const config = await repo.getAllConfig() @@ -867,6 +876,11 @@ export async function updateMonth(req: Request, res: Response): Promise { return } + if (existing.status === 'published' && data.status === 'draft') { + res.status(409).json({ error: 'Para volver a borrador, despublica el mes' }) + return + } + // Handle status changes if (data.status === 'published' && existing.status !== 'published') { await repo.updateMonth(monthId, { @@ -934,6 +948,7 @@ export async function updateDay(req: Request, res: Response): Promise { res.status(404).json({ error: 'Día no encontrado' }) return } + if (await refusePublished(existing.month_id, res)) return await repo.updateDay(dayId, data) const day = await repo.getDayById(dayId) @@ -961,6 +976,7 @@ export async function bulkUpdateDays(req: Request, res: Response): Promise res.status(404).json({ error: 'Mes no encontrado' }) return } + if (await refusePublished(monthId, res)) return const updatedCount = await repo.bulkUpdateDays(data.days) @@ -996,6 +1012,7 @@ export async function updateAssignment(req: Request, res: Response): Promise ({ + getMonthById: vi.fn(), + getDayById: vi.fn(async () => ({ id: 7, month_id: 140 })), + getAssignmentById: vi.fn(async () => ({ id: 9, month_id: 140, shift_code: 'M' })), + getConstraintById: vi.fn(async () => ({ id: 3, month_id: 140, status: 'pending' })), + updateMonth: vi.fn(), + updateDay: vi.fn(), + bulkUpdateDays: vi.fn(), + updateAssignment: vi.fn(), + bulkUpsertAssignments: vi.fn(), + createConstraint: vi.fn(), + updateConstraint: vi.fn(), + deleteConstraint: vi.fn(), + createHistory: vi.fn(), +})) + +const controller = await import('../../controllers/scheduling/scheduling-controller.js') +const repo = await import('../../repositories/scheduling/scheduling-repository.js') + +function call( + handler: (req: Request, res: Response) => Promise, + params: object, + body: object +) { + const res = { status: vi.fn().mockReturnThis(), json: vi.fn().mockReturnThis() } + const req = { user: { id: 'u1' }, params, body } as unknown as Request + return handler(req, res as unknown as Response).then(() => res) +} + +const writes = [ + 'updateMonth', + 'updateDay', + 'bulkUpdateDays', + 'updateAssignment', + 'bulkUpsertAssignments', + 'createConstraint', + 'updateConstraint', + 'deleteConstraint', +] as const + +const cases: Array<[string, () => ReturnType]> = [ + ['updateDay', () => call(controller.updateDay, { dayId: '7' }, { is_holiday: true })], + [ + 'bulkUpdateDays', + () => + call(controller.bulkUpdateDays, { id: '140' }, { days: [{ day_id: 7, is_holiday: true }] }), + ], + [ + 'updateAssignment', + () => call(controller.updateAssignment, { assignmentId: '9' }, { shift_code: 'T' }), + ], + [ + 'bulkUpdateAssignments', + () => + call( + controller.bulkUpdateAssignments, + { id: '140' }, + { assignments: [{ day_id: 7, employee_id: EMPLOYEE, shift_code: 'T' }] } + ), + ], + [ + 'createConstraint', + () => + call( + controller.createConstraint, + {}, + { + month_id: 140, + employee_id: EMPLOYEE, + constraint_type: 'vacation', + start_date: '2026-10-05', + end_date: '2026-10-06', + } + ), + ], + [ + 'updateConstraint', + () => call(controller.updateConstraint, { constraintId: '3' }, { priority: 2 }), + ], + [ + 'approveConstraint', + () => call(controller.approveConstraint, { constraintId: '3' }, { status: 'approved' }), + ], + ['deleteConstraint', () => call(controller.deleteConstraint, { constraintId: '3' }, {})], + [ + 'updateMonth back to draft', + () => call(controller.updateMonth, { id: '140' }, { status: 'draft' }), + ], +] + +describe('a published month is read-only', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(repo.getMonthById).mockResolvedValue({ id: 140, status: 'published' } as never) + }) + + it.each(cases)('%s answers 409 and writes nothing', async (_name, run) => { + const res = await run() + expect(res.status).toHaveBeenCalledWith(409) + for (const write of writes) expect(repo[write], write).not.toHaveBeenCalled() + }) + + it('notes can still be edited on a published month', async () => { + const res = await call(controller.updateMonth, { id: '140' }, { notes: 'Cerrado' }) + expect(res.status).not.toHaveBeenCalledWith(409) + expect(repo.updateMonth).toHaveBeenCalledOnce() + }) +}) From 9d96e603579dde8db0f5ec71a9e52e47fec3812a Mon Sep 17 00:00:00 2001 From: razyd Date: Sun, 11 Oct 2026 12:14:33 +0200 Subject: [PATCH 3/7] fix(scheduling): keep employee dates and order when saving the list - scheduling-repository.ts: setSchedulableEmployees removes only the employees left out and adds only the new ones, in one transaction; the rows that stay keep start_date, end_date, display_order and added_by. An unknown id still fails instead of being ignored. - schedulable-employees.test.ts: the queries it runs, the empty list and the rollback. Why: it cleared scheduling_employees and filled it again, so every save lost the start and end dates and the manual order. Checked with curl against the local database: a start date set on an employee survives saving the same list. --- .../scheduling/scheduling-repository.ts | 32 +++++++++--- .../scheduling/schedulable-employees.test.ts | 51 +++++++++++++++++++ 2 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 backend/tests/scheduling/schedulable-employees.test.ts diff --git a/backend/repositories/scheduling/scheduling-repository.ts b/backend/repositories/scheduling/scheduling-repository.ts index efb90374..fbc8e501 100644 --- a/backend/repositories/scheduling/scheduling-repository.ts +++ b/backend/repositories/scheduling/scheduling-repository.ts @@ -1361,13 +1361,31 @@ export async function setSchedulableEmployees( employeeIds: string[], addedBy?: string ): Promise { - // Clear existing - await db.query(`DELETE FROM scheduling_employees`) - - // Insert new ones - if (employeeIds.length > 0) { - const values = employeeIds.map((id) => [id, addedBy || null]) - await db.query(`INSERT INTO scheduling_employees (employee_id, added_by) VALUES ?`, [values]) + // Remove the ones left out and add the new ones; those that stay keep + // their row (start_date, end_date, display_order, added_by). Clearing the + // table and filling it again lost all of that + const connection = await db.getConnection() + try { + await connection.beginTransaction() + if (employeeIds.length === 0) { + await connection.query(`DELETE FROM scheduling_employees`) + } else { + await connection.query(`DELETE FROM scheduling_employees WHERE employee_id NOT IN (?)`, [ + employeeIds, + ]) + const values = employeeIds.map((id) => [id, addedBy || null]) + await connection.query( + `INSERT INTO scheduling_employees (employee_id, added_by) VALUES ? + ON DUPLICATE KEY UPDATE employee_id = employee_id`, + [values] + ) + } + await connection.commit() + } catch (error) { + await connection.rollback() + throw error + } finally { + connection.release() } } diff --git a/backend/tests/scheduling/schedulable-employees.test.ts b/backend/tests/scheduling/schedulable-employees.test.ts new file mode 100644 index 00000000..6cf7c435 --- /dev/null +++ b/backend/tests/scheduling/schedulable-employees.test.ts @@ -0,0 +1,51 @@ +// tests/scheduling/schedulable-employees.test.ts +// Regression tests: saving the list of schedulable employees cleared +// scheduling_employees and filled it again, so everyone lost start_date, +// end_date, display_order and added_by. Now only the ones left out are +// removed and only the new ones are added, in one transaction. + +import { describe, it, expect, vi, beforeEach } from 'vitest' + +const connection = { + beginTransaction: vi.fn(), + commit: vi.fn(), + rollback: vi.fn(), + release: vi.fn(), + query: vi.fn(async () => [{}]), +} + +vi.mock('../../config/db.js', () => ({ + default: { getConnection: vi.fn(async () => connection), query: vi.fn(), execute: vi.fn() }, +})) + +const { setSchedulableEmployees } = + await import('../../repositories/scheduling/scheduling-repository.js') + +const sql = () => connection.query.mock.calls.map((c) => String(c[0]).replace(/\s+/g, ' ')) + +describe('setSchedulableEmployees', () => { + beforeEach(() => vi.clearAllMocks()) + + it('keeps the rows of the employees that stay', async () => { + await setSchedulableEmployees(['a', 'b'], 'admin') + + expect(sql()).toEqual([ + 'DELETE FROM scheduling_employees WHERE employee_id NOT IN (?)', + 'INSERT INTO scheduling_employees (employee_id, added_by) VALUES ? ON DUPLICATE KEY UPDATE employee_id = employee_id', + ]) + expect(connection.query.mock.calls[0][1]).toEqual([['a', 'b']]) + expect(connection.commit).toHaveBeenCalledOnce() + }) + + it('an empty list clears the table', async () => { + await setSchedulableEmployees([]) + expect(sql()).toEqual(['DELETE FROM scheduling_employees']) + }) + + it('rolls back when the insert fails', async () => { + connection.query.mockResolvedValueOnce([{}]).mockRejectedValueOnce(new Error('fk')) + await expect(setSchedulableEmployees(['x'])).rejects.toThrow('fk') + expect(connection.rollback).toHaveBeenCalledOnce() + expect(connection.commit).not.toHaveBeenCalled() + }) +}) From 89e957ae14ee8bd2f0bf42d7d64ace30ce654cc6 Mon Sep 17 00:00:00 2001 From: razyd Date: Sun, 11 Oct 2026 12:22:28 +0200 Subject: [PATCH 4/7] docs(scheduling): record that the solver stays non-deterministic - decisions.md: 2026-10-11 entry with the measurements (current, seed, one thread, interleave_search) on the local month and the 30x31 benchmark, the decision and how to revisit it. Why: the owner chose generation speed over reproducibility; the TODO entry asked to decide it. --- docs/scheduling/decisions.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/docs/scheduling/decisions.md b/docs/scheduling/decisions.md index 982d97e6..0ec4e18e 100644 --- a/docs/scheduling/decisions.md +++ b/docs/scheduling/decisions.md @@ -1304,3 +1304,36 @@ an L,L pair). The validator (`consecutive-rest.constraint.ts`) matches the spec. The Python solver (`rest.py`) implements it as **sum(rest) ≥ 2** (it does not require them to be consecutive). A historical drift since the start of the Python work. Alignment pending. + +## 2026-10-11 — Solver determinism: left as it is + +**Context.** The solver gets no seed and CP-SAT searches with every core in +parallel, so generating the same month twice can give two different rosters. +Measured on the local October 2026 month (10 employees, 31 days) and on the +synthetic 30×31 benchmark, four runs per setting on 12 cores: + +| Setting | 10 employees | 30×31 | +| --------------------------------------- | --------------------------------------------- | ----------------------------------------- | +| Current (no seed, all threads) | 4 different rosters, all OPTIMAL, ~0.8 s | different rosters, FEASIBLE at 30 s | +| Seed 0, all threads | 3 different rosters out of 4 | — | +| Seed 0, 1 thread | always the same, but no solution in 30 s | — | +| Seed 0, `interleave_search`, 8 threads | always the same, OPTIMAL, ~3.5 s | the same roster, FEASIBLE at 30 s | + +Every roster in a row had the same soft penalty: they are equally good, only +different. With `interleave_search` the result also depends on `num_workers` +(8 and 12 give different rosters), and when the 30 s limit is hit the cut +point depends on the speed of the machine, so local and production could +still differ on large months. + +**Decision.** Keep the current behaviour: no seed, all threads, a wall-clock +limit. Two generations of the same month may differ; both satisfy every hard +rule and have the same penalty. + +**Why.** Reproducibility would cost about 4× the generation time for the +current hotel and would still not hold across machines for months that hit +the time limit. A roster is generated a few times a month and then edited by +hand; the owner chose speed over reproducibility. + +**If it is revisited.** Set `random_seed`, `interleave_search = True` and a +fixed `num_workers` in `model.py`, and replace `max_time_in_seconds` with +`max_deterministic_time` for months that reach the limit. From 1896adbee9048396d53fe5f5e974c646a17d737f Mon Sep 17 00:00:00 2001 From: razyd Date: Mon, 12 Oct 2026 00:38:14 +0200 Subject: [PATCH 5/7] fix(scheduling): keep bulk edits inside their month and reset a month in one transaction - Bulk edits of cells and days answer 400 when a day id is not a day of the month in the URL (countDaysInMonth). - bulkUpsertAssignments reads the locked cells FOR UPDATE and writes the edit in one transaction; a locked cell still answers 409 and now nothing is written. - resetMonthAssignments corrects the dates, deletes the cells and inserts the new grid in one transaction. - fixMonthDates, deleteAllAssignmentsByMonth and upsertAssignment go: they had no other user. - services/scheduling/AGENTS.md: both rules and the real bulk endpoint. - month-scope.test.ts: foreign days, the locked cell and the reset. Why: PUT /months/141/assignments with a day of month 140 rewrote that cell through uk_day_employee, so a published month could be edited from any draft month; and a reset that failed after the delete left the month empty. Checked against the local database: the cross-month edit changed October from November before and answers 400 now, a locked cell answers 409 with nothing written, a reset whose insert fails keeps its 280 cells, and editing a cell from the grid in the browser still saves. --- .../scheduling/scheduling-controller.ts | 83 ++++++---- .../scheduling/scheduling-repository.ts | 154 +++++++++++++----- backend/services/scheduling/AGENTS.md | 11 +- backend/tests/scheduling/month-scope.test.ts | 120 ++++++++++++++ 4 files changed, 285 insertions(+), 83 deletions(-) create mode 100644 backend/tests/scheduling/month-scope.test.ts diff --git a/backend/controllers/scheduling/scheduling-controller.ts b/backend/controllers/scheduling/scheduling-controller.ts index 0addb5cf..67c0cfa4 100644 --- a/backend/controllers/scheduling/scheduling-controller.ts +++ b/backend/controllers/scheduling/scheduling-controller.ts @@ -174,6 +174,16 @@ async function refusePublished(monthId: number, res: Response): Promise return true } +// A bulk edit carries day ids: all must be days of the month in the URL, or a +// day of another month (published or not) would be written through this one. +// Answers 400 and returns true when one is not +async function refuseForeignDays(monthId: number, dayIds: number[], res: Response) { + const unique = [...new Set(dayIds)] + if ((await repo.countDaysInMonth(monthId, unique)) === unique.length) return false + res.status(400).json({ error: 'Hay días que no pertenecen a este mes' }) + return true +} + export async function getAllConfig(_req: Request, res: Response): Promise { try { const config = await repo.getAllConfig() @@ -818,25 +828,23 @@ export async function resetMonth(req: Request, res: Response): Promise { return } - // Fix dates in scheduling_days (timezone issue fix) - const fixedCount = await repo.fixMonthDates(monthId, month.year, month.month) - logger.info({ monthId, fixedCount }, '[resetMonth] Fixed day dates') - - // Delete all assignments - await repo.deleteAllAssignmentsByMonth(monthId) - logger.info({ monthId }, '[resetMonth] Deleted all assignments') - - // Re-pre-load approved constraints as assignments + // The new grid: an L per day and employee, or the code of an approved request const assignments = await initializeMonthGrid(monthId) - // Save assignments if any - if (assignments.length > 0) { - await repo.createAssignmentsBulk(monthId, assignments) - logger.info( - { monthId, assignmentsCount: assignments.length }, - '[resetMonth] Initialized assignments' - ) + // Dates fixed, cells deleted and the grid inserted in one transaction: a + // failure leaves the month as it was, not empty + const fixedCount = await repo.resetMonthAssignments( + monthId, + month.year, + month.month, + assignments + ) + logger.info( + { monthId, fixedCount, assignmentsCount: assignments.length }, + '[resetMonth] Month reset' + ) + if (assignments.length > 0) { // Number libre pairs immediately after seeding const uniqueEmployees = new Set(assignments.map((a) => a.employee_id)) for (const empId of uniqueEmployees) { @@ -977,6 +985,14 @@ export async function bulkUpdateDays(req: Request, res: Response): Promise return } if (await refusePublished(monthId, res)) return + if ( + await refuseForeignDays( + monthId, + data.days.map((d) => d.day_id), + res + ) + ) + return const updatedCount = await repo.bulkUpdateDays(data.days) @@ -1085,29 +1101,24 @@ export async function bulkUpdateAssignments(req: Request, res: Response): Promis } } - // Prevent editing locked cells (preloaded from approved constraints) - for (const assignment of data.assignments) { - const existing = await repo.getAssignmentByDayEmployee( - assignment.day_id, - assignment.employee_id + if ( + await refuseForeignDays( + monthId, + data.assignments.map((a) => a.day_id), + res ) - if (existing?.source_constraint_id) { - res.status(409).json({ - error: - 'Hay celdas bloqueadas por peticiones aprobadas. No se pueden editar desde bulk update.', - }) - return - } - } + ) + return - // Upsert each assignment - for (const assignment of data.assignments) { - await repo.upsertAssignment({ - month_id: monthId, - day_id: assignment.day_id, - employee_id: assignment.employee_id, - shift_code: assignment.shift_code, + // Cells locked by approved requests are checked and the rest written in + // one transaction + const { locked } = await repo.bulkUpsertAssignments(monthId, data.assignments) + if (locked) { + res.status(409).json({ + error: + 'Hay celdas bloqueadas por peticiones aprobadas. No se pueden editar desde bulk update.', }) + return } const affectedEmployees = new Set(data.assignments.map((a) => a.employee_id)) diff --git a/backend/repositories/scheduling/scheduling-repository.ts b/backend/repositories/scheduling/scheduling-repository.ts index fbc8e501..954edc8c 100644 --- a/backend/repositories/scheduling/scheduling-repository.ts +++ b/backend/repositories/scheduling/scheduling-repository.ts @@ -22,7 +22,6 @@ import type { UpdateMonthDTO, CreateDayDTO, UpdateDayDTO, - CreateAssignmentDTO, UpdateAssignmentDTO, BulkAssignmentDTO, CreateConstraintDTO, @@ -572,6 +571,20 @@ export async function updateDay(id: number, data: UpdateDayDTO): Promise 0 } +/** + * How many of `dayIds` (distinct) are days of `monthId`. The bulk edits use it + * to refuse days of another month: uk_day_employee would otherwise rewrite + * that month's cell, published or not. + */ +export async function countDaysInMonth(monthId: number, dayIds: number[]): Promise { + if (dayIds.length === 0) return 0 + const [rows] = await db.query( + 'SELECT COUNT(*) AS n FROM scheduling_days WHERE month_id = ? AND id IN (?)', + [monthId, dayIds] + ) + return Number(rows[0].n) +} + export async function bulkUpdateDays( days: Array<{ day_id: number; is_holiday?: boolean; holiday_name?: string | null }> ): Promise { @@ -721,33 +734,49 @@ export async function updateAssignment(id: number, data: UpdateAssignmentDTO): P return result.affectedRows > 0 } -export async function upsertAssignment(data: CreateAssignmentDTO): Promise { - // Use INSERT ... ON DUPLICATE KEY UPDATE for better performance - // Avoids extra SELECT query before INSERT/UPDATE - const [result] = await db.execute( - `INSERT INTO scheduling_assignments - (month_id, day_id, employee_id, shift_code, source_constraint_id, notes, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, NOW(), NOW()) - ON DUPLICATE KEY UPDATE - shift_code = VALUES(shift_code), - notes = VALUES(notes), - updated_at = NOW()`, - [ - data.month_id, - data.day_id, - data.employee_id, - data.shift_code, - data.source_constraint_id ?? null, - data.notes ?? null, - ] - ) - // insertId is the new ID on insert, or 0 on update - // For updates, we need to fetch the existing ID - if (result.insertId === 0) { - const existing = await getAssignmentByDayEmployee(data.day_id, data.employee_id) - return existing?.id ?? 0 +/** + * Writes the cells of a bulk edit in one transaction. The cells locked by an + * approved request are read FOR UPDATE inside it: if any is in the edit, + * nothing is written and `locked` is true. + */ +export async function bulkUpsertAssignments( + monthId: number, + assignments: { day_id: number; employee_id: string; shift_code: string }[] +): Promise<{ locked: boolean }> { + const connection = await db.getConnection() + try { + await connection.beginTransaction() + + const pairs = assignments.map((a) => [a.day_id, a.employee_id]) + const [locked] = await connection.query( + `SELECT id FROM scheduling_assignments + WHERE (day_id, employee_id) IN (?) AND source_constraint_id IS NOT NULL + FOR UPDATE`, + [pairs] + ) + if (locked.length > 0) { + await connection.rollback() + return { locked: true } + } + + for (const a of assignments) { + await connection.execute( + `INSERT INTO scheduling_assignments + (month_id, day_id, employee_id, shift_code, source_constraint_id, notes, created_at, updated_at) + VALUES (?, ?, ?, ?, NULL, NULL, NOW(), NOW()) + ON DUPLICATE KEY UPDATE shift_code = VALUES(shift_code), updated_at = NOW()`, + [monthId, a.day_id, a.employee_id, a.shift_code] + ) + } + + await connection.commit() + return { locked: false } + } catch (err) { + await connection.rollback() + throw err + } finally { + connection.release() } - return result.insertId } export async function recalculateLibreNumbers(employeeId: string, year: number): Promise { @@ -784,8 +813,60 @@ export async function recalculateLibreNumbers(employeeId: string, year: number): ) } -export async function deleteAllAssignmentsByMonth(monthId: number): Promise { - await db.execute('DELETE FROM scheduling_assignments WHERE month_id = ?', [monthId]) +/** + * Resets a month in one transaction: corrects the day dates, deletes every + * cell and inserts `seed` (an L per day and employee, or the code of an + * approved request). A failure leaves the month as it was. Returns the days + * whose date was corrected. + */ +export async function resetMonthAssignments( + monthId: number, + year: number, + month: number, + seed: BulkAssignmentDTO[] +): Promise { + const connection = await db.getConnection() + try { + await connection.beginTransaction() + + const daysInMonth = new Date(year, month, 0).getDate() + let fixedCount = 0 + for (let day = 1; day <= daysInMonth; day++) { + const correctDate = `${year}-${String(month).padStart(2, '0')}-${String(day).padStart(2, '0')}` + const [result] = await connection.execute( + 'UPDATE scheduling_days SET date = ? WHERE month_id = ? AND day_number = ?', + [correctDate, monthId, day] + ) + fixedCount += result.affectedRows + } + + await connection.execute('DELETE FROM scheduling_assignments WHERE month_id = ?', [monthId]) + + if (seed.length > 0) { + const values = seed.map((a) => [ + monthId, + a.day_id, + a.employee_id, + a.shift_code, + a.source_constraint_id ?? null, + ]) + const placeholders = values.map(() => '(?, ?, ?, ?, ?, NULL, NOW(), NOW())').join(', ') + await connection.query( + `INSERT INTO scheduling_assignments + (month_id, day_id, employee_id, shift_code, source_constraint_id, notes, created_at, updated_at) + VALUES ${placeholders}`, + values.flat() + ) + } + + await connection.commit() + return fixedCount + } catch (err) { + await connection.rollback() + throw err + } finally { + connection.release() + } } /** Borra las asignaciones no bloqueadas e inserta las nuevas en una sola transacción. */ @@ -883,21 +964,6 @@ export async function getAnnualLCountByEmployee( return rows as { employee_id: string; employee_name: string; libre_count: number }[] } -export async function fixMonthDates(monthId: number, year: number, month: number): Promise { - const daysInMonth = new Date(year, month, 0).getDate() - - let fixedCount = 0 - for (let day = 1; day <= daysInMonth; day++) { - const correctDate = `${year}-${String(month).padStart(2, '0')}-${String(day).padStart(2, '0')}` - const [result] = await db.execute( - 'UPDATE scheduling_days SET date = ? WHERE month_id = ? AND day_number = ?', - [correctDate, monthId, day] - ) - fixedCount += (result as ResultSetHeader).affectedRows - } - return fixedCount -} - // ============================================ // CONSTRAINTS // ============================================ diff --git a/backend/services/scheduling/AGENTS.md b/backend/services/scheduling/AGENTS.md index b3a1515e..04672277 100644 --- a/backend/services/scheduling/AGENTS.md +++ b/backend/services/scheduling/AGENTS.md @@ -279,7 +279,11 @@ When you touch this file: - Almost every function takes `monthId` or `year/month` — they're per-month, not global. - Assignments with `source_constraint_id !== null` are locked: respect that - invariant in any bulk update query. + invariant in any bulk update query. `bulkUpsertAssignments` reads them + `FOR UPDATE` inside its transaction. +- A bulk edit must only carry days of its own month: check with + `countDaysInMonth` first. `uk_day_employee` would otherwise rewrite the cell + of another month, published or not. - `getSchedulableEmployeesForMonth(year, month)` applies `start_date`/`end_date` filtering. For raw employee lists (Totals tab, etc.) use `getAllEmployeesWithStatus`. @@ -299,10 +303,11 @@ setSchedulableEmployees". panel - `POST /months/:id/generate` — Run CP-SAT solver, apply matrix to assignments - `POST /months/:id/reset` — Wipe all assignments, re-seed from approved - constraints + constraints, in one transaction (`resetMonthAssignments`) - `POST /months/:id/unpublish` — Back to draft (controlled transition) - `PATCH /assignments/:id` — Update single cell (409 if locked) -- `POST /assignments/bulk` — Bulk update; respects locks +- `PUT /months/:id/assignments` — Bulk update; respects locks, 400 for a day + of another month - `POST /constraints/:id/approve` — Approve/reject + auto-sync assignments - `GET /employee-rules` — Rules list in camelCase for the panel - `PUT /scheduling/employees` — Set schedulable employees list (⚠️ see bug diff --git a/backend/tests/scheduling/month-scope.test.ts b/backend/tests/scheduling/month-scope.test.ts new file mode 100644 index 00000000..53e0bf67 --- /dev/null +++ b/backend/tests/scheduling/month-scope.test.ts @@ -0,0 +1,120 @@ +// tests/scheduling/month-scope.test.ts +// Regression tests: the bulk edits of a month accepted day ids of any other +// month. PUT /months/141/assignments with a day of month 140 rewrote that +// cell (uk_day_employee), so a published month could be edited through any +// draft month. Resetting a month deleted its cells and seeded them again in +// separate statements: a failure in between left the month without its +// locked cells. Both now check the days and write in one transaction. + +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { Request, Response } from 'express' + +const EMPLOYEE = '11111111-1111-4111-8111-111111111111' + +vi.mock('../../repositories/scheduling/scheduling-repository.js', () => ({ + getMonthById: vi.fn(), + countDaysInMonth: vi.fn(), + bulkUpdateDays: vi.fn(async () => 1), + bulkUpsertAssignments: vi.fn(async () => ({ locked: false })), + getAllShifts: vi.fn(async () => [{ code: 'M' }, { code: 'L' }]), + recalculateLibreNumbers: vi.fn(), + resetMonthAssignments: vi.fn(async () => 31), + createAssignmentsBulk: vi.fn(), + createHistory: vi.fn(), + // initializeMonthGrid (in the controller) seeds an L for every day and employee + getDaysByMonth: vi.fn(async () => [{ id: 7, date: '2026-11-01' }]), + getConstraintsByMonth: vi.fn(async () => []), + getSchedulableEmployees: vi.fn(async () => [{ id: EMPLOYEE }]), +})) + +const controller = await import('../../controllers/scheduling/scheduling-controller.js') +const repo = await import('../../repositories/scheduling/scheduling-repository.js') + +function call( + handler: (req: Request, res: Response) => Promise, + params: object, + body: object +) { + const res = { status: vi.fn().mockReturnThis(), json: vi.fn().mockReturnThis() } + const req = { user: { id: 'u1' }, params, body } as unknown as Request + return handler(req, res as unknown as Response).then(() => res) +} + +const assignments = { + assignments: [ + { day_id: 7, employee_id: EMPLOYEE, shift_code: 'M' }, + { day_id: 8, employee_id: EMPLOYEE, shift_code: 'L' }, + ], +} +const days = { + days: [ + { day_id: 7, is_holiday: true }, + { day_id: 8, is_holiday: false }, + ], +} + +beforeEach(() => { + vi.clearAllMocks() + vi.mocked(repo.getMonthById).mockResolvedValue({ + id: 141, + year: 2026, + month: 11, + status: 'draft', + } as never) +}) + +describe('bulk edits only touch days of their month', () => { + it('assignments with a day of another month answer 400 and write nothing', async () => { + vi.mocked(repo.countDaysInMonth).mockResolvedValue(1) + const res = await call(controller.bulkUpdateAssignments, { id: '141' }, assignments) + expect(repo.countDaysInMonth).toHaveBeenCalledWith(141, [7, 8]) + expect(res.status).toHaveBeenCalledWith(400) + expect(repo.bulkUpsertAssignments).not.toHaveBeenCalled() + }) + + it('days of another month answer 400 and write nothing', async () => { + vi.mocked(repo.countDaysInMonth).mockResolvedValue(1) + const res = await call(controller.bulkUpdateDays, { id: '141' }, days) + expect(res.status).toHaveBeenCalledWith(400) + expect(repo.bulkUpdateDays).not.toHaveBeenCalled() + }) + + it('assignments of the month are written in one transactional call', async () => { + vi.mocked(repo.countDaysInMonth).mockResolvedValue(2) + const res = await call(controller.bulkUpdateAssignments, { id: '141' }, assignments) + expect(res.status).not.toHaveBeenCalled() + expect(repo.bulkUpsertAssignments).toHaveBeenCalledWith(141, assignments.assignments) + }) + + it('a locked cell, checked inside the transaction, answers 409', async () => { + vi.mocked(repo.countDaysInMonth).mockResolvedValue(2) + vi.mocked(repo.bulkUpsertAssignments).mockResolvedValueOnce({ locked: true }) + const res = await call(controller.bulkUpdateAssignments, { id: '141' }, assignments) + expect(res.status).toHaveBeenCalledWith(409) + expect(repo.createHistory).not.toHaveBeenCalled() + }) + + it('the same day twice counts once', async () => { + vi.mocked(repo.countDaysInMonth).mockResolvedValue(1) + const twice = { + assignments: [ + { day_id: 7, employee_id: EMPLOYEE, shift_code: 'M' }, + { day_id: 7, employee_id: '22222222-2222-4222-8222-222222222222', shift_code: 'L' }, + ], + } + const res = await call(controller.bulkUpdateAssignments, { id: '141' }, twice) + expect(repo.countDaysInMonth).toHaveBeenCalledWith(141, [7]) + expect(res.status).not.toHaveBeenCalled() + }) +}) + +describe('resetting a month', () => { + it('deletes and seeds the cells in one transactional call', async () => { + const res = await call(controller.resetMonth, { id: '141' }, {}) + expect(res.status).not.toHaveBeenCalled() + expect(repo.resetMonthAssignments).toHaveBeenCalledWith(141, 2026, 11, [ + { day_id: 7, employee_id: EMPLOYEE, shift_code: 'L', source_constraint_id: null }, + ]) + expect(repo.createAssignmentsBulk).not.toHaveBeenCalled() + }) +}) From cbec8c85852f3407a4d61ff3404e699c3bdf1743 Mon Sep 17 00:00:00 2001 From: razyd Date: Mon, 12 Oct 2026 00:38:14 +0200 Subject: [PATCH 6/7] docs(scheduling): record that LI stays outside the rest rules of the solver - decisions.md: LI does not count towards the weekly free days nor the monthly minimum of L, and it stays that way. Why: decided by the owner on 2026-10-12; it matches the catalogue, which describes LI as an extra free day, and keeps every hard rule unchanged. --- docs/scheduling/decisions.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/scheduling/decisions.md b/docs/scheduling/decisions.md index 0ec4e18e..83cc37dc 100644 --- a/docs/scheduling/decisions.md +++ b/docs/scheduling/decisions.md @@ -1337,3 +1337,18 @@ hand; the owner chose speed over reproducibility. **If it is revisited.** Set `random_seed`, `interleave_search = True` and a fixed `num_workers` in `model.py`, and replace `max_time_in_seconds` with `max_deterministic_time` for months that reach the limit. + +## 2026-10-12 — `LI` does not count as rest in the solver + +**Context.** `LI` (Libre Disposición) is not in `REST_SHIFTS` of +`constraints/rest.py` nor in `_SPECIAL_REST` of `constraints/libres.py`: a +locked `LI` neither counts towards the two free days of any 7-day window nor +lowers the monthly minimum of `L`. Whoever has an `LI` also gets every +regular free day. + +**Decision.** Keep it. It matches the catalogue (`README.md`: "extra free +day"; `services/scheduling/AGENTS.md`: "outside the weekly rotation"). + +**Why.** The owner's priority is a solver that keeps working: counting `LI` +as rest would change two hard rules and the rosters of every month with an +`LI`, with no reported problem behind it. From 9f1344ef53d88d47ad4943c7384d3b568a9ca3ea Mon Sep 17 00:00:00 2001 From: razyd Date: Mon, 12 Oct 2026 00:56:43 +0200 Subject: [PATCH 7/7] docs(scheduling): record that the solver daemon queue stays as it is - decisions.md: the 30-minute startup limit and the unbounded queue are kept, with the reasons and what to change if it is revisited. Why: decided by the owner on 2026-10-12; only admins generate, the demo is rate limited and the daemon starts in seconds. --- docs/scheduling/decisions.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/scheduling/decisions.md b/docs/scheduling/decisions.md index 83cc37dc..5abb1862 100644 --- a/docs/scheduling/decisions.md +++ b/docs/scheduling/decisions.md @@ -1352,3 +1352,21 @@ day"; `services/scheduling/AGENTS.md`: "outside the weekly rotation"). **Why.** The owner's priority is a solver that keeps working: counting `LI` as rest would change two hard rules and the rosters of every month with an `LI`, with no reported problem behind it. + +## 2026-10-12 — Solver daemon: startup limit and queue left as they are + +**Context.** `services/scheduling/solver-client.ts` takes the semaphore before +waiting for the daemon, waits up to 30 minutes for it to start on every +platform (the limit was set for Windows with Defender) and queues requests +without a limit. + +**Decision.** Keep it. + +**Why.** The risk is low: only `admin` generates, the demo account is limited +to 5 generations per IP and 30 in total per hour, the daemon starts in 1–3 s +when it is warm, and a request cancelled while it waits returns before +calling the solver. + +**If it is revisited.** A shorter startup limit outside Windows and a cap on +the queue that answers "solver busy". +