From 17d58687c94ad17a7e72a9a280206276ad2cde70 Mon Sep 17 00:00:00 2001 From: Dmytro Svyrydenko Date: Fri, 19 Jan 2024 18:19:39 +0200 Subject: [PATCH] feat: Allow updating non-primary transfer txs --- .../update-transaction.e2e.ts | 160 ++++++++---------- .../transactions/update-transaction.ts | 18 +- 2 files changed, 77 insertions(+), 101 deletions(-) diff --git a/src/controllers/transactions.controller/update-transaction.e2e.ts b/src/controllers/transactions.controller/update-transaction.e2e.ts index 45a45a8c..a1a782ca 100644 --- a/src/controllers/transactions.controller/update-transaction.e2e.ts +++ b/src/controllers/transactions.controller/update-transaction.e2e.ts @@ -89,77 +89,23 @@ describe('Update transaction controller', () => { currencyCode: currencyEUR.code, }); }); - describe('should change expense to transfer and vice versa', () => { - let createdTransactions = []; - let accountA = null; - let accountB = null; - - beforeEach(async () => { - // Create account with a new currency - const OLD_DESTINATION_CURRENCY = 'UAH'; - const currencyA = global.MODELS_CURRENCIES.find( - (item) => item.code === OLD_DESTINATION_CURRENCY, - ); - await helpers.addUserCurrencies({ currencyCodes: [currencyA.code] }); - accountA = await helpers.createAccount({ - payload: { - ...helpers.buildAccountPayload(), - currencyId: currencyA.id, - }, - raw: true, - }); - - const New_DESTINATION_CURRENCY = 'EUR'; - const currencyB = global.MODELS_CURRENCIES.find( - (item) => item.code === New_DESTINATION_CURRENCY, - ); - await helpers.addUserCurrencies({ currencyCodes: [currencyA.code] }); - accountB = await helpers.createAccount({ - payload: { - ...helpers.buildAccountPayload(), - currencyId: currencyB.id, - }, - raw: true, - }); - - const txPayload = { - ...helpers.buildTransactionPayload({ accountId: accountA.id }), - transferNature: TRANSACTION_TRANSFER_NATURE.common_transfer, - destinationAmount: 30, - destinationAccountId: accountB.id, - }; - createdTransactions = await helpers.createTransaction({ - payload: txPayload, - raw: true, - }); - }); - - it('works with source transaction (`from`)', async () => { - const sourceTransaction = createdTransactions[0]; - - await helpers.updateTransaction({ - id: sourceTransaction.id, - payload: { - transactionType: TRANSACTION_TYPES.income, - }, + it.each([[TRANSACTION_TYPES.income], [TRANSACTION_TYPES.expense]])( + 'should change %s to transfer and vice versa', + async (txType) => { + const accountA = await helpers.createAccount({ raw: true }); + const accountB = await helpers.createAccount({ raw: true }); + + const [tx] = await helpers.createTransaction({ + payload: helpers.buildTransactionPayload({ + accountId: accountA.id, + transactionType: txType, + }), raw: true, }); - const txsAfterUpdation = await helpers.getTransactions({ raw: true }); - - expect(txsAfterUpdation.length).toBe(1); - expect(txsAfterUpdation[0].transactionType).toBe( - TRANSACTION_TYPES.income, - ); - expect(txsAfterUpdation[0].transferId).toBe(null); - expect(txsAfterUpdation[0].transferNature).toBe( - TRANSACTION_TRANSFER_NATURE.not_transfer, - ); - await helpers.updateTransaction({ - id: sourceTransaction.id, + id: tx.id, payload: { - ...helpers.buildTransactionPayload({ accountId: accountA.id }), transferNature: TRANSACTION_TRANSFER_NATURE.common_transfer, destinationAmount: 30, destinationAccountId: accountB.id, @@ -167,35 +113,20 @@ describe('Update transaction controller', () => { raw: true, }); - const txsAfterUpdation2 = await helpers.getTransactions({ raw: true }); - - expect(txsAfterUpdation2[0].id).toBe(sourceTransaction.id); - // Check that after making tx transfer type, source changes from `income` to `expense` - expect(txsAfterUpdation2[0].transactionType).toBe( - TRANSACTION_TYPES.expense, - ); - expect(txsAfterUpdation2[0].transferId).not.toBe(null); - expect(txsAfterUpdation2[0].transferNature).toBe( - TRANSACTION_TRANSFER_NATURE.common_transfer, - ); - }); - it('disallowd to change non-source transaction', async () => { - const destinationTransaction = createdTransactions[1]; - - const response = await helpers.updateTransaction({ - id: destinationTransaction.id, + await helpers.updateTransaction({ + id: tx.id, payload: { - transactionType: TRANSACTION_TYPES.expense, + transferNature: TRANSACTION_TRANSFER_NATURE.not_transfer, }, + raw: true, }); - const txsAfterUpdation = await helpers.getTransactions({ raw: true }); + const transactions = await helpers.getTransactions({ raw: true }); - expect(response.statusCode).toBe(ERROR_CODES.ValidationError); - // Check that after updation try nothing changed - expect(createdTransactions).toStrictEqual(txsAfterUpdation); - }); - }); + expect(transactions.length).toBe(1); + expect(transactions[0]).toEqual(tx); + }, + ); describe('test refAmount is correct when changing transfer transaction accounts to ref account', () => { it('EUR->UAH to EUR->USD, refAmount should be same as amount of USD. Because USD is a ref-currency', async () => { const { account: accountEUR } = @@ -580,9 +511,54 @@ describe('Update transaction controller', () => { }, ); - it.todo( - 'update transfer of two linked transactions back to their initial state', + it.each([[TRANSACTION_TYPES.expense], [TRANSACTION_TYPES.income]])( + 'update transfer of two linked transactions back to their initial state will just remove the opposite tx. testing %s', + async (txType) => { + const accountA = await helpers.createAccount({ raw: true }); + const accountB = await helpers.createAccount({ raw: true }); + + const oppositeTxType = + txType === TRANSACTION_TYPES.income + ? TRANSACTION_TYPES.expense + : TRANSACTION_TYPES.income; + + const [tx1] = await helpers.createTransaction({ + payload: helpers.buildTransactionPayload({ + accountId: accountA.id, + transactionType: txType, + }), + raw: true, + }); + const [tx2] = await helpers.createTransaction({ + payload: helpers.buildTransactionPayload({ + accountId: accountB.id, + transactionType: oppositeTxType, + }), + raw: true, + }); + + await helpers.updateTransaction({ + id: tx1.id, + payload: { + transferNature: TRANSACTION_TRANSFER_NATURE.common_transfer, + destinationTransactionId: tx2.id, + }, + }); + + await helpers.updateTransaction({ + id: tx1.id, + payload: { + transferNature: TRANSACTION_TRANSFER_NATURE.not_transfer, + }, + }); + + const txsAfterUpdation = await helpers.getTransactions({ raw: true }); + + expect(txsAfterUpdation.length).toBe(1); + expect(txsAfterUpdation[0]).toEqual(tx1); + }, ); + it.todo('test unlinking system transactions'); it.todo( 'update transfer of two EXTERNAL linked transactions back to their initial state', ); diff --git a/src/services/transactions/update-transaction.ts b/src/services/transactions/update-transaction.ts index 8013355a..84bb8643 100644 --- a/src/services/transactions/update-transaction.ts +++ b/src/services/transactions/update-transaction.ts @@ -69,15 +69,15 @@ const validateTransaction = ( // 2. To keep `refAmount` calculation correct abd be tied exactly to source tx. // Otherwise we will need to code additional logic to handle that // For now keep that logic only for system transactions - if ( - prevData.accountType === ACCOUNT_TYPES.system && - prevData.transferNature === TRANSACTION_TRANSFER_NATURE.common_transfer && - prevData.transactionType !== TRANSACTION_TYPES.expense - ) { - throw new ValidationError({ - message: 'You cannot edit non-primary transfer transaction', - }); - } + // if ( + // prevData.accountType === ACCOUNT_TYPES.system && + // prevData.transferNature === TRANSACTION_TRANSFER_NATURE.common_transfer && + // prevData.transactionType !== TRANSACTION_TYPES.expense + // ) { + // throw new ValidationError({ + // message: 'You cannot edit non-primary transfer transaction', + // }); + // } }; const makeBasicBaseTxUpdation = async (