From c49fb287be88ad71caed9f321faee833ccc95742 Mon Sep 17 00:00:00 2001 From: dbarkowsky Date: Wed, 16 Oct 2024 15:54:25 -0700 Subject: [PATCH 1/4] stop resending notifications on previous status --- express-api/src/services/projects/projectsServices.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/express-api/src/services/projects/projectsServices.ts b/express-api/src/services/projects/projectsServices.ts index d68e14393..42a9f6d8b 100644 --- a/express-api/src/services/projects/projectsServices.ts +++ b/express-api/src/services/projects/projectsServices.ts @@ -426,13 +426,23 @@ const handleProjectNotifications = async ( const notifsToSend: Array = []; + // If the status has been changed if (previousStatus !== projectWithRelations.StatusId) { + // Has the project previously been to this status? If so, don't re-queue notifications. + const previousStatuses = await queryRunner.manager.find(ProjectStatusHistory, { + where: { ProjectId: projectWithRelations.Id }, + }); + const statusPreviouslyVisited = previousStatuses.some( + (record: ProjectStatusHistory) => record.StatusId === projectWithRelations.StatusId, + ); + if (!statusPreviouslyVisited) { const statusChangeNotifs = await notificationServices.generateProjectNotifications( projectWithRelations, previousStatus, queryRunner, ); notifsToSend.push(...statusChangeNotifs); + } } if (projectAgencyResponses.length) { From 5f4b14b82d9e75caa5df41202a91ec96bf2a265f Mon Sep 17 00:00:00 2001 From: dbarkowsky Date: Wed, 16 Oct 2024 15:54:41 -0700 Subject: [PATCH 2/4] cancel notifications on cancelled status --- .../src/services/projects/projectsServices.ts | 40 +++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/express-api/src/services/projects/projectsServices.ts b/express-api/src/services/projects/projectsServices.ts index 42a9f6d8b..74b0a4d6a 100644 --- a/express-api/src/services/projects/projectsServices.ts +++ b/express-api/src/services/projects/projectsServices.ts @@ -25,7 +25,10 @@ import { import { ProjectFilter } from '@/services/projects/projectSchema'; import { PropertyType } from '@/constants/propertyType'; import { ProjectRisk } from '@/constants/projectRisk'; -import notificationServices, { AgencyResponseType } from '../notifications/notificationServices'; +import notificationServices, { + AgencyResponseType, + NotificationStatus, +} from '../notifications/notificationServices'; import { constructFindOptionFromQuery, constructFindOptionFromQuerySingleSelect, @@ -411,6 +414,29 @@ const handleProjectNotifications = async ( Id: projectId, }, }); + + // If the project is cancelled, cancel pending notifications + if (projectWithRelations.CancelledOn) { + const pendingNotifications = await queryRunner.manager.find(NotificationQueue, { + where: [ + { + ProjectId: projectWithRelations.Id, + Status: NotificationStatus.Accepted, + }, + { + ProjectId: projectWithRelations.Id, + Status: NotificationStatus.Pending, + }, + ], + }); + await Promise.allSettled( + pendingNotifications.map(async (notification) => { + await notificationServices.cancelNotificationById(notification.Id, user); + }), + ); + return []; + } + const projectAgency = await queryRunner.manager.findOne(Agency, { where: { Id: projectWithRelations.AgencyId }, }); @@ -436,12 +462,12 @@ const handleProjectNotifications = async ( (record: ProjectStatusHistory) => record.StatusId === projectWithRelations.StatusId, ); if (!statusPreviouslyVisited) { - const statusChangeNotifs = await notificationServices.generateProjectNotifications( - projectWithRelations, - previousStatus, - queryRunner, - ); - notifsToSend.push(...statusChangeNotifs); + const statusChangeNotifs = await notificationServices.generateProjectNotifications( + projectWithRelations, + previousStatus, + queryRunner, + ); + notifsToSend.push(...statusChangeNotifs); } } From 60afac300b98d1796148bb59700f83d28366a4d8 Mon Sep 17 00:00:00 2001 From: dbarkowsky Date: Thu, 17 Oct 2024 08:40:23 -0700 Subject: [PATCH 3/4] changed condition, added test --- .../src/services/projects/projectsServices.ts | 3 +- .../projects/projectsServices.test.ts | 148 +++++++++++------- 2 files changed, 90 insertions(+), 61 deletions(-) diff --git a/express-api/src/services/projects/projectsServices.ts b/express-api/src/services/projects/projectsServices.ts index 74b0a4d6a..dab8fd33e 100644 --- a/express-api/src/services/projects/projectsServices.ts +++ b/express-api/src/services/projects/projectsServices.ts @@ -416,7 +416,7 @@ const handleProjectNotifications = async ( }); // If the project is cancelled, cancel pending notifications - if (projectWithRelations.CancelledOn) { + if (projectWithRelations.StatusId === ProjectStatus.CANCELLED) { const pendingNotifications = await queryRunner.manager.find(NotificationQueue, { where: [ { @@ -1084,6 +1084,7 @@ const projectServices = { updateProject, getProjects, getProjectsForExport, + handleProjectNotifications, }; export default projectServices; diff --git a/express-api/tests/unit/services/projects/projectsServices.test.ts b/express-api/tests/unit/services/projects/projectsServices.test.ts index e243ea252..c07d85174 100644 --- a/express-api/tests/unit/services/projects/projectsServices.test.ts +++ b/express-api/tests/unit/services/projects/projectsServices.test.ts @@ -2,6 +2,7 @@ import { AppDataSource } from '@/appDataSource'; import { ProjectStatus } from '@/constants/projectStatus'; import { ProjectType } from '@/constants/projectType'; import { Roles } from '@/constants/roles'; +import { NotificationStatus } from '@/services/notifications/notificationServices'; import projectServices from '@/services/projects/projectsServices'; import userServices from '@/services/users/usersServices'; import { Agency } from '@/typeorm/Entities/Agency'; @@ -244,8 +245,9 @@ jest .mockImplementation(() => projectJoinQueryBuilder); const _generateProjectWatchNotifications = jest.fn(async () => [produceNotificationQueue()]); +const _generateProjectNotifications = jest.fn(async () => [produceNotificationQueue()]); jest.mock('@/services/notifications/notificationServices', () => ({ - generateProjectNotifications: jest.fn(async () => [produceNotificationQueue()]), + generateProjectNotifications: async () => _generateProjectNotifications(), sendNotification: jest.fn(async () => produceNotificationQueue()), generateProjectWatchNotifications: async () => _generateProjectWatchNotifications(), NotificationStatus: { Accepted: 0, Pending: 1, Cancelled: 2, Failed: 3, Completed: 4 }, @@ -616,70 +618,96 @@ describe('UNIT - Project Services', () => { ); expect(_generateProjectWatchNotifications).toHaveBeenCalled(); }); + }); - describe('getProjects', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - it('should return projects based on filter conditions', async () => { - const filter = { - statusId: 1, - agencyId: [3], - quantity: 10, - page: 1, - market: '$12', - netBook: '$12', - agency: 'contains,aaa', - status: 'contains,aaa', - projectNumber: 'contains,aaa', - name: 'contains,Project', - updatedOn: 'before,' + new Date(), - updatedBy: 'Jane', - sortOrder: 'asc', - sortKey: 'Status', - quickFilter: 'hi', - }; - - // Call the service function - const projectsResponse = await projectServices.getProjects(filter); // Pass the mocked projectRepo - // Returned project should be the one based on the agency and status id in the filter - expect(projectsResponse.totalCount).toEqual(1); - expect(projectsResponse.data.length).toEqual(1); + describe('handleProjectNotifications', () => { + it('should not send notifications when status becomes Cancelled', async () => { + const project = produceProject({ + AgencyResponses: [produceAgencyResponse()], + StatusId: ProjectStatus.CANCELLED, + CancelledOn: new Date(), }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const queryRunner: any = { + manager: { + findOne: async () => project, + find: () => [produceNotificationQueue({ Status: NotificationStatus.Pending })], + }, + }; + await projectServices.handleProjectNotifications( + project.Id, + ProjectStatus.ON_HOLD, + [produceAgencyResponse()], + producePimsRequestUser(), + queryRunner, + ); + expect(_generateProjectWatchNotifications).not.toHaveBeenCalled(); + expect(_generateProjectNotifications).not.toHaveBeenCalled(); }); + }); - describe('getProjectsForExport', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - it('should return projects based on filter conditions', async () => { - const filter = { - statusId: 1, - agencyId: [3], - quantity: 10, - page: 0, - }; - - _projectFind.mockImplementationOnce(async () => { - const mockProjects: Project[] = [ - produceProject({ Id: 1, Name: 'Project 1', StatusId: 1, AgencyId: 3 }), - produceProject({ Id: 2, Name: 'Project 2', StatusId: 4, AgencyId: 14 }), - ]; - // Check if the project matches the filter conditions - return mockProjects.filter( - (project) => - filter.statusId === project.StatusId && filter.agencyId.includes(project.AgencyId), - ); - }); - - // Call the service function - const projects = await projectServices.getProjectsForExport(filter); // Pass the mocked projectRepo - - // Assertions - expect(_projectFind).toHaveBeenCalled(); - // Returned project should be the one based on the agency and status id in the filter - expect(projects.length).toEqual(1); + describe('getProjects', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('should return projects based on filter conditions', async () => { + const filter = { + statusId: 1, + agencyId: [3], + quantity: 10, + page: 1, + market: '$12', + netBook: '$12', + agency: 'contains,aaa', + status: 'contains,aaa', + projectNumber: 'contains,aaa', + name: 'contains,Project', + updatedOn: 'before,' + new Date(), + updatedBy: 'Jane', + sortOrder: 'asc', + sortKey: 'Status', + quickFilter: 'hi', + }; + + // Call the service function + const projectsResponse = await projectServices.getProjects(filter); // Pass the mocked projectRepo + // Returned project should be the one based on the agency and status id in the filter + expect(projectsResponse.totalCount).toEqual(1); + expect(projectsResponse.data.length).toEqual(1); + }); + }); + + describe('getProjectsForExport', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('should return projects based on filter conditions', async () => { + const filter = { + statusId: 1, + agencyId: [3], + quantity: 10, + page: 0, + }; + + _projectFind.mockImplementationOnce(async () => { + const mockProjects: Project[] = [ + produceProject({ Id: 1, Name: 'Project 1', StatusId: 1, AgencyId: 3 }), + produceProject({ Id: 2, Name: 'Project 2', StatusId: 4, AgencyId: 14 }), + ]; + // Check if the project matches the filter conditions + return mockProjects.filter( + (project) => + filter.statusId === project.StatusId && filter.agencyId.includes(project.AgencyId), + ); }); + + // Call the service function + const projects = await projectServices.getProjectsForExport(filter); // Pass the mocked projectRepo + + // Assertions + expect(_projectFind).toHaveBeenCalled(); + // Returned project should be the one based on the agency and status id in the filter + expect(projects.length).toEqual(1); }); }); }); From f5834047f9b401e60d15dfeaeac7c8ddd1d3bb5c Mon Sep 17 00:00:00 2001 From: dbarkowsky Date: Thu, 17 Oct 2024 17:19:29 -0700 Subject: [PATCH 4/4] send cancelled message still --- express-api/src/constants/switches.ts | 2 +- .../src/services/projects/projectsServices.ts | 98 +++++++++++-------- .../projects/projectsServices.test.ts | 7 +- 3 files changed, 61 insertions(+), 46 deletions(-) diff --git a/express-api/src/constants/switches.ts b/express-api/src/constants/switches.ts index 9c5e42160..b5de974c7 100644 --- a/express-api/src/constants/switches.ts +++ b/express-api/src/constants/switches.ts @@ -1,5 +1,5 @@ const { NODE_ENV } = process.env; export default { - TESTING: NODE_ENV === 'test', // Can be used to disable certain features when testing + TESTING: NODE_ENV === 'test' || process.env.TESTING === 'true', // Can be used to disable certain features when testing }; diff --git a/express-api/src/services/projects/projectsServices.ts b/express-api/src/services/projects/projectsServices.ts index 0b8977911..8952e287b 100644 --- a/express-api/src/services/projects/projectsServices.ts +++ b/express-api/src/services/projects/projectsServices.ts @@ -417,28 +417,6 @@ const handleProjectNotifications = async ( }, }); - // If the project is cancelled, cancel pending notifications - if (projectWithRelations.StatusId === ProjectStatus.CANCELLED) { - const pendingNotifications = await queryRunner.manager.find(NotificationQueue, { - where: [ - { - ProjectId: projectWithRelations.Id, - Status: NotificationStatus.Accepted, - }, - { - ProjectId: projectWithRelations.Id, - Status: NotificationStatus.Pending, - }, - ], - }); - await Promise.allSettled( - pendingNotifications.map(async (notification) => { - await notificationServices.cancelNotificationById(notification.Id, user); - }), - ); - return []; - } - const projectAgency = await queryRunner.manager.findOne(Agency, { where: { Id: projectWithRelations.AgencyId }, }); @@ -453,33 +431,65 @@ const handleProjectNotifications = async ( projectWithRelations.Notes = projectNotes; const notifsToSend: Array = []; + const queueNotifications = async () => { + // If the status has been changed + if (previousStatus !== projectWithRelations.StatusId) { + // Has the project previously been to this status? If so, don't re-queue notifications. + const previousStatuses = await queryRunner.manager.find(ProjectStatusHistory, { + where: { ProjectId: projectWithRelations.Id }, + }); + const statusPreviouslyVisited = previousStatuses.some( + (record: ProjectStatusHistory) => record.StatusId === projectWithRelations.StatusId, + ); + if (!statusPreviouslyVisited) { + const statusChangeNotifs = await notificationServices.generateProjectNotifications( + projectWithRelations, + previousStatus, + queryRunner, + ); + return statusChangeNotifs; + } + } + return []; + }; - // If the status has been changed - if (previousStatus !== projectWithRelations.StatusId) { - // Has the project previously been to this status? If so, don't re-queue notifications. - const previousStatuses = await queryRunner.manager.find(ProjectStatusHistory, { - where: { ProjectId: projectWithRelations.Id }, - }); - const statusPreviouslyVisited = previousStatuses.some( - (record: ProjectStatusHistory) => record.StatusId === projectWithRelations.StatusId, - ); - if (!statusPreviouslyVisited) { - const statusChangeNotifs = await notificationServices.generateProjectNotifications( + const queueWatchNotifications = async () => { + if (projectAgencyResponses.length) { + const agencyResponseNotifs = await notificationServices.generateProjectWatchNotifications( projectWithRelations, - previousStatus, + responses, queryRunner, ); - notifsToSend.push(...statusChangeNotifs); + return agencyResponseNotifs; } - } + return []; + }; - if (projectAgencyResponses.length) { - const agencyResponseNotifs = await notificationServices.generateProjectWatchNotifications( - projectWithRelations, - responses, - queryRunner, + // If the project is cancelled, cancel pending notifications + if (projectWithRelations.StatusId === ProjectStatus.CANCELLED) { + const pendingNotifications = await queryRunner.manager.find(NotificationQueue, { + where: [ + { + ProjectId: projectWithRelations.Id, + Status: NotificationStatus.Accepted, + }, + { + ProjectId: projectWithRelations.Id, + Status: NotificationStatus.Pending, + }, + ], + }); + + await Promise.all( + pendingNotifications.map((notification) => { + notificationServices.cancelNotificationById(notification.Id, user); + }), ); - notifsToSend.push(...agencyResponseNotifs); + // Queue cancellation notifications + notifsToSend.push(...(await queueNotifications())); + } else { + notifsToSend.push(...(await queueNotifications())); + notifsToSend.push(...(await queueWatchNotifications())); } return Promise.all( @@ -817,8 +827,10 @@ const updateProject = async ( const returnProject = await projectRepo.findOne({ where: { Id: originalProject.Id } }); return returnProject; } catch (e) { - await queryRunner.rollbackTransaction(); logger.warn(e.message); + if (queryRunner.isTransactionActive) { + await queryRunner.rollbackTransaction(); + } if (e instanceof ErrorWithCode) throw e; throw new ErrorWithCode(`Error updating project: ${e.message}`, 500); } finally { diff --git a/express-api/tests/unit/services/projects/projectsServices.test.ts b/express-api/tests/unit/services/projects/projectsServices.test.ts index c07d85174..e7dbb303f 100644 --- a/express-api/tests/unit/services/projects/projectsServices.test.ts +++ b/express-api/tests/unit/services/projects/projectsServices.test.ts @@ -246,11 +246,13 @@ jest const _generateProjectWatchNotifications = jest.fn(async () => [produceNotificationQueue()]); const _generateProjectNotifications = jest.fn(async () => [produceNotificationQueue()]); +const _cancelNotificationById = jest.fn(async () => produceNotificationQueue()); jest.mock('@/services/notifications/notificationServices', () => ({ generateProjectNotifications: async () => _generateProjectNotifications(), sendNotification: jest.fn(async () => produceNotificationQueue()), generateProjectWatchNotifications: async () => _generateProjectWatchNotifications(), NotificationStatus: { Accepted: 0, Pending: 1, Cancelled: 2, Failed: 3, Completed: 4 }, + cancelNotificationById: async () => _cancelNotificationById, })); describe('UNIT - Project Services', () => { @@ -634,7 +636,7 @@ describe('UNIT - Project Services', () => { find: () => [produceNotificationQueue({ Status: NotificationStatus.Pending })], }, }; - await projectServices.handleProjectNotifications( + const result = await projectServices.handleProjectNotifications( project.Id, ProjectStatus.ON_HOLD, [produceAgencyResponse()], @@ -642,7 +644,8 @@ describe('UNIT - Project Services', () => { queryRunner, ); expect(_generateProjectWatchNotifications).not.toHaveBeenCalled(); - expect(_generateProjectNotifications).not.toHaveBeenCalled(); + expect(_generateProjectNotifications).toHaveBeenCalled(); + expect(result).toHaveLength(1); }); });