Commit 780d454c authored by Antoine's avatar Antoine
Browse files

Use accout Id instead of accountNumber to link operation to account. Fixes #626.

parent 178407fa
Pipeline #42473 passed with stages
in 9 minutes and 37 seconds
......@@ -45,7 +45,7 @@ class AddOperationModal extends React.Component {
amount: this.state.amount,
categoryId: this.state.categoryId,
type: this.state.type,
bankAccount: this.props.account.accountNumber
accountId: this.props.account.id
};
this.props.createOperation(operation);
......
......@@ -43,7 +43,7 @@ export default connect(
let accounts = get.accountsByAccessId(state, access.id);
for (let account of accounts) {
pairs.push({
key: account.accountNumber,
key: account.id,
val: `${access.name}${account.title}`
});
}
......
......@@ -40,7 +40,7 @@ class AlertCreationModal extends React.Component {
type: this.props.alertType,
limit,
order: this.orderSelector.value,
bankAccount: this.accountSelector.getWrappedInstance().value()
accountId: this.accountSelector.getWrappedInstance().value()
};
this.props.createAlert(newAlert);
......
......@@ -15,7 +15,7 @@ class ReportCreationModal extends React.Component {
handleCreate = () => {
let newAlert = {
type: 'report',
bankAccount: this.accountSelector.getWrappedInstance().value(),
accountId: this.accountSelector.getWrappedInstance().value(),
frequency: this.frequencySelector.value
};
this.props.createAlert(newAlert);
......
......@@ -76,7 +76,7 @@ export class Account {
export class Operation {
constructor(arg) {
this.bankAccount = assertHas(arg, 'bankAccount') && arg.bankAccount;
this.accountId = assertHas(arg, 'accountId') && arg.accountId;
this.title = assertHas(arg, 'title') && arg.title;
this.date = assertHas(arg, 'date') && new Date(arg.date);
this.amount = assertHas(arg, 'amount') && arg.amount;
......@@ -129,7 +129,7 @@ export class Setting {
export class Alert {
constructor(arg) {
this.id = assertHas(arg, 'id') && arg.id;
this.bankAccount = assertHas(arg, 'bankAccount') && arg.bankAccount;
this.accountId = assertHas(arg, 'accountId') && arg.accountId;
this.type = assertHas(arg, 'type') && arg.type;
......
......@@ -739,16 +739,16 @@ function reduceUpdateAccount(state, action) {
}
function reduceDeleteAccountInternal(state, accountId) {
let { accountNumber, bankAccess } = accountById(state, accountId);
let { bankAccess } = accountById(state, accountId);
// Remove account:
let ret = u.updateIn('accounts', u.reject(a => a.id === accountId), state);
// Remove operations:
ret = u.updateIn('operations', u.reject(o => o.bankAccount === accountNumber), ret);
ret = u.updateIn('operations', u.reject(o => o.accountId === accountId), ret);
// Remove alerts:
ret = u.updateIn('alerts', u.reject(a => a.bankAccount === accountNumber), ret);
ret = u.updateIn('alerts', u.reject(a => a.accountId === accountId), ret);
// If this was the last account of the access, remove the access too:
if (accountsByAccessId(state, bankAccess).length === 1) {
......@@ -1138,15 +1138,14 @@ export function operationById(state, operationId) {
}
export function operationsByAccountId(state, accountId) {
let { accountNumber } = accountById(state, accountId);
return state.operations.filter(op => op.bankAccount === accountNumber);
return state.operations.filter(op => op.accountId === accountId);
}
export function alertPairsByType(state, alertType) {
let pairs = [];
for (let al of state.alerts.filter(a => a.type === alertType)) {
let accounts = state.accounts.filter(acc => acc.accountNumber === al.bankAccount);
let accounts = state.accounts.filter(acc => acc.id === al.accountId);
if (!accounts.length) {
debug('alert tied to no accounts, skipping');
continue;
......
......@@ -29,10 +29,10 @@ export async function destroyWithOperations(account) {
log.info(`Removing account ${account.title} from database...`);
log.info(`\t-> Destroy operations for account ${account.title}`);
await Operation.destroyByAccount(account.accountNumber);
await Operation.destroyByAccount(account.id);
log.info(`\t-> Destroy alerts for account ${account.title}`);
await Alert.destroyByAccount(account.accountNumber);
await Alert.destroyByAccount(account.id);
log.info(`\t-> Checking if ${account.title} is the default account`);
let found = await Config.findOrCreateDefault('defaultAccountId');
......
......@@ -23,7 +23,7 @@ export async function create(req, res) {
let newAlert = req.body;
if (
!newAlert ||
typeof newAlert.bankAccount !== 'string' ||
typeof newAlert.accountId !== 'string' ||
typeof newAlert.type !== 'string'
) {
throw new KError('missing parameters', 400);
......@@ -34,7 +34,7 @@ export async function create(req, res) {
throw new KError(validationError, 400);
}
let account = await Account.byAccountNumber(newAlert.bankAccount);
let account = await Account.find(newAlert.accountId);
if (!account) {
throw new KError('bank account not found', 404);
}
......
......@@ -50,6 +50,7 @@ export async function all(req, res) {
function cleanMeta(obj) {
delete obj._id;
delete obj._rev;
delete obj.docType;
}
// Sync function
......@@ -94,6 +95,8 @@ function cleanData(world) {
}
}
o.accountId = accountMap[o.accountId];
// Strip away id.
delete o.id;
cleanMeta(o);
......@@ -121,6 +124,8 @@ function cleanData(world) {
world.alerts = world.alerts || [];
for (let a of world.alerts) {
a.accountId = accountMap[a.accountId];
delete a.bankAccount;
delete a.id;
cleanMeta(a);
}
......@@ -301,6 +306,9 @@ export async function import_(req, res) {
delete op.operationTypeID;
}
op.accountId = accountMap[op.accountId];
delete op.bankAccount;
// Remove attachments, if there were any.
delete op.attachments;
delete op.binary;
......@@ -367,6 +375,7 @@ export async function import_(req, res) {
log.info('Import alerts...');
for (let a of world.alerts) {
a.accountId = accountMap[a.accountId];
await Alert.create(a);
}
log.info('Done.');
......
......@@ -2,7 +2,6 @@ import moment from 'moment';
import Access from '../models/access';
import Account from '../models/account';
import Alert from '../models/alert';
import Bank from '../models/bank';
import Config from '../models/config';
import Operation from '../models/operation';
......@@ -57,19 +56,6 @@ function handler(access) {
// - known is the former Account instance (known in Kresus's database).
// - provided is the new Account instance provided by the source backend.
async function mergeAccounts(known, provided) {
// The accountNumber is used as a primary key in several models, update them if necessary.
if (known.accountNumber !== provided.accountNumber) {
let ops = await Operation.byAccount(known);
for (let op of ops) {
await op.updateAttributes({ bankAccount: provided.accountNumber });
}
let alerts = await Alert.byAccount(known);
for (let alert of alerts) {
await alert.updateAttributes({ bankAccount: provided.accountNumber });
}
}
let newProps = {
accountNumber: provided.accountNumber,
title: provided.title,
......@@ -127,7 +113,7 @@ async function notifyNewOperations(access, newOperations, accountMap) {
let newOpsPerAccount = new Map();
for (let newOp of newOperations) {
let opAccountId = newOp.bankAccount;
let opAccountId = newOp.accountId;
if (!newOpsPerAccount.has(opAccountId)) {
newOpsPerAccount.set(opAccountId, [newOp]);
} else {
......@@ -199,22 +185,15 @@ class AccountManager {
balanceOffset: 0
};
// Consider all the operations that could have been inserted before the fix in #405.
let existingOperations = await Operation.byAccount(account);
if (existingOperations.length) {
let offset = existingOperations.reduce((acc, op) => acc + +op.amount, 0);
newAccountInfo.balanceOffset += offset;
}
// Save the account in DB and in the new accounts map.
let newAccount = await Account.create(account);
newAccountInfo.account = newAccount;
this.newAccountsMap.set(newAccount.accountNumber, newAccountInfo);
this.newAccountsMap.set(newAccount.id, newAccountInfo);
}
for (let account of diff.knownOrphans) {
log.info("Orphan account found in Kresus's database: ", account.id);
log.info("Orphan account found in Kresus's database: ", account.accountNumber);
// TODO do something with orphan accounts!
}
......@@ -254,14 +233,16 @@ merging as per request`);
let allAccounts = await Account.byAccess(access);
let accountMap = new Map();
let accountIdNumberMap = new Map();
for (let account of allAccounts) {
if (this.newAccountsMap.has(account.accountNumber)) {
let oldEntry = this.newAccountsMap.get(account.accountNumber);
accountMap.set(account.accountNumber, oldEntry);
accountIdNumberMap.set(account.accountNumber, account.id);
if (this.newAccountsMap.has(account.id)) {
let oldEntry = this.newAccountsMap.get(account.id);
accountMap.set(account.id, oldEntry);
continue;
}
accountMap.set(account.accountNumber, {
accountMap.set(account.id, {
account,
balanceOffset: 0
});
......@@ -276,8 +257,12 @@ merging as per request`);
log.info('Normalizing source information...');
for (let sourceOp of sourceOps) {
if (!accountIdNumberMap.has(sourceOp.account)) {
log.error('Operation attached to an unknown account, skipping');
continue;
}
let operation = {
bankAccount: sourceOp.account,
accountId: accountIdNumberMap.get(sourceOp.account),
amount: sourceOp.amount,
raw: sourceOp.raw,
date: sourceOp.date,
......@@ -304,10 +289,7 @@ merging as per request`);
log.info('Comparing with database to ignore already known operations…');
let newOperations = [];
for (let operation of operations) {
// Ignore operations coming from unknown accounts.
if (!accountMap.has(operation.bankAccount)) {
continue;
}
let accountInfo = accountMap.get(operation.accountId);
// Ignore operations already known in database.
let similarOperations = await Operation.allLike(operation);
......@@ -322,7 +304,6 @@ merging as per request`);
newOperations.push(operation);
// Remember amounts of operations older than the import, to resync balance.
let accountInfo = accountMap.get(operation.bankAccount);
if (+debitDate < +accountInfo.account.importDate) {
accountInfo.balanceOffset += +operation.amount;
}
......@@ -358,8 +339,9 @@ offset of ${balanceOffset}.`);
// Carry over all the triggers on new operations.
log.info("Updating 'last checked' for linked accounts...");
let accounts = [];
let lastChecked = new Date();
for (let account of allAccounts) {
let updated = await account.updateAttributes({ lastChecked: new Date() });
let updated = await account.updateAttributes({ lastChecked });
accounts.push(updated);
}
......
......@@ -43,7 +43,7 @@ ${$t('server.email.signature')}
let accounts = await Account.byAccess(access);
let accountsMap = new Map();
for (let a of accounts) {
accountsMap.set(a.accountNumber, {
accountsMap.set(a.id, {
title: a.title,
formatCurrency: currency.makeFormat(a.currency || defaultCurrency)
});
......@@ -55,11 +55,11 @@ ${$t('server.email.signature')}
for (let operation of operations) {
// Memoize alerts by account
let alerts;
if (!alertsByAccount.has(operation.bankAccount)) {
alerts = await Alert.byAccountAndType(operation.bankAccount, 'transaction');
alertsByAccount.set(operation.bankAccount, alerts);
if (!alertsByAccount.has(operation.accountId)) {
alerts = await Alert.byAccountAndType(operation.accountId, 'transaction');
alertsByAccount.set(operation.accountId, alerts);
} else {
alerts = alertsByAccount.get(operation.bankAccount);
alerts = alertsByAccount.get(operation.accountId);
}
// Skip operations for which the account has no alerts
......@@ -68,7 +68,7 @@ ${$t('server.email.signature')}
}
// Set the account information
let { title: accountName, formatCurrency } = accountsMap.get(operation.bankAccount);
let { title: accountName, formatCurrency } = accountsMap.get(operation.accountId);
for (let alert of alerts) {
if (!alert.testTransaction(operation)) {
......
......@@ -89,7 +89,7 @@ export async function fullPoll() {
}
}
} catch (err) {
log.error(`Error when polling accounts: ${err.message}`);
log.error(`Error when polling accounts: ${err.message}\n`, err);
if (err.errCode && errorRequiresUserAction(err)) {
await manageCredentialsErrors(access, err);
}
......
......@@ -72,7 +72,7 @@ class ReportManager {
}
log.info('Report enabled and never sent, generating it...');
let includedAccounts = reports.map(report => report.bankAccount);
let includedAccounts = reports.map(report => report.accountId);
let accounts = await Account.findMany(includedAccounts);
if (!accounts || !accounts.length) {
throw new KError("report's account does not exist");
......@@ -84,7 +84,7 @@ class ReportManager {
for (let a of accounts) {
let curr = a.currency ? a.currency : defaultCurrency;
a.formatCurrency = currency.makeFormat(curr);
operationsByAccount.set(a.accountNumber, {
operationsByAccount.set(a.id, {
account: a,
operations: []
});
......@@ -92,25 +92,25 @@ class ReportManager {
let reportsMap = new Map();
for (let report of reports) {
reportsMap.set(report.bankAccount, report);
reportsMap.set(report.accountId, report);
}
let operations = await Operation.byAccounts(includedAccounts);
let count = 0;
for (let operation of operations) {
let account = operation.bankAccount;
let { accountId } = operation;
let report = reportsMap.get(account);
let report = reportsMap.get(accountId);
let includeAfter = report.lastTriggeredDate || this.computeIncludeAfter(frequencyKey);
includeAfter = moment(includeAfter);
let date = operation.dateImport || operation.date;
if (moment(date).isAfter(includeAfter)) {
if (!operationsByAccount.has(account)) {
if (!operationsByAccount.has(accountId)) {
throw new KError("operation's account does not exist");
}
operationsByAccount.get(account).operations.push(operation);
operationsByAccount.get(accountId).operations.push(operation);
++count;
}
}
......
......@@ -72,18 +72,7 @@ Account.findMany = async function findMany(accountIds) {
let params = {
keys: accountIds.slice()
};
return await request('allByAccountNumber', params);
};
Account.byAccountNumber = async function byAccountNumber(accountNumber) {
if (typeof accountNumber !== 'string') {
log.warn('Account.byAccountNumber misuse: 1st param must be a string');
}
let params = {
key: accountNumber
};
return await request('allByAccountNumber', params);
return await request('allByAccountIds', params);
};
Account.byAccess = async function byAccess(access) {
......
......@@ -5,8 +5,8 @@ import { makeLogger, promisify, promisifyModel, translate as $t, formatDate } fr
let log = makeLogger('models/alert');
let Alert = cozydb.getModel('bankalert', {
// external (backend) account id.
bankAccount: String,
// internal account id.
accountId: String,
// possible options are: report, balance, transaction.
type: String,
......@@ -21,7 +21,14 @@ let Alert = cozydb.getModel('bankalert', {
order: String,
// when did the alert get triggered for the last time?
lastTriggeredDate: Date
lastTriggeredDate: Date,
// ///////////////////////////////////////////////////
// // DEPRECATED
// //////////////////////////////////////////////////
// external (backend) account id.
bankAccount: String
});
Alert = promisifyModel(Alert);
......@@ -142,4 +149,12 @@ Alert.prototype.formatAccountMessage = function(title, balance, formatCurrency)
});
};
Alert.prototype.clone = function() {
let clone = { ...this };
delete clone.id;
delete clone._id;
delete clone._rev;
return clone;
};
module.exports = Alert;
......@@ -473,6 +473,77 @@ let migrations = [
} catch (e) {
log.error('Error while removing weboob-version: ', e.toString());
}
},
async function m16(cache) {
log.info('Linking operations to account by id instead of accountNumber');
try {
cache.operations = cache.operations || (await Operation.all());
cache.accounts = cache.accounts || (await Account.all());
let accountsMap = new Map();
for (let account of cache.accounts) {
if (accountsMap.has(account.accountNumber)) {
accountsMap.get(account.accountNumber).push(account);
} else {
accountsMap.set(account.accountNumber, [account]);
}
}
let newOperations = [];
let numberMigratedOps = 0;
for (let op of cache.operations) {
let cloneOperation = false;
for (let account of accountsMap.get(op.bankAccount)) {
if (cloneOperation) {
let newOp = op.clone();
newOp.accountId = account.id;
newOp = await Operation.create(newOp);
newOperations.push(newOp);
} else {
cloneOperation = true;
op.accountId = account.id;
delete op.bankAccount;
await op.save();
numberMigratedOps++;
}
}
}
cache.operations = cache.operations.concat(newOperations);
log.info(`${numberMigratedOps} operations migrated`);
log.info(`${newOperations.length} new operations created`);
log.info('All operations correctly migrated.');
log.info('Linking alerts to account by id instead of accountNumber');
cache.alerts = cache.alerts || (await Alert.all());
let newAlerts = [];
let numberMigratedAlerts = 0;
for (let alert of cache.alerts) {
let cloneAlert = false;
for (let account of accountsMap.get(alert.bankAccount)) {
if (cloneAlert) {
let newAlert = alert.clone();
newAlert.accountId = account.id;
newAlert = await Alert.create(newAlert);
newAlerts.push(newAlert);
} else {
cloneAlert = true;
alert.accountId = account.id;
delete alert.bankAccount;
await alert.save();
numberMigratedAlerts++;
}
}
}
cache.alerts = cache.alerts.concat(newAlerts);
log.info(`${numberMigratedAlerts} alerts migrated`);
log.info(`${newAlerts.length} new alerts created`);
log.info('All alerts correctly migrated.');
} catch (e) {
log.error('Error while linking operations and alerts to account by id: ', e.toString());
}
}
];
......
......@@ -12,8 +12,8 @@ let Operation = cozydb.getModel('bankoperation', {
// EXTERNAL LINKS
// ************************************************************************
// external (backend) account id.
bankAccount: String,
// Internal account id, to which the transaction is attached
accountId: String,
// internal category id.
categoryId: String,
......@@ -59,7 +59,7 @@ let Operation = cozydb.getModel('bankoperation', {
createdByUser: Boolean,
// ************************************************************************
// ATTACHMENTS
// DEPRECATED
// ************************************************************************
// TODO: remove linkPlainEnglish?
......@@ -72,12 +72,11 @@ let Operation = cozydb.getModel('bankoperation', {
// as attachment.
binary: x => x,
// ************************************************************************
// DEPRECATED
// ************************************************************************
// internal operation type id.
operationTypeID: String
operationTypeID: String,
// external (backend) account id.
bankAccount: String
});
Operation = promisifyModel(Operation);
......@@ -86,35 +85,35 @@ let request = promisify(Operation.request.bind(Operation));
let requestDestroy = promisify(Operation.requestDestroy.bind(Operation));
Operation.byAccount = async function byAccount(account) {
if (typeof account !== 'object' || typeof account.accountNumber !== 'string') {
if (typeof account !== 'object' || typeof account.id !== 'string') {
log.warn('Operation.byAccount misuse: account must be an Account');
}
let params = {
key: account.accountNumber
key: account.id
};
return await request('allByBankAccount', params);
};
Operation.byAccounts = async function byAccounts(accountNums) {
if (!(accountNums instanceof Array)) {
log.warn('Operation.byAccounts misuse: accountNums must be an array');
Operation.byAccounts = async function byAccounts(accountIds) {
if (!(accountIds instanceof Array)) {
log.warn('Operation.byAccounts misuse: accountIds must be an array');
}
let params = {
keys: accountNums
keys: accountIds
};
return await request('allByBankAccount', params);
};