feat(phase-2): wire notify() into admin controllers (#70)#76
Merged
Conversation
After a successful DB write, three admin actions now fan out a notification to every team-member wallet of the affected project: application accept/reject, M2 approve, and M2 request-changes. The notify call runs after the HTTP response is sent and is fully isolated — the prior-status lookup, the team fan-out, and the send are each guarded so a failure logs an error but never changes the response or rolls back the underlying admin action. Closes #70
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
notifyProjectTeam(projectId, eventType, sourceId, payload)tonotification.service.js— fetches the project, fillsprojectNameinto the payload, and fansnotify(...)out to every non-null team-member wallet. Fully guarded: a project-fetch failure resolves to[], never throws.updateApplicationStatus(on asubmitted → accepted/rejectedtransition),approveM2, andrequestChanges. Each firesnotifyProjectTeamafter the success response is sent, inside atry/catchthat logs vialogger.error.source_idper spec §4.2: application id for accept/reject; project id form2_approved;${projectId}:${changeRequestDate}form2_changes_requested(reusing the timestamp already written to the DB).getByIdtoprogram-application.repository.js+ service pass-through, used to read the application's prior status so the notify only fires on a genuinesubmitted →transition.Closes #70
Test plan
cd server && npm test— pass (14 files, 121 tests)cd client && npm run build— passcd client && npm run lint— pass (zero warnings)UI verification (stadium-tester)
Not applicable — server-only issue. Issue #70 wires service calls into admin controllers; it has no client surface. All five
## Test scenariosare HTTP/DB-state assertions, covered by Vitest controller tests, mapped 1:1:submitted → acceptedwrites notifications for every team wallet; HTTP response unchangedprogram-application.test.js— "calls notifyProjectTeam with application_accepted when submitted → accepted"m2_approved,source_id=projectId)project.controller.test.js—approveM2notify casem2_changes_requested,source_id=projectId:iso)project.controller.test.js—requestChangesnotify caseprogram-application.test.js+project.controller.test.js— "still returns 200 even when notifyProjectTeam rejects"submitted → submitted(no real change) writes no notificationsprogram-application.test.js— "does not call notifyProjectTeam when new status is submitted"Failure-isolation is additionally covered by
notifyProjectTeamfan-out tests innotification.service.test.js(null wallets skipped, no team members →[],getProjectByIdthrowing →[]) and a new case proving a failing prior-status lookup still returns 200.Preview
N/A — no client surface affected.
Backlog items logged
One non-blocking observation from the pre-PR review, appended to
docs/improvement-backlog.md:console.logcalls inproject.controller.js(lines 75, 188) — predate Phase 2, left untouched per minimal-diff; flagged for aloggercleanup pass.Pre-PR review found 1 blocker, fixed before this PR: the prior-status
getByIdlookup ran inside the main try block, so a Supabase transport error there would have 500'd the admin action. It is now wrapped in its owntry/catch— a lookup failure degrades to "skip the notify gate," and the status update + response proceed normally. A regression test covers this path.Invariants verified
loggerserver/api/**(Supabase via repositories)importsyntax only; norequire()notification.service.js→project.service.jsis one-directional