Commit 49de73a7 authored by Nicolas Frandeboeuf's avatar Nicolas Frandeboeuf Committed by Benjamin Bouvier
Browse files

Use bank's real balance instead of `initialBalance` + sum of transactions

parent 4888bf6a
Pipeline #491097 failed with stage
in 3 minutes and 2 seconds
......@@ -20,7 +20,7 @@ function roundDate(d: Date): number {
function createChartBalance(
chartId: string,
initialBalance: number,
currentBalance: number,
inputTransactions: Operation[],
theme: string
) {
......@@ -46,7 +46,7 @@ function createChartBalance(
}
}
let balance = initialBalance || 0;
let balance = currentBalance || 0;
if (!dateToAmount.size) {
// Should be an edge use-case, but if there are no transactions, just
......@@ -56,16 +56,18 @@ function createChartBalance(
dateToAmount.set(Date.now() - DAY, balance);
}
// Sort them from more recent to oldest to start from current/real balance and deduce each day's
// amount.
const sorted = Array.from(dateToAmount).sort((a, b) => {
if (a[0] < b[0]) return -1;
if (a[0] > b[0]) return 1;
if (a[0] > b[0]) return -1;
if (a[0] < b[0]) return 1;
return 0;
});
const data = [];
for (const [date, amount] of sorted) {
balance += amount;
data.push({
balance -= amount;
data.unshift({
x: +date,
y: round2(balance),
});
......@@ -146,17 +148,13 @@ function createChartBalance(
});
}
const BalanceChart = (props: {
initialBalance: number;
transactions: Operation[];
theme: string;
}) => {
const BalanceChart = (props: { balance: number; transactions: Operation[]; theme: string }) => {
const container = useRef<Chart>();
const redraw = useCallback(() => {
container.current = createChartBalance(
'barchart',
props.initialBalance,
props.balance,
props.transactions,
props.theme
);
......
......@@ -58,11 +58,7 @@ const Charts = () => {
const makeByCategoryCharts = () => <CategoryCharts transactions={view.transactions} />;
const makeBalanceCharts = () => (
<BalanceChart
transactions={view.transactions}
initialBalance={view.initialBalance}
theme={theme}
/>
<BalanceChart transactions={view.transactions} balance={view.balance} theme={theme} />
);
const tabs = new Map<string, TabDescriptor>();
......
......@@ -29,7 +29,7 @@ export class DriverAccount extends Driver {
account.lastCheckDate,
account.balance,
account.outstandingSum,
account.initialBalance,
account.balance,
account
);
}
......
......@@ -237,6 +237,7 @@ export class Account {
assertHas(arg, 'label');
assertHas(arg, 'vendorAccountId');
assertHas(arg, 'initialBalance');
assertHas(arg, 'balance');
assertHas(arg, 'lastCheckDate');
assertHas(arg, 'id');
......@@ -245,6 +246,7 @@ export class Account {
this.label = arg.label;
this.vendorAccountId = arg.vendorAccountId;
this.initialBalance = arg.initialBalance;
this.balance = arg.balance;
this.lastCheckDate = new Date(arg.lastCheckDate);
this.id = arg.id;
......@@ -262,7 +264,6 @@ export class Account {
// These fields will be updated when the operations are attached to the account.
// Make sure to update `updateFrom` if you add any fields here.
this.operationIds = [];
this.balance = this.initialBalance;
this.outstandingSum = 0;
}
......@@ -275,8 +276,7 @@ export class Account {
// Make sure to keep this in sync with the above ctor.
newAccount.operationIds = previousAccount.operationIds;
newAccount.balance =
previousAccount.balance - previousAccount.initialBalance + newAccount.initialBalance;
newAccount.balance = previousAccount.balance;
newAccount.outstandingSum = previousAccount.outstandingSum;
return newAccount;
......
......@@ -8,7 +8,6 @@ import {
NONE_CATEGORY_ID,
UNKNOWN_ACCOUNT_TYPE,
displayLabel,
shouldIncludeInBalance,
shouldIncludeInOutstandingSum,
assertDefined,
translate as $t,
......@@ -451,8 +450,7 @@ export function createOperation(operation: Partial<Operation>) {
const action = createTransactionAction({});
dispatch(action);
try {
const created = await backend.createOperation(serverOperation);
action.created = created;
action.created = await backend.createOperation(serverOperation);
dispatch(actionStatus.ok(action));
} catch (err) {
dispatch(actionStatus.err(action, err));
......@@ -462,7 +460,11 @@ export function createOperation(operation: Partial<Operation>) {
}
type CreateTransactionParams = {
created?: Operation;
created?: {
transaction: Operation;
accountBalance: number;
accountId: number;
};
};
const createTransactionAction = createActionCreator<CreateTransactionParams>(CREATE_OPERATION);
......@@ -472,7 +474,8 @@ function reduceCreateOperation(state: BankState, action: Action<CreateTransactio
const { created } = action;
assertDefined(created);
return mutateState(state, mut => {
addOperations(mut, [created]);
addOperations(mut, [created.transaction]);
updateAccountBalance(mut, created.accountId, created.accountBalance);
});
}
return state;
......@@ -481,10 +484,13 @@ function reduceCreateOperation(state: BankState, action: Action<CreateTransactio
// Deletes a given transaction.
export function deleteOperation(operationId: number) {
return async (dispatch: Dispatch) => {
const action = deleteTransactionAction({ operationId });
const action = deleteTransactionAction({});
dispatch(action);
try {
await backend.deleteOperation(operationId);
action.deleted = await backend.deleteOperation(operationId);
if (action.deleted) {
action.deleted.operationId = operationId;
}
dispatch(actionStatus.ok(action));
} catch (err) {
dispatch(actionStatus.err(action, err));
......@@ -493,15 +499,27 @@ export function deleteOperation(operationId: number) {
};
}
type DeleteTransactionParams = { operationId: number };
type DeleteTransactionParams = {
deleted?: {
operationId: number;
accountBalance?: number;
accountId: number;
};
};
const deleteTransactionAction = createActionCreator<DeleteTransactionParams>(DELETE_OPERATION);
function reduceDeleteOperation(state: BankState, action: Action<DeleteTransactionParams>) {
const { status } = action;
if (status === SUCCESS) {
const { operationId } = action;
const { deleted } = action;
assertDefined(deleted);
return mutateState(state, mut => {
removeOperation(mut, operationId);
removeOperation(mut, deleted.operationId);
if (typeof deleted.accountBalance === 'number') {
updateAccountBalance(mut, deleted.accountId, deleted.accountBalance);
}
});
}
return state;
......@@ -513,8 +531,14 @@ export function mergeOperations(toKeep: Operation, toRemove: Operation) {
const action = mergeTransactionAction({ toKeep, toRemove });
dispatch(action);
try {
const newToKeep = await backend.mergeOperations(toKeep.id, toRemove.id);
const {
transaction: newToKeep,
accountId,
accountBalance,
} = await backend.mergeOperations(toKeep.id, toRemove.id);
action.toKeep = newToKeep;
action.accountId = accountId;
action.accountBalance = accountBalance;
dispatch(actionStatus.ok(action));
} catch (err) {
dispatch(actionStatus.err(action, err));
......@@ -526,6 +550,8 @@ export function mergeOperations(toKeep: Operation, toRemove: Operation) {
type MergeTransactionParams = {
toKeep: Operation;
toRemove: Operation;
accountBalance?: number;
accountId?: number;
};
const mergeTransactionAction = createActionCreator<MergeTransactionParams>(MERGE_OPERATIONS);
......@@ -538,6 +564,10 @@ function reduceMergeOperations(state: BankState, action: Action<MergeTransaction
// Replace the kept one:
const newKept = new Operation(action.toKeep);
mergeInObject(mut.state.transactionMap, action.toKeep.id, newKept);
if (action.accountId && typeof action.accountBalance === 'number') {
updateAccountBalance(mut, action.accountId, action.accountBalance);
}
});
}
return state;
......@@ -697,6 +727,7 @@ export function resyncBalance(
return;
}
action.initialBalance = results.initialBalance;
action.balance = results.balance;
dispatch(actionStatus.ok(action));
} catch (err) {
dispatch(actionStatus.err(action, err));
......@@ -704,16 +735,14 @@ export function resyncBalance(
};
}
type ResyncBalanceParams = { accountId: number; initialBalance?: number };
type ResyncBalanceParams = { accountId: number; initialBalance?: number; balance?: number };
const resyncBalanceAction = createActionCreator<ResyncBalanceParams>(RUN_BALANCE_RESYNC);
function reduceResyncBalance(state: BankState, action: Action<ResyncBalanceParams>) {
if (action.status === SUCCESS) {
const { accountId, initialBalance } = action;
const { accountId, initialBalance, balance } = action;
assertDefined(initialBalance);
return mutateState(state, mut => {
const account = accountById(mut.state, accountId);
const balance = account.balance - account.initialBalance + initialBalance;
mergeInObject(mut.state.accountMap, accountId, { initialBalance, balance });
});
}
......@@ -1193,11 +1222,14 @@ function makeCompareOperationByIds(state: BankState) {
};
}
function updateAccountBalance(mut: MutableState, accountId: number, accountBalance: number): void {
mergeInObject(mut.state.accountMap, accountId, { balance: accountBalance });
}
function addOperations(mut: MutableState, operations: Partial<Operation>[]): void {
const accountsToSort = new Set<Account>();
const limitOngoingToCurrentMonth = mut.state.isOngoingLimitedToCurrentMonth;
const today = new Date();
for (const op of operations) {
const operation = new Operation(op);
......@@ -1206,9 +1238,9 @@ function addOperations(mut: MutableState, operations: Partial<Operation>[]): voi
account.operationIds.push(operation.id);
if (shouldIncludeInBalance(operation, today, account.type)) {
account.balance += operation.amount;
} else if (shouldIncludeInOutstandingSum(operation, limitOngoingToCurrentMonth)) {
// Do not include it in the balance, this should be computed by the server.
if (shouldIncludeInOutstandingSum(operation, limitOngoingToCurrentMonth)) {
account.outstandingSum += operation.amount;
}
......@@ -1415,18 +1447,16 @@ function removeOperation(mut: MutableState, operationId: number): void {
const account = accountById(mut.state, op.accountId);
const limitOngoingToCurrentMonth = mut.state.isOngoingLimitedToCurrentMonth;
let { balance, outstandingSum } = account;
const today = new Date();
let { outstandingSum } = account;
// Do not remove it from the balance, this should be computed by the server.
if (shouldIncludeInBalance(op, today, account.type)) {
balance -= op.amount;
} else if (shouldIncludeInOutstandingSum(op, limitOngoingToCurrentMonth)) {
if (shouldIncludeInOutstandingSum(op, limitOngoingToCurrentMonth)) {
outstandingSum -= op.amount;
}
mergeInObject(mut.state.accountMap, account.id, {
operationIds: account.operationIds.filter(id => id !== operationId),
balance,
outstandingSum,
});
......
import express from 'express';
import { IdentifiedRequest, PreloadedRequest } from './routes';
import { Category, Transaction } from '../models';
import { Account, Category, Transaction } from '../models';
import { isKnownTransactionTypeName } from '../lib/transaction-types';
import { KError, asyncErr, UNKNOWN_OPERATION_TYPE } from '../helpers';
......@@ -121,7 +121,17 @@ export async function merge(req: PreloadedRequest<Transaction>, res: express.Res
op = await Transaction.update(userId, op.id, newFields);
await Transaction.destroy(userId, otherOp.id);
res.status(200).json(op);
const account = await Account.find(userId, otherOp.accountId);
if (!account) {
throw new KError('bank account not found', 404);
}
res.status(200).json({
transaction: op,
accountBalance: account.balance,
accountId: otherOp.accountId,
});
} catch (err) {
asyncErr(res, err, 'when merging two operations');
}
......@@ -152,7 +162,18 @@ export async function create(req: IdentifiedRequest<Transaction>, res: express.R
operation.isUserDefinedType = true;
}
const op = await Transaction.create(userId, operation);
res.status(201).json(op);
// Send back the transaction as well as the (possibly) updated account balance.
const account = await Account.find(userId, op.accountId);
if (!account) {
throw new KError('bank account not found', 404);
}
res.status(201).json({
transaction: op,
accountBalance: account.balance,
accountId: op.accountId,
});
} catch (err) {
asyncErr(res, err, 'when creating operation for a bank account');
}
......@@ -163,8 +184,19 @@ export async function destroy(req: PreloadedRequest<Transaction>, res: express.R
try {
const { id: userId } = req.user;
const op = req.preloaded.operation;
await Transaction.destroy(userId, op.id);
res.status(204).end();
// Send back the transaction as well as the (possibly) updated account balance.
const account = await Account.find(userId, op.accountId);
if (!account) {
throw new KError('bank account not found', 404);
}
res.status(200).json({
accountBalance: account.balance,
accountId: op.accountId,
});
} catch (err) {
asyncErr(res, err, 'when deleting operation');
}
......
......@@ -68,7 +68,10 @@ function normalizeAccount(access: Access, source: ProviderAccount): Partial<Acco
accessId: access.id,
iban: source.iban ?? null,
label: source.label,
initialBalance: Number.parseFloat(source.balance) || 0,
initialBalance: Number.parseFloat(source.balance || '0') || 0,
balance: source.hasOwnProperty('balance')
? Number.parseFloat(source.balance || '0') || 0
: null,
lastCheckDate: new Date(),
importDate: new Date(),
};
......@@ -659,15 +662,16 @@ to be resynced, by an offset of ${balanceOffset}.`);
throw new KError('account not found', 404);
}
const realBalance = found.initialBalance ?? 0;
const kresusBalance = await account.computeBalance();
const balanceDelta = realBalance - kresusBalance;
const kresusBalance = await account.computeBalance(account.initialBalance);
const balanceDelta = (found.initialBalance ?? 0) - kresusBalance;
if (Math.abs(balanceDelta) > 0.001) {
log.info(`Updating balance for account ${account.vendorAccountId}`);
const initialBalance = account.initialBalance + balanceDelta;
const updatedAccount = await Account.update(userId, account.id, { initialBalance });
const updatedAccount = await Account.update(userId, account.id, {
initialBalance,
balance: found.balance,
});
return {
kind: 'value',
value: updatedAccount,
......
......@@ -142,7 +142,7 @@ ${$t(i18n, 'server.email.signature')}
continue;
}
const balance = await account.computeBalance();
const balance = account.balance as number;
for (const alert of alerts) {
if (!alert.testBalance(balance)) {
continue;
......
......@@ -219,9 +219,8 @@ class ReportManager {
const formatCurrency = await account.getCurrencyFormatter();
const lastCheckDate = formatDate(i18n.localeId).toShortString(account.lastCheckDate);
const balance = await account.computeBalance();
content += `\t* ${accountsNameMap.get(account.id)} : `;
content += `${formatCurrency(balance)} (`;
content += `${formatCurrency(account.balance as number)} (`;
content += $t(i18n, 'server.email.report.last_sync');
content += ` ${lastCheckDate})\n`;
}
......
......@@ -3,9 +3,16 @@ import { getManager, EntityManager } from 'typeorm';
import { updateBanks as banks20200414 } from './banks-20200414';
import { updateBanks as banks20210526 } from './banks-20210526';
import { updateBanks as banks20210814 } from './banks-20210814';
import { resetManualBankAccountsBalance } from './reset-manual-accounts-balance';
import { run as removeMigratedFromCozydb } from './remove-migrated-from-cozydb';
const MIGRATIONS = [banks20200414, removeMigratedFromCozydb, banks20210526, banks20210814];
const MIGRATIONS = [
banks20200414,
removeMigratedFromCozydb,
banks20210526,
banks20210814,
resetManualBankAccountsBalance,
];
export default async function runDataMigrations(userId: number): Promise<void> {
const manager: EntityManager = getManager();
......
/* eslint new-cap: ["error", { "capIsNewExceptions": ["In"] }]*/
import { EntityManager, In } from 'typeorm';
import { Access, Account } from '..';
import { makeLogger } from '../../helpers';
const log = makeLogger('models/data-migrations');
export async function resetManualBankAccountsBalance(
userId: number | null,
manager: EntityManager
): Promise<void> {
// Do not use transactions, as this code might be called from migrations which are already
// happening in a transaction.
log.info('Running data migration on manual bank (resetting balance)');
// Reset the manual accounts balance to NULL so that the balance is computed from the
// transactions.
const userCondition: { userId?: number } = {};
if (userId !== null) {
userCondition.userId = userId;
}
const accesses = await manager.find(Access, {
select: ['id'],
where: {
vendorId: In(['manual', 'demo']),
...userCondition,
},
});
if (accesses.length > 0) {
const accessesIds = accesses.map(acc => acc.id);
await manager.update(
Account,
{
accessId: In(accessesIds),
},
{ balance: null }
);
}
log.info('Finished running data migration on manual bank (resetting balance)');
}
......@@ -109,14 +109,19 @@ export default class Account {
@Column('boolean', { default: false })
excludeFromBalance = false;
// Balance on the account, updated at each account update.
@Column('numeric', { nullable: true, default: null, transformer: new ForceNumericColumn() })
balance: number | null = null;
// Methods.
computeBalance = async (): Promise<number> => {
computeBalance = async (offset = 0): Promise<number> => {
const ops = await Transaction.byAccount(this.userId, this);
const today = new Date();
const s = ops
.filter(op => shouldIncludeInBalance(op, today, this.type))
.reduce((sum, op) => sum + op.amount, this.initialBalance);
.reduce((sum, op) => sum + op.amount, offset);
return Math.round(s * 100) / 100;
};
......@@ -144,6 +149,14 @@ export default class Account {
return currencyFormatter(checkedCurrency);
};
static async ensureBalance(account: Account): Promise<void> {
// If there is no balance for an account, compute one based on the initial amount and the
// transactions.
if (account.balance === null) {
account.balance = await account.computeBalance();
}
}
// Static methods
static renamings = {
initialAmount: 'initialBalance',
......@@ -158,15 +171,21 @@ export default class Account {
userId: number,
{ uuid: vendorId }: { uuid: string }
): Promise<Account[]> {
return await Account.repo().find({ userId, vendorId });
const accounts = await Account.repo().find({ userId, vendorId });
await Promise.all(accounts.map(Account.ensureBalance));
return accounts;
}
static async findMany(userId: number, accountIds: number[]): Promise<Account[]> {
return await Account.repo().find({ userId, id: In(accountIds) });
const accounts = await Account.repo().find({ userId, id: In(accountIds) });
await Promise.all(accounts.map(Account.ensureBalance));
return accounts;
}
static async byAccess(userId: number, access: Access | { id: number }): Promise<Account[]> {
return await Account.repo().find({ userId, accessId: access.id });
const accounts = await Account.repo().find({ userId, accessId: access.id });
await Promise.all(accounts.map(Account.ensureBalance));
return accounts;
}
// Doesn't insert anything in db, only creates a new instance and normalizes its fields.
......@@ -176,15 +195,23 @@ export default class Account {
static async create(userId: number, attributes: Partial<Account>): Promise<Account> {
const entity = Account.repo().create({ ...attributes, userId });
return await Account.repo().save(entity);
const account = await Account.repo().save(entity);
await Account.ensureBalance(account);
return account;
}
static async find(userId: number, accessId: number): Promise<Account | undefined> {
return await Account.repo().findOne({ where: { userId, id: accessId } });
static async find(userId: number, accountId: number): Promise<Account | undefined> {
const account = await Account.repo().findOne({ where: { userId, id: accountId } });
if (account) {
await Account.ensureBalance(account);
}
return account;
}
static async all(userId: number): Promise<Account[]> {
return await Account.repo().find({ userId });
const accounts = await Account.repo().find({ userId });
await Promise.all(accounts.map(Account.ensureBalance));
return accounts;
}
static async exists(userId: number, accessId: number): Promise<boolean> {
......
import { MigrationInterface