From aadcc410cf1a47e793f19cc7a42c3f9796fb3c0d Mon Sep 17 00:00:00 2001 From: razyd Date: Sun, 11 Oct 2026 11:57:54 +0200 Subject: [PATCH] fix(groups): keep payment amounts, status and percentages coherent - payment-rules.ts: statusFor, reconcilePayment and checkPercentages, the rules decided on 2026-10-11: what was paid never exceeds what is owed, marking a payment paid fills in what was paid, the status follows the amounts ("requested" stays until money arrives), and a group's percentages add up to 100 at most. - group-payment-controller.ts: create, update, status and amount-paid apply them; a broken rule answers 400 with GROUP_PAYMENT_OVERPAID or GROUP_PAYMENT_PERCENTAGE_OVER_100. - group-payment-repository.ts: the total recalculation also sets the status from the new amount; updateAmountPaid goes, it had no other user. - error-codes.ts, errors.json (es, en), errorCodeMessage.ts: the two codes. - payment-rules.test.ts: the rules and the four endpoints. Why: more paid than owed was accepted, a payment could be paid with nothing paid, a new group total left statuses behind, and percentages could exceed 100. Checked in the browser and with curl against the local database: an overpayment is refused with its text, marking paid fills in 5000, a 120 % total is refused, and lowering and restoring the group total moves the statuses and puts them back. --- backend/config/error-codes.ts | 2 + .../group/group-payment-controller.ts | 90 ++++++++++- .../group/group-payment-repository.ts | 37 +---- backend/services/group/payment-rules.ts | 59 +++++++ backend/tests/group/payment-rules.test.ts | 151 ++++++++++++++++++ frontend/app/lib/helpers/errorCodeMessage.ts | 2 + frontend/messages/en/errors.json | 2 + frontend/messages/es/errors.json | 2 + 8 files changed, 314 insertions(+), 31 deletions(-) create mode 100644 backend/services/group/payment-rules.ts create mode 100644 backend/tests/group/payment-rules.test.ts diff --git a/backend/config/error-codes.ts b/backend/config/error-codes.ts index 39d90afe..16b7344b 100644 --- a/backend/config/error-codes.ts +++ b/backend/config/error-codes.ts @@ -253,6 +253,8 @@ export const ERROR_CODES = { // Group Payment GROUP_PAYMENT_MISSING_FIELDS: 'GROUP_PAYMENT_MISSING_FIELDS', GROUP_PAYMENT_INVALID_AMOUNT: 'GROUP_PAYMENT_INVALID_AMOUNT', + GROUP_PAYMENT_OVERPAID: 'GROUP_PAYMENT_OVERPAID', + GROUP_PAYMENT_PERCENTAGE_OVER_100: 'GROUP_PAYMENT_PERCENTAGE_OVER_100', GROUP_PAYMENT_NOT_FOUND: 'GROUP_PAYMENT_NOT_FOUND', GROUP_PAYMENT_NOT_IN_GROUP: 'GROUP_PAYMENT_NOT_IN_GROUP', GROUP_PAYMENT_FETCH_ERROR: 'GROUP_PAYMENT_FETCH_ERROR', diff --git a/backend/controllers/group/group-payment-controller.ts b/backend/controllers/group/group-payment-controller.ts index 1941c554..c502caef 100644 --- a/backend/controllers/group/group-payment-controller.ts +++ b/backend/controllers/group/group-payment-controller.ts @@ -5,10 +5,16 @@ import { GroupPaymentRepository } from '../../repositories/group/group-payment-r import { GroupRepository } from '../../repositories/group/group-repository' import { PaymentCalculatorService } from '../../services/group/payment-calculator-service' import { GroupHistoryService } from '../../services/group/group-history-service' +import { + checkPercentages, + PaymentRuleError, + reconcilePayment, +} from '../../services/group/payment-rules.js' import { CreateGroupPaymentDTO, UpdateGroupPaymentDTO, HistoryAction, + PaymentStatus, } from '../../models/group/index' import { ERROR_CODES, SUCCESS_CODES } from '../../config/error-codes.js' import { logger } from '../../config/logger.js' @@ -21,6 +27,12 @@ import { validationError, } from '../../validations/group/group-schemas.js' +// A broken payment rule answers 400 with its code; anything else is rethrown +function ruleError(res: Response, error: unknown): Response { + if (!(error instanceof PaymentRuleError)) throw error + return res.status(400).json({ success: false, error: error.code, code: error.code }) +} + export class GroupPaymentController { /** * GET /api/groups/:id/payments @@ -129,6 +141,25 @@ export class GroupPaymentController { }) } + try { + if (paymentData.percentage != null) { + checkPercentages( + await GroupPaymentRepository.getByGroupId(groupId), + paymentData.percentage + ) + } + Object.assign( + paymentData, + reconcilePayment({ + amount: paymentData.amount, + amountPaid: paymentData.amount_paid ?? 0, + status: paymentData.status, + }) + ) + } catch (error) { + return ruleError(res, error) + } + const newPayment = await GroupPaymentRepository.create(paymentData) await GroupHistoryService.logChange( @@ -209,6 +240,32 @@ export class GroupPaymentController { } const updateData = parsed.data as UpdateGroupPaymentDTO + try { + if (updateData.percentage != null) { + const others = (await GroupPaymentRepository.getByGroupId(groupId)).filter( + (p) => p.id !== paymentId + ) + checkPercentages(others, updateData.percentage) + } + if ( + updateData.amount != null || + updateData.amount_paid !== undefined || + updateData.status !== undefined + ) { + Object.assign( + updateData, + reconcilePayment({ + amount: updateData.amount ?? oldPayment.amount, + amountPaid: updateData.amount_paid ?? oldPayment.amount_paid, + status: updateData.status, + current: oldPayment.status, + }) + ) + } + } catch (error) { + return ruleError(res, error) + } + const updated = await GroupPaymentRepository.update(paymentId, updateData) if (!updated) { @@ -295,7 +352,12 @@ export class GroupPaymentController { }) } - const updated = await GroupPaymentRepository.updateStatus(paymentId, status) + // Marking it paid by hand fills in what was paid + const filled = status === PaymentStatus.PAID ? Number(oldPayment.amount) : null + const updated = + filled === null + ? await GroupPaymentRepository.updateStatus(paymentId, status) + : await GroupPaymentRepository.update(paymentId, { status, amount_paid: filled }) if (!updated) { return res.status(500).json({ @@ -316,6 +378,19 @@ export class GroupPaymentController { status, 'Estado de pago actualizado' ) + if (filled !== null && Number(oldPayment.amount_paid) !== filled) { + await GroupHistoryService.logChange( + oldPayment.group_id, + userId, + HistoryAction.PAYMENT_UPDATED, + 'group_payments', + paymentId, + 'amount_paid', + oldPayment.amount_paid, + filled, + 'Cantidad pagada completada al marcar el pago como pagado' + ) + } return res.status(200).json({ success: true, @@ -382,7 +457,18 @@ export class GroupPaymentController { }) } - const updated = await GroupPaymentRepository.updateAmountPaid(paymentId, amount_paid) + let next: ReturnType + try { + next = reconcilePayment({ + amount: oldPayment.amount, + amountPaid: amount_paid, + current: oldPayment.status, + }) + } catch (error) { + return ruleError(res, error) + } + + const updated = await GroupPaymentRepository.update(paymentId, next) if (!updated) { return res.status(500).json({ diff --git a/backend/repositories/group/group-payment-repository.ts b/backend/repositories/group/group-payment-repository.ts index f02f2532..bf00891e 100644 --- a/backend/repositories/group/group-payment-repository.ts +++ b/backend/repositories/group/group-payment-repository.ts @@ -159,34 +159,6 @@ export class GroupPaymentRepository { return result.affectedRows > 0 } - /** - * Actualizar cantidad pagada (pagos parciales) - */ - static async updateAmountPaid(id: number, amountPaid: number): Promise { - const payment = await this.getById(id) - - if (!payment) { - throw new Error('Pago no encontrado') - } - - // Calcular nuevo estado automáticamente - let newStatus = PaymentStatus.PENDING - if (amountPaid >= payment.amount) { - newStatus = PaymentStatus.PAID - } else if (amountPaid > 0) { - newStatus = PaymentStatus.PARTIAL - } - - const query = ` - UPDATE group_payments - SET amount_paid = ?, status = ?, status_updated_at = NOW() - WHERE id = ? - ` - - const [result] = await db.query(query, [amountPaid, newStatus, id]) - return result.affectedRows > 0 - } - /** * Eliminar pago */ @@ -203,7 +175,14 @@ export class GroupPaymentRepository { static async recalculateAmounts(groupId: number, totalAmount: number): Promise { const query = ` UPDATE group_payments - SET amount = (? * percentage / 100) + SET amount = (? * percentage / 100), + -- The status follows the new amount (MySQL assigns left to right) + status = CASE + WHEN amount > 0 AND amount_paid >= amount THEN 'paid' + WHEN amount_paid > 0 THEN 'partial' + WHEN status IN ('paid', 'partial') THEN 'pending' + ELSE status + END WHERE group_id = ? AND percentage IS NOT NULL ` diff --git a/backend/services/group/payment-rules.ts b/backend/services/group/payment-rules.ts new file mode 100644 index 00000000..a43be6b8 --- /dev/null +++ b/backend/services/group/payment-rules.ts @@ -0,0 +1,59 @@ +// services/group/payment-rules.ts +// Coherence rules for a group payment, decided on 2026-10-11: what was paid +// never exceeds what is owed, marking a payment paid by hand fills in what was +// paid, the status follows the amounts, and the percentages of a group's +// payments add up to 100 at most. DECIMAL columns arrive as strings, hence +// the Number() calls. + +import { PaymentStatus } from '../../models/group/index.js' +import { ERROR_CODES } from '../../config/error-codes.js' + +/** A rule the payment breaks; `code` is the ERROR_CODES entry to answer with */ +export class PaymentRuleError extends Error { + constructor(readonly code: string) { + super(code) + } +} + +/** Status that follows from what is owed and what was paid; "requested" stays until money arrives */ +export function statusFor( + amount: number, + amountPaid: number, + current?: PaymentStatus +): PaymentStatus { + if (amount > 0 && amountPaid >= amount) return PaymentStatus.PAID + if (amountPaid > 0) return PaymentStatus.PARTIAL + return current === PaymentStatus.REQUESTED ? PaymentStatus.REQUESTED : PaymentStatus.PENDING +} + +/** + * amount_paid and status to store after a change. An explicit status is kept, + * except that "paid" fills amount_paid with the amount; without one, the + * status follows the amounts. + */ +export function reconcilePayment(input: { + amount: number | string + amountPaid: number | string + status?: PaymentStatus + current?: PaymentStatus +}): { amount_paid: number; status: PaymentStatus } { + const amount = Number(input.amount) + const given = Number(input.amountPaid) + if (given > amount) throw new PaymentRuleError(ERROR_CODES.GROUP_PAYMENT_OVERPAID) + + const amountPaid = input.status === PaymentStatus.PAID ? amount : given + return { + amount_paid: amountPaid, + status: input.status ?? statusFor(amount, amountPaid, input.current), + } +} + +/** Throws when the group's percentages would add up to more than 100 */ +export function checkPercentages( + others: ReadonlyArray<{ percentage: number | string | null }>, + next: number | null | undefined +): void { + const total = others.reduce((sum, p) => sum + Number(p.percentage ?? 0), Number(next ?? 0)) + // Two decimals in the column: allow the rounding of thirds and the like + if (total > 100.001) throw new PaymentRuleError(ERROR_CODES.GROUP_PAYMENT_PERCENTAGE_OVER_100) +} diff --git a/backend/tests/group/payment-rules.test.ts b/backend/tests/group/payment-rules.test.ts new file mode 100644 index 00000000..64144300 --- /dev/null +++ b/backend/tests/group/payment-rules.test.ts @@ -0,0 +1,151 @@ +// tests/group/payment-rules.test.ts +// Regression tests for the group payments logic: more paid than owed was +// accepted, a payment could be marked paid with nothing paid, recalculating +// the group total left the status behind, and percentages could add up to +// more than 100. Rules decided on 2026-10-11 (services/group/payment-rules.ts). + +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { Request, Response } from 'express' +import { readFileSync } from 'node:fs' +import { join, dirname } from 'node:path' +import { fileURLToPath } from 'node:url' +import { PaymentStatus } from '../../models/group/index.js' +import { + checkPercentages, + reconcilePayment, + statusFor, +} from '../../services/group/payment-rules.js' +import { ERROR_CODES } from '../../config/error-codes.js' + +vi.mock('../../repositories/group/group-payment-repository', () => ({ + GroupPaymentRepository: { + getById: vi.fn(), + getByGroupId: vi.fn(async () => []), + update: vi.fn(async () => true), + updateStatus: vi.fn(async () => true), + create: vi.fn(async () => ({ id: 9 })), + }, +})) +vi.mock('../../repositories/group/group-repository', () => ({ + GroupRepository: { getById: vi.fn(async () => ({ id: 1, total_amount: 1000 })) }, +})) +vi.mock('../../services/group/group-history-service', () => ({ + GroupHistoryService: { logChange: vi.fn(), logPaymentUpdated: vi.fn() }, +})) + +const { GroupPaymentController } = + await import('../../controllers/group/group-payment-controller.js') +const { GroupPaymentRepository } = await import('../../repositories/group/group-payment-repository') + +const OVERPAID = ERROR_CODES.GROUP_PAYMENT_OVERPAID +const OVER_100 = ERROR_CODES.GROUP_PAYMENT_PERCENTAGE_OVER_100 + +describe('payment rules', () => { + it('the status follows the amounts and keeps "requested" until money arrives', () => { + expect(statusFor(100, 0)).toBe(PaymentStatus.PENDING) + expect(statusFor(100, 0, PaymentStatus.REQUESTED)).toBe(PaymentStatus.REQUESTED) + expect(statusFor(100, 40, PaymentStatus.REQUESTED)).toBe(PaymentStatus.PARTIAL) + expect(statusFor(100, 100)).toBe(PaymentStatus.PAID) + }) + + it('refuses more paid than owed, also when DECIMALs arrive as strings', () => { + expect(() => reconcilePayment({ amount: 100, amountPaid: 100.01 })).toThrow(OVERPAID) + expect(() => reconcilePayment({ amount: '100.00', amountPaid: '250.00' })).toThrow(OVERPAID) + expect(reconcilePayment({ amount: '100.00', amountPaid: '100.00' })).toEqual({ + amount_paid: 100, + status: PaymentStatus.PAID, + }) + }) + + it('marking it paid fills in what was paid', () => { + expect(reconcilePayment({ amount: 300, amountPaid: 0, status: PaymentStatus.PAID })).toEqual({ + amount_paid: 300, + status: PaymentStatus.PAID, + }) + }) + + it('percentages add up to 100 at most', () => { + expect(() => checkPercentages([{ percentage: '60.00' }], 40)).not.toThrow() + expect(() => + checkPercentages([{ percentage: 33.33 }, { percentage: 33.33 }], 33.34) + ).not.toThrow() + expect(() => checkPercentages([{ percentage: '60.00' }, { percentage: null }], 41)).toThrow( + OVER_100 + ) + }) + + it('the total recalculation sets the status from the new amount', () => { + const repo = readFileSync( + join( + dirname(fileURLToPath(import.meta.url)), + '..', + '..', + 'repositories', + 'group', + 'group-payment-repository.ts' + ), + 'utf8' + ) + const recalc = repo.slice(repo.indexOf('static async recalculateAmounts')) + expect(recalc.slice(0, 900)).toMatch( + /SET amount = \(\? \* percentage \/ 100\),\s+[\s\S]*status = CASE/ + ) + }) +}) + +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 payment = { id: 5, group_id: 1, amount: '300.00', amount_paid: '0.00', status: 'pending' } +const params = { id: '1', paymentId: '5' } + +describe('payment endpoints', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(GroupPaymentRepository.getById).mockResolvedValue(payment as never) + }) + + it('PATCH status paid stores amount_paid = amount', async () => { + const res = await call(GroupPaymentController.updatePaymentStatus, params, { status: 'paid' }) + expect(res.status).toHaveBeenCalledWith(200) + expect(GroupPaymentRepository.update).toHaveBeenCalledWith(5, { + status: 'paid', + amount_paid: 300, + }) + }) + + it('PATCH amount-paid over the amount answers 400 and writes nothing', async () => { + const res = await call(GroupPaymentController.updateAmountPaid, params, { amount_paid: 301 }) + expect(res.status).toHaveBeenCalledWith(400) + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ code: OVERPAID })) + expect(GroupPaymentRepository.update).not.toHaveBeenCalled() + }) + + it('PUT with a percentage over the group total answers 400', async () => { + vi.mocked(GroupPaymentRepository.getByGroupId).mockResolvedValue([ + { id: 6, percentage: '70.00' }, + ] as never) + const res = await call(GroupPaymentController.updatePayment, params, { percentage: 40 }) + expect(res.status).toHaveBeenCalledWith(400) + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ code: OVER_100 })) + }) + + it('POST paid by hand is created with amount_paid filled in', async () => { + const res = await call( + GroupPaymentController.createPayment, + { id: '1' }, + { payment_name: 'Anticipo', amount: 200, due_date: '2026-11-01', status: 'paid' } + ) + expect(res.status).toHaveBeenCalledWith(201) + expect(GroupPaymentRepository.create).toHaveBeenCalledWith( + expect.objectContaining({ amount_paid: 200, status: 'paid' }) + ) + }) +}) diff --git a/frontend/app/lib/helpers/errorCodeMessage.ts b/frontend/app/lib/helpers/errorCodeMessage.ts index 09360dee..bce4898f 100644 --- a/frontend/app/lib/helpers/errorCodeMessage.ts +++ b/frontend/app/lib/helpers/errorCodeMessage.ts @@ -201,6 +201,8 @@ const TRANSLATED = new Set([ 'GROUP_PAYMENT_DELETE_ERROR', 'GROUP_PAYMENT_FETCH_ERROR', 'GROUP_PAYMENT_INVALID_AMOUNT', + 'GROUP_PAYMENT_OVERPAID', + 'GROUP_PAYMENT_PERCENTAGE_OVER_100', 'GROUP_PAYMENT_MISSING_FIELDS', 'GROUP_PAYMENT_NOT_FOUND', 'GROUP_PAYMENT_NOT_IN_GROUP', diff --git a/frontend/messages/en/errors.json b/frontend/messages/en/errors.json index 409b8e3a..da1de2d4 100644 --- a/frontend/messages/en/errors.json +++ b/frontend/messages/en/errors.json @@ -290,6 +290,8 @@ "GROUP_ROOM_DELETE_ERROR": "Error deleting room", "GROUP_PAYMENT_MISSING_FIELDS": "Missing required fields: payment name, due date", "GROUP_PAYMENT_INVALID_AMOUNT": "Payment must have an amount greater than 0", + "GROUP_PAYMENT_OVERPAID": "The amount paid cannot exceed the payment amount", + "GROUP_PAYMENT_PERCENTAGE_OVER_100": "The group's payment percentages add up to more than 100", "GROUP_PAYMENT_NOT_FOUND": "Payment not found", "GROUP_PAYMENT_NOT_IN_GROUP": "Payment does not belong to this group", "GROUP_PAYMENT_FETCH_ERROR": "Error fetching payments", diff --git a/frontend/messages/es/errors.json b/frontend/messages/es/errors.json index c36f2733..0a0f4260 100644 --- a/frontend/messages/es/errors.json +++ b/frontend/messages/es/errors.json @@ -290,6 +290,8 @@ "GROUP_ROOM_DELETE_ERROR": "Error al eliminar habitación", "GROUP_PAYMENT_MISSING_FIELDS": "Faltan campos obligatorios: nombre del pago, fecha de vencimiento", "GROUP_PAYMENT_INVALID_AMOUNT": "El pago debe tener un monto mayor a 0", + "GROUP_PAYMENT_OVERPAID": "Lo pagado no puede superar el importe del pago", + "GROUP_PAYMENT_PERCENTAGE_OVER_100": "Los porcentajes de los pagos del grupo suman más de 100", "GROUP_PAYMENT_NOT_FOUND": "Pago no encontrado", "GROUP_PAYMENT_NOT_IN_GROUP": "El pago no pertenece a este grupo", "GROUP_PAYMENT_FETCH_ERROR": "Error al obtener pagos",