Skip to content

PIMS-2347 Project Agency Response Date Saving Bug #3027

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.

Sign up for GitHub

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

Open
dbarkowsky wants to merge 12 commits into
base: main
Choose a base branch
from
Open

PIMS-2347 Project Agency Response Date Saving Bug #3027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6c0e3ee
sent received on date with agency responses
dbarkowsky Jul 16, 2025
6f6137f
don't queue watch notifications if agency disabled
dbarkowsky Jul 16, 2025
b83bec2
prevent invalid agency response subscriptions
dbarkowsky Jul 16, 2025
37d97b4
agency response table formatting
dbarkowsky Jul 16, 2025
e264c3a
prevent frontend user from setting subscribe under certain conditions
dbarkowsky Jul 16, 2025
defef97
initial agency intrest sort state
dbarkowsky Jul 16, 2025
66e212f
return to cell edit mode due to saving complications
dbarkowsky Jul 16, 2025
763eeda
update test cases
dbarkowsky Jul 17, 2025
4b1a7df
Merge branch 'main' into PIMS-2347
dbarkowsky Jul 28, 2025
80fdf11
ensure lookup is refreshed on agency notification settings edit
dbarkowsky Nov 6, 2025
0f3f33c
Merge branch 'main' into PIMS-2347
dbarkowsky Nov 7, 2025
cc4e279
add patch-package and bump node versions
dbarkowsky Nov 7, 2025
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: 1 addition & 1 deletion express-api/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ RUN npm run build
#############################################
# Prod Build #
#############################################
FROM node:22.9-bullseye-slim as Prod
FROM node:22.15-bullseye-slim as Prod

# Set the working directory to /express-api
WORKDIR /express-api
Expand Down
14 changes: 12 additions & 2 deletions express-api/src/controllers/projects/projectsController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,21 @@ export const updateProjectAgencyResponses = async (req: Request, res: Response)
return res.status(403).send('Projects only editable by Administrator role.');
}

const notificationsSent = await projectServices.updateProjectAgencyResponses(
const updateResults = await projectServices.updateProjectAgencyResponses(
projectId,
req.body.responses,
req.pimsUser,
);

return res.status(200).send(notificationsSent);
// Let requestor know if any of the responses were not updated due to invalid changes.
// This can happen if they are trying to subscribe an agency that is disabled, has no email, or has opted out of notifications.
if (updateResults.invalidResponseChanges > 0) {
return res
.status(400)
.send(
`${updateResults.invalidResponseChanges} of the responses were not updated due to invalid changes.`,
);
}

return res.status(200).send(updateResults);
};
5 changes: 3 additions & 2 deletions express-api/src/services/notifications/notificationServices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,8 +677,9 @@ const generateProjectWatchNotifications = async (
const agency = await query.manager.findOne(Agency, {
where: { Id: response.AgencyId },
});
//No use in queueing an email for an agency with no email address or that doesn't want notifications
if (agency?.SendEmail && agency?.Email && agency?.Email.length) {
// No use in queueing an email for an agency with no email address or that doesn't want notifications
// Don't queue notifications for disabled agencies either.
if (agency?.SendEmail && agency?.Email && agency?.Email.length && !agency.IsDisabled) {
const dateInERP = project.ApprovedOn;
const daysSinceThisStatus = getDaysBetween(dateInERP, new Date());
const statusNotifs = await query.manager.find(ProjectStatusNotification, {
Expand Down
37 changes: 33 additions & 4 deletions express-api/src/services/projects/projectsServices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ const updateProjectAgencyResponses = async (
id: number,
updatedResponses: Partial<ProjectAgencyResponse>[],
user: User,
): Promise<NotificationQueue[]> => {
): Promise<{ sentNotifications: NotificationQueue[]; invalidResponseChanges: number }> => {
if (!(await projectRepo.exists({ where: { Id: id } }))) {
throw new ErrorWithCode('Project matching this internal ID not found.', 404);
}
Expand Down Expand Up @@ -851,14 +851,39 @@ const updateProjectAgencyResponses = async (
},
{ DeletedById: user.Id, DeletedOn: new Date() },
);
// Are any of the agencies involved disabled, not wanting notifications, or missing an email?
// We can't allow them to subcribe.
const relevantAgencies = await AppDataSource.getRepository(Agency).find({
where: {
Id: In(updatedResponses.map((r) => r.AgencyId)),
},
});
const validReponses = updatedResponses.filter((resp) => {
// Only worry about this if subscribing.
if (resp.Response === AgencyResponseType.Subscribe) {
const agency = relevantAgencies.find((a) => a.Id === resp.AgencyId);
// If agency is not found, skip it.
if (!agency) return false;
// If agency is disabled, skip it.
if (agency.IsDisabled) return false;
// If agency does not want notifications, skip it.
if (!agency.SendEmail) return false;
// If agency does not have an email, skip it.
if (!agency.Email || agency.Email.trim() === '') return false;
}
// Otherwise, keep the response.
return true;
});

// Save the remaining updated responses as current
await AppDataSource.getRepository(ProjectAgencyResponse).save(
updatedResponses.map(
validReponses.map(
(resp) =>
({
...resp,
ProjectId: id,
CreatedById: user.Id,
UpdatedById: user.Id,
DeletedById: null,
DeletedOn: null,
}) as ProjectAgencyResponse,
Expand All @@ -867,7 +892,7 @@ const updateProjectAgencyResponses = async (

// Identify which incoming responses are different than original ones on the project
// This counts deleted ones as unsubscribed
const changedResponses = await getAgencyResponseChanges(originalResponses, updatedResponses);
const changedResponses = await getAgencyResponseChanges(originalResponses, validReponses);
// For each of these changed/new responses, queue for send or cancel the notification as needed
// Notifications are cancelled in this function, but only ones to send are returned in the list
const notifsToSend = await notificationServices.generateProjectWatchNotifications(
Expand All @@ -876,9 +901,13 @@ const updateProjectAgencyResponses = async (
);

// Send new notifcations
return await Promise.all(
const sentNotifications = await Promise.all(
notifsToSend.map((notif) => notificationServices.sendNotification(notif, user)),
);
return {
sentNotifications,
invalidResponseChanges: updatedResponses.length - validReponses.length,
};
};

/**
Expand Down
34 changes: 29 additions & 5 deletions express-api/tests/unit/controllers/projects/projectsController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@ const _updateProjectAgencyResponses = jest
.fn()
// eslint-disable-next-line @typescript-eslint/no-unused-vars
.mockImplementation((_id: number, responses: ProjectAgencyResponse[], user: User) => {
return responses.map((r) =>
produceNotificationQueue({ ProjectId: r.ProjectId, ToAgencyId: r.AgencyId }),
) as NotificationQueue[];
return {
sentNotifications: responses.map((r) =>
produceNotificationQueue({ ProjectId: r.ProjectId, ToAgencyId: r.AgencyId }),
) as NotificationQueue[],
invalidResponseChanges: 0,
};
});

jest
Expand Down Expand Up @@ -426,7 +429,7 @@ describe('UNIT - Testing controllers for users routes.', () => {
mockRequest.params.projectId = '1';
await controllers.updateProjectAgencyResponses(mockRequest, mockResponse);
expect(mockResponse.statusValue).toBe(200);
expect(mockResponse.sendValue).toHaveLength(1);
expect(mockResponse.sendValue.sentNotifications).toHaveLength(1);
});

it('should return 400 when the body of responses is not properly formatted', async () => {
Expand All @@ -439,14 +442,35 @@ describe('UNIT - Testing controllers for users routes.', () => {
});

it('should return 400 when the projectId is not a number', async () => {
mockRequest.body = { responses: [produceAgencyResponse()] };
mockRequest.body = {
responses: [produceAgencyResponse({ Response: 0 })], // Response 0 is Subscribe
};
mockRequest.setPimsUser({ RoleId: Roles.ADMIN, hasOneOfRoles: () => true });
mockRequest.params.projectId = 'a';
await controllers.updateProjectAgencyResponses(mockRequest, mockResponse);
expect(mockResponse.statusValue).toBe(400);
expect(mockResponse.sendValue).toBe('Invalid Project ID');
});

it('should return 400 when some invalid subscribe requests are received', async () => {
mockRequest.body = {
responses: [produceAgencyResponse({ Response: 0 })], // Response 0 is Subscribe
};
mockRequest.setPimsUser({ RoleId: Roles.ADMIN, hasOneOfRoles: () => true });
mockRequest.params.projectId = '1';
_updateProjectAgencyResponses.mockImplementationOnce(() => {
return {
sentNotifications: [],
invalidResponseChanges: 1,
};
});
await controllers.updateProjectAgencyResponses(mockRequest, mockResponse);
expect(mockResponse.statusValue).toBe(400);
expect(mockResponse.sendValue).toBe(
'1 of the responses were not updated due to invalid changes.',
);
});

it('should return 403 when a non-admin attempts to make this change', async () => {
mockRequest.body = { responses: [produceAgencyResponse()] };
mockRequest.setPimsUser({ RoleId: Roles.GENERAL_USER, hasOneOfRoles: () => false });
Expand Down
142 changes: 137 additions & 5 deletions express-api/tests/unit/services/projects/projectsServices.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,9 +889,6 @@ describe('UNIT - Project Services', () => {
});

describe('updateProjectAgencyResponses', () => {
jest
.spyOn(AppDataSource.getRepository(Agency), 'findOne')
.mockImplementation(async () => produceAgency());
jest
.spyOn(AppDataSource.getRepository(ProjectNote), 'find')
.mockImplementation(async () => [produceNote()]);
Expand All @@ -908,18 +905,153 @@ describe('UNIT - Project Services', () => {
jest
.spyOn(AppDataSource.getRepository(ProjectAgencyResponse), 'save')
.mockImplementation(async () => produceAgencyResponse());

it('should return the list of sent notifications from this update', async () => {
const returnAgency = produceAgency({
SendEmail: true,
Email: 'email@email.com',
IsDisabled: false,
});
jest
.spyOn(AppDataSource.getRepository(Agency), 'findOne')
.mockImplementationOnce(async () => returnAgency);
jest
.spyOn(AppDataSource.getRepository(Agency), 'find')
.mockImplementationOnce(async () => [returnAgency]);

const project = produceProject();
const responses = [produceAgencyResponse({ Response: AgencyResponseType.Subscribe })];
const responses = [
produceAgencyResponse({
Response: AgencyResponseType.Subscribe,
AgencyId: returnAgency.Id,
}),
];
const user = producePimsRequestUser();
const result = await projectServices.updateProjectAgencyResponses(
project.Id,
responses,
user,
);
expect(result).toHaveLength(
expect(result.sentNotifications).toHaveLength(
responses.filter((r) => r.Response === AgencyResponseType.Subscribe).length,
);
expect(result.invalidResponseChanges).toBe(0);
});

it('should return a invalid response of 1 if the agency cannot be found', async () => {
const returnAgency = produceAgency({
SendEmail: true,
Email: 'email@email.com',
IsDisabled: false,
});
jest
.spyOn(AppDataSource.getRepository(Agency), 'findOne')
.mockImplementationOnce(async () => returnAgency);
jest
.spyOn(AppDataSource.getRepository(Agency), 'find')
.mockImplementationOnce(async () => []);

const project = produceProject();
const responses = [
produceAgencyResponse({
Response: AgencyResponseType.Subscribe,
AgencyId: returnAgency.Id,
}),
];
const user = producePimsRequestUser();
const result = await projectServices.updateProjectAgencyResponses(
project.Id,
responses,
user,
);
expect(result.invalidResponseChanges).toBe(1);
});

it('should return a invalid response of 1 if the agency is disabled', async () => {
const returnAgency = produceAgency({
SendEmail: true,
Email: 'email@email.com',
IsDisabled: true,
});
jest
.spyOn(AppDataSource.getRepository(Agency), 'findOne')
.mockImplementationOnce(async () => returnAgency);
jest
.spyOn(AppDataSource.getRepository(Agency), 'find')
.mockImplementationOnce(async () => [returnAgency]);

const project = produceProject();
const responses = [
produceAgencyResponse({
Response: AgencyResponseType.Subscribe,
AgencyId: returnAgency.Id,
}),
];
const user = producePimsRequestUser();
const result = await projectServices.updateProjectAgencyResponses(
project.Id,
responses,
user,
);
expect(result.invalidResponseChanges).toBe(1);
});

it('should return a invalid response of 1 if the agency has SendEmail disabled', async () => {
const returnAgency = produceAgency({
SendEmail: false,
Email: 'email@email.com',
IsDisabled: false,
});
jest
.spyOn(AppDataSource.getRepository(Agency), 'findOne')
.mockImplementationOnce(async () => returnAgency);
jest
.spyOn(AppDataSource.getRepository(Agency), 'find')
.mockImplementationOnce(async () => [returnAgency]);

const project = produceProject();
const responses = [
produceAgencyResponse({
Response: AgencyResponseType.Subscribe,
AgencyId: returnAgency.Id,
}),
];
const user = producePimsRequestUser();
const result = await projectServices.updateProjectAgencyResponses(
project.Id,
responses,
user,
);
expect(result.invalidResponseChanges).toBe(1);
});

it('should return a invalid response of 1 if the agency has a blank email', async () => {
const returnAgency = produceAgency({
SendEmail: true,
Email: '',
IsDisabled: false,
});
jest
.spyOn(AppDataSource.getRepository(Agency), 'findOne')
.mockImplementationOnce(async () => returnAgency);
jest
.spyOn(AppDataSource.getRepository(Agency), 'find')
.mockImplementationOnce(async () => [returnAgency]);

const project = produceProject();
const responses = [
produceAgencyResponse({
Response: AgencyResponseType.Subscribe,
AgencyId: returnAgency.Id,
}),
];
const user = producePimsRequestUser();
const result = await projectServices.updateProjectAgencyResponses(
project.Id,
responses,
user,
);
expect(result.invalidResponseChanges).toBe(1);
});
});
});
2 changes: 1 addition & 1 deletion react-app/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#############################################
# Base Build #
#############################################
FROM node:22.9-bullseye-slim as base
FROM node:22.15-bullseye-slim as base

# Set the working directory to /app
WORKDIR /app
Expand Down
1 change: 1 addition & 0 deletions react-app/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
"eslint-plugin-react": "7.37.1",
"jest": "^29.7.0",
"jest-environment-jsdom": "29.7.0",
"patch-package": "8.0.1",
"prettier": "3.6.2",
"react-test-renderer": "18.3.1",
"ts-jest": "29.4.0",
Expand Down
1 change: 1 addition & 0 deletions react-app/src/components/agencies/AgencyDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ const AgencyDetail = ({ onClose }: IAgencyDetail) => {
SendEmail,
}).then(() => {
refreshData();
refreshLookup();
setOpenNotificationsDialog(false);
});
}
Expand Down
Loading