Task #165: Add automated tests covering the notification fan-out logic
(executive-meeting-notify.ts + executive-meetings.ts route handlers).
What was added
- artifacts/api-server/tests/executive-meetings-notifications.test.mjs
— 7 integration tests, one per notification type:
1. meeting_created 2. request_submitted
3. request_approved 4. request_rejected
5. request_needs_edit 6. task_assigned
7. task_completed
Each test asserts (a) the actor is excluded, (b) recipients are
deduped across direct (user_roles) and group-derived (group_roles +
user_groups) role assignments, (c) one row is inserted into BOTH
executive_meeting_notifications and notifications in the same
transaction, and (d) the matching Socket.IO events fire
(notification_created per recipient + the
executive_meeting_notifications_changed broadcast) with the right
notificationType payload.
- Test setup creates one user (approver2) that holds the target role
both directly AND through a group, so the dedup invariant is
exercised on every fan-out path that uses getUserIdsForRoleNames.
Deviation from the task spec
- The task suggested the new file at
artifacts/api-server/src/routes/__tests__/executive-meetings-notifications.test.ts,
but the api-server's runner is `node --test 'tests/**/*.test.mjs'`
and every existing test (including notification-adjacent coverage)
lives in tests/ as .mjs. A .ts file in src/__tests__ would silently
never run, so the test file follows the established convention.
Hardening (post code-review)
- Added a scopeDiff() helper that filters each snapshot diff by
notificationType + meetingId/relatedType+relatedId before any
assertion runs. This protects the actor-exclusion check on the
seeded admin user from cross-file flakiness if other test files
happen to write notifications for admin while these tests run.
- expectSocketEventsFor() now accepts { expectExactlyOne: true }; the
meeting_created and request_submitted tests use it so a regression
that double-emitted notification_created on the dedupe-sensitive
paths would also be caught at the socket layer (not just in the DB).
- Trimmed verbose explanatory comments in the test file to match the
style of the surrounding tests/*.test.mjs.
Other notes
- While running the new test for the first time, the dev DB was
missing the executive_meeting_notification_prefs table (an
un-pushed migration), causing 500s in fan-out paths that read
prefs. Ran `pnpm --filter @workspace/db push` once to sync the
schema; no schema or runtime code was changed.
- Full api-server suite passes: 211/211 tests green (7 new + 204
pre-existing).
Riyadh
2980bf1bcb