Skip to content

Commit 64bf0c5

Browse files
[TTAHUB-5387] Add notification handlers (#3674)
* fix: cap getNotifications limit at 100 and add comprehensive tests - Fix Math.max -> Math.min bug in getNotifications limit calculation - Add handler unit tests (src/routes/notifications/handlers.test.ts) - Add policy unit tests (src/policies/notifications.test.ts) - Fix and expand service tests for sort field/direction validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add param middleware * Updates from code review * refactor: separate per-user notification state into NotificationUserStates table The Notifications table previously stored viewedAt and archivedAt directly on the notification row. Global notifications (userId=null) could not track whether different users had independently viewed or archived them — a single row cannot hold state for multiple users. Changes: - Migration: creates NotificationUserStates (notificationId, userId, viewedAt, archivedAt, UNIQUE on notificationId+userId with FK cascades), backfills existing per-user state, and drops viewedAt/archivedAt from Notifications - New model: NotificationUserState with belongsTo Notification+User - Notification model: removed archivedAt/viewedAt/isInformational, added hasMany(NotificationUserState, { as: 'userStates' }) - Service: updateNotification → updateNotificationState(notificationId, userId, { viewedAt?, archivedAt? }) — upserts per-user state row; getNotifications(userId, scopes, options) — LEFT JOINs state, filters archived, returns NotificationWithState[] - Types: added NotificationUserStateModel and NotificationWithState - Policy: added isGlobalNotification(); canUpdateNotification() now allows admin, owner, or global notification (any user can set their own state) - Handlers: getNotificationsHandler passes userId as first arg; updateNotificationHandler calls updateNotificationState - Spec: updated to document new table, service signatures, cleanup logic - Tests: 121/121 passing across 10 suites Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update to remove findOrCreate * Update seeder * Define relation in both directions * Update test * Map scopes to types directly to prevent drift * Updates from code review --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 42b970c commit 64bf0c5

21 files changed

Lines changed: 1200 additions & 198 deletions

docs/logical_data_model.encoded

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

docs/logical_data_model.puml

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,20 +1053,28 @@ class NextSteps{
10531053
completeDate : date
10541054
}
10551055

1056+
class NotificationUserStates{
1057+
* id : integer : <generated>
1058+
* notificationId : integer : REFERENCES "Notifications".id
1059+
* userId : integer : REFERENCES "Users".id
1060+
* createdAt : timestamp with time zone
1061+
* updatedAt : timestamp with time zone
1062+
archivedAt : date
1063+
viewedAt : date
1064+
}
1065+
10561066
class Notifications{
10571067
* id : integer : <generated>
10581068
userId : integer : REFERENCES "Users".id
10591069
* createdAt : timestamp with time zone
10601070
* type : enum
10611071
* updatedAt : timestamp with time zone
1062-
archivedAt : date
10631072
displayId : varchar(255)
10641073
entityId : integer
10651074
label : text
10661075
link : text
10671076
text : text
10681077
triggeredAt : date
1069-
viewedAt : date
10701078
}
10711079

10721080
class ObjectiveCollaborators{
@@ -2537,6 +2545,20 @@ class ZALNextSteps{
25372545
session_sig : text
25382546
}
25392547

2548+
class ZALNotificationUserStates{
2549+
* id : bigint : <generated>
2550+
* data_id : bigint
2551+
* dml_as : bigint
2552+
* dml_by : bigint
2553+
* dml_timestamp : timestamp with time zone
2554+
* dml_txid : uuid
2555+
* dml_type : enum
2556+
descriptor_id : integer
2557+
new_row_data : jsonb
2558+
old_row_data : jsonb
2559+
session_sig : text
2560+
}
2561+
25402562
class ZALNotifications{
25412563
* id : bigint : <generated>
25422564
* data_id : bigint
@@ -3065,6 +3087,7 @@ NationalCenters "1" --[#black,dashed,thickness=2]--{ "n" EventReportPilotNation
30653087
NationalCenters "1" --[#black,dashed,thickness=2]--{ "n" NationalCenterUsers : nationalCenter, nationalCenterUsers
30663088
NationalCenters "1" --[#black,dashed,thickness=2]--{ "n" NationalCenters : mapsToNationalCenter, mapsFromNationalCenters
30673089
NextSteps "1" --[#black,dashed,thickness=2]--{ "n" NextStepResources : nextStep, nextStepResources
3090+
Notifications "1" --[#black,dashed,thickness=2]--{ "n" NotificationUserStates : notification, userStates
30683091
ObjectiveTemplates "1" --[#black,dashed,thickness=2]--{ "n" GoalTemplateObjectiveTemplates : objectiveTemplate, goalTemplateObjectiveTemplates
30693092
ObjectiveTemplates "1" --[#black,dashed,thickness=2]--{ "n" Objectives : objectives, objectiveTemplate
30703093
Objectives "1" --[#black,dashed,thickness=2]--{ "n" ActivityReportObjectives : objective, originalObjective, activityReportObjectives, reassignedActivityReportObjectives
@@ -3114,6 +3137,7 @@ Users "1" --[#black,dashed,thickness=2]--{ "n" GoalCollaborators : user, goalCo
31143137
Users "1" --[#black,dashed,thickness=2]--{ "n" GoalStatusChanges : user, goalStatusChanges
31153138
Users "1" --[#black,dashed,thickness=2]--{ "n" GroupCollaborators : user, groupCollaborators
31163139
Users "1" --[#black,dashed,thickness=2]--{ "n" NationalCenterUsers : user, nationalCenterUsers
3140+
Users "1" --[#black,dashed,thickness=2]--{ "n" NotificationUserStates : user, notificationUserStates
31173141
Users "1" --[#black,dashed,thickness=2]--{ "n" Notifications : user, notifications
31183142
Users "1" --[#black,dashed,thickness=2]--{ "n" ObjectiveCollaborators : user, objectiveCollaborators
31193143
Users "1" --[#black,dashed,thickness=2]--{ "n" Permissions : user, permissions

src/constants.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,11 @@ const NOTIFICATION_CONFIGURATION = {
223223
},
224224
};
225225

226+
const ADMIN_BROADCASTABLE_NOTIFICATION_TYPES = [
227+
NOTIFICATION_TYPES.SYSTEM_PLANNED_OUTAGE,
228+
NOTIFICATION_TYPES.SYSTEM_UNPLANNED_OUTAGE,
229+
];
230+
226231
const EMAIL_ACTIONS = {
227232
COLLABORATOR_ADDED: 'collaboratorAssigned',
228233
NEEDS_ACTION: 'changesRequested',
@@ -377,6 +382,7 @@ module.exports = {
377382
USER_SETTINGS,
378383
NOTIFICATION_TYPES,
379384
NOTIFICATION_CONFIGURATION,
385+
ADMIN_BROADCASTABLE_NOTIFICATION_TYPES,
380386
EMAIL_ACTIONS,
381387
S3_ACTIONS,
382388
EMAIL_DIGEST_FREQ,

src/middleware/checkIdParamMiddleware.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,7 @@ export function checkGrantIdQueryParam(req, res, next) {
283283
req.query.parsedGrantId = parsed;
284284
return next();
285285
}
286+
287+
export function checkNotificationIdParam(req, res, next) {
288+
return checkIdParam(req, res, next, 'notificationId');
289+
}

src/middleware/checkIdParamMiddleware.test.js

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
checkGroupIdParam,
1414
checkIdIdParam,
1515
checkIdParam,
16+
checkNotificationIdParam,
1617
checkObjectiveIdParam,
1718
checkObjectiveTemplateIdParam,
1819
checkRecipientIdParam,
@@ -405,6 +406,56 @@ describe('checkIdParamMiddleware', () => {
405406
});
406407
});
407408

409+
describe('checkNotificationIdParam', () => {
410+
it('calls next if notification id is string or integer', () => {
411+
const mockRequest = {
412+
path: '/api/endpoint',
413+
params: {
414+
notificationId: '2',
415+
},
416+
};
417+
418+
checkNotificationIdParam(mockRequest, mockResponse, mockNext);
419+
expect(mockResponse.status).not.toHaveBeenCalled();
420+
expect(mockNext).toHaveBeenCalled();
421+
});
422+
423+
it('throw 400 if param is not string or integer', () => {
424+
const mockRequest = {
425+
path: '/api/endpoint',
426+
params: {
427+
notificationId: '2D',
428+
},
429+
};
430+
431+
checkNotificationIdParam(mockRequest, mockResponse, mockNext);
432+
expect(mockResponse.status).toHaveBeenCalledWith(400);
433+
expect(auditLogger.error).toHaveBeenCalled();
434+
expect(mockNext).not.toHaveBeenCalled();
435+
});
436+
437+
it('throw 400 if param is missing', () => {
438+
const mockRequest = {
439+
path: '/api/endpoint',
440+
params: {},
441+
};
442+
443+
checkNotificationIdParam(mockRequest, mockResponse, mockNext);
444+
expect(mockResponse.status).toHaveBeenCalledWith(400);
445+
expect(auditLogger.error).toHaveBeenCalled();
446+
expect(mockNext).not.toHaveBeenCalled();
447+
});
448+
449+
it('throw 400 if notificationId param is undefined', () => {
450+
const mockRequest = { path: '/api/endpoint', params: {} };
451+
452+
checkNotificationIdParam(mockRequest, mockResponse, mockNext);
453+
expect(mockResponse.status).toHaveBeenCalledWith(400);
454+
expect(auditLogger.error).toHaveBeenCalledWith(`${errorMessage}: notificationId undefined`);
455+
expect(mockNext).not.toHaveBeenCalled();
456+
});
457+
});
458+
408459
describe('checkGroupIdParam', () => {
409460
it('calls next if objective id is string or integer', () => {
410461
const mockRequest = {
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
const { prepMigration, removeTables } = require('../lib/migration');
2+
3+
/** @type {import('sequelize-cli').Migration} */
4+
module.exports = {
5+
async up(queryInterface, Sequelize) {
6+
await queryInterface.sequelize.transaction(async (transaction) => {
7+
const sessionSig = __filename;
8+
await prepMigration(queryInterface, transaction, sessionSig);
9+
10+
await queryInterface.createTable(
11+
'NotificationUserStates',
12+
{
13+
id: {
14+
type: Sequelize.INTEGER,
15+
autoIncrement: true,
16+
primaryKey: true,
17+
},
18+
notificationId: {
19+
type: Sequelize.INTEGER,
20+
allowNull: false,
21+
references: {
22+
model: 'Notifications',
23+
key: 'id',
24+
},
25+
onDelete: 'CASCADE',
26+
},
27+
userId: {
28+
type: Sequelize.INTEGER,
29+
allowNull: false,
30+
references: {
31+
model: 'Users',
32+
key: 'id',
33+
},
34+
onDelete: 'CASCADE',
35+
},
36+
viewedAt: {
37+
type: Sequelize.DATEONLY,
38+
allowNull: true,
39+
},
40+
archivedAt: {
41+
type: Sequelize.DATEONLY,
42+
allowNull: true,
43+
},
44+
createdAt: {
45+
type: Sequelize.DATE,
46+
allowNull: false,
47+
},
48+
updatedAt: {
49+
type: Sequelize.DATE,
50+
allowNull: false,
51+
},
52+
},
53+
{ transaction }
54+
);
55+
56+
await queryInterface.addIndex('NotificationUserStates', ['notificationId', 'userId'], {
57+
unique: true,
58+
transaction,
59+
});
60+
61+
await queryInterface.sequelize.query(
62+
/* sql */ `
63+
INSERT INTO "NotificationUserStates" (
64+
"notificationId",
65+
"userId",
66+
"viewedAt",
67+
"archivedAt",
68+
"createdAt",
69+
"updatedAt"
70+
)
71+
SELECT
72+
n.id,
73+
n."userId",
74+
n."viewedAt",
75+
n."archivedAt",
76+
n."createdAt",
77+
n."updatedAt"
78+
FROM "Notifications" n
79+
WHERE n."userId" IS NOT NULL
80+
AND (
81+
n."viewedAt" IS NOT NULL
82+
OR n."archivedAt" IS NOT NULL
83+
);
84+
`,
85+
{ transaction }
86+
);
87+
88+
await queryInterface.removeColumn('Notifications', 'viewedAt', { transaction });
89+
await queryInterface.removeColumn('Notifications', 'archivedAt', { transaction });
90+
});
91+
},
92+
93+
async down(queryInterface, Sequelize) {
94+
await queryInterface.sequelize.transaction(async (transaction) => {
95+
const sessionSig = __filename;
96+
await prepMigration(queryInterface, transaction, sessionSig);
97+
98+
await queryInterface.addColumn(
99+
'Notifications',
100+
'archivedAt',
101+
{
102+
type: Sequelize.DATEONLY,
103+
allowNull: true,
104+
},
105+
{ transaction }
106+
);
107+
108+
await queryInterface.addColumn(
109+
'Notifications',
110+
'viewedAt',
111+
{
112+
type: Sequelize.DATEONLY,
113+
allowNull: true,
114+
},
115+
{ transaction }
116+
);
117+
118+
await queryInterface.sequelize.query(
119+
/* sql */ `
120+
UPDATE "Notifications" n
121+
SET
122+
"archivedAt" = nus."archivedAt",
123+
"viewedAt" = nus."viewedAt"
124+
FROM "NotificationUserStates" nus
125+
WHERE n.id = nus."notificationId"
126+
AND n."userId" = nus."userId"
127+
AND n."userId" IS NOT NULL;
128+
`,
129+
{ transaction }
130+
);
131+
132+
await removeTables(queryInterface, transaction, ['NotificationUserStates']);
133+
});
134+
},
135+
};

src/models/notification.js

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ export default (sequelize, DataTypes) => {
55
class Notification extends Model {
66
static associate(models) {
77
Notification.belongsTo(models.User, { foreignKey: 'userId', as: 'user' });
8+
Notification.hasMany(models.NotificationUserState, {
9+
foreignKey: 'notificationId',
10+
as: 'userStates',
11+
});
812
}
913
}
1014

@@ -44,30 +48,16 @@ export default (sequelize, DataTypes) => {
4448
type: DataTypes.STRING,
4549
allowNull: true,
4650
},
47-
archivedAt: {
48-
type: DataTypes.DATEONLY,
49-
allowNull: true,
50-
},
5151
triggeredAt: {
5252
type: DataTypes.DATEONLY,
5353
allowNull: true,
5454
},
55-
viewedAt: {
56-
type: DataTypes.DATEONLY,
57-
allowNull: true,
58-
},
5955
isGlobal: {
6056
type: DataTypes.VIRTUAL,
6157
get() {
6258
return this.userId === null;
6359
},
6460
},
65-
isInformational: {
66-
type: DataTypes.VIRTUAL,
67-
get() {
68-
return this.triggeredAt === null;
69-
},
70-
},
7161
},
7262
{
7363
sequelize,
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
const { Model } = require('sequelize');
2+
3+
export default (sequelize, DataTypes) => {
4+
class NotificationUserState extends Model {
5+
static associate(models) {
6+
NotificationUserState.belongsTo(models.Notification, {
7+
foreignKey: 'notificationId',
8+
as: 'notification',
9+
});
10+
NotificationUserState.belongsTo(models.User, {
11+
foreignKey: 'userId',
12+
as: 'user',
13+
});
14+
}
15+
}
16+
17+
NotificationUserState.init(
18+
{
19+
notificationId: {
20+
type: DataTypes.INTEGER,
21+
allowNull: false,
22+
references: {
23+
model: {
24+
tableName: 'Notifications',
25+
},
26+
key: 'id',
27+
},
28+
},
29+
userId: {
30+
type: DataTypes.INTEGER,
31+
allowNull: false,
32+
references: {
33+
model: {
34+
tableName: 'Users',
35+
},
36+
key: 'id',
37+
},
38+
},
39+
viewedAt: {
40+
type: DataTypes.DATEONLY,
41+
allowNull: true,
42+
},
43+
archivedAt: {
44+
type: DataTypes.DATEONLY,
45+
allowNull: true,
46+
},
47+
},
48+
{
49+
sequelize,
50+
modelName: 'NotificationUserState',
51+
timestamps: true,
52+
paranoid: false,
53+
}
54+
);
55+
56+
return NotificationUserState;
57+
};

0 commit comments

Comments
 (0)