diff --git a/backend/config/error-codes.ts b/backend/config/error-codes.ts index 3d4dc258..5c2bf850 100644 --- a/backend/config/error-codes.ts +++ b/backend/config/error-codes.ts @@ -54,6 +54,9 @@ export const ERROR_CODES = { BACKOFFICE_SUPPLIER_NOT_FOUND: 'BACKOFFICE_SUPPLIER_NOT_FOUND', BACKOFFICE_REJECTION_REASON_REQUIRED: 'BACKOFFICE_REJECTION_REASON_REQUIRED', BACKOFFICE_PAYMENT_DATE_REQUIRED: 'BACKOFFICE_PAYMENT_DATE_REQUIRED', + BACKOFFICE_INVALID_PAYMENT_DATE: 'BACKOFFICE_INVALID_PAYMENT_DATE', + BACKOFFICE_INVALID_AMOUNT: 'BACKOFFICE_INVALID_AMOUNT', + BACKOFFICE_INVOICE_DUPLICATE: 'BACKOFFICE_INVOICE_DUPLICATE', BACKOFFICE_NO_FILE_SENT: 'BACKOFFICE_NO_FILE_SENT', BACKOFFICE_PDF_ONLY: 'BACKOFFICE_PDF_ONLY', BACKOFFICE_FILE_TOO_LARGE: 'BACKOFFICE_FILE_TOO_LARGE', diff --git a/backend/controllers/backoffice/backoffice-controller.ts b/backend/controllers/backoffice/backoffice-controller.ts index 6f4e128e..4dd2c151 100644 --- a/backend/controllers/backoffice/backoffice-controller.ts +++ b/backend/controllers/backoffice/backoffice-controller.ts @@ -18,6 +18,12 @@ import { executeBatchPaymentSchema, revertBatchPaymentSchema, } from '../../validations/backoffice/batch-payment.js' +import { + hasValidAmounts, + isDuplicateEntry, + isValidPaidDate, +} from '../../validations/backoffice/invoice.js' +import { formatDateMadrid, getTodayMadrid } from '../../config/date-utils.js' import { isOwnCloudinaryUrl, isPublicIdInFolder, @@ -26,6 +32,23 @@ import { import { fetchStoredFile, sendPrivateFile } from '../../services/uploads/private-files.js' import { presentAsset, presentInvoice, presentInvoiceOrNull } from './backoffice-files.js' +function respondInvalidAmount(res: Response): void { + res.status(400).json({ + success: false, + error: ERROR_CODES.BACKOFFICE_INVALID_AMOUNT, + code: ERROR_CODES.BACKOFFICE_INVALID_AMOUNT, + }) +} + +// uk_invoice_supplier_number: the supplier already has an invoice with that number +function respondDuplicateInvoice(res: Response): void { + res.status(409).json({ + success: false, + error: ERROR_CODES.BACKOFFICE_INVOICE_DUPLICATE, + code: ERROR_CODES.BACKOFFICE_INVOICE_DUPLICATE, + }) +} + // An invoice action changed no row: the invoice does not exist (404) or its // status does not allow the action (409) async function respondActionNotApplied(res: Response, id: number): Promise { @@ -570,6 +593,11 @@ export class BackofficeController { return } + if (!hasValidAmounts(req.body)) { + respondInvalidAmount(res) + return + } + // The PDF must be one we uploaded: the server later downloads this URL if ( (req.body.original_pdf_url && !isOwnCloudinaryUrl(req.body.original_pdf_url)) || @@ -611,6 +639,10 @@ export class BackofficeController { invoice: presentInvoiceOrNull(invoice), }) } catch (error) { + if (isDuplicateEntry(error)) { + respondDuplicateInvoice(res) + return + } logger.error({ err: error }, '[BackofficeController.createInvoice] Error') res.status(500).json({ success: false, @@ -637,6 +669,11 @@ export class BackofficeController { const { id } = req.params + if (!hasValidAmounts(req.body)) { + respondInvalidAmount(res) + return + } + const updated = await BackofficeRepository.updateInvoice(Number(id), req.body, req.user.id) if (!updated) { @@ -653,6 +690,10 @@ export class BackofficeController { invoice: presentInvoiceOrNull(invoice), }) } catch (error) { + if (isDuplicateEntry(error)) { + respondDuplicateInvoice(res) + return + } logger.error({ err: error }, '[BackofficeController.updateInvoice] Error') res.status(500).json({ success: false, @@ -872,6 +913,26 @@ export class BackofficeController { return } + const current = await BackofficeRepository.getInvoiceById(Number(id)) + if (!current) { + res.status(404).json({ + success: false, + error: ERROR_CODES.BACKOFFICE_INVOICE_NOT_FOUND, + code: ERROR_CODES.BACKOFFICE_INVOICE_NOT_FOUND, + }) + return + } + + // A real day, not before the invoice nor in the future + if (!isValidPaidDate(paid_date, formatDateMadrid(current.invoice_date), getTodayMadrid())) { + res.status(400).json({ + success: false, + error: ERROR_CODES.BACKOFFICE_INVALID_PAYMENT_DATE, + code: ERROR_CODES.BACKOFFICE_INVALID_PAYMENT_DATE, + }) + return + } + const paid = await BackofficeRepository.markAsPaid(Number(id), paid_date, req.user.id) if (!paid) { diff --git a/backend/db-mysql/INDEX.md b/backend/db-mysql/INDEX.md index f4eb998b..3ba27b5d 100644 --- a/backend/db-mysql/INDEX.md +++ b/backend/db-mysql/INDEX.md @@ -109,6 +109,7 @@ db-mysql/ | 2026-10-05 | `20261005_fix_bo_monthly_summary_view.sql` | Recrea `v_bo_monthly_summary` con `period` en el `GROUP BY`. Con `ONLY_FULL_GROUP_BY` (activo en Aiven y local) toda consulta a la vista fallaba con el error 1055 y `GET /api/backoffice/stats/monthly` respondía siempre 500. Mismas columnas y mismos grupos; comprobada contra un cálculo directo sobre `bo_invoices`. | ✅ local · ✅ Aiven | | 2026-10-05 | `20261005_drop_demo_admin_role.sql` | Borra el rol `demo-admin` si ningún usuario lo tiene (ADR-038): la cuenta demo es un admin con `is_demo` y el código ya no conoce ese rol. En local no existía. | ✅ local (sin cambios) · ✅ Aiven | | 2026-10-11 | `20261011_keep_group_history_on_delete.sql` | Quita la clave foránea `group_history_ibfk_1` (`ON DELETE CASCADE` a `hotel_groups`): borrar un grupo ya no borra su historial, y el borrado deja una fila `deleted` con la foto del grupo. **Aplicar en Aiven antes de desplegar el código**: con la clave dentro, borrar un grupo falla. | ✅ local · ✅ Aiven | +| 2026-10-12 | `20261012_unique_invoice_supplier_number.sql` | Añade `UNIQUE KEY uk_invoice_supplier_number (supplier_id, invoice_number)` a `bo_invoices`: la misma factura de un proveedor se podía registrar dos veces y se pagaban las dos en el lote mensual. El backend responde 409 `BACKOFFICE_INVOICE_DUPLICATE`. Antes del `ALTER` lista las duplicadas; si hay alguna, el `ALTER` falla y hay que corregirlas. Idempotente. El código funciona sin ella (no rechaza nada hasta que exista). | ✅ local · ✅ Aiven | ### Verificación (99) diff --git a/backend/db-mysql/scripts/20261012_unique_invoice_supplier_number.sql b/backend/db-mysql/scripts/20261012_unique_invoice_supplier_number.sql new file mode 100644 index 00000000..2fdc12ca --- /dev/null +++ b/backend/db-mysql/scripts/20261012_unique_invoice_supplier_number.sql @@ -0,0 +1,45 @@ +-- ============================================================================= +-- 20261012_unique_invoice_supplier_number.sql +-- ============================================================================= +-- Adds UNIQUE KEY uk_invoice_supplier_number (supplier_id, invoice_number) to +-- bo_invoices. The same invoice of the same supplier could be entered twice +-- and both copies were paid in the monthly batch. Decided on 2026-10-11: a +-- second invoice with the same supplier and number is refused (409 +-- BACKOFFICE_INVOICE_DUPLICATE, see validations/backoffice/invoice.ts). +-- Deleting an invoice stays permanent, so a deleted number can be reused. +-- The column collation is case-insensitive: "F-1" and "f-1" count as the same. +-- +-- Idempotent: if the key already exists the ALTER is skipped. +-- Safe to run multiple times. +-- +-- If the table already holds duplicates, the first SELECT lists them and the +-- ALTER fails with "Duplicate entry": fix those rows and run it again. +-- +-- Applies to: LOCAL + AIVEN. The code works without it (nothing is refused +-- until the key exists), so it can go before or after the deploy. +-- Rollback: +-- ALTER TABLE bo_invoices DROP INDEX uk_invoice_supplier_number; +-- ============================================================================= + +SELECT supplier_id, invoice_number, COUNT(*) AS copies +FROM bo_invoices +GROUP BY supplier_id, invoice_number +HAVING COUNT(*) > 1; + +SET @key_exists := ( + SELECT COUNT(*) + FROM information_schema.STATISTICS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'bo_invoices' + AND INDEX_NAME = 'uk_invoice_supplier_number' +); + +SET @ddl := IF( + @key_exists = 0, + 'ALTER TABLE bo_invoices ADD UNIQUE KEY uk_invoice_supplier_number (supplier_id, invoice_number)', + 'SELECT ''uk_invoice_supplier_number already exists, skipping'' AS info' +); + +PREPARE stmt FROM @ddl; +EXECUTE stmt; +DEALLOCATE PREPARE stmt; diff --git a/backend/tests/backoffice/invoice-rules.test.ts b/backend/tests/backoffice/invoice-rules.test.ts new file mode 100644 index 00000000..546d91f9 --- /dev/null +++ b/backend/tests/backoffice/invoice-rules.test.ts @@ -0,0 +1,109 @@ +// tests/backoffice/invoice-rules.test.ts +// Regression tests for the invoice data rules decided on 2026-10-11: a +// negative amount, a second invoice with the same supplier and number, and a +// payment date before the invoice, in the future or that does not exist were +// all saved. Pure rules plus the controller and migration sources; no database. + +import { describe, it, expect } from 'vitest' +import { readFileSync } from 'node:fs' +import { join, dirname } from 'node:path' +import { fileURLToPath } from 'node:url' +import { + hasValidAmounts, + isDuplicateEntry, + isValidAmount, + isValidPaidDate, +} from '../../validations/backoffice/invoice.js' + +const root = join(dirname(fileURLToPath(import.meta.url)), '..', '..') +const read = (...p: string[]) => readFileSync(join(root, ...p), 'utf8') +const controller = read('controllers', 'backoffice', 'backoffice-controller.ts') + +// Body of `static async name(` up to the next static method +function method(name: string): string { + const start = controller.indexOf(`static async ${name}(`) + expect(start, name).toBeGreaterThan(-1) + const next = controller.indexOf('static async ', start + 1) + return controller.slice(start, next === -1 ? undefined : next) +} + +describe('isValidAmount', () => { + it('accepts zero and positive numbers, and the DECIMAL strings the API returns', () => { + for (const v of [0, 12.5, 2680, '2680.00', '0', ' 15 ']) { + expect(isValidAmount(v), String(v)).toBe(true) + } + }) + + it('refuses negatives, non-numbers and empty values', () => { + for (const v of [-0.01, -100, '-5', '1e3', 'abc', '', NaN, Infinity, null, {}, [1]]) { + expect(isValidAmount(v), String(v)).toBe(false) + } + }) +}) + +describe('hasValidAmounts', () => { + it('checks only the amounts present, as a partial edit sends', () => { + expect(hasValidAmounts({ notes: 'x' })).toBe(true) + expect(hasValidAmounts({ amount_with_vat: '121.00' })).toBe(true) + expect(hasValidAmounts({ amount_without_vat: 100, amount_with_vat: -121 })).toBe(false) + }) +}) + +describe('isValidPaidDate', () => { + const invoice = '2026-09-30' + const today = '2026-10-12' + + it('accepts the invoice day, today and any day between', () => { + for (const d of [invoice, '2026-10-01', today]) { + expect(isValidPaidDate(d, invoice, today)).toBe(true) + } + }) + + it('refuses a day before the invoice, a future day and days that do not exist', () => { + for (const d of ['2026-09-29', '2026-10-13', '2026-02-31', '30/09/2026', '', 20261001]) { + expect(isValidPaidDate(d, invoice, today), String(d)).toBe(false) + } + }) +}) + +describe('isDuplicateEntry', () => { + it('recognises only MySQL duplicate-key errors', () => { + expect(isDuplicateEntry({ code: 'ER_DUP_ENTRY' })).toBe(true) + expect(isDuplicateEntry(new Error('x'))).toBe(false) + expect(isDuplicateEntry(null)).toBe(false) + }) +}) + +describe('controller', () => { + it.each(['createInvoice', 'updateInvoice'])( + '%s refuses negative amounts (400) and duplicates (409)', + (name) => { + const body = method(name) + expect(body).toMatch(/if \(!hasValidAmounts\(req\.body\)\) \{\s*respondInvalidAmount\(res\)/) + expect(body).toMatch(/if \(isDuplicateEntry\(error\)\) \{\s*respondDuplicateInvoice\(res\)/) + } + ) + + it('answers 400 BACKOFFICE_INVALID_AMOUNT and 409 BACKOFFICE_INVOICE_DUPLICATE', () => { + expect(controller).toMatch( + /function respondInvalidAmount[\s\S]*?res\.status\(400\)[\s\S]*?ERROR_CODES\.BACKOFFICE_INVALID_AMOUNT/ + ) + expect(controller).toMatch( + /function respondDuplicateInvoice[\s\S]*?res\.status\(409\)[\s\S]*?ERROR_CODES\.BACKOFFICE_INVOICE_DUPLICATE/ + ) + }) + + it('markAsPaid checks the date against the invoice and today', () => { + const body = method('markAsPaid') + expect(body).toContain('isValidPaidDate(') + expect(body).toContain('getTodayMadrid()') + expect(body).toContain('ERROR_CODES.BACKOFFICE_INVALID_PAYMENT_DATE') + }) +}) + +describe('migration', () => { + it('adds the unique key on supplier and number', () => { + const sql = read('db-mysql', 'scripts', '20261012_unique_invoice_supplier_number.sql') + expect(sql).toContain('ADD UNIQUE KEY uk_invoice_supplier_number (supplier_id, invoice_number)') + }) +}) diff --git a/backend/validations/backoffice/invoice.ts b/backend/validations/backoffice/invoice.ts new file mode 100644 index 00000000..07e85750 --- /dev/null +++ b/backend/validations/backoffice/invoice.ts @@ -0,0 +1,40 @@ +// validations/backoffice/invoice.ts +// Rules decided on 2026-10-11: no negative amounts, one invoice per supplier +// and number (UNIQUE uk_invoice_supplier_number, scripts/20261012_*), and a +// payment date that is a real day, not before the invoice nor in the future. + +import { isCalendarDate } from '../common/calendar-date.js' + +export const INVOICE_AMOUNT_FIELDS = ['amount_without_vat', 'amount_with_vat'] as const + +/** + * A number, or the numeric string the API itself returns for DECIMAL columns + * (the edit form sends them back as they came), and not negative. + */ +export function isValidAmount(value: unknown): boolean { + if (typeof value === 'number') return Number.isFinite(value) && value >= 0 + return typeof value === 'string' && /^\d+(\.\d+)?$/.test(value.trim()) +} + +/** True when every amount present in `body` is valid; absent ones are not checked. */ +export function hasValidAmounts(body: Record): boolean { + return INVOICE_AMOUNT_FIELDS.every((f) => body[f] === undefined || isValidAmount(body[f])) +} + +/** + * `paidDate` (YYYY-MM-DD) is a real day between the invoice date and today, + * both included. + */ +export function isValidPaidDate(paidDate: unknown, invoiceDate: string, today: string): boolean { + return ( + typeof paidDate === 'string' && + isCalendarDate(paidDate) && + paidDate >= invoiceDate && + paidDate <= today + ) +} + +/** MySQL's duplicate-key error, raised by uk_invoice_supplier_number. */ +export function isDuplicateEntry(error: unknown): boolean { + return (error as { code?: string } | null)?.code === 'ER_DUP_ENTRY' +} diff --git a/docs/backoffice/README.md b/docs/backoffice/README.md index 5deba492..72a41900 100644 --- a/docs/backoffice/README.md +++ b/docs/backoffice/README.md @@ -62,8 +62,18 @@ PDFs and images are stored in **Cloudinary**; the database keeps their URLs. or payment. - **The batch payment only takes validated invoices**, and only validated or paid invoices with a stamped PDF can go into a ZIP. +- **An invoice can be validated without a stamped PDF**, on purpose (decided on + 2026-10-11): the screen validates without a PDF and in bulk. The stamp is + placed in the browser, so the server cannot tell whether the stamped copy + comes from the original. +- **Amounts cannot be negative**, a supplier cannot have two invoices with the + same number (`uk_invoice_supplier_number`, case-insensitive), and a payment + date must be a real day between the invoice date and today (decided on + 2026-10-11). - **Changes are logged** in `bo_invoice_history`. -- **Deleting an invoice is permanent** and also deletes its history. +- **Deleting an invoice is permanent** and also deletes its history and its + PDFs. Kept on purpose (decided on 2026-10-11): deleting is how a wrong entry + is corrected, since rejecting has no screen. - **The automatic payment on the 10th does not work today**: it records the change as a system user that does not exist, and the database rejects it. - **PDFs are downloaded through the backend**, which fetches them from diff --git a/frontend/app/lib/helpers/errorCodeMessage.ts b/frontend/app/lib/helpers/errorCodeMessage.ts index d8b4715d..f5287f1e 100644 --- a/frontend/app/lib/helpers/errorCodeMessage.ts +++ b/frontend/app/lib/helpers/errorCodeMessage.ts @@ -67,6 +67,9 @@ const TRANSLATED = new Set([ 'BACKOFFICE_NO_FILE_SENT', 'BACKOFFICE_NO_IMAGE_SENT', 'BACKOFFICE_PAYMENT_DATE_REQUIRED', + 'BACKOFFICE_INVALID_PAYMENT_DATE', + 'BACKOFFICE_INVALID_AMOUNT', + 'BACKOFFICE_INVOICE_DUPLICATE', 'BACKOFFICE_PDF_ONLY', 'BACKOFFICE_PDF_TYPE_REQUIRED', 'BACKOFFICE_REJECTION_REASON_REQUIRED', diff --git a/frontend/messages/en/errors.json b/frontend/messages/en/errors.json index bd4676b4..881d6472 100644 --- a/frontend/messages/en/errors.json +++ b/frontend/messages/en/errors.json @@ -111,6 +111,9 @@ "BACKOFFICE_SUPPLIER_NOT_FOUND": "Supplier not found", "BACKOFFICE_REJECTION_REASON_REQUIRED": "Rejection reason is required", "BACKOFFICE_PAYMENT_DATE_REQUIRED": "Payment date is required", + "BACKOFFICE_INVALID_PAYMENT_DATE": "The payment date must be a real day, not before the invoice nor in the future", + "BACKOFFICE_INVALID_AMOUNT": "Amounts must be numbers and cannot be negative", + "BACKOFFICE_INVOICE_DUPLICATE": "This supplier already has an invoice with that number", "BACKOFFICE_NO_FILE_SENT": "No file was sent", "BACKOFFICE_PDF_ONLY": "Only PDF files are allowed", "BACKOFFICE_FILE_TOO_LARGE": "File is too large. Maximum 10MB", diff --git a/frontend/messages/es/errors.json b/frontend/messages/es/errors.json index 4659097c..3f683c05 100644 --- a/frontend/messages/es/errors.json +++ b/frontend/messages/es/errors.json @@ -111,6 +111,9 @@ "BACKOFFICE_SUPPLIER_NOT_FOUND": "Proveedor no encontrado", "BACKOFFICE_REJECTION_REASON_REQUIRED": "Se requiere un motivo de rechazo", "BACKOFFICE_PAYMENT_DATE_REQUIRED": "Se requiere la fecha de pago", + "BACKOFFICE_INVALID_PAYMENT_DATE": "La fecha de pago debe ser un día real, no anterior a la factura ni futura", + "BACKOFFICE_INVALID_AMOUNT": "Los importes deben ser números y no pueden ser negativos", + "BACKOFFICE_INVOICE_DUPLICATE": "Ya existe una factura con ese número para este proveedor", "BACKOFFICE_NO_FILE_SENT": "No se envió ningún archivo", "BACKOFFICE_PDF_ONLY": "Solo se permiten archivos PDF", "BACKOFFICE_FILE_TOO_LARGE": "El archivo es demasiado grande. Máximo 10MB",