From 40e52265807a8f9a9c08844b43c6660280dfaddf Mon Sep 17 00:00:00 2001 From: Mahendra Date: Wed, 11 Oct 2023 19:51:52 -0300 Subject: [PATCH 01/12] feat: add idempotency interceptor --- .cspell.json | 5 +- apps/api/e2e/idempotency.e2e.ts | 283 ++++++++++++++++++ apps/api/src/.env.development | 2 + apps/api/src/.env.production | 2 + apps/api/src/.env.test | 2 + apps/api/src/app.module.ts | 8 +- .../framework/idempotency.interceptor.ts | 246 +++++++++++++++ .../src/app/testing/dtos/idempotency.dto.ts | 3 + .../api/src/app/testing/testing.controller.ts | 30 +- apps/api/src/types/env.d.ts | 1 + .../src/services/cache/cache-service.spec.ts | 10 + .../src/services/cache/cache.service.ts | 16 + 12 files changed, 605 insertions(+), 3 deletions(-) create mode 100644 apps/api/e2e/idempotency.e2e.ts create mode 100644 apps/api/src/app/shared/framework/idempotency.interceptor.ts create mode 100644 apps/api/src/app/testing/dtos/idempotency.dto.ts diff --git a/.cspell.json b/.cspell.json index 42a732732e9..b536d14745a 100644 --- a/.cspell.json +++ b/.cspell.json @@ -513,7 +513,10 @@ "Clicksend", "Kamil", "Myƛliwiec", - "nestframework" + "nestframework", + "idempotency", + "IDEMPOTENCY", + "Idempotency" ], "flagWords": [], "patterns": [ diff --git a/apps/api/e2e/idempotency.e2e.ts b/apps/api/e2e/idempotency.e2e.ts new file mode 100644 index 00000000000..b48f111feaf --- /dev/null +++ b/apps/api/e2e/idempotency.e2e.ts @@ -0,0 +1,283 @@ +import { UserSession } from '@novu/testing'; +import { CacheService } from '@novu/application-generic'; +import { expect } from 'chai'; +describe('Idempotency Test', async () => { + let session: UserSession; + const path = '/v1/testing/idempotency'; + const HEADER_KEYS = { + IDEMPOTENCY: 'idempotency-key', + RETRY_AFTER: 'retry-after', + IDEMPOTENCY_CONFLICT: 'x-idempotency-conflict', + }; + let cacheService: CacheService | null = null; + + describe('when enabled', () => { + before(async () => { + session = new UserSession(); + await session.initialize(); + cacheService = session.testServer?.getService(CacheService); + process.env.API_IDEMPOTENCY_ENABLED = 'true'; + }); + + it('should return cached same response for duplicate requests', async () => { + const key = `1`; + const { body, headers } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 201 }) + .expect(201); + const { body: bodyDupe, headers: headerDupe } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 201 }) + .expect(201); + expect(typeof body.data.number === 'number').to.be.true; + expect(body.data.number).to.equal(bodyDupe.data.number); + expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + }); + it('should return cached and use correct cache key when apiKey is used', async () => { + const key = `2`; + const { body, headers } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 201 }) + .expect(201); + const cacheKey = `test-${session.organization._id}-${key}`; + session.testServer?.getHttpServer(); + // eslint-disable-next-line @typescript-eslint/no-non-null-asserted-optional-chain + const cacheVal = JSON.stringify(JSON.parse(await cacheService?.get(cacheKey)!).data); + expect(JSON.stringify(body)).to.eq(cacheVal); + const { body: bodyDupe, headers: headerDupe } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 201 }) + .expect(201); + expect(typeof body.data.number === 'number').to.be.true; + expect(body.data.number).to.equal(bodyDupe.data.number); + expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + }); + it('should return cached and use correct cache key when authToken and apiKey combination is used', async () => { + const key = `3`; + const { body, headers } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', session.token) + .send({ data: 201 }) + .expect(201); + const cacheKey = `test-${session.organization._id}-${key}`; + session.testServer?.getHttpServer(); + // eslint-disable-next-line @typescript-eslint/no-non-null-asserted-optional-chain + const cacheVal = JSON.stringify(JSON.parse(await cacheService?.get(cacheKey)!).data); + expect(JSON.stringify(body)).to.eq(cacheVal); + const { body: bodyDupe, headers: headerDupe } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 201 }) + .expect(201); + expect(typeof body.data.number === 'number').to.be.true; + expect(body.data.number).to.equal(bodyDupe.data.number); + expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + }); + it('should return conflict when concurrent requests are made', async () => { + const key = `4`; + const [{ headers, body, status }, { headers: headerDupe, body: bodyDupe, status: statusDupe }] = + await Promise.all([ + session.testAgent.post(path).set(HEADER_KEYS.IDEMPOTENCY, key).send({ data: 250 }), + session.testAgent.post(path).set(HEADER_KEYS.IDEMPOTENCY, key).send({ data: 250 }), + ]); + const oneSuccess = status === 201 || statusDupe === 201; + const oneConflict = status === 429 || statusDupe === 429; + const conflictBody = status === 201 ? bodyDupe : body; + const retryHeader = headers[HEADER_KEYS.RETRY_AFTER] || headerDupe[HEADER_KEYS.RETRY_AFTER]; + const conflictHeader = headers[HEADER_KEYS.IDEMPOTENCY_CONFLICT] || headerDupe[HEADER_KEYS.IDEMPOTENCY_CONFLICT]; + expect(oneSuccess).to.be.true; + expect(oneConflict).to.be.true; + expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(retryHeader).to.eq(`1`); + expect(conflictHeader).to.eq('REQUEST_PROCESSING'); + expect(JSON.stringify(conflictBody)).to.eq( + JSON.stringify({ + statusCode: 429, + message: `Request with key "${key}" is currently being processed. Please retry after 1 second`, + }) + ); + }); + it('should return conflict when different body is sent for same key', async () => { + const key = '5'; + const { headers, body, status } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 250 }); + const { + headers: headerDupe, + body: bodyDupe, + status: statusDupe, + } = await session.testAgent.post(path).set(HEADER_KEYS.IDEMPOTENCY, key).send({ data: 251 }); + + const oneSuccess = status === 201 || statusDupe === 201; + const oneConflict = status === 422 || statusDupe === 422; + const conflictBody = status === 201 ? bodyDupe : body; + const conflictHeader = headers[HEADER_KEYS.IDEMPOTENCY_CONFLICT] || headerDupe[HEADER_KEYS.IDEMPOTENCY_CONFLICT]; + expect(oneSuccess).to.be.true; + expect(oneConflict).to.be.true; + expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(conflictHeader).to.eq('DIFFERENT_BODY'); + expect(JSON.stringify(conflictBody)).to.eq( + JSON.stringify({ + message: `Request with key "${key}" is being reused for a different body`, + error: 'Unprocessable Entity', + statusCode: 422, + }) + ); + }); + it('should return non cached response for unique requests', async () => { + const key = '6'; + const key1 = '7'; + const { body, headers } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 201 }) + .expect(201); + + const { body: bodyDupe, headers: headerDupe } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key1) + .send({ data: 201 }) + .expect(201); + expect(typeof body.data.number === 'number').to.be.true; + expect(typeof bodyDupe.data.number === 'number').to.be.true; + expect(body.data.number).not.to.equal(bodyDupe.data.number); + expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key1); + }); + it('should return non cached response for GET requests', async () => { + const key = '8'; + const { body, headers } = await session.testAgent + .get(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({}) + .expect(200); + + const { body: bodyDupe } = await session.testAgent + .get(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({}) + .expect(200); + expect(typeof body.data.number === 'number').to.be.true; + expect(typeof bodyDupe.data.number === 'number').to.be.true; + expect(body.data.number).not.to.equal(bodyDupe.data.number); + expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(undefined); + }); + it('should return cached error response for duplicate requests', async () => { + const key = '9'; + const { body, headers } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 422 }) + .expect(422); + + const { body: bodyDupe, headers: headerDupe } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 422 }) + .expect(422); + expect(JSON.stringify(body)).to.equal(JSON.stringify(bodyDupe)); + + expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + }); + }); + + describe('when disabled', () => { + before(async () => { + session = new UserSession(); + await session.initialize(); + process.env.API_IDEMPOTENCY_ENABLED = 'false'; + }); + + it('should not return cached same response for duplicate requests', async () => { + const key = '10'; + const { body } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 201 }) + .expect(201); + + const { body: bodyDupe } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 201 }) + .expect(201); + expect(typeof body.data.number === 'number').to.be.true; + expect(body.data.number).not.to.equal(bodyDupe.data.number); + }); + it('should return non cached response for unique requests', async () => { + const key = '11'; + const key1 = '12'; + const { body } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 201 }) + .expect(201); + + const { body: bodyDupe } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key1) + .send({ data: 201 }) + .expect(201); + expect(typeof body.data.number === 'number').to.be.true; + expect(typeof bodyDupe.data.number === 'number').to.be.true; + expect(body.data.number).not.to.equal(bodyDupe.data.number); + }); + it('should return non cached response for GET requests', async () => { + const key = '13'; + const { body } = await session.testAgent.get(path).set(HEADER_KEYS.IDEMPOTENCY, key).send({}).expect(200); + + const { body: bodyDupe } = await session.testAgent + .get(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({}) + .expect(200); + expect(typeof body.data.number === 'number').to.be.true; + expect(typeof bodyDupe.data.number === 'number').to.be.true; + expect(body.data.number).not.to.equal(bodyDupe.data.number); + }); + it('should not return cached error response for duplicate requests', async () => { + const key = '14'; + const { body } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 500 }) + .expect(500); + + const { body: bodyDupe } = await session.testAgent + .post(path) + .set(HEADER_KEYS.IDEMPOTENCY, key) + .set('authorization', `ApiKey ${session.apiKey}`) + .send({ data: 500 }) + .expect(500); + expect(JSON.stringify(body)).not.to.equal(JSON.stringify(bodyDupe)); + }); + }); +}); diff --git a/apps/api/src/.env.development b/apps/api/src/.env.development index 8af859ce673..554a7dcb6ab 100644 --- a/apps/api/src/.env.development +++ b/apps/api/src/.env.development @@ -64,3 +64,5 @@ NOVU_SMS_INTEGRATION_SENDER= INTERCOM_IDENTITY_VERIFICATION_SECRET_KEY= LAUNCH_DARKLY_SDK_KEY= + +API_IDEMPOTENCY_ENABLED=false diff --git a/apps/api/src/.env.production b/apps/api/src/.env.production index 988ff2d4d9a..d0e79b9bfec 100644 --- a/apps/api/src/.env.production +++ b/apps/api/src/.env.production @@ -53,3 +53,5 @@ NOVU_SMS_INTEGRATION_SENDER= INTERCOM_IDENTITY_VERIFICATION_SECRET_KEY= LAUNCH_DARKLY_SDK_KEY= + +API_IDEMPOTENCY_ENABLED=false diff --git a/apps/api/src/.env.test b/apps/api/src/.env.test index 5b43216c298..301e6f52efd 100644 --- a/apps/api/src/.env.test +++ b/apps/api/src/.env.test @@ -90,3 +90,5 @@ MAX_NOVU_INTEGRATION_SMS_REQUESTS=20 NOVU_SMS_INTEGRATION_ACCOUNT_SID=test NOVU_SMS_INTEGRATION_TOKEN=test NOVU_SMS_INTEGRATION_SENDER=1234567890 + +API_IDEMPOTENCY_ENABLED=true diff --git a/apps/api/src/app.module.ts b/apps/api/src/app.module.ts index 00418026038..05373ac753e 100644 --- a/apps/api/src/app.module.ts +++ b/apps/api/src/app.module.ts @@ -31,6 +31,7 @@ import { TopicsModule } from './app/topics/topics.module'; import { InboundParseModule } from './app/inbound-parse/inbound-parse.module'; import { BlueprintModule } from './app/blueprint/blueprint.module'; import { TenantModule } from './app/tenant/tenant.module'; +import { IdempotencyInterceptor } from './app/shared/framework/idempotency.interceptor'; const enterpriseImports = (): Array | ForwardReference> => { const modules: Array | ForwardReference> = []; @@ -78,7 +79,12 @@ const enterpriseModules = enterpriseImports(); const modules = baseModules.concat(enterpriseModules); -const providers: Provider[] = []; +const providers: Provider[] = [ + { + provide: APP_INTERCEPTOR, + useClass: IdempotencyInterceptor, + }, +]; if (process.env.SENTRY_DSN) { modules.push(RavenModule); diff --git a/apps/api/src/app/shared/framework/idempotency.interceptor.ts b/apps/api/src/app/shared/framework/idempotency.interceptor.ts new file mode 100644 index 00000000000..b93d0e608f1 --- /dev/null +++ b/apps/api/src/app/shared/framework/idempotency.interceptor.ts @@ -0,0 +1,246 @@ +import { + Injectable, + NestInterceptor, + ExecutionContext, + CallHandler, + Logger, + HttpException, + InternalServerErrorException, + ServiceUnavailableException, + UnprocessableEntityException, +} from '@nestjs/common'; +import { CacheService } from '@novu/application-generic'; +import { Observable, of, throwError } from 'rxjs'; +import { catchError, map } from 'rxjs/operators'; +import { createHash } from 'crypto'; +import * as jwt from 'jsonwebtoken'; +import { IJwtPayload } from '@novu/shared'; + +const LOG_CONTEXT = 'IdempotencyInterceptor'; +const IDEMPOTENCY_CACHE_TTL = 60 * 60 * 24; //24h +const IDEMPOTENCY_PROGRESS_TTL = 60 * 5; //5min + +const HEADER_KEYS = { + IDEMPOTENCY_KEY: 'idempotency-key', + RETRY_AFTER: 'retry-after', + IDEMPOTENCY_CONFLICT: 'x-idempotency-conflict', + IDEMPOTENCY_REPLAYED: 'idempotency-replayed', + LINK: 'link', +}; + +const DOCS_LINK = 'docs.novu.co/idempotency'; + +enum IdempotencyConflictEnum { + REQUEST_PROCESSING = 'REQUEST_PROCESSING', + DIFFERENT_BODY = 'DIFFERENT_BODY', +} + +enum ReqStatusEnum { + PROGRESS = 'in-progress', + SUCCESS = 'success', + ERROR = 'error', +} + +@Injectable() +export class IdempotencyInterceptor implements NestInterceptor { + constructor(private readonly cacheService: CacheService) {} + + async intercept(context: ExecutionContext, next: CallHandler): Promise> { + const request = context.switchToHttp().getRequest(); + const idempotencyKey = this.getIdempotencyKey(context); + const isEnabled = process.env.API_IDEMPOTENCY_ENABLED == 'true'; + if (!isEnabled || !idempotencyKey || !['post', 'patch'].includes(request.method.toLowerCase())) { + return next.handle(); + } + const cacheKey = this.getCacheKey(context); + + try { + const bodyHash = this.hashRequestBody(request.body); + //if 1st time we are seeing the request, marks the request as in-progress if not, does nothing + const isNewReq = await this.setCache( + cacheKey, + { status: ReqStatusEnum.PROGRESS, bodyHash }, + IDEMPOTENCY_PROGRESS_TTL, + true + ); + // Check if the idempotency key is in the cache + if (isNewReq) { + return await this.handleNewRequest(context, next, bodyHash); + } else { + return await this.handlerDuplicateRequest(context, bodyHash); + } + } catch (err) { + Logger.warn( + `An error occurred while making idempotency check, proceeding without idempotency. key:${idempotencyKey}. error: ${err.message}`, + LOG_CONTEXT + ); + } + + //something unexpected happened, both cached response and handler did not execute as expected + return throwError(() => new ServiceUnavailableException()); + } + + private getIdempotencyKey(context: ExecutionContext): string | undefined { + const request = context.switchToHttp().getRequest(); + + return request.headers[HEADER_KEYS.IDEMPOTENCY_KEY]; + } + + private getReqUser(context: ExecutionContext): IJwtPayload | null { + const req = context.switchToHttp().getRequest(); + if (req?.user?.organizationId) { + return req.user; + } + if (req.headers?.authorization?.length) { + const token = req.headers.authorization.split(' ')[1]; + if (token) { + return jwt.decode(token); + } + } + + return null; + } + + private getCacheKey(context: ExecutionContext): string { + const { organizationId } = this.getReqUser(context) || {}; + const env = process.env.NODE_ENV; + + return `${env}-${organizationId}-${this.getIdempotencyKey(context)}`; + } + + async setCache( + key: string, + val: { status: ReqStatusEnum; bodyHash: string; data?: any; statusCode?: number }, + ttl: number, + ifNotExists?: boolean + ): Promise { + try { + if (ifNotExists) { + return await this.cacheService.setIfNotExist(key, JSON.stringify(val), { ttl }); + } + await this.cacheService.set(key, JSON.stringify(val), { ttl }); + } catch (err) { + Logger.warn(`An error occurred while setting idempotency cache, key:${key} error: ${err.message}`, LOG_CONTEXT); + } + + return null; + } + + private buildError(error: any): HttpException { + const statusCode = error.status || error.response?.statusCode || 500; + if (statusCode == 500 && !error.response) { + //some unhandled exception occured + return new InternalServerErrorException(); + } + + return new HttpException(error.response || error.message, statusCode, error.response?.options); + } + + private setHeaders(response: any, headers: Record) { + Object.keys(headers).map((key) => { + if (headers[key]) { + response.set(key, headers[key]); + } + }); + } + + private hashRequestBody(body: object): string { + const hash = createHash('blake2s256'); + hash.update(Buffer.from(JSON.stringify(body))); + + return hash.digest('hex'); + } + + private async handlerDuplicateRequest(context: ExecutionContext, bodyHash: string): Promise> { + const cacheKey = this.getCacheKey(context); + const idempotencyKey = this.getIdempotencyKey(context)!; + const data = await this.cacheService.get(cacheKey); + this.setHeaders(context.switchToHttp().getResponse(), { [HEADER_KEYS.IDEMPOTENCY_KEY]: idempotencyKey }); + const parsed = JSON.parse(data); + if (parsed.status === ReqStatusEnum.PROGRESS) { + // api call is in progress, so client need to handle this case + Logger.error(`previous api call in progress rejecting the request. key:${idempotencyKey}`, LOG_CONTEXT); + this.setHeaders(context.switchToHttp().getResponse(), { + [HEADER_KEYS.IDEMPOTENCY_CONFLICT]: IdempotencyConflictEnum.REQUEST_PROCESSING, + [HEADER_KEYS.RETRY_AFTER]: `1`, + }); + + return throwError( + () => + new HttpException( + `Request with key "${idempotencyKey}" is currently being processed. Please retry after 1 second`, + 429 + ) + ); + } + if (bodyHash !== parsed.bodyHash) { + //different body sent than before + Logger.error(`idempotency key is being reused for different bodies. key:${idempotencyKey}`, LOG_CONTEXT); + this.setHeaders(context.switchToHttp().getResponse(), { + [HEADER_KEYS.IDEMPOTENCY_CONFLICT]: IdempotencyConflictEnum.DIFFERENT_BODY, + [HEADER_KEYS.LINK]: DOCS_LINK, + }); + + return throwError( + () => + new UnprocessableEntityException(`Request with key "${idempotencyKey}" is being reused for a different body`) + ); + } + this.setHeaders(context.switchToHttp().getResponse(), { [HEADER_KEYS.IDEMPOTENCY_REPLAYED]: 'true' }); + + //already seen the request return cached response + if (parsed.status === ReqStatusEnum.ERROR) { + Logger.error(`returning cached error response. key:${idempotencyKey}`, LOG_CONTEXT); + + return throwError(() => this.buildError(parsed.data)); + } + + return of(parsed.data); + } + + private async handleNewRequest( + context: ExecutionContext, + next: CallHandler, + bodyHash: string + ): Promise> { + const cacheKey = this.getCacheKey(context); + const idempotencyKey = this.getIdempotencyKey(context)!; + + return next.handle().pipe( + map(async (response) => { + const httpResponse = context.switchToHttp().getResponse(); + const statusCode = httpResponse.statusCode; + + // Cache the success response and return it + await this.setCache( + cacheKey, + { status: ReqStatusEnum.SUCCESS, bodyHash, statusCode: statusCode, data: response }, + IDEMPOTENCY_CACHE_TTL + ); + Logger.verbose(`cached the success response for idempotency key:${idempotencyKey}`, LOG_CONTEXT); + this.setHeaders(httpResponse, { [HEADER_KEYS.IDEMPOTENCY_KEY]: idempotencyKey }); + + return response; + }), + catchError((err) => { + const httpException = this.buildError(err); + // Cache the error response and return it + const error = err instanceof HttpException ? err : httpException; + this.setCache( + cacheKey, + { + status: ReqStatusEnum.ERROR, + statusCode: httpException.getStatus(), + bodyHash, + data: error, + }, + IDEMPOTENCY_CACHE_TTL + ).catch(() => {}); + Logger.verbose(`cached the error response for idempotency key:${idempotencyKey}`, LOG_CONTEXT); + this.setHeaders(context.switchToHttp().getResponse(), { [HEADER_KEYS.IDEMPOTENCY_KEY]: idempotencyKey }); + + return throwError(() => error); + }) + ); + } +} diff --git a/apps/api/src/app/testing/dtos/idempotency.dto.ts b/apps/api/src/app/testing/dtos/idempotency.dto.ts new file mode 100644 index 00000000000..6f885116da1 --- /dev/null +++ b/apps/api/src/app/testing/dtos/idempotency.dto.ts @@ -0,0 +1,3 @@ +export class IdempotencyBodyDto { + data: number; +} diff --git a/apps/api/src/app/testing/testing.controller.ts b/apps/api/src/app/testing/testing.controller.ts index f23c67f0e76..1939ca49148 100644 --- a/apps/api/src/app/testing/testing.controller.ts +++ b/apps/api/src/app/testing/testing.controller.ts @@ -1,12 +1,16 @@ -import { Body, Controller, NotFoundException, Post } from '@nestjs/common'; +import { Body, Controller, Get, HttpException, NotFoundException, Post, UseGuards } from '@nestjs/common'; import { DalService } from '@novu/dal'; import { IUserEntity } from '@novu/shared'; import { ISeedDataResponseDto, SeedDataBodyDto } from './dtos/seed-data.dto'; +import { IdempotencyBodyDto } from './dtos/idempotency.dto'; + import { SeedData } from './usecases/seed-data/seed-data.usecase'; import { SeedDataCommand } from './usecases/seed-data/seed-data.command'; import { CreateSession } from './usecases/create-session/create-session.usecase'; import { CreateSessionCommand } from './usecases/create-session/create-session.command'; import { ApiExcludeController } from '@nestjs/swagger'; +import { JwtAuthGuard } from '../auth/framework/auth.guard'; +import { ExternalApiAccessible } from '../auth/framework/external-api.decorator'; @Controller('/testing') @ApiExcludeController() @@ -47,4 +51,28 @@ export class TestingController { return await this.seedDataUsecase.execute(command); } + + @ExternalApiAccessible() + @UseGuards(JwtAuthGuard) + @Post('/idempotency') + async idempotency(@Body() body: IdempotencyBodyDto): Promise<{ number: number }> { + if (process.env.NODE_ENV !== 'test') throw new NotFoundException(); + + if (body.data > 300) { + throw new HttpException(`` + Math.random(), body.data); + } + if (body.data === 250) { + //for testing conflict + await new Promise((resolve) => setTimeout(resolve, 500)); + } + + return { number: Math.random() }; + } + + @Get('/idempotency') + async idempotencyGet(): Promise<{ number: number }> { + if (process.env.NODE_ENV !== 'test') throw new NotFoundException(); + + return { number: Math.random() }; + } } diff --git a/apps/api/src/types/env.d.ts b/apps/api/src/types/env.d.ts index d5650e5ab3c..f48a79d75de 100644 --- a/apps/api/src/types/env.d.ts +++ b/apps/api/src/types/env.d.ts @@ -11,6 +11,7 @@ declare namespace NodeJS { NODE_ENV: 'test' | 'production' | 'dev' | 'ci' | 'local'; PORT: string; DISABLE_USER_REGISTRATION: 'true' | 'false'; + API_IDEMPOTENCY_ENABLED: 'true' | 'false'; FRONT_BASE_URL: string; SENTRY_DSN: string; } diff --git a/packages/application-generic/src/services/cache/cache-service.spec.ts b/packages/application-generic/src/services/cache/cache-service.spec.ts index 573f9a57452..545bf45b548 100644 --- a/packages/application-generic/src/services/cache/cache-service.spec.ts +++ b/packages/application-generic/src/services/cache/cache-service.spec.ts @@ -113,6 +113,16 @@ describe('Cache Service - Cluster Mode', () => { expect(value).toBe('value1'); }); + it('should be able to add a key / value in the Redis Cluster if key not exist', async () => { + const result = await cacheService.setIfNotExist('key1-not-exist', 'value1'); + expect(result).toBeDefined(); + const result1 = await cacheService.setIfNotExist( + 'key1-not-exist', + 'value1' + ); + expect(result1).toBeFalsy(); + }); + it('should be able to delete a key / value in the Redis Cluster', async () => { const result = await cacheService.del('key1'); expect(result).toBe(1); diff --git a/packages/application-generic/src/services/cache/cache.service.ts b/packages/application-generic/src/services/cache/cache.service.ts index 03fa14518bf..3623d420bde 100644 --- a/packages/application-generic/src/services/cache/cache.service.ts +++ b/packages/application-generic/src/services/cache/cache.service.ts @@ -84,6 +84,22 @@ export class CacheService implements ICacheService { return result; } + public async setIfNotExist( + key: string, + value: string, + options?: CachingConfig + ): Promise { + const result = await this.client?.set( + key, + value, + 'EX', + this.getTtlInSeconds(options), + 'NX' + ); + + return result; + } + public async setQuery( key: string, value: string, From c3feff1326d129e81db9405d53e58e6f782069a1 Mon Sep 17 00:00:00 2001 From: Richard Fontein <32132657+rifont@users.noreply.github.com> Date: Wed, 25 Oct 2023 14:57:45 +0100 Subject: [PATCH 02/12] fix: Idempotency-Replay noun convention --- apps/api/src/app/shared/framework/idempotency.interceptor.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/api/src/app/shared/framework/idempotency.interceptor.ts b/apps/api/src/app/shared/framework/idempotency.interceptor.ts index b93d0e608f1..a29d5f0694c 100644 --- a/apps/api/src/app/shared/framework/idempotency.interceptor.ts +++ b/apps/api/src/app/shared/framework/idempotency.interceptor.ts @@ -24,7 +24,7 @@ const HEADER_KEYS = { IDEMPOTENCY_KEY: 'idempotency-key', RETRY_AFTER: 'retry-after', IDEMPOTENCY_CONFLICT: 'x-idempotency-conflict', - IDEMPOTENCY_REPLAYED: 'idempotency-replayed', + IDEMPOTENCY_REPLAY: 'idempotency-replay', LINK: 'link', }; @@ -186,7 +186,7 @@ export class IdempotencyInterceptor implements NestInterceptor { new UnprocessableEntityException(`Request with key "${idempotencyKey}" is being reused for a different body`) ); } - this.setHeaders(context.switchToHttp().getResponse(), { [HEADER_KEYS.IDEMPOTENCY_REPLAYED]: 'true' }); + this.setHeaders(context.switchToHttp().getResponse(), { [HEADER_KEYS.IDEMPOTENCY_REPLAY]: 'true' }); //already seen the request return cached response if (parsed.status === ReqStatusEnum.ERROR) { From 4403ca89ebd7c6ed0589d7ba3ca1a3632a82902d Mon Sep 17 00:00:00 2001 From: Richard Fontein <32132657+rifont@users.noreply.github.com> Date: Wed, 25 Oct 2023 15:01:03 +0100 Subject: [PATCH 03/12] test(api): Fix idempotency header fixture naming --- apps/api/e2e/idempotency.e2e.ts | 84 ++++++++++++++++----------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/apps/api/e2e/idempotency.e2e.ts b/apps/api/e2e/idempotency.e2e.ts index b48f111feaf..fb5803f9e26 100644 --- a/apps/api/e2e/idempotency.e2e.ts +++ b/apps/api/e2e/idempotency.e2e.ts @@ -5,7 +5,7 @@ describe('Idempotency Test', async () => { let session: UserSession; const path = '/v1/testing/idempotency'; const HEADER_KEYS = { - IDEMPOTENCY: 'idempotency-key', + IDEMPOTENCY_KEY: 'idempotency-key', RETRY_AFTER: 'retry-after', IDEMPOTENCY_CONFLICT: 'x-idempotency-conflict', }; @@ -23,26 +23,26 @@ describe('Idempotency Test', async () => { const key = `1`; const { body, headers } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 201 }) .expect(201); const { body: bodyDupe, headers: headerDupe } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 201 }) .expect(201); expect(typeof body.data.number === 'number').to.be.true; expect(body.data.number).to.equal(bodyDupe.data.number); - expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); - expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headers[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); }); it('should return cached and use correct cache key when apiKey is used', async () => { const key = `2`; const { body, headers } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 201 }) .expect(201); @@ -53,20 +53,20 @@ describe('Idempotency Test', async () => { expect(JSON.stringify(body)).to.eq(cacheVal); const { body: bodyDupe, headers: headerDupe } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 201 }) .expect(201); expect(typeof body.data.number === 'number').to.be.true; expect(body.data.number).to.equal(bodyDupe.data.number); - expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); - expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headers[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); }); it('should return cached and use correct cache key when authToken and apiKey combination is used', async () => { const key = `3`; const { body, headers } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', session.token) .send({ data: 201 }) .expect(201); @@ -77,31 +77,31 @@ describe('Idempotency Test', async () => { expect(JSON.stringify(body)).to.eq(cacheVal); const { body: bodyDupe, headers: headerDupe } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 201 }) .expect(201); expect(typeof body.data.number === 'number').to.be.true; expect(body.data.number).to.equal(bodyDupe.data.number); - expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); - expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headers[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); }); it('should return conflict when concurrent requests are made', async () => { const key = `4`; const [{ headers, body, status }, { headers: headerDupe, body: bodyDupe, status: statusDupe }] = await Promise.all([ - session.testAgent.post(path).set(HEADER_KEYS.IDEMPOTENCY, key).send({ data: 250 }), - session.testAgent.post(path).set(HEADER_KEYS.IDEMPOTENCY, key).send({ data: 250 }), + session.testAgent.post(path).set(HEADER_KEYS.IDEMPOTENCY_KEY, key).send({ data: 250 }), + session.testAgent.post(path).set(HEADER_KEYS.IDEMPOTENCY_KEY, key).send({ data: 250 }), ]); const oneSuccess = status === 201 || statusDupe === 201; const oneConflict = status === 429 || statusDupe === 429; const conflictBody = status === 201 ? bodyDupe : body; const retryHeader = headers[HEADER_KEYS.RETRY_AFTER] || headerDupe[HEADER_KEYS.RETRY_AFTER]; - const conflictHeader = headers[HEADER_KEYS.IDEMPOTENCY_CONFLICT] || headerDupe[HEADER_KEYS.IDEMPOTENCY_CONFLICT]; + const conflictHeader = headers[HEADER_KEYS.IDEMPOTENCY_KEY_CONFLICT] || headerDupe[HEADER_KEYS.IDEMPOTENCY_KEY_CONFLICT]; expect(oneSuccess).to.be.true; expect(oneConflict).to.be.true; - expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); - expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headers[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); expect(retryHeader).to.eq(`1`); expect(conflictHeader).to.eq('REQUEST_PROCESSING'); expect(JSON.stringify(conflictBody)).to.eq( @@ -115,23 +115,23 @@ describe('Idempotency Test', async () => { const key = '5'; const { headers, body, status } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 250 }); const { headers: headerDupe, body: bodyDupe, status: statusDupe, - } = await session.testAgent.post(path).set(HEADER_KEYS.IDEMPOTENCY, key).send({ data: 251 }); + } = await session.testAgent.post(path).set(HEADER_KEYS.IDEMPOTENCY_KEY, key).send({ data: 251 }); const oneSuccess = status === 201 || statusDupe === 201; const oneConflict = status === 422 || statusDupe === 422; const conflictBody = status === 201 ? bodyDupe : body; - const conflictHeader = headers[HEADER_KEYS.IDEMPOTENCY_CONFLICT] || headerDupe[HEADER_KEYS.IDEMPOTENCY_CONFLICT]; + const conflictHeader = headers[HEADER_KEYS.IDEMPOTENCY_KEY_CONFLICT] || headerDupe[HEADER_KEYS.IDEMPOTENCY_KEY_CONFLICT]; expect(oneSuccess).to.be.true; expect(oneConflict).to.be.true; - expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); - expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headers[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); expect(conflictHeader).to.eq('DIFFERENT_BODY'); expect(JSON.stringify(conflictBody)).to.eq( JSON.stringify({ @@ -146,61 +146,61 @@ describe('Idempotency Test', async () => { const key1 = '7'; const { body, headers } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 201 }) .expect(201); const { body: bodyDupe, headers: headerDupe } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key1) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key1) .send({ data: 201 }) .expect(201); expect(typeof body.data.number === 'number').to.be.true; expect(typeof bodyDupe.data.number === 'number').to.be.true; expect(body.data.number).not.to.equal(bodyDupe.data.number); - expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); - expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key1); + expect(headers[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key1); }); it('should return non cached response for GET requests', async () => { const key = '8'; const { body, headers } = await session.testAgent .get(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({}) .expect(200); const { body: bodyDupe } = await session.testAgent .get(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({}) .expect(200); expect(typeof body.data.number === 'number').to.be.true; expect(typeof bodyDupe.data.number === 'number').to.be.true; expect(body.data.number).not.to.equal(bodyDupe.data.number); - expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(undefined); + expect(headers[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(undefined); }); it('should return cached error response for duplicate requests', async () => { const key = '9'; const { body, headers } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 422 }) .expect(422); const { body: bodyDupe, headers: headerDupe } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 422 }) .expect(422); expect(JSON.stringify(body)).to.equal(JSON.stringify(bodyDupe)); - expect(headers[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); - expect(headerDupe[HEADER_KEYS.IDEMPOTENCY]).to.eq(key); + expect(headers[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); + expect(headerDupe[HEADER_KEYS.IDEMPOTENCY_KEY]).to.eq(key); }); }); @@ -215,14 +215,14 @@ describe('Idempotency Test', async () => { const key = '10'; const { body } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 201 }) .expect(201); const { body: bodyDupe } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 201 }) .expect(201); @@ -234,14 +234,14 @@ describe('Idempotency Test', async () => { const key1 = '12'; const { body } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 201 }) .expect(201); const { body: bodyDupe } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key1) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key1) .send({ data: 201 }) .expect(201); expect(typeof body.data.number === 'number').to.be.true; @@ -250,11 +250,11 @@ describe('Idempotency Test', async () => { }); it('should return non cached response for GET requests', async () => { const key = '13'; - const { body } = await session.testAgent.get(path).set(HEADER_KEYS.IDEMPOTENCY, key).send({}).expect(200); + const { body } = await session.testAgent.get(path).set(HEADER_KEYS.IDEMPOTENCY_KEY, key).send({}).expect(200); const { body: bodyDupe } = await session.testAgent .get(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({}) .expect(200); @@ -266,14 +266,14 @@ describe('Idempotency Test', async () => { const key = '14'; const { body } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 500 }) .expect(500); const { body: bodyDupe } = await session.testAgent .post(path) - .set(HEADER_KEYS.IDEMPOTENCY, key) + .set(HEADER_KEYS.IDEMPOTENCY_KEY, key) .set('authorization', `ApiKey ${session.apiKey}`) .send({ data: 500 }) .expect(500); From 35042d3f350820c2b5d57edfdbc876feea18a5d7 Mon Sep 17 00:00:00 2001 From: Richard Fontein <32132657+rifont@users.noreply.github.com> Date: Wed, 25 Oct 2023 15:02:35 +0100 Subject: [PATCH 04/12] fix: Idempotency interceptor typo --- apps/api/src/app/shared/framework/idempotency.interceptor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/src/app/shared/framework/idempotency.interceptor.ts b/apps/api/src/app/shared/framework/idempotency.interceptor.ts index a29d5f0694c..c47bae9b47c 100644 --- a/apps/api/src/app/shared/framework/idempotency.interceptor.ts +++ b/apps/api/src/app/shared/framework/idempotency.interceptor.ts @@ -129,7 +129,7 @@ export class IdempotencyInterceptor implements NestInterceptor { private buildError(error: any): HttpException { const statusCode = error.status || error.response?.statusCode || 500; if (statusCode == 500 && !error.response) { - //some unhandled exception occured + //some unhandled exception occurred return new InternalServerErrorException(); } From 1c85adff5d749bb04a617e74984fe0a81817ffc4 Mon Sep 17 00:00:00 2001 From: Richard Fontein <32132657+rifont@users.noreply.github.com> Date: Wed, 25 Oct 2023 15:04:41 +0100 Subject: [PATCH 05/12] fix: Remove redundant throwError wrapper --- apps/api/src/app/shared/framework/idempotency.interceptor.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/apps/api/src/app/shared/framework/idempotency.interceptor.ts b/apps/api/src/app/shared/framework/idempotency.interceptor.ts index c47bae9b47c..02f7899ba6b 100644 --- a/apps/api/src/app/shared/framework/idempotency.interceptor.ts +++ b/apps/api/src/app/shared/framework/idempotency.interceptor.ts @@ -181,10 +181,7 @@ export class IdempotencyInterceptor implements NestInterceptor { [HEADER_KEYS.LINK]: DOCS_LINK, }); - return throwError( - () => - new UnprocessableEntityException(`Request with key "${idempotencyKey}" is being reused for a different body`) - ); + throw new UnprocessableEntityException(`Request with key "${idempotencyKey}" is being reused for a different body`) } this.setHeaders(context.switchToHttp().getResponse(), { [HEADER_KEYS.IDEMPOTENCY_REPLAY]: 'true' }); From 8c15f66bf373932c08fc7e4f25fbf4f281dd639e Mon Sep 17 00:00:00 2001 From: p-fernandez Date: Tue, 24 Oct 2023 11:48:32 +0100 Subject: [PATCH 06/12] fix(api): few tests were flakey in the ci/cd --- .../src/app/events/e2e/send-message-push.e2e.ts | 4 +++- apps/api/src/app/events/e2e/trigger-event.e2e.ts | 1 + pnpm-lock.yaml | 16 +++++++++++++--- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/apps/api/src/app/events/e2e/send-message-push.e2e.ts b/apps/api/src/app/events/e2e/send-message-push.e2e.ts index e9cafbbea4b..18e5b0869b7 100644 --- a/apps/api/src/app/events/e2e/send-message-push.e2e.ts +++ b/apps/api/src/app/events/e2e/send-message-push.e2e.ts @@ -12,6 +12,8 @@ import { UserSession } from '@novu/testing'; const axiosInstance = axios.create(); +const ORIGINAL_IS_MULTI_PROVIDER_CONFIGURATION_ENABLED = process.env.IS_MULTI_PROVIDER_CONFIGURATION_ENABLED; + describe('Trigger event - Send Push Notification - /v1/events/trigger (POST)', () => { let session: UserSession; let template: NotificationTemplateEntity; @@ -39,7 +41,7 @@ describe('Trigger event - Send Push Notification - /v1/events/trigger (POST)', ( }); after(() => { - process.env.IS_MULTI_PROVIDER_CONFIGURATION_ENABLED = 'false'; + process.env.IS_MULTI_PROVIDER_CONFIGURATION_ENABLED = ORIGINAL_IS_MULTI_PROVIDER_CONFIGURATION_ENABLED; }); describe('Multiple providers active', () => { diff --git a/apps/api/src/app/events/e2e/trigger-event.e2e.ts b/apps/api/src/app/events/e2e/trigger-event.e2e.ts index 8d1eb8a7d2b..533f284b8b6 100644 --- a/apps/api/src/app/events/e2e/trigger-event.e2e.ts +++ b/apps/api/src/app/events/e2e/trigger-event.e2e.ts @@ -60,6 +60,7 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { describe(`Trigger Event - ${eventTriggerPath} (POST)`, function () { beforeEach(async () => { + process.env.LAUNCH_DARKLY_SDK_KEY = ''; session = new UserSession(); await session.initialize(); template = await session.createTemplate(); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7db83cc83a5..b92b26b0984 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -17266,22 +17266,27 @@ packages: /@protobufjs/aspromise@1.1.2: resolution: {integrity: sha512-j+gKExEuLmKwvz3OgROXtrJ2UG2x8Ch2YZUxahh+s1F2HZ+wAceUNLkvy6zKCPVRkU++ZWQrdxsUeQXmcg4uoQ==} + requiresBuild: true dev: false /@protobufjs/base64@1.1.2: resolution: {integrity: sha512-AZkcAA5vnN/v4PDqKyMR5lx7hZttPDgClv83E//FMNhR2TMcLUhfRUBHCmSl0oi9zMgDDqRUJkSxO3wm85+XLg==} + requiresBuild: true dev: false /@protobufjs/codegen@2.0.4: resolution: {integrity: sha512-YyFaikqM5sH0ziFZCN3xDC7zeGaB/d0IUb9CATugHWbd1FRFwWwt4ld4OYMPWu5a3Xe01mGAULCdqhMlPl29Jg==} + requiresBuild: true dev: false /@protobufjs/eventemitter@1.1.0: resolution: {integrity: sha512-j9ednRT81vYJ9OfVuXG6ERSTdEL1xVsNgqpkxMsbIabzSo3goCjDIveeGv5d03om39ML71RdmrGNjG5SReBP/Q==} + requiresBuild: true dev: false /@protobufjs/fetch@1.1.0: resolution: {integrity: sha512-lljVXpqXebpsijW71PZaCYeIcE5on1w5DlQy5WH6GLbFryLUrBD4932W/E2BSpfRJWseIL4v/KPgBFxDOIdKpQ==} + requiresBuild: true dependencies: '@protobufjs/aspromise': 1.1.2 '@protobufjs/inquire': 1.1.0 @@ -17289,22 +17294,27 @@ packages: /@protobufjs/float@1.0.2: resolution: {integrity: sha512-Ddb+kVXlXst9d+R9PfTIxh1EdNkgoRe5tOX6t01f1lYWOvJnSPDBlG241QLzcyPdoNTsblLUdujGSE4RzrTZGQ==} + requiresBuild: true dev: false /@protobufjs/inquire@1.1.0: resolution: {integrity: sha512-kdSefcPdruJiFMVSbn801t4vFK7KB/5gd2fYvrxhuJYg8ILrmn9SKSX2tZdV6V+ksulWqS7aXjBcRXl3wHoD9Q==} + requiresBuild: true dev: false /@protobufjs/path@1.1.2: resolution: {integrity: sha512-6JOcJ5Tm08dOHAbdR3GrvP+yUUfkjG5ePsHYczMFLq3ZmMkAD98cDgcT2iA1lJ9NVwFd4tH/iSSoe44YWkltEA==} + requiresBuild: true dev: false /@protobufjs/pool@1.1.0: resolution: {integrity: sha512-0kELaGSIDBKvcgS4zkjz1PeddatrjYcmMWOlAuAPwAeccUrPHdUqo/J6LiymHHEiJT5NrF1UVwxY14f+fy4WQw==} + requiresBuild: true dev: false /@protobufjs/utf8@1.1.0: resolution: {integrity: sha512-Vvn3zZrhQZkkBE8LSuW3em98c0FwgO4nxzv6OdSxPKJIEKY2bGbHn+mhGIPerzI4twdxaP8/0+06HBpwf345Lw==} + requiresBuild: true dev: false /@radix-ui/number@0.1.0: @@ -30688,7 +30698,7 @@ packages: eslint: 8.48.0 eslint-import-resolver-node: 0.3.7 eslint-module-utils: 2.8.0(@typescript-eslint/parser@5.58.0)(eslint-import-resolver-node@0.3.7)(eslint-import-resolver-webpack@0.13.7)(eslint@8.48.0) - has: 1.0.3 + has: 1.0.4 is-core-module: 2.13.0 is-glob: 4.0.3 minimatch: 3.1.2 @@ -30766,7 +30776,7 @@ packages: damerau-levenshtein: 1.0.8 emoji-regex: 9.2.2 eslint: 8.48.0 - has: 1.0.3 + has: 1.0.4 jsx-ast-utils: 3.3.3 language-tags: 1.0.5 minimatch: 3.1.2 @@ -30858,7 +30868,7 @@ packages: object.values: 1.1.6 prop-types: 15.8.1 resolve: 2.0.0-next.4 - semver: 6.3.1 + semver: 6.3.0 string.prototype.matchall: 4.0.8 dev: true From c9ecd3f57b70021627c58f68b6bc6cb442cad64e Mon Sep 17 00:00:00 2001 From: p-fernandez Date: Tue, 24 Oct 2023 19:57:06 +0100 Subject: [PATCH 07/12] feat(shared): tidy up filters operators to reuse in events broadcast --- .../e2e/get-grouped-blueprints.e2e.ts | 13 +- .../api/src/app/change/e2e/get-changes.e2e.ts | 12 +- .../src/app/change/e2e/promote-changes.e2e.ts | 54 ++++--- .../src/app/events/e2e/trigger-event.e2e.ts | 54 +++---- .../e2e/create-integration.e2e.ts | 3 +- .../shared/helpers/content.service.spec.ts | 6 +- .../e2e/create-notification-templates.e2e.ts | 10 +- .../src/components/conditions/Conditions.tsx | 24 +-- .../ExecutionDetailsConditionItem.cy.tsx | 4 +- .../ExecutionDetailsConditions.cy.tsx | 8 +- .../pages/templates/filter/FilterModal.tsx | 28 ++-- .../src/pages/templates/filter/Filters.cy.tsx | 23 +-- .../src/pages/templates/filter/Filters.tsx | 18 +-- .../templates/filter/OnlineFiltersForms.tsx | 9 +- .../message-matcher.usecase.spec.ts | 147 ++++++++++-------- .../message-matcher.usecase.ts | 10 +- .../shared/src/types/builder/builder.types.ts | 55 +++++-- .../application-generic/src/utils/filter.ts | 21 +-- .../src/lib/integrations/integrations.spec.ts | 30 +++- 19 files changed, 309 insertions(+), 220 deletions(-) diff --git a/apps/api/src/app/blueprint/e2e/get-grouped-blueprints.e2e.ts b/apps/api/src/app/blueprint/e2e/get-grouped-blueprints.e2e.ts index cf619d79b55..67e4a5893f3 100644 --- a/apps/api/src/app/blueprint/e2e/get-grouped-blueprints.e2e.ts +++ b/apps/api/src/app/blueprint/e2e/get-grouped-blueprints.e2e.ts @@ -3,7 +3,14 @@ import * as sinon from 'sinon'; import { UserSession } from '@novu/testing'; import { NotificationTemplateRepository, EnvironmentRepository } from '@novu/dal'; -import { EmailBlockTypeEnum, FilterPartTypeEnum, INotificationTemplate, StepTypeEnum } from '@novu/shared'; +import { + EmailBlockTypeEnum, + FieldLogicalOperatorEnum, + FieldOperatorEnum, + FilterPartTypeEnum, + INotificationTemplate, + StepTypeEnum, +} from '@novu/shared'; import { buildGroupedBlueprintsKey, CacheService, @@ -177,13 +184,13 @@ export async function createTemplateFromBlueprint({ { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, diff --git a/apps/api/src/app/change/e2e/get-changes.e2e.ts b/apps/api/src/app/change/e2e/get-changes.e2e.ts index 5ac9ea173fa..7406192ba12 100644 --- a/apps/api/src/app/change/e2e/get-changes.e2e.ts +++ b/apps/api/src/app/change/e2e/get-changes.e2e.ts @@ -1,6 +1,12 @@ import { expect } from 'chai'; import { ChangeRepository } from '@novu/dal'; -import { EmailBlockTypeEnum, StepTypeEnum, FilterPartTypeEnum } from '@novu/shared'; +import { + EmailBlockTypeEnum, + StepTypeEnum, + FilterPartTypeEnum, + FieldLogicalOperatorEnum, + FieldOperatorEnum, +} from '@novu/shared'; import { UserSession } from '@novu/testing'; import { CreateWorkflowRequestDto, UpdateWorkflowRequestDto } from '../../workflows/dto'; @@ -32,13 +38,13 @@ describe('Get changes', () => { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, diff --git a/apps/api/src/app/change/e2e/promote-changes.e2e.ts b/apps/api/src/app/change/e2e/promote-changes.e2e.ts index 05ac9b983ca..0c7f5b029b8 100644 --- a/apps/api/src/app/change/e2e/promote-changes.e2e.ts +++ b/apps/api/src/app/change/e2e/promote-changes.e2e.ts @@ -13,6 +13,8 @@ import { ChangeEntityTypeEnum, ChannelCTATypeEnum, EmailBlockTypeEnum, + FieldLogicalOperatorEnum, + FieldOperatorEnum, StepTypeEnum, FilterPartTypeEnum, TemplateVariableTypeEnum, @@ -69,13 +71,13 @@ describe('Promote changes', () => { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, @@ -96,7 +98,7 @@ describe('Promote changes', () => { _parentId: notificationTemplateId, }); - expect(prodVersion._notificationGroupId).to.eq(prodGroup._id); + expect(prodVersion?._notificationGroupId).to.eq(prodGroup._id); }); it('should promote step variables default values', async () => { @@ -204,13 +206,13 @@ describe('Promote changes', () => { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, @@ -242,7 +244,7 @@ describe('Promote changes', () => { _parentId: notificationTemplateId, } as any); - expect(prodVersion.steps.length).to.eq(0); + expect(prodVersion?.steps.length).to.eq(0); }); it('update active flag on notification template', async () => { @@ -274,7 +276,7 @@ describe('Promote changes', () => { _parentId: notificationTemplateId, }); - expect(prodVersion.active).to.eq(true); + expect(prodVersion?.active).to.eq(true); }); it('update existing message', async () => { @@ -295,13 +297,13 @@ describe('Promote changes', () => { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, @@ -352,7 +354,7 @@ describe('Promote changes', () => { _parentId: step._templateId, }); - expect(prodVersion.name).to.eq('test'); + expect(prodVersion?.name).to.eq('test'); }); it('add one more message', async () => { @@ -373,13 +375,13 @@ describe('Promote changes', () => { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, @@ -431,13 +433,13 @@ describe('Promote changes', () => { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'secondName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, @@ -479,13 +481,13 @@ describe('Promote changes', () => { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, @@ -521,13 +523,13 @@ describe('Promote changes', () => { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, @@ -632,13 +634,13 @@ describe('Promote changes', () => { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, @@ -659,7 +661,7 @@ describe('Promote changes', () => { _parentId: notificationTemplateId, }); - expect(prodVersion.isBlueprint).to.equal(true); + expect(prodVersion?.isBlueprint).to.equal(true); }); it('should merge creation, and status changes to one change', async () => { @@ -724,9 +726,15 @@ describe('Promote changes', () => { }); }); - async function getProductionEnvironment() { - return await environmentRepository.findOne({ + async function getProductionEnvironment(): Promise { + const production = await environmentRepository.findOne({ _parentId: session.environment._id, }); + + if (!production) { + throw new Error('No production environment'); + } + + return production; } }); diff --git a/apps/api/src/app/events/e2e/trigger-event.e2e.ts b/apps/api/src/app/events/e2e/trigger-event.e2e.ts index 533f284b8b6..66474841a14 100644 --- a/apps/api/src/app/events/e2e/trigger-event.e2e.ts +++ b/apps/api/src/app/events/e2e/trigger-event.e2e.ts @@ -19,13 +19,15 @@ import { UserSession, SubscribersService } from '@novu/testing'; import { ChannelTypeEnum, EmailBlockTypeEnum, + FieldLogicalOperatorEnum, + FieldOperatorEnum, + FilterPartTypeEnum, StepTypeEnum, IEmailBlock, ISubscribersDefine, TemplateVariableTypeEnum, EmailProviderIdEnum, SmsProviderIdEnum, - FilterPartTypeEnum, DigestUnitEnum, DelayTypeEnum, PreviousStepTypeEnum, @@ -91,11 +93,11 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.PAYLOAD, - operator: 'IS_DEFINED', + operator: FieldOperatorEnum.IS_DEFINED, field: 'exclude', value: '', }, @@ -171,11 +173,11 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.PAYLOAD, - operator: 'IS_DEFINED', + operator: FieldOperatorEnum.IS_DEFINED, field: 'exclude', value: '', }, @@ -251,11 +253,11 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.PAYLOAD, - operator: 'IS_DEFINED', + operator: FieldOperatorEnum.IS_DEFINED, field: 'exclude', value: '', }, @@ -332,11 +334,11 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.PAYLOAD, - operator: 'IS_DEFINED', + operator: FieldOperatorEnum.IS_DEFINED, field: 'exclude', value: '', }, @@ -398,7 +400,7 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { _environmentId: session.environment._id, conditions: [ { - children: [{ field: 'identifier', value: 'test', operator: 'EQUAL', on: 'tenant' }], + children: [{ field: 'identifier', value: 'test', operator: FieldOperatorEnum.EQUAL, on: 'tenant' }], }, ], active: true, @@ -437,10 +439,10 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { _environmentId: session.environment._id, conditions: [ { - value: 'OR', + value: FieldLogicalOperatorEnum.OR, children: [ - { field: 'identifier', value: 'test3', operator: 'EQUAL', on: 'tenant' }, - { field: 'identifier', value: 'test2', operator: 'EQUAL', on: 'tenant' }, + { field: 'identifier', value: 'test3', operator: FieldOperatorEnum.EQUAL, on: 'tenant' }, + { field: 'identifier', value: 'test2', operator: FieldOperatorEnum.EQUAL, on: 'tenant' }, ], }, ], @@ -497,7 +499,7 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { _environmentId: session.environment._id, conditions: [ { - children: [{ field: 'identifier', value: 'test1', operator: 'EQUAL', on: 'tenant' }], + children: [{ field: 'identifier', value: 'test1', operator: FieldOperatorEnum.EQUAL, on: 'tenant' }], }, ], active: true, @@ -1702,13 +1704,13 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { field: 'run', value: 'true', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, on: FilterPartTypeEnum.PAYLOAD, }, ], @@ -1735,13 +1737,13 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { field: 'subscriberId', value: subscriber.subscriberId, - operator: 'NOT_EQUAL', + operator: FieldOperatorEnum.NOT_EQUAL, on: FilterPartTypeEnum.SUBSCRIBER, }, ], @@ -1800,12 +1802,12 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { field: 'isOnline', value: 'true', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, on: FilterPartTypeEnum.WEBHOOK, webhookUrl: 'www.user.com/webhook', }, @@ -1902,12 +1904,12 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { field: 'isOnline', value: 'true', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, on: FilterPartTypeEnum.WEBHOOK, webhookUrl: 'www.user.com/webhook', }, @@ -1967,12 +1969,12 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { field: 'isOnline', value: 'true', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, on: FilterPartTypeEnum.WEBHOOK, webhookUrl: 'www.user.com/webhook', }, @@ -2229,7 +2231,7 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.PREVIOUS_STEP, @@ -2321,7 +2323,7 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.PREVIOUS_STEP, diff --git a/apps/api/src/app/integrations/e2e/create-integration.e2e.ts b/apps/api/src/app/integrations/e2e/create-integration.e2e.ts index 8d99417548d..6fb6813fffa 100644 --- a/apps/api/src/app/integrations/e2e/create-integration.e2e.ts +++ b/apps/api/src/app/integrations/e2e/create-integration.e2e.ts @@ -4,6 +4,7 @@ import { ChannelTypeEnum, ChatProviderIdEnum, EmailProviderIdEnum, + FieldOperatorEnum, InAppProviderIdEnum, PushProviderIdEnum, SmsProviderIdEnum, @@ -106,7 +107,7 @@ describe('Create Integration - /integration (POST)', function () { check: false, conditions: [ { - children: [{ field: 'identifier', value: 'test', operator: 'EQUAL', on: 'tenant' }], + children: [{ field: 'identifier', value: 'test', operator: FieldOperatorEnum.EQUAL, on: 'tenant' }], }, ], }; diff --git a/apps/api/src/app/shared/helpers/content.service.spec.ts b/apps/api/src/app/shared/helpers/content.service.spec.ts index 70f80455447..32984c527b1 100644 --- a/apps/api/src/app/shared/helpers/content.service.spec.ts +++ b/apps/api/src/app/shared/helpers/content.service.spec.ts @@ -3,6 +3,8 @@ import { DelayTypeEnum, DigestTypeEnum, DigestUnitEnum, + FieldLogicalOperatorEnum, + FieldOperatorEnum, FilterPartTypeEnum, StepTypeEnum, TriggerContextTypeEnum, @@ -292,13 +294,13 @@ describe('ContentService', function () { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.PAYLOAD, field: 'counter', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, diff --git a/apps/api/src/app/workflows/e2e/create-notification-templates.e2e.ts b/apps/api/src/app/workflows/e2e/create-notification-templates.e2e.ts index 0f404d73e90..68cb6190a25 100644 --- a/apps/api/src/app/workflows/e2e/create-notification-templates.e2e.ts +++ b/apps/api/src/app/workflows/e2e/create-notification-templates.e2e.ts @@ -4,6 +4,8 @@ import { ChannelCTATypeEnum, ChannelTypeEnum, EmailBlockTypeEnum, + FieldLogicalOperatorEnum, + FieldOperatorEnum, StepTypeEnum, INotificationTemplate, TriggerTypeEnum, @@ -80,13 +82,13 @@ describe('Create Workflow - /workflows (POST)', async () => { { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, @@ -504,13 +506,13 @@ export async function createTemplateFromBlueprint({ { isNegated: false, type: 'GROUP', - value: 'AND', + value: FieldLogicalOperatorEnum.AND, children: [ { on: FilterPartTypeEnum.SUBSCRIBER, field: 'firstName', value: 'test value', - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, }, ], }, diff --git a/apps/web/src/components/conditions/Conditions.tsx b/apps/web/src/components/conditions/Conditions.tsx index 12486d47273..cc1e3167cb8 100644 --- a/apps/web/src/components/conditions/Conditions.tsx +++ b/apps/web/src/components/conditions/Conditions.tsx @@ -3,7 +3,7 @@ import styled from '@emotion/styled'; import { useMemo } from 'react'; import { Control, Controller, useFieldArray, useForm, useWatch } from 'react-hook-form'; -import { FILTER_TO_LABEL, FilterPartTypeEnum } from '@novu/shared'; +import { FILTER_TO_LABEL, FilterPartTypeEnum, FieldLogicalOperatorEnum, FieldOperatorEnum } from '@novu/shared'; import { Button, @@ -135,13 +135,13 @@ export function Conditions({ { return ( ; i - {operator !== 'IS_DEFINED' && ( + {operator !== FieldOperatorEnum.IS_DEFINED && ( { append({ - operator: 'EQUAL', + operator: FieldOperatorEnum.EQUAL, on: 'payload', }); }} @@ -332,28 +332,28 @@ function EqualityForm({ { return ( +