From 4e6945edefddf5f95941d47bec7a41a97c472875 Mon Sep 17 00:00:00 2001 From: liorzam <6435752+liorzam@users.noreply.github.com> Date: Thu, 19 Dec 2024 19:13:55 +0200 Subject: [PATCH] [bal-3193] - backward compatbility for previous alert's subject (#2898) --- .../src/alert/alert.repository.ts | 3 +- .../src/alert/alert.service.ts | 16 +++++-- .../data-investigation.service.ts | 46 +++++++++++++++++-- .../test/helpers/create-alert-definition.ts | 4 +- .../src/test/helpers/create-alert.ts | 11 ++++- .../test/helpers/create-transaction-record.ts | 27 +++++++++-- ...ansaction.controller.external.intg.test.ts | 34 ++++++++------ .../transaction.controller.external.ts | 24 +++++----- .../workflows-service/tsconfig.build.json | 1 - 9 files changed, 127 insertions(+), 39 deletions(-) diff --git a/services/workflows-service/src/alert/alert.repository.ts b/services/workflows-service/src/alert/alert.repository.ts index 31010f290e..3157798498 100644 --- a/services/workflows-service/src/alert/alert.repository.ts +++ b/services/workflows-service/src/alert/alert.repository.ts @@ -18,12 +18,13 @@ export class AlertRepository { } async findFirst>( - args: Prisma.SelectSubset>, + args: Prisma.SelectSubset>, projectIds: TProjectIds, ) { const queryArgs = this.scopeService.scopeFindFirst(args, projectIds); return await this.prisma.extendedClient.alert.findFirst({ + ...queryArgs, where: queryArgs.where, orderBy: { createdAt: 'desc', diff --git a/services/workflows-service/src/alert/alert.service.ts b/services/workflows-service/src/alert/alert.service.ts index e4625a1a47..2f45f9b862 100644 --- a/services/workflows-service/src/alert/alert.service.ts +++ b/services/workflows-service/src/alert/alert.service.ts @@ -24,6 +24,7 @@ import { DedupeWindow, TDedupeStrategy, TExecutionDetails } from './types'; import { computeHash } from '@ballerine/common'; import { convertTimeUnitToMilliseconds } from '@/data-analytics/utils'; import { DataInvestigationService } from '@/data-analytics/data-investigation.service'; +import { equals } from 'class-validator'; const DEFAULT_DEDUPE_STRATEGIES = { cooldownTimeframeInMinutes: 60 * 24, @@ -63,12 +64,17 @@ export class AlertService { async getAlertWithDefinition( alertId: string, projectId: string, + monitoringType: MonitoringType, ): Promise<(Alert & { alertDefinition: AlertDefinition }) | null> { - const alert = await this.alertRepository.findById( - alertId, + const alert = await this.alertRepository.findFirst( { where: { id: alertId, + alertDefinition: { + monitoringType: { + equals: monitoringType, + }, + }, }, include: { alertDefinition: true, @@ -346,7 +352,7 @@ export class AlertService { const projectId = alertDef.projectId; const now = new Date(); - return this.alertRepository.create({ + const alertData = { data: { projectId, alertDefinitionId: alertDef.id, @@ -371,7 +377,9 @@ export class AlertService { createdAt: now, dataTimestamp: now, }, - }); + }; + + return this.alertRepository.create(alertData); } private async isDuplicateAlert( diff --git a/services/workflows-service/src/data-analytics/data-investigation.service.ts b/services/workflows-service/src/data-analytics/data-investigation.service.ts index e150e85cec..0dde69b9ef 100644 --- a/services/workflows-service/src/data-analytics/data-investigation.service.ts +++ b/services/workflows-service/src/data-analytics/data-investigation.service.ts @@ -1,4 +1,5 @@ -import { SubjectRecord } from '@/alert/types'; +import { ALERT_DEFINITIONS } from './../../scripts/alerts/generate-alerts'; +import { SubjectRecord, TExecutionDetails } from '@/alert/types'; import { AppLoggerService } from '@/common/app-logger/app-logger.service'; import { Injectable } from '@nestjs/common'; import { Alert, PaymentMethod, Prisma, TransactionRecordType } from '@prisma/client'; @@ -15,12 +16,13 @@ import { TPeerGroupTransactionAverageOptions, TransactionsAgainstDynamicRulesType, } from './types'; +import type { AlertService } from '@/alert/alert.service'; @Injectable() export class DataInvestigationService { constructor(protected readonly logger: AppLoggerService) {} - getInvestigationFilter(projectId: string, inlineRule: InlineRule, subject: SubjectRecord) { + getInvestigationFilter(projectId: string, inlineRule: InlineRule, subject?: SubjectRecord) { let investigationFilter; switch (inlineRule.fnInvestigationName) { @@ -91,13 +93,51 @@ export class DataInvestigationService { } return { - ...subject, + // TODO: Backward compatibility, Remove this when all rules are updated, this is a temporary fix ...investigationFilter, + ...this.buildSubjectFilterCompetability(inlineRule, subject), ...this._buildTransactionsFiltersByAlert(inlineRule), projectId, } satisfies Prisma.TransactionRecordWhereInput; } + // TODO: can be removed after all rules are updated, support for subjects in the alert + buildSubjectFilterCompetabilityByAlert( + alert: NonNullable>>, + ) { + const inlineRule = + ALERT_DEFINITIONS[alert.alertDefinition.ruleId as keyof typeof ALERT_DEFINITIONS]?.inlineRule; + + if (!inlineRule) { + this.logger.error(`Couldnt find related alert definition by ruleId`, { + alert, + }); + + return {}; + } + + const subject = (alert.executionDetails as TExecutionDetails).subject; + + return this.buildSubjectFilterCompetability(inlineRule, subject); + } + + // TODO: can be removed after all rules are updated + buildSubjectFilterCompetability(inlineRule: InlineRule, subject?: SubjectRecord) { + return { + ...(subject?.counterpartyId && + (inlineRule.subjects[0] === 'counterpartyOriginatorId' || + inlineRule.subjects[0] === 'counterpartyBeneficiaryId') && { + [inlineRule.subjects[0]]: subject.counterpartyId, + }), + ...(subject?.counterpartyOriginatorId && { + counterpartyOriginatorId: subject.counterpartyOriginatorId, + }), + ...(subject?.counterpartyBeneficiaryId && { + counterpartyBeneficiaryId: subject?.counterpartyBeneficiaryId, + }), + }; + } + investigateTransactionsAgainstDynamicRules(options: TransactionsAgainstDynamicRulesType) { const { amountBetween, diff --git a/services/workflows-service/src/test/helpers/create-alert-definition.ts b/services/workflows-service/src/test/helpers/create-alert-definition.ts index 7c525c993a..db804a7e29 100644 --- a/services/workflows-service/src/test/helpers/create-alert-definition.ts +++ b/services/workflows-service/src/test/helpers/create-alert-definition.ts @@ -58,7 +58,9 @@ export const createAlertDefinition = async ( }, excludePaymentMethods: faker.datatype.boolean(), }, - subjects: [faker.helpers.arrayElement(['counterpartyId', 'businessId', 'transactionId'])], + subjects: [ + faker.helpers.arrayElement(['counterpartyBeneficiaryId', 'counterpartyOriginatorId']), + ], }, }; diff --git a/services/workflows-service/src/test/helpers/create-alert.ts b/services/workflows-service/src/test/helpers/create-alert.ts index 15e50bdf6c..3b10028fdc 100644 --- a/services/workflows-service/src/test/helpers/create-alert.ts +++ b/services/workflows-service/src/test/helpers/create-alert.ts @@ -1,18 +1,27 @@ import { AlertService } from '@/alert/alert.service'; import { AlertDefinition } from '@prisma/client'; +import { createTransactionRecord } from './create-transaction-record'; +import { InlineRule } from '@/data-analytics/types'; export const createAlert = async ( projectId: string, alertDefinition: AlertDefinition, alertService: AlertService, + transactions: Awaited>, ) => { + const subject = (alertDefinition.inlineRule as InlineRule).subjects[0] as + | 'counterpartyBeneficiaryId' + | 'counterpartyOriginatorId'; + + const subjectValue = transactions[0]?.[subject]; + // Accessing private method for testing purposes while maintaining types return await alertService.createAlert( { ...alertDefinition, projectId, }, - [], + [{ [subject]: subjectValue }], {}, {}, ); diff --git a/services/workflows-service/src/test/helpers/create-transaction-record.ts b/services/workflows-service/src/test/helpers/create-transaction-record.ts index 46c9200298..e93ee3c73a 100644 --- a/services/workflows-service/src/test/helpers/create-transaction-record.ts +++ b/services/workflows-service/src/test/helpers/create-transaction-record.ts @@ -22,6 +22,7 @@ import { Test } from '@nestjs/testing'; import { ClsModule } from 'nestjs-cls'; import { ProjectScopeService } from '@/project/project-scope.service'; import { DataAnalyticsService } from '@/data-analytics/data-analytics.service'; +import { TransactionCreatedDto } from '@/transaction/dtos/transaction-created.dto'; export const createTransactionRecord = async ( prisma: PrismaClient, @@ -122,7 +123,7 @@ export const createTransactionRecord = async ( brand: faker.helpers.arrayElement(['Visa', 'Mastercard', 'Amex']), expiryMonth: faker.date.future().getMonth().toString().padStart(2, '0'), expiryYear: faker.date.future().getFullYear().toString(), - holderName: faker.name.findName(), + holderName: faker.name.fullName(), tokenized: faker.random.alphaNumeric(24), cardBin: parseInt(faker.finance.creditCardNumber().slice(0, 6)), }, @@ -131,10 +132,30 @@ export const createTransactionRecord = async ( }; // Use createBulk to create the transaction - const createdTransactions = await transactionService.createBulk({ + const createdTransactions = (await transactionService.createBulk({ transactionsPayload: [transactionCreateDto], projectId: project.id, + })) satisfies Array< + | TransactionCreatedDto + | { + errorMessage: string; + correlationId: string; + } + >; + + const result = await prisma.transactionRecord.findMany({ + include: { + counterpartyOriginator: true, + counterpartyBeneficiary: true, + }, + where: { + id: { + in: createdTransactions + .map(transaction => 'id' in transaction && transaction.id) + .filter(Boolean), + }, + }, }); - return createdTransactions; + return result; }; diff --git a/services/workflows-service/src/transaction/transaction.controller.external.intg.test.ts b/services/workflows-service/src/transaction/transaction.controller.external.intg.test.ts index f1430945c7..1d7a0dc4b6 100644 --- a/services/workflows-service/src/transaction/transaction.controller.external.intg.test.ts +++ b/services/workflows-service/src/transaction/transaction.controller.external.intg.test.ts @@ -624,7 +624,7 @@ describe('#TransactionControllerExternal', () => { const createTransactionWithDate = async (daysAgo: number) => { const currentDate = new Date(); - await createTransactionRecord(app.get(PrismaService), project, { + return await createTransactionRecord(app.get(PrismaService), project, { date: new Date(currentDate.getTime() - daysAgo * 24 * 60 * 60 * 1000), }); }; @@ -635,9 +635,8 @@ describe('#TransactionControllerExternal', () => { getAlertDefinitionWithTimeOptions('days', 7) as any, alertService, ); - const alert = await createAlert(project.id, alertDefinition, alertService); - await Promise.all([ + const result = await Promise.all([ // 5 transactions in the past 7 days createTransactionWithDate(1), createTransactionWithDate(3), @@ -652,12 +651,14 @@ describe('#TransactionControllerExternal', () => { createTransactionWithDate(20), ]); + const alert = await createAlert(project.id, alertDefinition, alertService, result[0]); + const response = await request(app.getHttpServer()) .get(`/external/transactions/by-alert?alertId=${alert.id}`) .set('authorization', `Bearer ${API_KEY}`); expect(response.status).toBe(200); - expect(response.body).toHaveLength(5); + expect(response.body).toHaveLength(1); }); it('returns 404 when alertId is not found', async () => { const nonExistentAlertId = faker.datatype.uuid(); @@ -674,12 +675,15 @@ describe('#TransactionControllerExternal', () => { getAlertDefinitionWithTimeOptions('days', 1) as any, alertService, ); - const alert = await createAlert(project.id, alertDefinition, alertService); + // TODO: shouldnt happen, might we remove this test? + const alert = await createAlert(project.id, alertDefinition, alertService, []); // Create a transaction older than the alert criteria - await createTransactionRecord(app.get(PrismaService), project, { - date: new Date(Date.now() - 2 * 24 * 60 * 60 * 1000), // 2 days ago - }); + const tx1 = ( + await createTransactionRecord(app.get(PrismaService), project, { + date: new Date(Date.now() - 2 * 24 * 60 * 60 * 1000), // 2 days ago + }) + )[0]; const response = await request(app.getHttpServer()) .get(`/external/transactions/by-alert?alertId=${alert.id}`) @@ -710,7 +714,7 @@ describe('#TransactionControllerExternal', () => { alertService, ); - const alert = await createAlert(otherProject.id, alertDefinition, alertService); + const alert = await createAlert(otherProject.id, alertDefinition, alertService, []); const response = await request(app.getHttpServer()) .get(`/external/transactions/by-alert?alertId=${alert.id}`) @@ -726,9 +730,11 @@ describe('#TransactionControllerExternal', () => { const fiveDaysAgo = new Date(Date.now() - 5 * 24 * 60 * 60 * 1000); // Create transactions at different times - await createTransactionRecord(app.get(PrismaService), project, { date: fifteenDaysAgo }); - await createTransactionRecord(app.get(PrismaService), project, { date: tenDaysAgo }); - await createTransactionRecord(app.get(PrismaService), project, { date: fiveDaysAgo }); + const tx1 = ( + await createTransactionRecord(app.get(PrismaService), project, { + date: fifteenDaysAgo, + }) + )[0]; alertDefinition = await createAlertDefinition( project.id, @@ -736,14 +742,14 @@ describe('#TransactionControllerExternal', () => { alertService, ); - const alert = await createAlert(project.id, alertDefinition, alertService); + const alert = await createAlert(project.id, alertDefinition, alertService, [tx1!]); const response = await request(app.getHttpServer()) .get(`/external/transactions/by-alert?alertId=${alert.id}`) .set('authorization', `Bearer ${API_KEY}`); expect(response.status).toBe(200); - expect(response.body).toHaveLength(3); + expect(response.body).toHaveLength(1); // Verify that all returned transactions are within the last 15 days response.body.forEach((transaction: any) => { diff --git a/services/workflows-service/src/transaction/transaction.controller.external.ts b/services/workflows-service/src/transaction/transaction.controller.external.ts index ededfca4f7..9d1ef8a163 100644 --- a/services/workflows-service/src/transaction/transaction.controller.external.ts +++ b/services/workflows-service/src/transaction/transaction.controller.external.ts @@ -37,7 +37,7 @@ import { GetTransactionsDto, } from '@/transaction/dtos/get-transactions.dto'; import { TransactionCreatedDto } from '@/transaction/dtos/transaction-created.dto'; -import { PaymentMethod } from '@prisma/client'; +import { MonitoringType, PaymentMethod } from '@prisma/client'; import { isEmpty } from 'lodash'; import { TransactionEntityMapper } from './transaction.mapper'; import { DataInvestigationService } from '@/data-analytics/data-investigation.service'; @@ -292,13 +292,6 @@ export class TransactionControllerExternal { @Get('/by-alert') // @UseCustomerAuthGuard() - @swagger.ApiOkResponse({ description: 'Returns an array of transactions.' }) - @swagger.ApiQuery({ name: 'businessId', description: 'Filter by business ID.', required: false }) - @swagger.ApiQuery({ - name: 'counterpartyId', - description: 'Filter by counterparty ID.', - required: false, - }) @swagger.ApiQuery({ name: 'startDate', type: Date, @@ -339,13 +332,20 @@ export class TransactionControllerExternal { @Query() filters: GetTransactionsByAlertDto, @CurrentProject() projectId: types.TProjectId, ) { - const alert = await this.alertService.getAlertWithDefinition(filters.alertId, projectId); + const alert = await this.alertService.getAlertWithDefinition( + filters.alertId, + projectId, + MonitoringType.transaction_monitoring, + ); if (!alert) { throw new errors.NotFoundException(`Alert with id ${filters.alertId} not found`); } - if (!alert.alertDefinition) { + if ( + !alert.alertDefinition || + alert.alertDefinition.monitoringType !== MonitoringType.transaction_monitoring + ) { throw new errors.NotFoundException(`Alert definition not found for alert ${alert.id}`); } @@ -423,8 +423,10 @@ export class TransactionControllerExternal { alert: NonNullable>>; filters: Pick; }) { + const subject = this.dataInvestigationService.buildSubjectFilterCompetabilityByAlert(alert); + return this.service.getTransactions(projectId, filters, { - where: alert.executionDetails.filters, + where: { ...alert.executionDetails.filters, ...subject }, include: { counterpartyBeneficiary: { select: { diff --git a/services/workflows-service/tsconfig.build.json b/services/workflows-service/tsconfig.build.json index 5415ff6a43..d67a561579 100644 --- a/services/workflows-service/tsconfig.build.json +++ b/services/workflows-service/tsconfig.build.json @@ -3,7 +3,6 @@ "sourceRoot": "/", // Shortens file paths on Sentry "exclude": [ "node_modules", - "prisma/migrations", "prisma/schema.prisma", "test", "dist",