diff --git a/backend/controllers/group/group-controller.ts b/backend/controllers/group/group-controller.ts index 5083b020..fd5f09e5 100644 --- a/backend/controllers/group/group-controller.ts +++ b/backend/controllers/group/group-controller.ts @@ -253,7 +253,7 @@ export class GroupController { }) } - const deleted = await GroupRepository.delete(groupId) + const deleted = await GroupRepository.delete(groupId, userId, group) if (!deleted) { return res.status(500).json({ diff --git a/backend/db-mysql/INDEX.md b/backend/db-mysql/INDEX.md index 30eef9bf..f4eb998b 100644 --- a/backend/db-mysql/INDEX.md +++ b/backend/db-mysql/INDEX.md @@ -108,6 +108,7 @@ db-mysql/ | 2026-10-05 | `20261005_complete_fresh_install.sql` | Completa una instalación desde cero: el baseline congelado no crea `cashier_history` (nombre de clave duplicado), `scheduling_assignments` (referencia a una tabla creada después) ni `demo_activity_log`, y deja `roles.name` en `varchar(50)`. Definiciones copiadas de Aiven. En Aiven no cambia nada; en local creó `demo_activity_log`, que faltaba. `pnpm setup:local` además corrige esos dos fallos del baseline en memoria. | ✅ local · ✅ Aiven (sin cambios) | | 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 | ### Verificación (99) diff --git a/backend/db-mysql/scripts/20261011_keep_group_history_on_delete.sql b/backend/db-mysql/scripts/20261011_keep_group_history_on_delete.sql new file mode 100644 index 00000000..96f77455 --- /dev/null +++ b/backend/db-mysql/scripts/20261011_keep_group_history_on_delete.sql @@ -0,0 +1,40 @@ +-- ============================================================================= +-- 20261011_keep_group_history_on_delete.sql +-- ============================================================================= +-- Drops the foreign key from group_history.group_id to hotel_groups.id. It was +-- ON DELETE CASCADE, so deleting a group also deleted its whole history and +-- left no trace of the deletion. Decided on 2026-10-11: a group is still +-- deleted, but its history stays and gains a "deleted" row with a snapshot of +-- the group (GroupRepository.delete). group_id keeps its index +-- (idx_group_id); rows of a deleted group point to an id that no longer +-- exists. +-- +-- Idempotent: if the foreign key is already gone the ALTER is a no-op. +-- Safe to run multiple times. +-- +-- Applies to: LOCAL + AIVEN. Apply it before deploying the code that writes +-- the deletion row: with the foreign key in place that insert fails and the +-- delete rolls back. +-- Rollback (only while no history row points to a deleted group): +-- ALTER TABLE group_history ADD CONSTRAINT group_history_ibfk_1 +-- FOREIGN KEY (group_id) REFERENCES hotel_groups (id) ON DELETE CASCADE; +-- ============================================================================= + +SET @fk_exists := ( + SELECT COUNT(*) + FROM information_schema.TABLE_CONSTRAINTS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'group_history' + AND CONSTRAINT_NAME = 'group_history_ibfk_1' + AND CONSTRAINT_TYPE = 'FOREIGN KEY' +); + +SET @ddl := IF( + @fk_exists = 1, + 'ALTER TABLE group_history DROP FOREIGN KEY group_history_ibfk_1', + 'SELECT ''group_history_ibfk_1 already dropped, skipping'' AS info' +); + +PREPARE stmt FROM @ddl; +EXECUTE stmt; +DEALLOCATE PREPARE stmt; diff --git a/backend/repositories/group/group-repository.ts b/backend/repositories/group/group-repository.ts index 90b96155..13bfa0f2 100644 --- a/backend/repositories/group/group-repository.ts +++ b/backend/repositories/group/group-repository.ts @@ -269,9 +269,33 @@ export class GroupRepository { /** * Eliminar grupo */ - static async delete(id: number): Promise { - const [result] = await db.query('DELETE FROM hotel_groups WHERE id = ?', [id]) - return result.affectedRows > 0 + static async delete(id: number, deletedBy: string, snapshot: unknown): Promise { + // The group goes, its history stays: group_history has no foreign key to + // hotel_groups since 20261011_keep_group_history_on_delete.sql. The delete + // and its history row commit or roll back together + const connection = await db.getConnection() + try { + await connection.beginTransaction() + const [result] = await connection.query( + 'DELETE FROM hotel_groups WHERE id = ?', + [id] + ) + if (result.affectedRows > 0) { + await connection.query( + `INSERT INTO group_history + (group_id, action, table_affected, record_id, old_value, changed_by, notes) + VALUES (?, 'deleted', 'hotel_groups', ?, ?, ?, 'Grupo eliminado')`, + [id, id, JSON.stringify(snapshot), deletedBy] + ) + } + await connection.commit() + return result.affectedRows > 0 + } catch (error) { + await connection.rollback() + throw error + } finally { + connection.release() + } } /** diff --git a/backend/tests/group/group-deletion.test.ts b/backend/tests/group/group-deletion.test.ts new file mode 100644 index 00000000..0d287e74 --- /dev/null +++ b/backend/tests/group/group-deletion.test.ts @@ -0,0 +1,78 @@ +// tests/group/group-deletion.test.ts +// Regression tests: deleting a group deleted its whole history too +// (group_history had ON DELETE CASCADE) and logged nothing. Decided on +// 2026-10-11: the group is still deleted, its history stays, and the delete +// writes a "deleted" row with a snapshot of the group, in the same +// transaction. + +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { readFileSync } from 'node:fs' +import { join, dirname } from 'node:path' +import { fileURLToPath } from 'node:url' + +const connection = { + beginTransaction: vi.fn(), + commit: vi.fn(), + rollback: vi.fn(), + release: vi.fn(), + query: vi.fn(), +} + +vi.mock('../../config/db.js', () => ({ + default: { getConnection: vi.fn(async () => connection), query: vi.fn() }, +})) + +const { GroupRepository } = await import('../../repositories/group/group-repository.js') + +const group = { id: 4, name: 'Congreso', total_amount: '12500.00' } + +describe('deleting a group', () => { + beforeEach(() => { + vi.clearAllMocks() + connection.query.mockReset() + }) + + it('deletes the group and writes a deleted row with its snapshot', async () => { + connection.query.mockResolvedValueOnce([{ affectedRows: 1 }]).mockResolvedValueOnce([{}]) + + expect(await GroupRepository.delete(4, 'u1', group)).toBe(true) + + const [del, history] = connection.query.mock.calls + expect(del[0]).toMatch(/DELETE FROM hotel_groups/) + expect(history[0]).toMatch(/INSERT INTO group_history[\s\S]*'deleted', 'hotel_groups'/) + expect(history[1]).toEqual([4, 4, JSON.stringify(group), 'u1']) + expect(connection.commit).toHaveBeenCalledOnce() + }) + + it('rolls the delete back when the history row fails', async () => { + connection.query + .mockResolvedValueOnce([{ affectedRows: 1 }]) + .mockRejectedValueOnce(new Error('history insert failed')) + + await expect(GroupRepository.delete(4, 'u1', group)).rejects.toThrow('history insert failed') + expect(connection.rollback).toHaveBeenCalledOnce() + expect(connection.commit).not.toHaveBeenCalled() + }) + + it('writes no history when there was no group to delete', async () => { + connection.query.mockResolvedValueOnce([{ affectedRows: 0 }]) + + expect(await GroupRepository.delete(4, 'u1', group)).toBe(false) + expect(connection.query).toHaveBeenCalledOnce() + }) + + it('a migration drops the cascade from group_history', () => { + const script = readFileSync( + join( + dirname(fileURLToPath(import.meta.url)), + '..', + '..', + 'db-mysql', + 'scripts', + '20261011_keep_group_history_on_delete.sql' + ), + 'utf8' + ) + expect(script).toContain('ALTER TABLE group_history DROP FOREIGN KEY group_history_ibfk_1') + }) +})