From 91dd6627c49b5e31148ae2e6cd406ec4a6c8c46c Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:03:03 +0200 Subject: [PATCH 1/3] fix(bank-tx): count bank account fee bookings in the monthly bank cost getBankTxFee summed chargeAmountChf only, a field the statement import stopped populating with the bank migration in December 2025 - the monthly bank cost has read zero ever since while the real fees arrive as dedicated BankAccountFee rows. Those rows now aggregate per currency (debits as cost, credits netted) and convert to CHF alongside the legacy per-tx charge sum; a missing price throws instead of hiding the position. --- .../bank-tx/__tests__/bank-tx.service.spec.ts | 186 ++++++++++++++++++ .../bank-tx/services/bank-tx.service.ts | 47 ++++- .../log/__tests__/log-job.service.spec.ts | 12 ++ 3 files changed, 244 insertions(+), 1 deletion(-) create mode 100644 src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts diff --git a/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts b/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts new file mode 100644 index 0000000000..6ab08d239e --- /dev/null +++ b/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts @@ -0,0 +1,186 @@ +import { createMock } from '@golevelup/ts-jest'; +import { ConfigService } from 'src/config/config'; +import { OlkypayService } from 'src/integration/bank/services/olkypay.service'; +import { YapealService } from 'src/integration/bank/services/yapeal.service'; +import { createCustomPrice } from 'src/integration/exchange/dto/__mocks__/price.dto.mock'; +import { createCustomFiat } from 'src/shared/models/fiat/__mocks__/fiat.entity.mock'; +import { FiatService } from 'src/shared/models/fiat/fiat.service'; +import { SettingService } from 'src/shared/models/setting/setting.service'; +import { BuyCryptoService } from 'src/subdomains/core/buy-crypto/process/services/buy-crypto.service'; +import { BuyService } from 'src/subdomains/core/buy-crypto/routes/buy/buy.service'; +import { BankDataService } from 'src/subdomains/generic/user/models/bank-data/bank-data.service'; +import { BankService } from 'src/subdomains/supporting/bank/bank/bank.service'; +import { VirtualIbanService } from 'src/subdomains/supporting/bank/virtual-iban/virtual-iban.service'; +import { NotificationService } from 'src/subdomains/supporting/notification/services/notification.service'; +import { SpecialExternalAccountService } from 'src/subdomains/supporting/payment/services/special-external-account.service'; +import { TransactionNotificationService } from 'src/subdomains/supporting/payment/services/transaction-notification.service'; +import { TransactionService } from 'src/subdomains/supporting/payment/services/transaction.service'; +import { PricingService } from 'src/subdomains/supporting/pricing/services/pricing.service'; +import { BankTxRepeatService } from '../../bank-tx-repeat/bank-tx-repeat.service'; +import { BankTxReturnService } from '../../bank-tx-return/bank-tx-return.service'; +import { createCustomBankTx } from '../__mocks__/bank-tx.entity.mock'; +import { BankTx, BankTxIndicator, BankTxType } from '../entities/bank-tx.entity'; +import { BankTxBatchRepository } from '../repositories/bank-tx-batch.repository'; +import { BankTxRepository } from '../repositories/bank-tx.repository'; +import { BankTxService } from '../services/bank-tx.service'; +import { SepaParser } from '../services/sepa-parser.service'; + +describe('BankTxService', () => { + let service: BankTxService; + + let bankTxRepo: BankTxRepository; + let pricingService: PricingService; + let fiatService: FiatService; + + let qb: any; + + const from = new Date('2026-07-01'); + + beforeAll(() => { + new ConfigService(); // sets module-level Config (defaultVolumeDecimal read by the CHF conversion) + }); + + beforeEach(() => { + jest.clearAllMocks(); + + bankTxRepo = createMock(); + pricingService = createMock(); + fiatService = createMock(); + + // legacy chargeAmountChf sum query builder + qb = { select: jest.fn(() => qb), where: jest.fn(() => qb), getRawOne: jest.fn() }; + (bankTxRepo.createQueryBuilder as jest.Mock).mockReturnValue(qb); + + (fiatService.getFiatByName as jest.Mock).mockImplementation((name: string) => createCustomFiat({ name })); + + service = new BankTxService( + bankTxRepo, + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + pricingService, + fiatService, + ); + }); + + function mockLegacyFee(fee: number | null): void { + qb.getRawOne.mockResolvedValue({ fee }); + } + + function mockFeeRows(rows: Partial[]): void { + (bankTxRepo.findBy as jest.Mock).mockResolvedValue(rows.map((r) => createCustomBankTx(r))); + } + + // identity conversion: Price.convert divides by price, so price 1 keeps the amount unchanged + function mockIdentityChfPrice(): void { + (pricingService.getPrice as jest.Mock).mockResolvedValue( + createCustomPrice({ source: 'EUR', target: 'CHF', price: 1 }), + ); + } + + it('adds DBIT BankAccountFee rows converted to CHF into the fee', async () => { + mockLegacyFee(null); + mockFeeRows([ + { + id: 1, + type: BankTxType.BANK_ACCOUNT_FEE, + creditDebitIndicator: BankTxIndicator.DEBIT, + amount: 100, + currency: 'EUR', + }, + { + id: 2, + type: BankTxType.BANK_ACCOUNT_FEE, + creditDebitIndicator: BankTxIndicator.DEBIT, + amount: 200, + currency: 'CHF', + }, + ]); + mockIdentityChfPrice(); + + await expect(service.getBankTxFee(from)).resolves.toBe(300); + + expect(fiatService.getFiatByName).toHaveBeenCalledTimes(2); + expect(pricingService.getPrice).toHaveBeenCalledTimes(2); + }); + + it('nets a CRDT refund against a DBIT fee in the same currency', async () => { + mockLegacyFee(null); + mockFeeRows([ + { + id: 1, + type: BankTxType.BANK_ACCOUNT_FEE, + creditDebitIndicator: BankTxIndicator.DEBIT, + amount: 100, + currency: 'EUR', + }, + { + id: 2, + type: BankTxType.BANK_ACCOUNT_FEE, + creditDebitIndicator: BankTxIndicator.CREDIT, + amount: 30, + currency: 'EUR', + }, + ]); + mockIdentityChfPrice(); + + await expect(service.getBankTxFee(from)).resolves.toBe(70); + + expect(fiatService.getFiatByName).toHaveBeenCalledTimes(1); + expect(pricingService.getPrice).toHaveBeenCalledTimes(1); + }); + + it('adds the legacy chargeAmountChf sum on top of the BankAccountFee rows', async () => { + mockLegacyFee(5); + mockFeeRows([ + { + id: 1, + type: BankTxType.BANK_ACCOUNT_FEE, + creditDebitIndicator: BankTxIndicator.DEBIT, + amount: 100, + currency: 'EUR', + }, + ]); + mockIdentityChfPrice(); + + await expect(service.getBankTxFee(from)).resolves.toBe(105); + }); + + it('fails loud when the price is unavailable', async () => { + mockLegacyFee(null); + mockFeeRows([ + { + id: 1, + type: BankTxType.BANK_ACCOUNT_FEE, + creditDebitIndicator: BankTxIndicator.DEBIT, + amount: 100, + currency: 'EUR', + }, + ]); + (pricingService.getPrice as jest.Mock).mockRejectedValue(new Error('No valid price')); + + await expect(service.getBankTxFee(from)).rejects.toThrow(); + }); + + it('returns only the legacy sum when there are no BankAccountFee rows', async () => { + mockLegacyFee(5); + mockFeeRows([]); + + await expect(service.getBankTxFee(from)).resolves.toBe(5); + + expect(pricingService.getPrice).not.toHaveBeenCalled(); + }); +}); diff --git a/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts b/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts index 64c7c74838..aed4ee54d9 100644 --- a/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts +++ b/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts @@ -9,7 +9,9 @@ import { } from '@nestjs/common'; import { CronExpression } from '@nestjs/schedule'; import { Observable, Subject } from 'rxjs'; +import { Config } from 'src/config/config'; import { YapealService } from 'src/integration/bank/services/yapeal.service'; +import { FiatService } from 'src/shared/models/fiat/fiat.service'; import { SettingService } from 'src/shared/models/setting/setting.service'; import { DfxLogger } from 'src/shared/services/dfx-logger'; import { Process } from 'src/shared/services/process.service'; @@ -26,6 +28,11 @@ import { MailContext, MailType } from 'src/subdomains/supporting/notification/en import { NotificationService } from 'src/subdomains/supporting/notification/services/notification.service'; import { SpecialExternalAccount } from 'src/subdomains/supporting/payment/entities/special-external-account.entity'; import { TransactionNotificationService } from 'src/subdomains/supporting/payment/services/transaction-notification.service'; +import { + PriceCurrency, + PriceValidity, + PricingService, +} from 'src/subdomains/supporting/pricing/services/pricing.service'; import { DeepPartial, FindOptionsRelations, @@ -114,6 +121,8 @@ export class BankTxService implements OnModuleInit { private readonly virtualIbanService: VirtualIbanService, @Inject(forwardRef(() => TransactionNotificationService)) private readonly transactionNotificationService: TransactionNotificationService, + private readonly pricingService: PricingService, + private readonly fiatService: FiatService, ) {} onModuleInit() { @@ -430,6 +439,10 @@ export class BankTxService implements OnModuleInit { ]); } + // Bank fees come from two sources: legacy per-tx charges (chargeAmountChf) plus dedicated BankAccountFee rows. + // Charge fields ceased with the bank migration in Dec 2025; fees arrive as dedicated BankAccountFee rows since, + // carrying the amount in the account currency (no amountChf column), so they are aggregated per currency and + // converted to CHF. async getBankTxFee(from: Date): Promise { const { fee } = await this.bankTxRepo .createQueryBuilder('bankTx') @@ -437,7 +450,39 @@ export class BankTxService implements OnModuleInit { .where('bankTx.created >= :from', { from }) .getRawOne<{ fee: number }>(); - return fee ?? 0; + let totalFeeChf = fee ?? 0; + + const feeRows = await this.bankTxRepo.findBy({ + type: BankTxType.BANK_ACCOUNT_FEE, + created: MoreThanOrEqual(from), + }); + + const amountByCurrency = new Map(); + for (const tx of feeRows) { + let signedAmount: number; + if (tx.creditDebitIndicator === BankTxIndicator.DEBIT) { + signedAmount = +tx.amount; + } else if (tx.creditDebitIndicator === BankTxIndicator.CREDIT) { + signedAmount = -tx.amount; + } else { + this.logger.error( + `BankAccountFee bankTx ${tx.id} has unexpected creditDebitIndicator ${tx.creditDebitIndicator}, skipping`, + ); + continue; + } + + amountByCurrency.set(tx.currency, (amountByCurrency.get(tx.currency) ?? 0) + signedAmount); + } + + for (const [currency, amount] of amountByCurrency) { + if (!amount) continue; + + const fiat = await this.fiatService.getFiatByName(currency); + const price = await this.pricingService.getPrice(fiat, PriceCurrency.CHF, PriceValidity.ANY); + totalFeeChf += price.convert(amount, Config.defaultVolumeDecimal); + } + + return totalFeeChf; } async getRecentBankToBankTx(fromIban: string, toIban: string): Promise { diff --git a/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts b/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts index e513fce5ce..8832bd7ba9 100644 --- a/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts +++ b/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts @@ -160,6 +160,18 @@ describe('LogJobService', () => { expect(result.minus.scrypt).toBeUndefined(); expect(result.minus.mexc).toBeUndefined(); }); + + it('flows the bank tx fee into minus.bank and the totals', async () => { + setupEmpty(); + jest.spyOn(exchangeTxService, 'getExchangeTx').mockResolvedValue([] as any); + jest.spyOn(bankTxService, 'getBankTxFee').mockResolvedValue(4196 as any); + + const result = await (service as any).getChangeLog(); + + expect(result.minus.bank).toBe(4196); + expect(result.minus.total).toBe(4196); + expect(result.total).toBe(-4196); + }); }); describe('saveTradingLog (referral-credit liability)', () => { From 3bb4658966c125e0b4d2b366b32f6a024edfa3b7 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:24:15 +0200 Subject: [PATCH 2/3] fix(bank-tx): address review findings (safety-mode isolation, aggregation, rounding) - the change log moved behind the equity path and runs in its own try/catch: a reporting price failure logs and skips the minute's changes entry instead of arming the equity safety mode and suppressing the financial data log - bank account fee rows aggregate server-side (GROUP BY currency and indicator) with a new composite index on bank_tx (type, created) instead of hydrating full entities every minute - the CHF total rounds like its sepa-parser sibling; conversion tests use distinct non-unit prices per currency so direction and mapping errors fail --- ...1784117860216-AddBankTxTypeCreatedIndex.js | 33 +++++ .../bank-tx/__tests__/bank-tx.service.spec.ts | 139 +++++++++--------- .../bank-tx/services/bank-tx.service.ts | 36 +++-- .../log/__tests__/log-job.service.spec.ts | 41 ++++++ .../supporting/log/log-job.service.ts | 29 ++-- 5 files changed, 184 insertions(+), 94 deletions(-) create mode 100644 migration/1784117860216-AddBankTxTypeCreatedIndex.js diff --git a/migration/1784117860216-AddBankTxTypeCreatedIndex.js b/migration/1784117860216-AddBankTxTypeCreatedIndex.js new file mode 100644 index 0000000000..a2d61642f8 --- /dev/null +++ b/migration/1784117860216-AddBankTxTypeCreatedIndex.js @@ -0,0 +1,33 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * Backs the monthly bank-cost aggregation (BankTxService.getBankTxFee), which runs every minute from + * the financial-log cron and filters bank_tx by (type = BANK_ACCOUNT_FEE, created >= from). Without a + * matching composite index the query degrades to a full scan over a table that grows continuously. + * + * bank_tx carries no primary key in production (MSSQL->Postgres cutover drift), so this migration adds + * ONLY a secondary index: no foreign key, no column alter, nothing that requires a unique constraint. + * + * @class + * @implements {MigrationInterface} + */ +module.exports = class AddBankTxTypeCreatedIndex1784117860216 { + name = 'AddBankTxTypeCreatedIndex1784117860216'; + + /** + * @param {QueryRunner} queryRunner + */ + async up(queryRunner) { + await queryRunner.query(`CREATE INDEX "IDX_bank_tx_type_created" ON "bank_tx" ("type", "created")`); + } + + /** + * @param {QueryRunner} queryRunner + */ + async down(queryRunner) { + await queryRunner.query(`DROP INDEX "public"."IDX_bank_tx_type_created"`); + } +}; diff --git a/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts b/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts index 6ab08d239e..6602816634 100644 --- a/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts +++ b/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts @@ -18,13 +18,19 @@ import { TransactionService } from 'src/subdomains/supporting/payment/services/t import { PricingService } from 'src/subdomains/supporting/pricing/services/pricing.service'; import { BankTxRepeatService } from '../../bank-tx-repeat/bank-tx-repeat.service'; import { BankTxReturnService } from '../../bank-tx-return/bank-tx-return.service'; -import { createCustomBankTx } from '../__mocks__/bank-tx.entity.mock'; -import { BankTx, BankTxIndicator, BankTxType } from '../entities/bank-tx.entity'; +import { BankTxIndicator } from '../entities/bank-tx.entity'; import { BankTxBatchRepository } from '../repositories/bank-tx-batch.repository'; import { BankTxRepository } from '../repositories/bank-tx.repository'; import { BankTxService } from '../services/bank-tx.service'; import { SepaParser } from '../services/sepa-parser.service'; +// one raw aggregate row as returned by the GROUP BY currency, creditDebitIndicator query +interface FeeAggregate { + currency: string; + creditDebitIndicator: BankTxIndicator; + amount: string; // Postgres returns SUM() as a string +} + describe('BankTxService', () => { let service: BankTxService; @@ -36,6 +42,11 @@ describe('BankTxService', () => { const from = new Date('2026-07-01'); + // Distinct per-currency CHF prices. Price.convert divides the amount by the price + // (getPrice(fiat, CHF).convert(x) = x / price), so a wrong currency->price mapping or an + // inverted (multiply) direction produces a different total and turns these tests red. + const chfPriceByCurrency: Record = { EUR: 0.8, USD: 2 }; + beforeAll(() => { new ConfigService(); // sets module-level Config (defaultVolumeDecimal read by the CHF conversion) }); @@ -47,8 +58,18 @@ describe('BankTxService', () => { pricingService = createMock(); fiatService = createMock(); - // legacy chargeAmountChf sum query builder - qb = { select: jest.fn(() => qb), where: jest.fn(() => qb), getRawOne: jest.fn() }; + // one chainable query builder mock serves both queries: the legacy chargeAmountChf sum + // (getRawOne) and the per-currency/direction BankAccountFee aggregation (getRawMany). + qb = { + select: jest.fn(() => qb), + addSelect: jest.fn(() => qb), + where: jest.fn(() => qb), + andWhere: jest.fn(() => qb), + groupBy: jest.fn(() => qb), + addGroupBy: jest.fn(() => qb), + getRawOne: jest.fn(), + getRawMany: jest.fn(), + }; (bankTxRepo.createQueryBuilder as jest.Mock).mockReturnValue(qb); (fiatService.getFiatByName as jest.Mock).mockImplementation((name: string) => createCustomFiat({ name })); @@ -80,104 +101,80 @@ describe('BankTxService', () => { qb.getRawOne.mockResolvedValue({ fee }); } - function mockFeeRows(rows: Partial[]): void { - (bankTxRepo.findBy as jest.Mock).mockResolvedValue(rows.map((r) => createCustomBankTx(r))); + function mockFeeAggregates(rows: FeeAggregate[]): void { + qb.getRawMany.mockResolvedValue(rows); } - // identity conversion: Price.convert divides by price, so price 1 keeps the amount unchanged - function mockIdentityChfPrice(): void { - (pricingService.getPrice as jest.Mock).mockResolvedValue( - createCustomPrice({ source: 'EUR', target: 'CHF', price: 1 }), + // per-currency prices from chfPriceByCurrency (distinct so mapping/direction errors are caught) + function mockChfPrices(): void { + (pricingService.getPrice as jest.Mock).mockImplementation(async (fiat: { name: string }) => + createCustomPrice({ source: fiat.name, target: 'CHF', price: chfPriceByCurrency[fiat.name] }), ); } - it('adds DBIT BankAccountFee rows converted to CHF into the fee', async () => { + it('converts each DBIT currency aggregate with its own price and sums the result', async () => { mockLegacyFee(null); - mockFeeRows([ - { - id: 1, - type: BankTxType.BANK_ACCOUNT_FEE, - creditDebitIndicator: BankTxIndicator.DEBIT, - amount: 100, - currency: 'EUR', - }, - { - id: 2, - type: BankTxType.BANK_ACCOUNT_FEE, - creditDebitIndicator: BankTxIndicator.DEBIT, - amount: 200, - currency: 'CHF', - }, + mockFeeAggregates([ + { currency: 'EUR', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '100' }, + { currency: 'USD', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '200' }, ]); - mockIdentityChfPrice(); + mockChfPrices(); - await expect(service.getBankTxFee(from)).resolves.toBe(300); + // EUR 100 / 0.8 = 125 CHF; USD 200 / 2 = 100 CHF; total 225 CHF + // (a multiply direction -> 80 + 400 = 480; a swapped currency->price -> 125 + 250 = 375) + await expect(service.getBankTxFee(from)).resolves.toBe(225); - expect(fiatService.getFiatByName).toHaveBeenCalledTimes(2); + expect(fiatService.getFiatByName).toHaveBeenCalledWith('EUR'); + expect(fiatService.getFiatByName).toHaveBeenCalledWith('USD'); expect(pricingService.getPrice).toHaveBeenCalledTimes(2); }); - it('nets a CRDT refund against a DBIT fee in the same currency', async () => { + it('nets a CRDT refund against a DBIT fee in the same currency before converting', async () => { mockLegacyFee(null); - mockFeeRows([ - { - id: 1, - type: BankTxType.BANK_ACCOUNT_FEE, - creditDebitIndicator: BankTxIndicator.DEBIT, - amount: 100, - currency: 'EUR', - }, - { - id: 2, - type: BankTxType.BANK_ACCOUNT_FEE, - creditDebitIndicator: BankTxIndicator.CREDIT, - amount: 30, - currency: 'EUR', - }, + mockFeeAggregates([ + { currency: 'EUR', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '100' }, + { currency: 'EUR', creditDebitIndicator: BankTxIndicator.CREDIT, amount: '30' }, ]); - mockIdentityChfPrice(); + mockChfPrices(); - await expect(service.getBankTxFee(from)).resolves.toBe(70); + // net EUR 70 / 0.8 = 87.5 CHF (converted once, after netting) + await expect(service.getBankTxFee(from)).resolves.toBe(87.5); - expect(fiatService.getFiatByName).toHaveBeenCalledTimes(1); expect(pricingService.getPrice).toHaveBeenCalledTimes(1); }); - it('adds the legacy chargeAmountChf sum on top of the BankAccountFee rows', async () => { + it('adds the legacy chargeAmountChf sum on top of the converted BankAccountFee aggregates', async () => { mockLegacyFee(5); - mockFeeRows([ - { - id: 1, - type: BankTxType.BANK_ACCOUNT_FEE, - creditDebitIndicator: BankTxIndicator.DEBIT, - amount: 100, - currency: 'EUR', - }, - ]); - mockIdentityChfPrice(); + mockFeeAggregates([{ currency: 'EUR', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '100' }]); + mockChfPrices(); + + // legacy 5 + EUR 100 / 0.8 (125) = 130 + await expect(service.getBankTxFee(from)).resolves.toBe(130); + }); + + it('rounds the assembled total to the default volume decimals', async () => { + mockLegacyFee(0.1); + mockFeeAggregates([{ currency: 'EUR', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '0.2' }]); + // price 1 so the converted amount is exactly 0.2 and only the final sum needs rounding + (pricingService.getPrice as jest.Mock).mockResolvedValue( + createCustomPrice({ source: 'EUR', target: 'CHF', price: 1 }), + ); - await expect(service.getBankTxFee(from)).resolves.toBe(105); + // legacy 0.1 + EUR 0.2 = 0.30000000000000004 in float -> must be rounded to 0.3 + await expect(service.getBankTxFee(from)).resolves.toBe(0.3); }); it('fails loud when the price is unavailable', async () => { mockLegacyFee(null); - mockFeeRows([ - { - id: 1, - type: BankTxType.BANK_ACCOUNT_FEE, - creditDebitIndicator: BankTxIndicator.DEBIT, - amount: 100, - currency: 'EUR', - }, - ]); + mockFeeAggregates([{ currency: 'EUR', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '100' }]); (pricingService.getPrice as jest.Mock).mockRejectedValue(new Error('No valid price')); await expect(service.getBankTxFee(from)).rejects.toThrow(); }); - it('returns only the legacy sum when there are no BankAccountFee rows', async () => { + it('returns only the legacy sum when there are no BankAccountFee aggregates', async () => { mockLegacyFee(5); - mockFeeRows([]); + mockFeeAggregates([]); await expect(service.getBankTxFee(from)).resolves.toBe(5); diff --git a/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts b/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts index aed4ee54d9..1cb4cf61d4 100644 --- a/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts +++ b/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts @@ -452,26 +452,38 @@ export class BankTxService implements OnModuleInit { let totalFeeChf = fee ?? 0; - const feeRows = await this.bankTxRepo.findBy({ - type: BankTxType.BANK_ACCOUNT_FEE, - created: MoreThanOrEqual(from), - }); + // Aggregate the dedicated BankAccountFee rows in the database (summed per currency and direction) + // rather than hydrating every row: this runs every minute from the financial-log cron and the table + // grows continuously. Column references use the alias.property form so TypeORM quotes the camelCase + // identifiers correctly for Postgres. + const feeAggregates = await this.bankTxRepo + .createQueryBuilder('bankTx') + .select('bankTx.currency', 'currency') + .addSelect('bankTx.creditDebitIndicator', 'creditDebitIndicator') + .addSelect('SUM(bankTx.amount)', 'amount') + .where('bankTx.type = :type', { type: BankTxType.BANK_ACCOUNT_FEE }) + .andWhere('bankTx.created >= :from', { from }) + .groupBy('bankTx.currency') + .addGroupBy('bankTx.creditDebitIndicator') + .getRawMany<{ currency: string; creditDebitIndicator: string; amount: string }>(); const amountByCurrency = new Map(); - for (const tx of feeRows) { + for (const row of feeAggregates) { + const groupAmount = Number(row.amount); + let signedAmount: number; - if (tx.creditDebitIndicator === BankTxIndicator.DEBIT) { - signedAmount = +tx.amount; - } else if (tx.creditDebitIndicator === BankTxIndicator.CREDIT) { - signedAmount = -tx.amount; + if (row.creditDebitIndicator === BankTxIndicator.DEBIT) { + signedAmount = groupAmount; + } else if (row.creditDebitIndicator === BankTxIndicator.CREDIT) { + signedAmount = -groupAmount; } else { this.logger.error( - `BankAccountFee bankTx ${tx.id} has unexpected creditDebitIndicator ${tx.creditDebitIndicator}, skipping`, + `BankAccountFee aggregate (currency ${row.currency}) has unexpected creditDebitIndicator ${row.creditDebitIndicator}, skipping`, ); continue; } - amountByCurrency.set(tx.currency, (amountByCurrency.get(tx.currency) ?? 0) + signedAmount); + amountByCurrency.set(row.currency, (amountByCurrency.get(row.currency) ?? 0) + signedAmount); } for (const [currency, amount] of amountByCurrency) { @@ -482,7 +494,7 @@ export class BankTxService implements OnModuleInit { totalFeeChf += price.convert(amount, Config.defaultVolumeDecimal); } - return totalFeeChf; + return Util.round(totalFeeChf, Config.defaultVolumeDecimal); } async getRecentBankToBankTx(fromIban: string, toIban: string): Promise { diff --git a/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts b/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts index 8832bd7ba9..093d1bdef3 100644 --- a/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts +++ b/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts @@ -174,6 +174,47 @@ describe('LogJobService', () => { }); }); + describe('FinancialChangesLog isolation (reporting failure must not arm the equity safety mode)', () => { + // a healthy, finite book comfortably above the minimum -> the equity path leaves safety mode off + function setup() { + jest.spyOn(service as any, 'getTradingLog').mockResolvedValue({}); + jest.spyOn(service as any, 'getAssetLog').mockResolvedValue({}); + jest + .spyOn(service as any, 'getBalancesByFinancialType') + .mockReturnValue({ EUR: { plusBalance: 5000, plusBalanceChf: 5000, minusBalance: 0, minusBalanceChf: 0 } }); + jest.spyOn(assetService, 'getAssetsWith').mockResolvedValue([] as any); + jest.spyOn(settingService, 'getObj').mockResolvedValue(100 as any); + jest.spyOn(refRewardService, 'getOpenRefCreditLiability').mockResolvedValue({ amountEur: 0, amountChf: 0 }); + jest + .spyOn(logService, 'maxEntity') + .mockResolvedValue({ message: JSON.stringify({ balancesTotal: { totalBalanceChf: 5000 } }) } as any); + return jest.spyOn(logService, 'create').mockResolvedValue({} as any); + } + + it('writes the FinancialDataLog, keeps safety mode off, logs the error and omits the changes entry when getChangeLog throws', async () => { + const errorSpy = jest.spyOn(service['logger'], 'error'); + const createSpy = setup(); + // a transient reporting-price failure while building the changes log + jest.spyOn(service as any, 'getChangeLog').mockRejectedValue(new Error('No valid price')); + + await service.saveTradingLog(); + + // the equity path ran and still persisted the FinancialDataLog entry for this minute + const dataLog = createSpy.mock.calls.find(([dto]) => dto.subsystem === 'FinancialDataLog'); + expect(dataLog).toBeDefined(); + + // the reporting-price failure did NOT arm the equity safety mode: the healthy book set it to false + // (and the outer catch, which would set it true, never ran) + expect(processService.setSafetyModeActive).toHaveBeenCalledWith(false); + expect(processService.setSafetyModeActive).not.toHaveBeenCalledWith(true); + + // the failure is logged and this minute's changes entry is omitted (absence, not a wrong value) + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('financial changes log'), expect.any(Error)); + const changesLog = createSpy.mock.calls.find(([dto]) => dto.subsystem === 'FinancialChangesLog'); + expect(changesLog).toBeUndefined(); + }); + }); + describe('saveTradingLog (referral-credit liability)', () => { // a positive base book so the referral-credit assertions work on round numbers const baseBuckets = () => ({ diff --git a/src/subdomains/supporting/log/log-job.service.ts b/src/subdomains/supporting/log/log-job.service.ts index de96da777a..3a293c117b 100644 --- a/src/subdomains/supporting/log/log-job.service.ts +++ b/src/subdomains/supporting/log/log-job.service.ts @@ -129,9 +129,6 @@ export class LogJobService { const refCreditLiability = await this.getRefCreditLiability(); if (refCreditLiability) balancesByFinancialType[REF_CREDIT_FINANCIAL_TYPE] = refCreditLiability; - // changes - const changeLog = await this.getChangeLog(); - // total balances — customer flow is balance-neutral, so totalBalanceChf moves only on // operating profit, FX, or an error/realised loss (see BalancesTotal). Hence the guardrails below. const plusBalanceChf = Util.sumObjValue(Object.values(balancesByFinancialType), 'plusBalanceChf'); @@ -190,14 +187,24 @@ export class LogJobService { category: null, }); - await this.logService.create({ - system: 'LogService', - subsystem: 'FinancialChangesLog', - severity: LogSeverity.INFO, - message: JSON.stringify({ changes: changeLog }), - valid: null, - category: null, - }); + // The changeLog feeds only the informative FinancialChangesLog and is independent of the equity + // path above, so it runs in its own try/catch: a reporting-price failure must not arm the equity + // safety mode; the equity path above has already run and set it correctly. On failure we log the + // error and omit this minute's changes entry (absence instead of a wrong value). + try { + const changeLog = await this.getChangeLog(); + + await this.logService.create({ + system: 'LogService', + subsystem: 'FinancialChangesLog', + severity: LogSeverity.INFO, + message: JSON.stringify({ changes: changeLog }), + valid: null, + category: null, + }); + } catch (e) { + this.logger.error("Failed to build the financial changes log; skipping this minute's changes entry", e); + } } catch (e) { await this.processService.setSafetyModeActive(true); throw e; From 6b5fb9701266b38b95ca4abd9b879da5cfc965c3 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Wed, 15 Jul 2026 15:45:21 +0200 Subject: [PATCH 3/3] test(bank-tx): pass the Frick service in the positional service construction develop added BankTxFrickService to the constructor; the spec built the service positionally and fell one argument short. --- .../bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts b/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts index 6602816634..a48b323daf 100644 --- a/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts +++ b/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts @@ -21,6 +21,7 @@ import { BankTxReturnService } from '../../bank-tx-return/bank-tx-return.service import { BankTxIndicator } from '../entities/bank-tx.entity'; import { BankTxBatchRepository } from '../repositories/bank-tx-batch.repository'; import { BankTxRepository } from '../repositories/bank-tx.repository'; +import { BankTxFrickService } from '../services/bank-tx-frick.service'; import { BankTxService } from '../services/bank-tx.service'; import { SepaParser } from '../services/sepa-parser.service'; @@ -81,6 +82,7 @@ describe('BankTxService', () => { createMock(), createMock(), createMock(), + createMock(), createMock(), createMock(), createMock(),