Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PIMS-2254 Late Notification Warning #2884

Merged
merged 8 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions express-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"express-rate-limit": "7.4.0",
"morgan": "1.10.0",
"multer": "1.4.5-lts.1",
"node-cron": "3.0.3",
"node-sql-reader": "0.1.3",
"nunjucks": "3.2.4",
"pg": "8.13.0",
Expand All @@ -54,6 +55,7 @@
"@types/morgan": "1.9.9",
"@types/multer": "1.4.11",
"@types/node": "22.10.1",
"@types/node-cron": "3.0.11",
"@types/nunjucks": "3.2.6",
"@types/supertest": "6.0.2",
"@types/swagger-jsdoc": "6.0.4",
Expand Down
1 change: 1 addition & 0 deletions express-api/src/constants/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const config = {
},
contact: {
toEmail: process.env.CONTACT_EMAIL,
systemEmail: 'PIMS System <[email protected]>',
},
notificationTemplate: {
title: 'PIMS',
Expand Down
2 changes: 1 addition & 1 deletion express-api/src/controllers/reports/reportsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const submitErrorReport = async (req: Request, res: Response) => {
const email: IEmail = {
to: [config.contact.toEmail],
cc: [req.user.email],
from: '[email protected]', // Made up for this purpose.
from: config.contact.systemEmail, // Made up for this purpose.
bodyType: EmailBody.Html,
subject: 'PIMS Error Report Submission',
body: emailBody,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<html>
<head>
<title>PIMS Notifications Warning</title>
</head>
<body>
<h2>Some PIMS notifications may not have been sent.</h2>
<p>PIMS has detected that some projects have notifications in a pending state that have passed their intended send date.</p>
<p>Please review the following projects for notification issues:</p>
<ul>
{% for project in Projects %}
<li><p><b><a href={{project.Link}}>{{project.ProjectNumber}}</a></b> {{project.Name}}</p></li>
{% endfor %}
</ul>
</body>
</html>
9 changes: 9 additions & 0 deletions express-api/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import app from '@/express';
import { AppDataSource } from '@/appDataSource';
import { Application } from 'express';
import { IncomingMessage, Server, ServerResponse } from 'http';
import cron from 'node-cron';
import failedEmailCheck from '@/utilities/failedEmailCheck';

const { API_PORT } = constants;

Expand All @@ -18,6 +20,13 @@ const startApp = (app: Application) => {
if (err) logger.error(err);
logger.info(`Server started on port ${API_PORT}.`);

// 0 2 * * * == Triggers every 2:00AM
cron.schedule('0 2 * * *', async () => {
logger.info('Failed email check: Starting');
await failedEmailCheck();
logger.info('Failed email check: Completed');
});

// creating connection to database
await AppDataSource.initialize()
.then(() => {
Expand Down
16 changes: 10 additions & 6 deletions express-api/src/services/properties/propertiesServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,11 @@ const propertiesFuzzySearch = async (keyword: string, limit?: number, agencyIds?
}),
)
// Only include surplus properties
.andWhere(`classification.Name in ('Surplus Encumbered', 'Surplus Active')`)
// Exclude if already is a project property in a project that's in a disallowed status
.andWhere(`parcel.id NOT IN(:...excludedParcelIds)`, { excludedParcelIds });
.andWhere(`classification.Name in ('Surplus Encumbered', 'Surplus Active')`);
// Exclude if already is a project property in a project that's in a disallowed status
if (excludedParcelIds && excludedParcelIds.length > 0) {
parcelsQuery.andWhere(`parcel.id NOT IN(:...excludedParcelIds)`, { excludedParcelIds });
}

// Add the optional agencyIds filter if provided
if (agencyIds && agencyIds.length > 0) {
Expand Down Expand Up @@ -130,9 +132,11 @@ const propertiesFuzzySearch = async (keyword: string, limit?: number, agencyIds?
}),
)
// Only include surplus properties
.andWhere(`classification.Name in ('Surplus Encumbered', 'Surplus Active')`)
// Exclude if already is a project property in a project that's in a disallowed status
.andWhere(`building.id NOT IN(:...excludedBuildingIds)`, { excludedBuildingIds });
.andWhere(`classification.Name in ('Surplus Encumbered', 'Surplus Active')`);
// Exclude if already is a project property in a project that's in a disallowed status
if (excludedBuildingIds && excludedBuildingIds.length > 0) {
buildingsQuery.andWhere(`building.id NOT IN(:...excludedBuildingIds)`, { excludedBuildingIds });
}

if (agencyIds && agencyIds.length > 0) {
buildingsQuery.andWhere(`building.agency_id IN (:...agencyIds)`, { agencyIds });
Expand Down
86 changes: 86 additions & 0 deletions express-api/src/utilities/failedEmailCheck.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { AppDataSource } from '@/appDataSource';
import notificationServices, {
NotificationStatus,
} from '@/services/notifications/notificationServices';
import { NotificationQueue } from '@/typeorm/Entities/NotificationQueue';
import { User } from '@/typeorm/Entities/User';
import logger from '@/utilities/winstonLogger';
import { IsNull, LessThan, Not } from 'typeorm';
import nunjucks from 'nunjucks';
import getConfig from '@/constants/config';
import chesServices, { EmailBody, IEmail } from '@/services/ches/chesServices';
import { PimsRequestUser } from '@/middleware/userAuthCheck';
import { Project } from '@/typeorm/Entities/Project';
import networking from '@/constants/networking';

// Retrieves overdue notifications belonging to projects
const getOverdueNotifications = async (joinProjects: boolean = false) =>
await AppDataSource.getRepository(NotificationQueue).find({
where: {
Status: NotificationStatus.Pending,
SendOn: LessThan(new Date()),
ProjectId: Not(IsNull()),
},
relations: {
Project: joinProjects,
},
});

/**
* Used to check for notifications in the Pending state that have past their send date.
* If any notifications are found, an attempt to update their status is made.
* If any are still Pending after update, an email is created and sent to the business contact email.
*/
const failedEmailCheck = async () => {
try {
// Get all the notification records that are still pending and past their send date.
const overdueNotifications = await getOverdueNotifications();
// Get system user
const system = await AppDataSource.getRepository(User).findOneOrFail({
where: { Username: 'system' },
});
// Update these records
await Promise.all(
overdueNotifications.map((notif) =>
notificationServices.updateNotificationStatus(notif.Id, system),
),
);
// Temporary interface to allow for URL to project
interface ProjectWithLink extends Project {
Link?: string;
}
// If any of those records are still Pending, send an email to business with project names.
const stillOverdueNotifications = await getOverdueNotifications(true);
if (stillOverdueNotifications.length > 0) {
const uniqueProjects: Record<string, ProjectWithLink> = {};
stillOverdueNotifications.forEach(
(notif) =>
(uniqueProjects[notif.ProjectId] = {
...notif.Project,
Link: `${networking.FRONTEND_URL}/projects/${notif.ProjectId}`,
}),
);
const emailBody = nunjucks.render('FailedProjectNotifications.njk', {
Projects: Object.values(uniqueProjects) ?? [],
});
const config = getConfig();
const email: IEmail = {
to: [config.contact.toEmail],
cc: [],
from: config.contact.systemEmail, // Made up for this purpose.
bodyType: EmailBody.Html,
subject: 'PIMS Notifications Warning',
body: emailBody,
};
const sendResult = await chesServices.sendEmailAsync(email, system as PimsRequestUser);
// If the email fails to send, throw an error for logging purposes.
if (sendResult == null) {
throw new Error('Email was attempted but not sent. This feature could be disabled.');
}
}
} catch (e) {
logger.error(`Error in failedEmailCheck: ${(e as Error).message}`);
}
};

export default failedEmailCheck;
7 changes: 5 additions & 2 deletions express-api/tests/testUtils/factories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,10 @@ export const produceProjectNotification = (props?: Partial<ProjectStatusNotifica
return notif;
};

export const produceNotificationQueue = (props?: Partial<NotificationQueue>) => {
export const produceNotificationQueue = (
props?: Partial<NotificationQueue>,
includeProject: boolean = false,
) => {
const queue: NotificationQueue = {
Id: faker.number.int(),
Key: randomUUID(),
Expand All @@ -928,7 +931,7 @@ export const produceNotificationQueue = (props?: Partial<NotificationQueue>) =>
Cc: '',
Tag: faker.lorem.word(),
ProjectId: faker.number.int(),
Project: undefined,
Project: includeProject ? produceProject() : undefined,
ToAgencyId: faker.number.int(),
ToAgency: undefined,
TemplateId: faker.number.int(),
Expand Down
64 changes: 64 additions & 0 deletions express-api/tests/unit/utilities/failedEmailCheck.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { AppDataSource } from '@/appDataSource';
import chesServices, { IEmailSentResponse } from '@/services/ches/chesServices';
import notificationServices from '@/services/notifications/notificationServices';
import { NotificationQueue } from '@/typeorm/Entities/NotificationQueue';
import { User } from '@/typeorm/Entities/User';
import failedEmailCheck from '@/utilities/failedEmailCheck';
import logger from '@/utilities/winstonLogger';
import { faker } from '@faker-js/faker/.';
import { produceNotificationQueue, produceUser } from 'tests/testUtils/factories';
import nunjucks from 'nunjucks';

const _notificationFindSpy = jest
.spyOn(AppDataSource.getRepository(NotificationQueue), 'find')
.mockImplementation(async () => [produceNotificationQueue({}, true)]);

const _notificationServicesUpdateSpy = jest
.spyOn(notificationServices, 'updateNotificationStatus')
.mockImplementation(async () => produceNotificationQueue());

const _userFindSpy = jest
.spyOn(AppDataSource.getRepository(User), 'findOneOrFail')
.mockImplementation(async () => produceUser({ Username: 'system' }));

const _nunjucksSpy = jest.spyOn(nunjucks, 'render').mockImplementation(() => {
'mock html goes here';
});

const _chesSendEmailAsyncSpy = jest
.spyOn(chesServices, 'sendEmailAsync')
.mockImplementation(async () => {
return {
messages: [{ msgId: faker.string.uuid, to: faker.internet.email }],
txId: faker.string.uuid,
} as unknown as IEmailSentResponse;
});

const _loggerErrorSpy = jest.spyOn(logger, 'error');

describe('UNIT - failedEmailCheck', () => {
beforeEach(() => {
jest.clearAllMocks();
});
it('should attempt to send an email if notifications are outstanding', async () => {
await failedEmailCheck();
expect(_notificationServicesUpdateSpy).toHaveBeenCalledTimes(1);
expect(_notificationFindSpy).toHaveBeenCalledTimes(2);
expect(_userFindSpy).toHaveBeenCalledTimes(1);
expect(_nunjucksSpy).toHaveBeenCalledTimes(1);
expect(_chesSendEmailAsyncSpy).toHaveBeenCalledTimes(1);
});

it('should report an error if the email failed to send', async () => {
_chesSendEmailAsyncSpy.mockImplementationOnce(async () => null);
await failedEmailCheck();
expect(_loggerErrorSpy).toHaveBeenCalledWith(
'Error in failedEmailCheck: Email was attempted but not sent. This feature could be disabled.',
);
expect(_notificationServicesUpdateSpy).toHaveBeenCalledTimes(1);
expect(_notificationFindSpy).toHaveBeenCalledTimes(2);
expect(_userFindSpy).toHaveBeenCalledTimes(1);
expect(_nunjucksSpy).toHaveBeenCalledTimes(1);
expect(_chesSendEmailAsyncSpy).toHaveBeenCalledTimes(1);
});
});
Loading