fix: PR A — concurrency + money bugs (round 4, bugs 1-4) (#78)

Bug 1 (CRITICAL): payment capture/refund/void race
- Conditional UPDATE-by-status claims the row atomically
- Stripe idempotency keys (cap_/ref_/void_) prevent gateway double-charge
- Two-phase pattern: short tx claims state, network call outside, tx finalizes

Bug 2 (HIGH): folio.transferCharge non-atomic
- Wrapped in db.transaction, both folios locked FOR UPDATE in id-order
- Helpers accept optional tx; recalculateBalance uses decimal.js (no parseFloat)

Bug 3 (HIGH): transferToCityLedger non-atomic
- All 4 steps wrapped in db.transaction
- Monetary comparisons use decimal.js on string representations

Bug 4 (HIGH): night audit concurrent execution
- Unique index on (property_id, business_date)
- onConflictDoNothing + re-select: completed → ConflictException,
  running → return as active audit

All financial math now uses decimal.js on Drizzle's string numerics.
No parseFloat on monetary values.

552/552 tests passing, build + typecheck clean.

Co-authored-by: Dušan Milićević <dusanmilicevic33@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: telivity-otaip

apps/api/package.json

@@ -31,6 +31,7 @@
     "@nestjs/websockets": "^10.0.0",
     "class-transformer": "^0.5.1",
     "class-validator": "^0.14.1",
+    "decimal.js": "^10.4.3",
     "drizzle-orm": "^0.39.0",
     "eventemitter2": "^6.4.9",
     "fast-xml-parser": "^5.5.10",

apps/api/src/modules/folio/folio-routing.service.spec.ts

@@ -46,7 +46,7 @@ const mockFolioService = {
 };
 
 function createMockDb(returnData: any[] = [mockReservation]) {
-  return {
+  const db: any = {
     select: vi.fn().mockImplementation(() => ({
       from: vi.fn().mockReturnValue({
         where: vi.fn().mockReturnValue({
@@ -68,6 +68,8 @@ function createMockDb(returnData: any[] = [mockReservation]) {
     }),
     delete: vi.fn(),
   };
+  db.transaction = (cb: any) => cb(db);
+  return db;
 }
 
 describe('FolioRoutingService', () => {
@@ -213,6 +215,7 @@ describe('FolioRoutingService', () => {
           companyName: 'Acme Corp',
           paymentTermsDays: 'NET30',
         }),
+        expect.anything(), // Bug 3: now called inside db.transaction with tx
       );
       expect(result.transferredAmount).toBe('300.00');
       expect(result.cityLedgerFolioId).toBe('folio-cl-001');

apps/api/src/modules/folio/folio-routing.service.ts

@@ -4,6 +4,7 @@ import {
   NotFoundException,
 } from '@nestjs/common';
 import { eq, and } from 'drizzle-orm';
+import Decimal from 'decimal.js';
 import { folios, reservations, payments } from '@haip/database';
 import { DRIZZLE } from '../../database/database.module';
 import { FolioService } from './folio.service';
@@ -77,53 +78,70 @@ export class FolioRoutingService {
     dto: TransferCityLedgerDto,
   ) {
     const sourceFolio = await this.folioService.findById(folioId, propertyId);
-    const remainingBalance = parseFloat(sourceFolio.balance);
+    // Monetary compare on string representation via decimal.js (numeric-as-string).
+    const remainingBalance = new Decimal(sourceFolio.balance);
 
-    if (remainingBalance <= 0) {
+    if (remainingBalance.lte(0)) {
       return { message: 'No outstanding balance to transfer' };
     }
 
-    // Create city ledger folio
-    const cityLedgerFolio = await this.folioService.create({
-      propertyId,
-      guestId: sourceFolio.guestId,
-      type: 'city_ledger',
-      currencyCode: sourceFolio.currencyCode,
-      companyName: dto.companyName,
-      billingAddress: dto.billingAddress,
-      paymentTermsDays: dto.paymentTermsDays,
-    });
+    const amountStr = remainingBalance.toFixed(2);
+
+    // Bug 3: wrap all mutating steps in a single transaction so partial failure
+    // (e.g. CL folio created but payment insert fails) is impossible.
+    // Idempotency-by-transferId is out of scope for this PR.
+    const { cityLedgerFolio } = await this.db.transaction(async (tx: any) => {
+      // Create city ledger folio
+      const cityLedgerFolio = await this.folioService.create(
+        {
+          propertyId,
+          guestId: sourceFolio.guestId,
+          type: 'city_ledger',
+          currencyCode: sourceFolio.currencyCode,
+          companyName: dto.companyName,
+          billingAddress: dto.billingAddress,
+          paymentTermsDays: dto.paymentTermsDays,
+        },
+        tx,
+      );
+
+      // Record city_ledger payment on the source folio (zeroes out the guest folio)
+      await tx
+        .insert(payments)
+        .values({
+          folioId,
+          propertyId,
+          method: 'city_ledger',
+          amount: amountStr,
+          currencyCode: sourceFolio.currencyCode,
+          status: 'captured',
+          processedAt: new Date(),
+          notes: `Transferred to city ledger: ${dto.companyName}`,
+        });
+
+      await this.folioService.recalculateBalance(folioId, propertyId, tx);
+
+      // Post matching charge on the city ledger folio
+      await this.folioService.postCharge(
+        cityLedgerFolio.id,
+        {
+          propertyId,
+          type: 'fee',
+          description: `Transfer from folio ${sourceFolio.folioNumber}`,
+          amount: amountStr,
+          currencyCode: sourceFolio.currencyCode,
+          serviceDate: new Date().toISOString(),
+        },
+        tx,
+      );
 
-    // Record city_ledger payment on the source folio (zeroes out the guest folio)
-    await this.db
-      .insert(payments)
-      .values({
-        folioId,
-        propertyId,
-        method: 'city_ledger',
-        amount: remainingBalance.toFixed(2),
-        currencyCode: sourceFolio.currencyCode,
-        status: 'captured',
-        processedAt: new Date(),
-        notes: `Transferred to city ledger: ${dto.companyName}`,
-      });
-
-    await this.folioService.recalculateBalance(folioId, propertyId);
-
-    // Post matching charge on the city ledger folio
-    await this.folioService.postCharge(cityLedgerFolio.id, {
-      propertyId,
-      type: 'fee',
-      description: `Transfer from folio ${sourceFolio.folioNumber}`,
-      amount: remainingBalance.toFixed(2),
-      currencyCode: sourceFolio.currencyCode,
-      serviceDate: new Date().toISOString(),
+      return { cityLedgerFolio };
     });
 
     return {
       sourceFolioId: folioId,
       cityLedgerFolioId: cityLedgerFolio.id,
-      transferredAmount: remainingBalance.toFixed(2),
+      transferredAmount: amountStr,
     };
   }
 }

apps/api/src/modules/folio/folio.service.spec.ts

@@ -233,20 +233,23 @@ describe('FolioService', () => {
     it('should transfer charge between folios', async () => {
       let selectCallCount = 0;
       const targetFolio = { ...mockFolio, id: 'folio-002' };
-      const db = {
+      const thenResolver = (resolve: any) => {
+        selectCallCount++;
+        // Bug 2: new order is deterministic by folio.id: folio-001 (source), folio-002 (target), charge lookup, then recalc sums.
+        if (selectCallCount === 1) resolve([mockFolio]);
+        else if (selectCallCount === 2) resolve([targetFolio]);
+        else if (selectCallCount === 3) resolve([mockCharge]);
+        else resolve([{ total: '0' }]);
+      };
+      const whereChain = () => ({
+        then: thenResolver,
+        // .for('update') returns a thenable that also resolves the current row
+        for: vi.fn().mockReturnValue({ then: thenResolver }),
+      });
+      const db: any = {
         select: vi.fn().mockImplementation(() => ({
           from: vi.fn().mockReturnValue({
-            where: vi.fn().mockReturnValue({
-              then: (resolve: any) => {
-                selectCallCount++;
-                // 1: source folio, 2: target folio, 3: charge lookup
-                // 4-5: recalculate charge sums, 6-7: recalculate payment sums
-                if (selectCallCount === 1) resolve([mockFolio]);
-                else if (selectCallCount === 2) resolve([targetFolio]);
-                else if (selectCallCount === 3) resolve([mockCharge]);
-                else resolve([{ total: '0' }]);
-              },
-            }),
+            where: vi.fn().mockImplementation(whereChain),
           }),
         })),
         insert: vi.fn(),
@@ -259,6 +262,8 @@ describe('FolioService', () => {
         }),
         delete: vi.fn(),
       };
+      // Bug 2: transferCharge wraps everything in db.transaction — pass tx = db.
+      db.transaction = (cb: any) => cb(db);
 
       const module: TestingModule = await Test.createTestingModule({
         providers: [