From 1b06b5b043a0d06a326a8e4a443529a3f19d746f Mon Sep 17 00:00:00 2001 From: Graham Stewart Date: Tue, 6 Aug 2024 16:54:52 -0700 Subject: [PATCH 1/9] Added some logic largely copied from the old PIMS API to send notifications for agencies that have newly been added as subscribed to the project --- .../projects/projectsController.ts | 2 +- .../notifications/notificationServices.ts | 84 +++++++++++++++++-- .../src/services/projects/projectsServices.ts | 74 +++++++++++++--- express-api/src/utilities/helperFunctions.ts | 4 + .../projects/projectsController.test.ts | 2 +- .../notificationServices.test.ts | 4 +- 6 files changed, 149 insertions(+), 21 deletions(-) diff --git a/express-api/src/controllers/projects/projectsController.ts b/express-api/src/controllers/projects/projectsController.ts index 54c3825a8..91bc06efe 100644 --- a/express-api/src/controllers/projects/projectsController.ts +++ b/express-api/src/controllers/projects/projectsController.ts @@ -119,7 +119,7 @@ export const deleteDisposalProject = async (req: Request, res: Response) => { projectId, req.user.preferred_username, ); - const notifications = await notificationServices.cancelAllProjectNotifications(projectId); + const notifications = await notificationServices.cancelProjectNotifications(projectId); return res.status(200).send({ project: delProject, notifications }); }; diff --git a/express-api/src/services/notifications/notificationServices.ts b/express-api/src/services/notifications/notificationServices.ts index 251f7a2c3..42bf17a33 100644 --- a/express-api/src/services/notifications/notificationServices.ts +++ b/express-api/src/services/notifications/notificationServices.ts @@ -7,7 +7,7 @@ import { ProjectStatusNotification } from '@/typeorm/Entities/ProjectStatusNotif import { User } from '@/typeorm/Entities/User'; import { UUID, randomUUID } from 'crypto'; import nunjucks from 'nunjucks'; -import { In, IsNull, QueryRunner } from 'typeorm'; +import { In, IsNull, MoreThan, QueryRunner } from 'typeorm'; import chesServices, { EmailBody, EmailEncoding, @@ -19,6 +19,7 @@ import { SSOUser } from '@bcgov/citz-imb-sso-express'; import { ProjectAgencyResponse } from '@/typeorm/Entities/ProjectAgencyResponse'; import logger from '@/utilities/winstonLogger'; import getConfig from '@/constants/config'; +import { getDaysBetween } from '@/utilities/helperFunctions'; export interface AccessRequestData { FirstName: string; @@ -522,11 +523,11 @@ const getProjectNotificationsInQueue = async ( return pageModel; }; -const cancelAllProjectNotifications = async (projectId: number) => { +const cancelProjectNotifications = async (projectId: number, agencyId?: number) => { const notifications = await AppDataSource.getRepository(NotificationQueue).find({ where: [ - { ProjectId: projectId, Status: NotificationStatus.Accepted }, - { ProjectId: projectId, Status: NotificationStatus.Pending }, + { ProjectId: projectId, Status: NotificationStatus.Accepted, ToAgencyId: agencyId }, + { ProjectId: projectId, Status: NotificationStatus.Pending, ToAgencyId: agencyId }, ], }); const chesCancelPromises = notifications.map((notification) => { @@ -550,14 +551,87 @@ const cancelAllProjectNotifications = async (projectId: number) => { }; }; +const generateProjectWatchNotifications = async ( + project: Project, + responses: ProjectAgencyResponse[], + queryRunner?: QueryRunner, +) => { + const query = queryRunner ?? AppDataSource.createQueryRunner(); + const notificationsInserted: Array = []; + try { + for (const response of responses) { + switch (response.Response) { + case AgencyResponseType.Unsubscribe: + case AgencyResponseType.Watch: + await cancelProjectNotifications(response.ProjectId, response.AgencyId); + break; + case AgencyResponseType.Subscribe: { + const daysSinceCreated = getDaysBetween(project.CreatedOn, new Date()); + const statusNotifs = await AppDataSource.getRepository(ProjectStatusNotification).find({ + relations: { + Template: true, + }, + where: { + ToStatusId: project.StatusId, //Confirm this is correct. + Template: { + Audience: NotificationAudience.WatchingAgencies, + }, + DelayDays: MoreThan(daysSinceCreated), + }, + }); + for (const statusNotif of statusNotifs) { + const notifExists = await AppDataSource.getRepository(NotificationQueue).exists({ + where: [ + { + ProjectId: response.ProjectId, + ToAgencyId: response.AgencyId, + Status: NotificationStatus.Accepted, + }, + { + ProjectId: response.ProjectId, + ToAgencyId: response.AgencyId, + Status: NotificationStatus.Pending, + }, + ], + }); + if (!notifExists) { + const agency = await AppDataSource.getRepository(Agency).findOne({ + where: { Id: response.AgencyId }, + }); + const inserted = await insertProjectNotificationQueue( + statusNotif.Template, + statusNotif, + project, + agency, + ); + notificationsInserted.push(inserted); + } + } + break; + } + } + } + } catch (e) { + logger.error( + `Error: Some notification actions triggered by an agency response may have failed to cancel or update. Project ID: ${project.Id}, Error msg: ${e.message}`, + ); + } finally { + if (queryRunner === undefined) { + query.release(); + } + } + return notificationsInserted; +}; + const notificationServices = { generateProjectNotifications, generateAccessRequestNotification, + generateProjectWatchNotifications, sendNotification, updateNotificationStatus, getProjectNotificationsInQueue, convertChesStatusToNotificationStatus, - cancelAllProjectNotifications, + cancelProjectNotifications, }; export default notificationServices; diff --git a/express-api/src/services/projects/projectsServices.ts b/express-api/src/services/projects/projectsServices.ts index 5121b02ed..8c823233a 100644 --- a/express-api/src/services/projects/projectsServices.ts +++ b/express-api/src/services/projects/projectsServices.ts @@ -176,7 +176,13 @@ const addProject = async ( await handleProjectMonetary(newProject, queryRunner); await handleProjectNotes(newProject, queryRunner); await handleProjectTimestamps(newProject, queryRunner); - await handleProjectNotifications(newProject, ssoUser, queryRunner); + await handleProjectNotifications( + newProject.Id, + null, + newProject.AgencyResponses ?? [], + ssoUser, + queryRunner, + ); await queryRunner.commitTransaction(); return newProject; } catch (e) { @@ -386,7 +392,9 @@ const removeProjectBuildingRelations = async ( * @returns A promise that resolves when all notifications are sent. */ const handleProjectNotifications = async ( - oldProject: DeepPartial, + projectId: number, + previousStatus: number, + responses: ProjectAgencyResponse[], user: SSOUser, queryRunner: QueryRunner, ) => { @@ -407,7 +415,7 @@ const handleProjectNotifications = async ( }, }, where: { - Id: oldProject.Id, + Id: projectId, }, }); const projectAgency = await queryRunner.manager.findOne(Agency, { @@ -422,13 +430,29 @@ const handleProjectNotifications = async ( projectWithRelations.Agency = projectAgency; projectWithRelations.AgencyResponses = projectAgencyResponses; projectWithRelations.Notes = projectNotes; - const notifs = await notificationServices.generateProjectNotifications( - projectWithRelations, - oldProject.StatusId, - queryRunner, - ); + + const notifsToSend: Array = []; + + if (previousStatus !== projectWithRelations.StatusId) { + const statusChangeNotifs = await notificationServices.generateProjectNotifications( + projectWithRelations, + previousStatus, + queryRunner, + ); + notifsToSend.push(...statusChangeNotifs); + } + + if (projectAgencyResponses.length) { + const agencyResponseNotifs = await notificationServices.generateProjectWatchNotifications( + projectWithRelations, + responses, + queryRunner, + ); + notifsToSend.push(...agencyResponseNotifs); + } + return Promise.all( - notifs.map((notif) => notificationServices.sendNotification(notif, user, queryRunner)), + notifsToSend.map((notif) => notificationServices.sendNotification(notif, user, queryRunner)), ); }; @@ -601,6 +625,24 @@ const handleProjectMonetary = async ( } }; +const getAgencyResponseChanges = async ( + oldProject: Project, + newProject: DeepPartial, +): Promise> => { + const retResponses: Array = []; + for (const response of newProject.AgencyResponses as ProjectAgencyResponse[]) { + const originalResponse = oldProject.AgencyResponses.find( + (r) => r.ProjectId === response.ProjectId && r.AgencyId === response.AgencyId, + ); + if (originalResponse == null || originalResponse.Response != response.Response) { + retResponses.push(response); + } + } + // There is additional logic here for managing "removed" responses, but we don't expect agency responses to be ignored anymore. + // Instead, we guide users to just mark them as Unsubscribed, in which case the above logic should be sufficient. + return retResponses; +}; + /** * Updates a project with the given changes and property IDs. * @@ -713,10 +755,18 @@ const updateProject = async ( } queryRunner.commitTransaction(); - - if (project.StatusId !== undefined && originalProject.StatusId !== project.StatusId) { - await handleProjectNotifications(originalProject, user, queryRunner); //Do this after committing transaction so that we don't send emails to CHES unless the rest of the project metadata actually saved. + const changedResponses = []; + if (project.AgencyResponses) { + changedResponses.push(...(await getAgencyResponseChanges(originalProject, project))); } + await handleProjectNotifications( + project.Id, + originalProject.StatusId, + changedResponses, + user, + queryRunner, + ); //Do this after committing transaction so that we don't send emails to CHES unless the rest of the project metadata actually saved. + // Get project to return const returnProject = await projectRepo.findOne({ where: { Id: originalProject.Id } }); return returnProject; diff --git a/express-api/src/utilities/helperFunctions.ts b/express-api/src/utilities/helperFunctions.ts index 770762c96..012a962d2 100644 --- a/express-api/src/utilities/helperFunctions.ts +++ b/express-api/src/utilities/helperFunctions.ts @@ -202,3 +202,7 @@ export const toPostgresTimestamp = (date: Date) => { const seconds = pad(date.getUTCSeconds()); return `${year}-${month}-${day} ${hours}:${minutes}:${seconds}`; }; + +export const getDaysBetween = (earlierDate: Date, laterDate: Date): number => { + return Math.trunc((laterDate.getTime() - earlierDate.getTime()) / (1000 * 60 * 60 * 24)); +}; diff --git a/express-api/tests/unit/controllers/projects/projectsController.test.ts b/express-api/tests/unit/controllers/projects/projectsController.test.ts index 29d6ee313..04b2d19cf 100644 --- a/express-api/tests/unit/controllers/projects/projectsController.test.ts +++ b/express-api/tests/unit/controllers/projects/projectsController.test.ts @@ -62,7 +62,7 @@ jest.mock('@/services/users/usersServices', () => ({ })); jest.mock('@/services/notifications/notificationServices', () => ({ - cancelAllProjectNotifications: () => ({ + cancelProjectNotifications: () => ({ succeeded: faker.number.int(), failed: faker.number.int(), }), diff --git a/express-api/tests/unit/services/notifications/notificationServices.test.ts b/express-api/tests/unit/services/notifications/notificationServices.test.ts index 40ff574f6..14c1d9c57 100644 --- a/express-api/tests/unit/services/notifications/notificationServices.test.ts +++ b/express-api/tests/unit/services/notifications/notificationServices.test.ts @@ -324,9 +324,9 @@ describe('getProjectNotificationsInQueue', () => { } }); }); -describe('cancelAllProjectNotifications', () => { +describe('cancelProjectNotifications', () => { it('should return a count of successful and failed cancellations', async () => { - const result = await notificationServices.cancelAllProjectNotifications(faker.number.int()); + const result = await notificationServices.cancelProjectNotifications(faker.number.int()); expect(isNaN(result.succeeded)).toBe(false); expect(isNaN(result.failed)).toBe(false); }); From e6aefd131402acbf5fbb264cf0c203f2c1f9eaff Mon Sep 17 00:00:00 2001 From: Graham Stewart Date: Wed, 7 Aug 2024 11:39:35 -0700 Subject: [PATCH 2/9] Fixed some bugs in how original responses was being compared to new responses. Fixed some issues with determening whether an existing notification was queued that was preventing new notifications from being queued. --- .../notifications/notificationServices.ts | 149 ++++++++++-------- .../src/services/projects/projectsServices.ts | 18 ++- 2 files changed, 101 insertions(+), 66 deletions(-) diff --git a/express-api/src/services/notifications/notificationServices.ts b/express-api/src/services/notifications/notificationServices.ts index 42bf17a33..8006fe5b0 100644 --- a/express-api/src/services/notifications/notificationServices.ts +++ b/express-api/src/services/notifications/notificationServices.ts @@ -182,11 +182,11 @@ const insertProjectNotificationQueue = async ( ProjectId: project.Id, ToAgencyId: agency?.Id, }; + const insertedNotif = await query.manager.save(NotificationQueue, queueObject); if (queryRunner === undefined) { //If no arg passed we spawned a new query runner and we must release that! await query.release(); } - const insertedNotif = await query.manager.save(NotificationQueue, queueObject); return insertedNotif; }; @@ -523,32 +523,50 @@ const getProjectNotificationsInQueue = async ( return pageModel; }; -const cancelProjectNotifications = async (projectId: number, agencyId?: number) => { - const notifications = await AppDataSource.getRepository(NotificationQueue).find({ - where: [ - { ProjectId: projectId, Status: NotificationStatus.Accepted, ToAgencyId: agencyId }, - { ProjectId: projectId, Status: NotificationStatus.Pending, ToAgencyId: agencyId }, - ], - }); - const chesCancelPromises = notifications.map((notification) => { - return chesServices.cancelEmailByIdAsync(notification.ChesMessageId); - }); - const chesCancelResolved = await Promise.allSettled(chesCancelPromises); - const cancelledMessageIds = chesCancelResolved - .filter((a) => a.status === 'fulfilled' && a.value.status === 'cancelled') - .map((c) => (c as PromiseFulfilledResult).value.msgId); - await AppDataSource.getRepository(NotificationQueue).update( - { ChesMessageId: In(cancelledMessageIds) }, - { Status: convertChesStatusToNotificationStatus('cancelled') }, - ); - return { - succeeded: chesCancelResolved.filter( - (c) => c.status === 'fulfilled' && c.value.status === 'cancelled', - ).length, - failed: chesCancelResolved.filter( - (c) => c.status === 'rejected' || c.value?.status !== 'cancelled', - ).length, - }; +const cancelProjectNotifications = async ( + projectId: number, + agencyId?: number, + queryRunner?: QueryRunner, +) => { + const query = queryRunner ?? AppDataSource.createQueryRunner(); + try { + const notifications = await query.manager.find(NotificationQueue, { + where: [ + { ProjectId: projectId, Status: NotificationStatus.Accepted, ToAgencyId: agencyId }, + { ProjectId: projectId, Status: NotificationStatus.Pending, ToAgencyId: agencyId }, + ], + }); + const chesCancelPromises = notifications.map((notification) => { + return chesServices.cancelEmailByIdAsync(notification.ChesMessageId); + }); + const chesCancelResolved = await Promise.allSettled(chesCancelPromises); + const cancelledMessageIds = chesCancelResolved + .filter((a) => a.status === 'fulfilled' && a.value.status === 'cancelled') + .map((c) => (c as PromiseFulfilledResult).value.msgId); + await query.manager.update( + NotificationQueue, + { ChesMessageId: In(cancelledMessageIds) }, + { Status: convertChesStatusToNotificationStatus('cancelled') }, + ); + return { + succeeded: chesCancelResolved.filter( + (c) => c.status === 'fulfilled' && c.value.status === 'cancelled', + ).length, + failed: chesCancelResolved.filter( + (c) => c.status === 'rejected' || c.value?.status !== 'cancelled', + ).length, + }; + } catch (e) { + logger.error(`Error: Something went wrong when trying to cancel project notifications.`); + return { + succeeded: 0, + failed: 0, + }; + } finally { + if (queryRunner === undefined) { + query.release(); + } + } }; const generateProjectWatchNotifications = async ( @@ -566,45 +584,51 @@ const generateProjectWatchNotifications = async ( await cancelProjectNotifications(response.ProjectId, response.AgencyId); break; case AgencyResponseType.Subscribe: { - const daysSinceCreated = getDaysBetween(project.CreatedOn, new Date()); - const statusNotifs = await AppDataSource.getRepository(ProjectStatusNotification).find({ - relations: { - Template: true, - }, - where: { - ToStatusId: project.StatusId, //Confirm this is correct. - Template: { - Audience: NotificationAudience.WatchingAgencies, - }, - DelayDays: MoreThan(daysSinceCreated), - }, + const agency = await queryRunner.manager.findOne(Agency, { + where: { Id: response.AgencyId }, }); - for (const statusNotif of statusNotifs) { - const notifExists = await AppDataSource.getRepository(NotificationQueue).exists({ - where: [ - { - ProjectId: response.ProjectId, - ToAgencyId: response.AgencyId, - Status: NotificationStatus.Accepted, - }, - { - ProjectId: response.ProjectId, - ToAgencyId: response.AgencyId, - Status: NotificationStatus.Pending, + if (agency?.Email) { + const daysSinceCreated = getDaysBetween(project.CreatedOn, new Date()); + const statusNotifs = await query.manager.find(ProjectStatusNotification, { + relations: { + Template: true, + }, + where: { + ToStatusId: project.StatusId, //Confirm this is correct. + Template: { + Audience: NotificationAudience.WatchingAgencies, }, - ], + DelayDays: MoreThan(daysSinceCreated), + }, }); - if (!notifExists) { - const agency = await AppDataSource.getRepository(Agency).findOne({ - where: { Id: response.AgencyId }, + for (const statusNotif of statusNotifs) { + const notifExists = await query.manager.exists(NotificationQueue, { + where: [ + { + ProjectId: project.Id, + ToAgencyId: response.AgencyId, + TemplateId: statusNotif.TemplateId, + Status: NotificationStatus.Accepted, + }, + { + ProjectId: project.Id, + ToAgencyId: response.AgencyId, + TemplateId: statusNotif.TemplateId, + Status: NotificationStatus.Pending, + }, + ], }); - const inserted = await insertProjectNotificationQueue( - statusNotif.Template, - statusNotif, - project, - agency, - ); - notificationsInserted.push(inserted); + if (!notifExists) { + const inserted = await insertProjectNotificationQueue( + statusNotif.Template, + statusNotif, + project, + agency, + undefined, + queryRunner, + ); + notificationsInserted.push(inserted); + } } } break; @@ -615,6 +639,7 @@ const generateProjectWatchNotifications = async ( logger.error( `Error: Some notification actions triggered by an agency response may have failed to cancel or update. Project ID: ${project.Id}, Error msg: ${e.message}`, ); + logger.error(e.stack); } finally { if (queryRunner === undefined) { query.release(); diff --git a/express-api/src/services/projects/projectsServices.ts b/express-api/src/services/projects/projectsServices.ts index 8c823233a..d00898327 100644 --- a/express-api/src/services/projects/projectsServices.ts +++ b/express-api/src/services/projects/projectsServices.ts @@ -26,7 +26,7 @@ import { import { ProjectFilter } from '@/services/projects/projectSchema'; import { PropertyType } from '@/constants/propertyType'; import { ProjectRisk } from '@/constants/projectRisk'; -import notificationServices from '../notifications/notificationServices'; +import notificationServices, { AgencyResponseType } from '../notifications/notificationServices'; import { SSOUser } from '@bcgov/citz-imb-sso-express'; import userServices from '../users/usersServices'; import { constructFindOptionFromQuery } from '@/utilities/helperFunctions'; @@ -632,14 +632,21 @@ const getAgencyResponseChanges = async ( const retResponses: Array = []; for (const response of newProject.AgencyResponses as ProjectAgencyResponse[]) { const originalResponse = oldProject.AgencyResponses.find( - (r) => r.ProjectId === response.ProjectId && r.AgencyId === response.AgencyId, + (r) => r.AgencyId === response.AgencyId, ); if (originalResponse == null || originalResponse.Response != response.Response) { retResponses.push(response); } } - // There is additional logic here for managing "removed" responses, but we don't expect agency responses to be ignored anymore. - // Instead, we guide users to just mark them as Unsubscribed, in which case the above logic should be sufficient. + for (const response of oldProject.AgencyResponses) { + const updatedResponse = newProject.AgencyResponses.find( + (r) => r.AgencyId === response.AgencyId, + ); + if (updatedResponse == null) { + response.Response = AgencyResponseType.Unsubscribe; + retResponses.push(response); + } + } return retResponses; }; @@ -662,7 +669,10 @@ const updateProject = async ( if (project.Name === null || project.Name === '') { throw new ErrorWithCode('Projects must have a name.', 400); } + //We get the AgencyResponses relation here so that we have a copy of the state of those responses before being updated. + //We need this to check which responses were updated and thus require new notifications later after the transaction commit. const originalProject = await projectRepo.findOne({ + relations: { AgencyResponses: true }, where: { Id: project.Id }, }); if (!originalProject) { From 9ec5cf69c4e46d3b0dcbb48fd3c0bb708b0fe199 Mon Sep 17 00:00:00 2001 From: Graham Stewart Date: Wed, 7 Aug 2024 13:06:15 -0700 Subject: [PATCH 3/9] Updated unit tests and fixed incorrect queryRunner use --- .../notifications/notificationServices.ts | 3 ++- .../notificationServices.test.ts | 25 +++++++++++++++++++ .../projects/projectsServices.test.ts | 9 +++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/express-api/src/services/notifications/notificationServices.ts b/express-api/src/services/notifications/notificationServices.ts index 8006fe5b0..8a4f722bb 100644 --- a/express-api/src/services/notifications/notificationServices.ts +++ b/express-api/src/services/notifications/notificationServices.ts @@ -584,9 +584,10 @@ const generateProjectWatchNotifications = async ( await cancelProjectNotifications(response.ProjectId, response.AgencyId); break; case AgencyResponseType.Subscribe: { - const agency = await queryRunner.manager.findOne(Agency, { + const agency = await query.manager.findOne(Agency, { where: { Id: response.AgencyId }, }); + if (agency?.Email) { const daysSinceCreated = getDaysBetween(project.CreatedOn, new Date()); const statusNotifs = await query.manager.find(ProjectStatusNotification, { diff --git a/express-api/tests/unit/services/notifications/notificationServices.test.ts b/express-api/tests/unit/services/notifications/notificationServices.test.ts index 14c1d9c57..73b52a6dd 100644 --- a/express-api/tests/unit/services/notifications/notificationServices.test.ts +++ b/express-api/tests/unit/services/notifications/notificationServices.test.ts @@ -1,5 +1,6 @@ import { AppDataSource } from '@/appDataSource'; import notificationServices, { + AgencyResponseType, NotificationStatus, } from '@/services/notifications/notificationServices'; import { NotificationQueue } from '@/typeorm/Entities/NotificationQueue'; @@ -7,6 +8,7 @@ import { faker } from '@faker-js/faker'; import { randomUUID } from 'crypto'; import { produceAgency, + produceAgencyResponse, produceNotificationQueue, produceNotificationTemplate, produceProject, @@ -71,9 +73,12 @@ const _statusNotifFind = jest.fn().mockImplementation(async (options) => [ FromStatusId: (options.where as FindOptionsWhere) .FromStatusId as number, ToStatusId: (options.where as FindOptionsWhere).ToStatusId as number, + Template: produceNotificationTemplate(), }), ]); +const _agencyFindOne = jest.fn().mockImplementation(async () => produceAgency()); + jest.spyOn(AppDataSource, 'createQueryRunner').mockReturnValue({ ...jest.requireActual('@/appDataSource').createQueryRunner, release: () => {}, @@ -102,6 +107,8 @@ jest.spyOn(AppDataSource, 'createQueryRunner').mockReturnValue({ return _notifTemplateFindOne(options); } else if (entityClass === NotificationQueue) { return _notifQueueFindOne(options); + } else if (entityClass === Agency) { + return _agencyFindOne(); } else { return {}; } @@ -112,6 +119,12 @@ jest.spyOn(AppDataSource, 'createQueryRunner').mockReturnValue({ ) => { return _notifQueueSave(obj); }, + exists: () => { + return false; + }, + update: () => { + return { raw: {}, generatedMaps: [] }; + }, }, }); @@ -331,3 +344,15 @@ describe('cancelProjectNotifications', () => { expect(isNaN(result.failed)).toBe(false); }); }); +describe('generateProjectWatchNotifications', () => { + it('should generate notifications for list of responses', async () => { + const project = produceProject({ Id: 1 }); + const responses = [ + produceAgencyResponse({ Response: AgencyResponseType.Subscribe }), + produceAgencyResponse({ Response: AgencyResponseType.Unsubscribe }), + ]; + const result = await notificationServices.generateProjectWatchNotifications(project, responses); + expect(Array.isArray(result)).toBe(true); + expect(result.length).toBe(1); + }); +}); diff --git a/express-api/tests/unit/services/projects/projectsServices.test.ts b/express-api/tests/unit/services/projects/projectsServices.test.ts index 9fa95d85e..a988b0552 100644 --- a/express-api/tests/unit/services/projects/projectsServices.test.ts +++ b/express-api/tests/unit/services/projects/projectsServices.test.ts @@ -243,9 +243,11 @@ jest .spyOn(AppDataSource.getRepository(ProjectJoin), 'createQueryBuilder') .mockImplementation(() => projectJoinQueryBuilder); +const _generateProjectWatchNotifications = jest.fn(async () => [produceNotificationQueue()]); jest.mock('@/services/notifications/notificationServices', () => ({ generateProjectNotifications: jest.fn(async () => [produceNotificationQueue()]), sendNotification: jest.fn(async () => produceNotificationQueue()), + generateProjectWatchNotifications: async () => _generateProjectWatchNotifications(), NotificationStatus: { Accepted: 0, Pending: 1, Cancelled: 2, Failed: 3, Completed: 4 }, })); @@ -613,6 +615,13 @@ describe('UNIT - Project Services', () => { ).rejects.toThrow(new ErrorWithCode('Error updating project: bad save', 500)); }); + it('should send notifications when agency responses changed', async () => { + _projectFindOne.mockImplementationOnce(async () => originalProject); + const projUpd = { ...projectUpdate, AgencyResponses: [produceAgencyResponse()] }; + await projectServices.updateProject(projUpd, { parcels: [], buildings: [] }, produceSSO()); + expect(_generateProjectWatchNotifications).toHaveBeenCalled(); + }); + describe('getProjects', () => { beforeEach(() => { jest.clearAllMocks(); From f8c19c2ded0176b28d8b52dad65da36bdd838b2d Mon Sep 17 00:00:00 2001 From: Graham Stewart Date: Wed, 7 Aug 2024 14:50:03 -0700 Subject: [PATCH 4/9] insertProjectNotificationQueue will now let you override with a different send date. Modified the logic in generateWatchNotifications to use the most recent project update date to determine the send date --- .../notifications/notificationServices.ts | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/express-api/src/services/notifications/notificationServices.ts b/express-api/src/services/notifications/notificationServices.ts index 8a4f722bb..d834c5388 100644 --- a/express-api/src/services/notifications/notificationServices.ts +++ b/express-api/src/services/notifications/notificationServices.ts @@ -20,6 +20,7 @@ import { ProjectAgencyResponse } from '@/typeorm/Entities/ProjectAgencyResponse' import logger from '@/utilities/winstonLogger'; import getConfig from '@/constants/config'; import { getDaysBetween } from '@/utilities/helperFunctions'; +import { ProjectStatusHistory } from '@/typeorm/Entities/ProjectStatusHistory'; export interface AccessRequestData { FirstName: string; @@ -155,17 +156,22 @@ const insertProjectNotificationQueue = async ( project: Project, agency?: Agency, overrideTo?: string, + overrideSend?: Date, queryRunner?: QueryRunner, ) => { const query = queryRunner ?? AppDataSource.createQueryRunner(); - const sendDate = new Date(); - sendDate.setDate(sendDate.getDate() + projStatusNotif.DelayDays); + let emailSendDate = overrideSend; + if (emailSendDate == undefined) { + const sendDelayFromToday = new Date(); + sendDelayFromToday.setDate(sendDelayFromToday.getDate() + projStatusNotif.DelayDays); + emailSendDate = sendDelayFromToday; + } const queueObject = { Key: randomUUID(), Status: NotificationStatus.Pending, Priority: template.Priority, Encoding: template.Encoding, - SendOn: sendDate, + SendOn: emailSendDate, Subject: nunjucks.renderString(template.Subject, { Project: project }), BodyType: template.BodyType, Body: nunjucks.renderString(template.Body, { @@ -229,6 +235,7 @@ const generateProjectNotifications = async ( project, project.Agency, overrideTo, + undefined, queryRunner, ), ); @@ -240,6 +247,7 @@ const generateProjectNotifications = async ( project, project.Agency, undefined, + undefined, queryRunner, ), ); @@ -272,6 +280,7 @@ const generateProjectNotifications = async ( project, agc, undefined, + undefined, queryRunner, ), ), @@ -306,6 +315,7 @@ const generateProjectNotifications = async ( project, agc, undefined, + undefined, queryRunner, ), ), @@ -335,6 +345,7 @@ const generateProjectNotifications = async ( project, agc, undefined, + undefined, queryRunner, ), ), @@ -347,6 +358,7 @@ const generateProjectNotifications = async ( project, undefined, undefined, + undefined, queryRunner, ), ); @@ -589,7 +601,13 @@ const generateProjectWatchNotifications = async ( }); if (agency?.Email) { - const daysSinceCreated = getDaysBetween(project.CreatedOn, new Date()); + const mostRecentStatusChange = await query.manager.findOne(ProjectStatusHistory, { + where: { ProjectId: project.Id }, + order: { CreatedOn: 'DESC' }, + }); + const mostRecentStatusChangeDate = + mostRecentStatusChange?.CreatedOn ?? project.CreatedOn; + const daysSinceThisStatus = getDaysBetween(mostRecentStatusChangeDate, new Date()); const statusNotifs = await query.manager.find(ProjectStatusNotification, { relations: { Template: true, @@ -599,7 +617,7 @@ const generateProjectWatchNotifications = async ( Template: { Audience: NotificationAudience.WatchingAgencies, }, - DelayDays: MoreThan(daysSinceCreated), + DelayDays: MoreThan(daysSinceThisStatus), }, }); for (const statusNotif of statusNotifs) { @@ -619,13 +637,17 @@ const generateProjectWatchNotifications = async ( }, ], }); + if (!notifExists) { + const sendOn = new Date(project.UpdatedOn.getTime()); + sendOn.setDate(project.UpdatedOn.getDate() + statusNotif.DelayDays); const inserted = await insertProjectNotificationQueue( statusNotif.Template, statusNotif, project, agency, undefined, + sendOn, queryRunner, ); notificationsInserted.push(inserted); From 8905fb25e690a3a0297a899f94f2e573644155ec Mon Sep 17 00:00:00 2001 From: Graham Stewart Date: Wed, 7 Aug 2024 15:41:11 -0700 Subject: [PATCH 5/9] Was using the wrong date object for overriding the sendOn value of delayed watch notifications. Also added some comments. --- .../notifications/notificationServices.ts | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/express-api/src/services/notifications/notificationServices.ts b/express-api/src/services/notifications/notificationServices.ts index d834c5388..e88609ea3 100644 --- a/express-api/src/services/notifications/notificationServices.ts +++ b/express-api/src/services/notifications/notificationServices.ts @@ -581,6 +581,16 @@ const cancelProjectNotifications = async ( } }; +/** + * Generates project notifications based off agency responses. + * Agencies that opt out of notifications will have any pending notifications cancelled here. + * Agencies that were not previously registered for notifications will have any delayed notifications, + * such as 30,60,90 ERP reminders, queued as if they had been registered for these from the start. + * @param project Up to date project entity. + * @param responses Agency responses to process based off response type. + * @param queryRunner Optional queryRunner, include if inside transaction. + * @returns {NotificationQueue[]} A list of notifications queued by this function call + */ const generateProjectWatchNotifications = async ( project: Project, responses: ProjectAgencyResponse[], @@ -593,18 +603,23 @@ const generateProjectWatchNotifications = async ( switch (response.Response) { case AgencyResponseType.Unsubscribe: case AgencyResponseType.Watch: + //The simple case. Calling this will cancel all pending notifications for this project/agency pair. await cancelProjectNotifications(response.ProjectId, response.AgencyId); break; case AgencyResponseType.Subscribe: { const agency = await query.manager.findOne(Agency, { where: { Id: response.AgencyId }, }); - + //No use in queueing an email for an agency with no email address. if (agency?.Email) { + /*We get the most recent entry in the status history since the date value of this row would have been populated + at the time this project was placed into its current status value. Note that this value is not necessarily the same as + Project.UpdatedOn, as projects can be updated without the status being changed. */ const mostRecentStatusChange = await query.manager.findOne(ProjectStatusHistory, { where: { ProjectId: project.Id }, order: { CreatedOn: 'DESC' }, }); + const mostRecentStatusChangeDate = mostRecentStatusChange?.CreatedOn ?? project.CreatedOn; const daysSinceThisStatus = getDaysBetween(mostRecentStatusChangeDate, new Date()); @@ -613,7 +628,7 @@ const generateProjectWatchNotifications = async ( Template: true, }, where: { - ToStatusId: project.StatusId, //Confirm this is correct. + ToStatusId: project.StatusId, //We will only send notifications relevant to the current status. Template: { Audience: NotificationAudience.WatchingAgencies, }, @@ -621,6 +636,7 @@ const generateProjectWatchNotifications = async ( }, }); for (const statusNotif of statusNotifs) { + //If there is already a pending notification for this agency with this template, skip. const notifExists = await query.manager.exists(NotificationQueue, { where: [ { @@ -639,8 +655,11 @@ const generateProjectWatchNotifications = async ( }); if (!notifExists) { - const sendOn = new Date(project.UpdatedOn.getTime()); - sendOn.setDate(project.UpdatedOn.getDate() + statusNotif.DelayDays); + //If there is no notification like this already pending, we send one. + const sendOn = new Date(mostRecentStatusChangeDate.getTime()); + sendOn.setDate(mostRecentStatusChangeDate.getDate() + statusNotif.DelayDays); + //We set the delay by the most recent status change date plus the number of delay days. This should make these new emails + //send around the same time as the emails that previously sent when this project changed into its current status. const inserted = await insertProjectNotificationQueue( statusNotif.Template, statusNotif, From dd9f61a51c8a8bf57aff7cd63ede77a230ad3c68 Mon Sep 17 00:00:00 2001 From: Graham Stewart Date: Wed, 7 Aug 2024 15:43:45 -0700 Subject: [PATCH 6/9] Included a notification refresh call in the post submit function of the ProjectAgencyResponseDialog --- react-app/src/components/projects/ProjectDetail.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/react-app/src/components/projects/ProjectDetail.tsx b/react-app/src/components/projects/ProjectDetail.tsx index c000971b2..f4b51af99 100644 --- a/react-app/src/components/projects/ProjectDetail.tsx +++ b/react-app/src/components/projects/ProjectDetail.tsx @@ -516,6 +516,7 @@ const ProjectDetail = (props: IProjectDetail) => { postSubmit={() => { setOpenAgencyInterestDialog(false); refreshData(); + refreshNotifications(); }} onCancel={() => { setOpenAgencyInterestDialog(false); From c63e3518797096eabb87aa34448b3b1d588215fb Mon Sep 17 00:00:00 2001 From: Graham Stewart Date: Wed, 7 Aug 2024 16:03:58 -0700 Subject: [PATCH 7/9] Fixed unit tests --- express-api/tests/testUtils/factories.ts | 2 +- .../unit/services/notifications/notificationServices.test.ts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/express-api/tests/testUtils/factories.ts b/express-api/tests/testUtils/factories.ts index 17d2b5e12..8a59c4ce5 100644 --- a/express-api/tests/testUtils/factories.ts +++ b/express-api/tests/testUtils/factories.ts @@ -802,7 +802,7 @@ export const produceProjectProperty = (props?: Partial): Projec return projectProperty; }; -export const productProjectStatusHistory = (props?: Partial) => { +export const produceProjectStatusHistory = (props?: Partial) => { const history: ProjectStatusHistory = { Id: faker.number.int(), CreatedById: randomUUID(), diff --git a/express-api/tests/unit/services/notifications/notificationServices.test.ts b/express-api/tests/unit/services/notifications/notificationServices.test.ts index 73b52a6dd..6b67ddc3e 100644 --- a/express-api/tests/unit/services/notifications/notificationServices.test.ts +++ b/express-api/tests/unit/services/notifications/notificationServices.test.ts @@ -13,6 +13,7 @@ import { produceNotificationTemplate, produceProject, produceProjectNotification, + produceProjectStatusHistory, produceSSO, produceUser, } from 'tests/testUtils/factories'; @@ -22,6 +23,7 @@ import { NotificationTemplate } from '@/typeorm/Entities/NotificationTemplate'; import { ProjectStatusNotification } from '@/typeorm/Entities/ProjectStatusNotification'; import { User } from '@/typeorm/Entities/User'; import { Agency } from '@/typeorm/Entities/Agency'; +import { ProjectStatusHistory } from '@/typeorm/Entities/ProjectStatusHistory'; const _notifQueueSave = jest .fn() @@ -109,6 +111,8 @@ jest.spyOn(AppDataSource, 'createQueryRunner').mockReturnValue({ return _notifQueueFindOne(options); } else if (entityClass === Agency) { return _agencyFindOne(); + } else if (entityClass === ProjectStatusHistory) { + return produceProjectStatusHistory({}); } else { return {}; } From 4ae34e9b0b4d76dbfdf8c30b908c1c4f43186294 Mon Sep 17 00:00:00 2001 From: Graham Stewart Date: Wed, 7 Aug 2024 16:49:24 -0700 Subject: [PATCH 8/9] fixed buggy unit test --- express-api/src/services/projects/projectsServices.ts | 1 + .../tests/unit/services/projects/projectsServices.test.ts | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/express-api/src/services/projects/projectsServices.ts b/express-api/src/services/projects/projectsServices.ts index d00898327..2000ee4c1 100644 --- a/express-api/src/services/projects/projectsServices.ts +++ b/express-api/src/services/projects/projectsServices.ts @@ -643,6 +643,7 @@ const getAgencyResponseChanges = async ( (r) => r.AgencyId === response.AgencyId, ); if (updatedResponse == null) { + console.log(oldProject); response.Response = AgencyResponseType.Unsubscribe; retResponses.push(response); } diff --git a/express-api/tests/unit/services/projects/projectsServices.test.ts b/express-api/tests/unit/services/projects/projectsServices.test.ts index a988b0552..349f7ade9 100644 --- a/express-api/tests/unit/services/projects/projectsServices.test.ts +++ b/express-api/tests/unit/services/projects/projectsServices.test.ts @@ -616,8 +616,9 @@ describe('UNIT - Project Services', () => { }); it('should send notifications when agency responses changed', async () => { - _projectFindOne.mockImplementationOnce(async () => originalProject); - const projUpd = { ...projectUpdate, AgencyResponses: [produceAgencyResponse()] }; + const oldProject = produceProject({}); + const projUpd = { ...oldProject, AgencyResponses: [produceAgencyResponse()] }; + _projectFindOne.mockImplementationOnce(async () => oldProject); await projectServices.updateProject(projUpd, { parcels: [], buildings: [] }, produceSSO()); expect(_generateProjectWatchNotifications).toHaveBeenCalled(); }); From 3e17b17548c5470bcedd4ff168d56fd00ab90bce Mon Sep 17 00:00:00 2001 From: Graham Stewart Date: Wed, 7 Aug 2024 16:49:46 -0700 Subject: [PATCH 9/9] removed console --- express-api/src/services/projects/projectsServices.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/express-api/src/services/projects/projectsServices.ts b/express-api/src/services/projects/projectsServices.ts index 2000ee4c1..d00898327 100644 --- a/express-api/src/services/projects/projectsServices.ts +++ b/express-api/src/services/projects/projectsServices.ts @@ -643,7 +643,6 @@ const getAgencyResponseChanges = async ( (r) => r.AgencyId === response.AgencyId, ); if (updatedResponse == null) { - console.log(oldProject); response.Response = AgencyResponseType.Unsubscribe; retResponses.push(response); }