From 259ea9f6b3a97e109de4e23d5085bdc3ccc1c79c Mon Sep 17 00:00:00 2001 From: quantum-grit <91589884+quantum-grit@users.noreply.github.com> Date: Tue, 3 Oct 2023 09:11:09 +0300 Subject: [PATCH] fixed skipped bank transactions (#557) * added: env variable for CAMPAIGN_ADMIN_MAIL in deployment config * transactions now requested with dateTo = dateFrom+1 to ensure all transactions are returned from bank API * removed checking for existing records by count as it may result in skipped transactions --- .../bank-transactions.controller.ts | 31 ++++--- .../bank-transactions.service.ts | 2 +- .../import-transactions.task.spec.ts | 80 +------------------ .../bank-import/import-transactions.task.ts | 61 +++++--------- 4 files changed, 39 insertions(+), 135 deletions(-) diff --git a/apps/api/src/bank-transactions/bank-transactions.controller.ts b/apps/api/src/bank-transactions/bank-transactions.controller.ts index f4e111217..813e3bc6e 100644 --- a/apps/api/src/bank-transactions/bank-transactions.controller.ts +++ b/apps/api/src/bank-transactions/bank-transactions.controller.ts @@ -23,6 +23,7 @@ import { } from './dto/bank-transactions-query-dto' import { CampaignService } from '../campaign/campaign.service' import { BankDonationStatus } from '@prisma/client' +import { DateTime, Interval } from 'luxon' @ApiTags('bank-transaction') @Controller('bank-transaction') @@ -121,21 +122,27 @@ export class BankTransactionsController { mode: RoleMatchingMode.ANY, }) async rerunBankTransactionsForDate(@Body() body: { startDate: string; endDate: string }) { - Logger.debug('rerunBankTransactionsForDate startDate: ', body.startDate) + Logger.debug( + 'rerunBankTransactionsForDate startDate: ' + body.startDate + ' endDate: ' + body.endDate, + ) if (!body.startDate) throw new BadRequestException('Missing startDate in Request') if (!body.endDate) throw new BadRequestException('Missing endDate in Request') - const startDate = new Date(body.startDate.split('T')[0]) - const endDate = new Date(body.endDate.split('T')[0]) + const startDate = DateTime.fromISO(body.startDate) + const endDate = DateTime.fromISO(body.endDate).plus({ days: 1 }) //include endDate in the interval + const interval = Interval.fromDateTimes(startDate, endDate) + const dayCount = interval.length('days') - //rerun transactions iterating from startDate to endDate - for ( - const dateToCheck = startDate; - dateToCheck <= endDate; - dateToCheck.setDate(dateToCheck.getDate() + 1) - ) { - Logger.debug('Getting transactions for date: ' + dateToCheck.toISOString().split('T')[0]) - await this.bankTransactionsService.rerunBankTransactionsForDate(dateToCheck) - } + Logger.debug('rerunBankTransactionsForDate days: ' + dayCount) + if (dayCount > 31) + throw new BadRequestException( + 'Date range is more than 31 days. Please select a smaller date range', + ) + + //iterate over all dates in the interval + interval.splitBy({ days: 1 }).map(async (d) => { + Logger.debug('rerunBankTransactionsForDate date: ', d.start.toISODate()) + await this.bankTransactionsService.rerunBankTransactionsForDate(new Date(d.start.toISODate())) + }) } } diff --git a/apps/api/src/bank-transactions/bank-transactions.service.ts b/apps/api/src/bank-transactions/bank-transactions.service.ts index 99c004600..aa08b9535 100644 --- a/apps/api/src/bank-transactions/bank-transactions.service.ts +++ b/apps/api/src/bank-transactions/bank-transactions.service.ts @@ -163,6 +163,6 @@ export class BankTransactionsService { } async rerunBankTransactionsForDate(transactionsDate: Date) { - this.irisBankImport.importBankTransactionsTASK(transactionsDate) + await this.irisBankImport.importBankTransactionsTASK(transactionsDate) } } diff --git a/apps/api/src/tasks/bank-import/import-transactions.task.spec.ts b/apps/api/src/tasks/bank-import/import-transactions.task.spec.ts index 8602e1a59..ebdff5032 100644 --- a/apps/api/src/tasks/bank-import/import-transactions.task.spec.ts +++ b/apps/api/src/tasks/bank-import/import-transactions.task.spec.ts @@ -232,9 +232,6 @@ describe('ImportTransactionsTask', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any .spyOn(IrisTasks.prototype as any, 'getTransactions') .mockImplementation(() => mockIrisTransactions) - const checkTrxsSpy = jest - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .spyOn(IrisTasks.prototype as any, 'hasNewOrNonImportedTransactions') const prepareBankTrxSpy = jest.spyOn( // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -287,19 +284,7 @@ describe('ImportTransactionsTask', () => { expect(getIBANSpy).toHaveBeenCalled() // 2. Should get IBAN transactions from IRIS expect(getTrxSpy).toHaveBeenCalledWith(irisIBANAccountMock, transactionsDate) - // 3. Should check if transactions are up-to-date - expect(checkTrxsSpy).toHaveBeenCalledWith(mockIrisTransactions, transactionsDate) - expect(prismaMock.bankTransaction.count).toHaveBeenCalledWith( - expect.objectContaining({ - where: { - transactionDate: { - gte: new Date(transactionsDate.toISOString().split('T')[0]), - lte: new Date(transactionsDate.toISOString().split('T')[0]), - }, - }, - }), - ) - // 4.Should prepare the bank-transaction records + // 3.Should prepare the bank-transaction records expect(prepareBankTrxSpy).toHaveBeenCalledWith(mockIrisTransactions, irisIBANAccountMock) // 5.Should process transactions and parse donations @@ -420,69 +405,6 @@ describe('ImportTransactionsTask', () => { ) }) - it('should not run if all current transactions for the day have been processed', async () => { - const donationService = testModule.get(DonationsService) - const getIBANSpy = jest - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .spyOn(IrisTasks.prototype as any, 'getIrisUserIBANaccount') - .mockImplementation(() => irisIBANAccountMock) - const getTrxSpy = jest - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .spyOn(IrisTasks.prototype as any, 'getTransactions') - .mockImplementation(() => mockIrisTransactions) - const checkTrxsSpy = jest.spyOn( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - IrisTasks.prototype as any, - 'hasNewOrNonImportedTransactions', - ) - const prepareBankTrxSpy = jest.spyOn( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - IrisTasks.prototype as any, - 'prepareBankTransactionRecords', - ) - const processDonationsSpy = jest.spyOn( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - IrisTasks.prototype as any, - 'processDonations', - ) - const prepareBankPaymentSpy = jest.spyOn( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - IrisTasks.prototype as any, - 'prepareBankPaymentObject', - ) - const donationSpy = jest.spyOn(donationService, 'createUpdateBankPayment') - const saveTrxSpy = jest - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .spyOn(IrisTasks.prototype as any, 'saveBankTrxRecords') - - // The length of the imported transactions is the same as the ones received from IRIS -meaning everything is up-to date - jest.spyOn(prismaMock.bankTransaction, 'count').mockResolvedValue(mockIrisTransactions.length) - jest.spyOn(prismaMock, '$transaction').mockResolvedValue('SUCCESS') - jest.spyOn(prismaMock.campaign, 'findMany').mockResolvedValue(mockDonatedCampaigns) - - const transactionsDate = new Date() - // Run task - await irisTasks.importBankTransactionsTASK(transactionsDate) - - // 1. Should get IRIS iban account - expect(getIBANSpy).toHaveBeenCalled() - // 2. Should get IBAN transactions from IRIS - expect(getTrxSpy).toHaveBeenCalledWith(irisIBANAccountMock, transactionsDate) - // 3. Should check if transactions are up-to-date - expect(checkTrxsSpy).toHaveBeenCalledWith(mockIrisTransactions, transactionsDate) - // The rest of the flow should not have been executed - // 4. Should not be run - expect(prepareBankTrxSpy).not.toHaveBeenCalled() - // 5. Should not be run - expect(processDonationsSpy).not.toHaveBeenCalled() - // 6. Should not be run - expect(prepareBankPaymentSpy).not.toHaveBeenCalled() - // 7. Should not be run - expect(donationSpy).not.toHaveBeenCalled() - // 8. Should not be run - expect(saveTrxSpy).not.toHaveBeenCalled() - }) - it('should not run if no transactions have been fetched', async () => { jest // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/apps/api/src/tasks/bank-import/import-transactions.task.ts b/apps/api/src/tasks/bank-import/import-transactions.task.ts index 9ff1ac896..990d06327 100644 --- a/apps/api/src/tasks/bank-import/import-transactions.task.ts +++ b/apps/api/src/tasks/bank-import/import-transactions.task.ts @@ -141,37 +141,25 @@ export class IrisTasks { ibanAccount = account } catch (e) { - return Logger.error('Failed to get iban data from Iris') + return Logger.error('Failed to get iban data from Iris' + e.message) } // 2. Get transactions from IRIS let transactions: IrisTransactionInfo[] try { transactions = await this.getTransactions(ibanAccount, transactionsDate) + //Logger.debug(`Received ${transactions.length} for date: ${transactionsDate} ` + JSON.stringify(transactions)) // No transactions for the day yet if (!transactions.length) return } catch (e) { - return Logger.error('Failed to get transactions data from Iris') + return Logger.error('Failed to get transactions data from Iris' + e.message) } - // 3. Check if the cron should actually run - try { - const isUpToDate = await this.hasNewOrNonImportedTransactions(transactions, transactionsDate) - - /** - Should we let it run every time, (giving it a chance to import some previously failed donation for example, because DB was down for 0.5 sec). - This would also mean that the whole flow will run for all transactions every time - **/ - - if (isUpToDate) return - } catch (e) { - // Failure of this check is not critical - } - - // 4. Prepare the BankTransaction Records + // 3. Prepare the BankTransaction Records let bankTrxRecords: filteredTransaction[] try { bankTrxRecords = this.prepareBankTransactionRecords(transactions, ibanAccount) + Logger.debug(`Transactions for import after filtering: ${bankTrxRecords.length}`) } catch (e) { return Logger.error('Error while preparing BankTransaction records') } @@ -181,21 +169,22 @@ export class IrisTasks { try { processedBankTrxRecords = await this.processDonations(bankTrxRecords) } catch (e) { - return Logger.error('Failed to process transaction donations') + return Logger.error('Failed to process transaction donations' + e.message) } // 6. Save BankTransactions to DB try { - await this.saveBankTrxRecords(processedBankTrxRecords) + const savedTransactions = await this.saveBankTrxRecords(processedBankTrxRecords) + Logger.debug('Saved transactions count: ' + savedTransactions.count) } catch (e) { - return Logger.error('Failed to import transactions into DB') + return Logger.error('Failed to import transactions into DB: ' + e.message) } //7. Notify about unrecognized donations try { await this.sendUnrecognizedDonationsMail(processedBankTrxRecords) } catch (e) { - return Logger.error('Failed to notify about bad transaction donations') + return Logger.error('Failed to notify about bad transaction donations ' + e.message) } return @@ -241,13 +230,18 @@ export class IrisTasks { private async getTransactions(ibanAccount: IrisIbanAccountInfo, transactionsDate: Date) { const endpoint = this.config.get('iris.transactionsEndPoint', '') - const dateToCheck = transactionsDate.toISOString().split('T')[0] + const dateFrom = DateTime.fromJSDate(transactionsDate) + const dateTo = dateFrom.plus({ days: 1 }) - Logger.debug('Getting transactions for date:' + dateToCheck) + Logger.debug( + `Getting transactions from date: ${dateFrom.toISODate()} to date: ${dateTo.toISODate()}`, + ) const response = ( await this.httpService.axiosRef.get( - endpoint + `/${ibanAccount.id}` + `?dateFrom=${dateToCheck}&dateTo=${dateToCheck}`, + endpoint + + `/${ibanAccount.id}` + + `?dateFrom=${dateFrom.toISODate()}&dateTo=${dateTo.toISODate()}`, { headers: { 'x-user-hash': this.userHash, @@ -260,25 +254,6 @@ export class IrisTasks { return response.transactions } - // Checks to see if all transactions have been processed already - private async hasNewOrNonImportedTransactions( - transactions: IrisTransactionInfo[], - transactionsDate: Date, - ) { - const dateToCheck = new Date(transactionsDate.toISOString().split('T')[0]) - - const count = await this.prisma.bankTransaction.count({ - where: { - transactionDate: { - gte: dateToCheck, - lte: dateToCheck, - }, - }, - }) - - return transactions.length === count - } - private extractAmountFromTransactionId(transactionId, transactionValueDate): number { const formattedDate = DateTime.fromISO(transactionValueDate).toFormat('yyyyMMdd') const matchAmountRegex = new RegExp(`${formattedDate}(?[0-9.]+)_${formattedDate}`)