chore(application-generic): move shared services to application-generic package #3088
Conversation
| @@ -11,7 +11,7 @@ import { | |||
| UserRepository, | |||
| } from '@novu/dal'; | |||
| import { AuthProviderEnum, IJwtPayload, ISubscriberJwt, MemberRoleEnum, SignUpOriginEnum } from '@novu/shared'; | |||
| import { AnalyticsService } from '@novu/application-generic'; | |||
| import { AnalyticsService, CacheKeyPrefixEnum, Cached } from '@novu/application-generic'; | |||
There was a problem hiding this comment.
moved caching service and all the interceptors to the application-generic package
| import { CancelDelayed, CancelDelayedCommand } from './usecases/cancel-delayed'; | ||
| import { SendMessageCommand, SendTestEmail, SendTestEmailCommand } from './usecases/send-message'; |
There was a problem hiding this comment.
SendMessageCommand was unused
| import { Injectable, Logger } from '@nestjs/common'; | ||
| import * as Sentry from '@sentry/node'; | ||
| import { JobEntity, JobRepository, NotificationTemplateEntity, NotificationTemplateRepository } from '@novu/dal'; | ||
| import { ChannelTypeEnum, InAppProviderIdEnum, STEP_TYPE_TO_CHANNEL_TYPE } from '@novu/shared'; | ||
| import { EventsPerformanceService } from '@novu/application-generic'; |
There was a problem hiding this comment.
events performance service was also moved to application-generic package
| { | ||
| provide: PerformanceService, | ||
| useFactory: () => { | ||
| return new PerformanceService(); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
the PerformanceService is not injected into any other services in the constructor, so no need to add it to the Nest DI context
| @@ -20,7 +20,8 @@ | |||
| "lint": "eslint src", | |||
| "lint:fix": "pnpm lint -- --fix", | |||
| "test": "cross-env TS_NODE_COMPILER_OPTIONS='{\"strictNullChecks\": false}' TZ=UTC NODE_ENV=test E2E_RUNNER=true mocha --timeout 10000 --require ts-node/register --exit --file e2e/setup.ts src/**/**/*.spec.ts", | |||
| "test:e2e": "cross-env TS_NODE_COMPILER_OPTIONS='{\"strictNullChecks\": false}' TZ=UTC NODE_ENV=test E2E_RUNNER=true mocha --timeout 10000 --require ts-node/register --exit --file e2e/setup.ts src/**/*.e2e.ts" | |||
| "test:e2e": "cross-env TS_NODE_COMPILER_OPTIONS='{\"strictNullChecks\": false}' TZ=UTC NODE_ENV=test E2E_RUNNER=true mocha --timeout 10000 --require ts-node/register --exit --file e2e/setup.ts src/**/*.e2e.ts", | |||
| "test:performance": "cross-env TS_NODE_COMPILER_OPTIONS='{\"strictNullChecks\": false}' TZ=UTC NODE_ENV=test E2E_RUNNER=true mocha --timeout 10000 --require ts-node/register --exit --file e2e/setup.ts performance/**/*.performance.ts src/**/*.performance.ts" | |||
There was a problem hiding this comment.
copied from the API package
| import { CachingConfig, ICacheService } from './cache.service'; | ||
|
|
||
| describe('cache-service', function () { | ||
| let cacheService: ICacheService; | ||
| before(function () { | ||
|
|
||
| beforeEach(function () { |
There was a problem hiding this comment.
had to rewrite it to jest test
There was a problem hiding this comment.
@djabarovgeorge just to get your second eyes here
| keyPrefix: this.config.keyPrefix ?? this.DEFAULT_KEY_PREFIX, | ||
| tls: this.config.tls, | ||
| }); | ||
| this.client = new Redis( |
There was a problem hiding this comment.
Seems quite harsh change, do we have different prettier config here? (width)
There was a problem hiding this comment.
I've found the problem, the ESLint config has 'max-len': ['warn', { code: 140 }], and Prettier "printWidth": 120,
So it looks like when the code is not formatted and is committed to Git the ESLint prefers its own rules ;)
| @@ -0,0 +1,245 @@ | |||
| import { File } from '@google-cloud/storage'; | |||
There was a problem hiding this comment.
had to rewrite it to jest test
There was a problem hiding this comment.
Wonder if we should continue to use mocha here and rewrite the old tests to use mocha aswell. But maybe not now. Wdyt?
There was a problem hiding this comment.
yeah, it will be more consistent with the rest...
…ared-services-to-application-generic-folder
| @@ -31,7 +26,7 @@ describe('Performance - Events', () => { | |||
| let subscriber: SubscriberEntity; | |||
| let subscriberService: SubscribersService; | |||
| const jobRepository = new JobRepository(); | |||
| let workflowQueueProducerService: WorkflowQueueProducerService; | |||
| let workflowQueueService: WorkflowQueueService; | |||
There was a problem hiding this comment.
It looks like you previous change is reverted? Or is it ok?
There was a problem hiding this comment.
this file was moved from the API folder where the queue service I called WorkflowQueueProducerService, but in this package worker-service it's called WorkflowQueueService :)
| import { CachingConfig, ICacheService } from './cache.service'; | ||
|
|
||
| describe('cache-service', function () { | ||
| let cacheService: ICacheService; | ||
| before(function () { | ||
|
|
||
| beforeEach(function () { |
There was a problem hiding this comment.
@djabarovgeorge just to get your second eyes here
| keyPrefix: this.config.keyPrefix ?? this.DEFAULT_KEY_PREFIX, | ||
| tls: this.config.tls, | ||
| }); | ||
| this.client = new Redis( |
There was a problem hiding this comment.
Seems quite harsh change, do we have different prettier config here? (width)
| @@ -0,0 +1,245 @@ | |||
| import { File } from '@google-cloud/storage'; | |||
There was a problem hiding this comment.
Wonder if we should continue to use mocha here and rewrite the old tests to use mocha aswell. But maybe not now. Wdyt?
…ared-services-to-application-generic-folder
LetItRock
deleted the
nv-1916-move-shared-services-to-application-generic-folder
branch
What change does this PR introduce?
Moved some shared services to the
application-genericpackage:Why was this change needed?
We want to move the jobs processing logic into a separate service that will be easier to dynamically scale based on the queue size.
Other information (Screenshots)