diff --git a/docs/improvement-backlog.md b/docs/improvement-backlog.md index cc94704..f173f7c 100644 --- a/docs/improvement-backlog.md +++ b/docs/improvement-backlog.md @@ -125,3 +125,9 @@ Do **not** manually edit `- **Promoted**` lines. - **File(s)**: `server/api/services/notification-templates/m2-changes-requested.js` - **Observed during**: issue #69 (revamp-P2-03 Resend integration) — reviewer flagged - **Suggestion**: `payload.feedback` is rendered into the email body verbatim (now HTML-escaped). Very long admin feedback produces an oversized email. Consider truncating with a "see the full feedback on your project page" link once P2-05 ships the project-page feedback surface. + +## [2026-05-18] Pre-existing console.log calls in project.controller.js +- **Severity**: nit +- **File(s)**: `server/api/controllers/project.controller.js:75`, `server/api/controllers/project.controller.js:188` +- **Observed during**: issue #70 (revamp-P2-04 notify trigger wiring) — reviewer flagged +- **Suggestion**: two pre-existing `console.log` calls (a debug payload preview in `updateProject`, and an M2-agreement confirmation log) predate Phase 2 and were left untouched. Server code elsewhere uses the `logger` utility (`server/api/utils/logger.js`). Convert these to `logger.debug`/`logger.info` (or remove the debug preview) in a dedicated cleanup pass — out of scope for #70's minimal diff. diff --git a/server/api/controllers/__tests__/program-application.test.js b/server/api/controllers/__tests__/program-application.test.js index 34946fc..f71ad31 100644 --- a/server/api/controllers/__tests__/program-application.test.js +++ b/server/api/controllers/__tests__/program-application.test.js @@ -8,6 +8,7 @@ vi.mock('../../services/project.service.js', () => ({ })); vi.mock('../../services/program-application.service.js', () => ({ default: { + getById: vi.fn(), listByProgram: vi.fn(), listByProject: vi.fn(), findOne: vi.fn(), @@ -15,10 +16,14 @@ vi.mock('../../services/program-application.service.js', () => ({ updateStatus: vi.fn(), }, })); +vi.mock('../../services/notification.service.js', () => ({ + default: { notifyProjectTeam: vi.fn() }, +})); const programService = (await import('../../services/program.service.js')).default; const projectService = (await import('../../services/project.service.js')).default; const applicationService = (await import('../../services/program-application.service.js')).default; +const notificationService = (await import('../../services/notification.service.js')).default; const programController = (await import('../program.controller.js')).default; const mockRes = () => { @@ -215,8 +220,10 @@ describe('ProgramController.updateApplicationStatus', () => { }); it('updates on valid payload', async () => { - programService.findBySlug.mockResolvedValue({ id: 'p1' }); - applicationService.updateStatus.mockResolvedValue({ id: 'a1', status: 'accepted' }); + programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding' }); + applicationService.getById.mockResolvedValue({ id: 'a1', status: 'submitted' }); + applicationService.updateStatus.mockResolvedValue({ id: 'a1', status: 'accepted', projectId: 'proj-1' }); + notificationService.notifyProjectTeam.mockResolvedValue([]); const req = { params: { slug: 's', applicationId: 'a1' }, body: { status: 'accepted', reviewNotes: ' looks great ' }, @@ -234,7 +241,8 @@ describe('ProgramController.updateApplicationStatus', () => { }); it('maps PostgREST no-rows error to 404', async () => { - programService.findBySlug.mockResolvedValue({ id: 'p1' }); + programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding' }); + applicationService.getById.mockResolvedValue({ id: 'missing', status: 'submitted' }); const err = Object.assign(new Error('no rows'), { code: 'PGRST116' }); applicationService.updateStatus.mockRejectedValue(err); const req = { @@ -246,4 +254,93 @@ describe('ProgramController.updateApplicationStatus', () => { await programController.updateApplicationStatus(req, res); expect(res.status).toHaveBeenCalledWith(404); }); + + it('calls notifyProjectTeam with application_accepted when submitted → accepted', async () => { + programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); + applicationService.getById.mockResolvedValue({ id: 'app-1', status: 'submitted' }); + applicationService.updateStatus.mockResolvedValue({ id: 'app-1', status: 'accepted', projectId: 'proj-1' }); + notificationService.notifyProjectTeam.mockResolvedValue([]); + const req = { + params: { slug: 'dogfooding-2026', applicationId: 'app-1' }, + body: { status: 'accepted' }, + user: { address: 'admin' }, + }; + const res = mockRes(); + await programController.updateApplicationStatus(req, res); + expect(res.status).toHaveBeenCalledWith(200); + expect(notificationService.notifyProjectTeam).toHaveBeenCalledWith( + 'proj-1', + 'application_accepted', + 'app-1', + expect.objectContaining({ programName: 'Dogfooding 2026', programSlug: 'dogfooding-2026' }), + ); + }); + + it('calls notifyProjectTeam with application_rejected when submitted → rejected', async () => { + programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); + applicationService.getById.mockResolvedValue({ id: 'app-2', status: 'submitted' }); + applicationService.updateStatus.mockResolvedValue({ id: 'app-2', status: 'rejected', projectId: 'proj-2' }); + notificationService.notifyProjectTeam.mockResolvedValue([]); + const req = { + params: { slug: 'dogfooding-2026', applicationId: 'app-2' }, + body: { status: 'rejected' }, + user: { address: 'admin' }, + }; + const res = mockRes(); + await programController.updateApplicationStatus(req, res); + expect(res.status).toHaveBeenCalledWith(200); + expect(notificationService.notifyProjectTeam).toHaveBeenCalledWith( + 'proj-2', + 'application_rejected', + 'app-2', + expect.objectContaining({ programName: 'Dogfooding 2026', programSlug: 'dogfooding-2026' }), + ); + }); + + it('does not call notifyProjectTeam when new status is submitted (no real transition)', async () => { + programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); + applicationService.getById.mockResolvedValue({ id: 'app-3', status: 'submitted' }); + applicationService.updateStatus.mockResolvedValue({ id: 'app-3', status: 'submitted', projectId: 'proj-3' }); + const req = { + params: { slug: 'dogfooding-2026', applicationId: 'app-3' }, + body: { status: 'submitted' }, + user: { address: 'admin' }, + }; + const res = mockRes(); + await programController.updateApplicationStatus(req, res); + expect(res.status).toHaveBeenCalledWith(200); + expect(notificationService.notifyProjectTeam).not.toHaveBeenCalled(); + }); + + it('still returns 200 even when notifyProjectTeam rejects', async () => { + programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); + applicationService.getById.mockResolvedValue({ id: 'app-4', status: 'submitted' }); + applicationService.updateStatus.mockResolvedValue({ id: 'app-4', status: 'accepted', projectId: 'proj-4' }); + notificationService.notifyProjectTeam.mockRejectedValue(new Error('boom')); + const req = { + params: { slug: 'dogfooding-2026', applicationId: 'app-4' }, + body: { status: 'accepted' }, + user: { address: 'admin' }, + }; + const res = mockRes(); + await programController.updateApplicationStatus(req, res); + expect(res.status).toHaveBeenCalledWith(200); + }); + + it('still updates and returns 200 when the prior-status lookup (getById) fails', async () => { + programService.findBySlug.mockResolvedValue({ id: 'p1', name: 'Dogfooding 2026' }); + applicationService.getById.mockRejectedValue(new Error('supabase transport error')); + applicationService.updateStatus.mockResolvedValue({ id: 'app-5', status: 'accepted', projectId: 'proj-5' }); + const req = { + params: { slug: 'dogfooding-2026', applicationId: 'app-5' }, + body: { status: 'accepted' }, + user: { address: 'admin' }, + }; + const res = mockRes(); + await programController.updateApplicationStatus(req, res); + // The status update still succeeds; getById failure only skips the notify gate. + expect(applicationService.updateStatus).toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + expect(notificationService.notifyProjectTeam).not.toHaveBeenCalled(); + }); }); diff --git a/server/api/controllers/__tests__/project.controller.test.js b/server/api/controllers/__tests__/project.controller.test.js new file mode 100644 index 0000000..523f5bb --- /dev/null +++ b/server/api/controllers/__tests__/project.controller.test.js @@ -0,0 +1,112 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../../services/project.service.js', () => ({ + default: { updateProject: vi.fn(), getProjectById: vi.fn() }, +})); +vi.mock('../../services/project-update.service.js', () => ({ + default: { listByProject: vi.fn(), create: vi.fn() }, +})); +vi.mock('../../services/funding-signal.service.js', () => ({ + default: { getByProject: vi.fn(), upsert: vi.fn() }, +})); +vi.mock('../../services/payment.service.js', () => ({ + default: { parseBalance: vi.fn(), constructTransfer: vi.fn() }, +})); +vi.mock('../../services/notification.service.js', () => ({ + default: { notifyProjectTeam: vi.fn() }, +})); +vi.mock('../../../config/polkadot-config.js', () => ({ + getAuthorizedAddresses: vi.fn(() => []), +})); + +const projectService = (await import('../../services/project.service.js')).default; +const notificationService = (await import('../../services/notification.service.js')).default; +const projectController = (await import('../project.controller.js')).default; + +const mockRes = () => { + const res = {}; + res.status = vi.fn(() => res); + res.json = vi.fn(() => res); + return res; +}; + +describe('ProjectController.approveM2', () => { + beforeEach(() => vi.clearAllMocks()); + + it('returns 200 and calls notifyProjectTeam with m2_approved', async () => { + const updated = { id: 'proj-1', m2Status: 'completed' }; + projectService.updateProject.mockResolvedValue(updated); + notificationService.notifyProjectTeam.mockResolvedValue([]); + const req = { params: { projectId: 'proj-1' } }; + const res = mockRes(); + await projectController.approveM2(req, res); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ status: 'success', data: updated }); + expect(notificationService.notifyProjectTeam).toHaveBeenCalledWith('proj-1', 'm2_approved', 'proj-1', {}); + }); + + it('returns 200 even when notifyProjectTeam rejects', async () => { + projectService.updateProject.mockResolvedValue({ id: 'proj-1', m2Status: 'completed' }); + notificationService.notifyProjectTeam.mockRejectedValue(new Error('boom')); + const req = { params: { projectId: 'proj-1' } }; + const res = mockRes(); + await projectController.approveM2(req, res); + expect(res.status).toHaveBeenCalledWith(200); + }); + + it('returns 404 when project is not found', async () => { + projectService.updateProject.mockResolvedValue(null); + const req = { params: { projectId: 'nope' } }; + const res = mockRes(); + await projectController.approveM2(req, res); + expect(res.status).toHaveBeenCalledWith(404); + expect(notificationService.notifyProjectTeam).not.toHaveBeenCalled(); + }); +}); + +describe('ProjectController.requestChanges', () => { + beforeEach(() => vi.clearAllMocks()); + + it('returns 200 and calls notifyProjectTeam with m2_changes_requested', async () => { + const updated = { id: 'proj-1', m2Status: 'building' }; + projectService.updateProject.mockResolvedValue(updated); + notificationService.notifyProjectTeam.mockResolvedValue([]); + const req = { params: { projectId: 'proj-1' }, body: { feedback: 'Fix the docs' } }; + const res = mockRes(); + await projectController.requestChanges(req, res); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ status: 'success', data: updated }); + expect(notificationService.notifyProjectTeam).toHaveBeenCalledWith( + 'proj-1', + 'm2_changes_requested', + expect.stringMatching(/^proj-1:/), + expect.objectContaining({ feedback: 'Fix the docs' }), + ); + }); + + it('returns 200 even when notifyProjectTeam rejects', async () => { + projectService.updateProject.mockResolvedValue({ id: 'proj-1', m2Status: 'building' }); + notificationService.notifyProjectTeam.mockRejectedValue(new Error('boom')); + const req = { params: { projectId: 'proj-1' }, body: { feedback: 'Some feedback' } }; + const res = mockRes(); + await projectController.requestChanges(req, res); + expect(res.status).toHaveBeenCalledWith(200); + }); + + it('returns 422 when feedback is missing', async () => { + const req = { params: { projectId: 'proj-1' }, body: {} }; + const res = mockRes(); + await projectController.requestChanges(req, res); + expect(res.status).toHaveBeenCalledWith(422); + expect(notificationService.notifyProjectTeam).not.toHaveBeenCalled(); + }); + + it('returns 404 when project is not found', async () => { + projectService.updateProject.mockResolvedValue(null); + const req = { params: { projectId: 'nope' }, body: { feedback: 'some feedback' } }; + const res = mockRes(); + await projectController.requestChanges(req, res); + expect(res.status).toHaveBeenCalledWith(404); + expect(notificationService.notifyProjectTeam).not.toHaveBeenCalled(); + }); +}); diff --git a/server/api/controllers/program.controller.js b/server/api/controllers/program.controller.js index efcc523..80f38a6 100644 --- a/server/api/controllers/program.controller.js +++ b/server/api/controllers/program.controller.js @@ -1,9 +1,11 @@ import programService from '../services/program.service.js'; import programApplicationService from '../services/program-application.service.js'; import projectService from '../services/project.service.js'; +import notificationService from '../services/notification.service.js'; import { validateApplicationFields } from '../utils/application-fields.validator.js'; import { validateProgram } from '../utils/validation.js'; import { randomUUID } from 'node:crypto'; +import logger from '../utils/logger.js'; class ProgramController { async list(req, res) { @@ -104,6 +106,16 @@ class ProgramController { return res.status(404).json({ status: 'error', message: 'Program not found' }); } + // Prior-status lookup feeds only the notification gate below — a failure + // here must not break the status update or change the HTTP response. + let prevStatus; + try { + const existing = await programApplicationService.getById(applicationId); + prevStatus = existing?.status; + } catch (err) { + logger.error('Failed to read application prior status for notification gating:', err); + } + const reviewedBy = req.user?.address || req.auth?.address || 'unknown'; const updated = await programApplicationService.updateStatus({ id: applicationId, @@ -115,6 +127,20 @@ class ProgramController { // updateStatus uses .single() which throws if no row matched; translate // to 404 via the Postgres PGRST116 code we see on not-found. res.status(200).json({ status: 'success', data: updated }); + + if (prevStatus === 'submitted' && (updated.status === 'accepted' || updated.status === 'rejected')) { + const eventType = updated.status === 'accepted' ? 'application_accepted' : 'application_rejected'; + try { + await notificationService.notifyProjectTeam( + updated.projectId, + eventType, + updated.id, + { programName: program.name, programSlug: req.params.slug }, + ); + } catch (err) { + logger.error('notifyProjectTeam failed for application status change:', err); + } + } } catch (error) { if (error?.code === 'PGRST116' || /no rows/i.test(error?.message || '')) { return res.status(404).json({ status: 'error', message: 'Application not found' }); diff --git a/server/api/controllers/project.controller.js b/server/api/controllers/project.controller.js index 2983074..1e3dd74 100644 --- a/server/api/controllers/project.controller.js +++ b/server/api/controllers/project.controller.js @@ -2,6 +2,7 @@ import projectService from '../services/project.service.js'; import projectUpdateService from '../services/project-update.service.js'; import fundingSignalService from '../services/funding-signal.service.js'; import paymentService from '../services/payment.service.js'; +import notificationService from '../services/notification.service.js'; import { ALLOWED_CATEGORIES } from '../constants/allowedTech.js'; import { validateSS58, validateM2Submission, validateSimpleUrl, validateProjectUpdate, validateFundingSignal } from '../utils/validation.js'; import { canEditM2Agreement, isSubmissionWindowOpen } from '../utils/dateHelpers.js'; @@ -328,6 +329,11 @@ class ProjectController { return res.status(404).json({ status: "error", message: "Project not found" }); } res.status(200).json({ status: "success", data: updated }); + try { + await notificationService.notifyProjectTeam(projectId, 'm2_approved', projectId, {}); + } catch (err) { + logger.error('notifyProjectTeam failed for m2_approved:', err); + } } catch (error) { console.error("❌ Error approving M2:", error); res.status(500).json({ status: "error", message: "Failed to approve M2" }); @@ -344,15 +350,26 @@ class ProjectController { if (!feedback) { return res.status(422).json({ status: "error", message: "Feedback is required" }); } + const changeRequestDate = new Date().toISOString(); const updated = await projectService.updateProject(projectId, { m2Status: 'building', changeRequestFeedback: feedback, - changeRequestDate: new Date().toISOString(), + changeRequestDate, }); if (!updated) { return res.status(404).json({ status: "error", message: "Project not found" }); } res.status(200).json({ status: "success", data: updated }); + try { + await notificationService.notifyProjectTeam( + projectId, + 'm2_changes_requested', + `${projectId}:${changeRequestDate}`, + { feedback }, + ); + } catch (err) { + logger.error('notifyProjectTeam failed for m2_changes_requested:', err); + } } catch (error) { console.error("❌ Error requesting changes:", error); res.status(500).json({ status: "error", message: "Failed to request changes" }); diff --git a/server/api/repositories/program-application.repository.js b/server/api/repositories/program-application.repository.js index 7cf7b97..15b5b70 100644 --- a/server/api/repositories/program-application.repository.js +++ b/server/api/repositories/program-application.repository.js @@ -64,6 +64,16 @@ class ProgramApplicationRepository { return transform(data); } + async getById(id) { + const { data, error } = await supabase + .from('program_applications') + .select('*') + .eq('id', id) + .maybeSingle(); + if (error) throw error; + return transform(data); + } + /** Phase 1: used by admin review flow (#47, Block F). */ async updateStatus({ id, status, reviewedBy, reviewNotes }) { const { data, error } = await supabase diff --git a/server/api/services/__tests__/notification.service.test.js b/server/api/services/__tests__/notification.service.test.js index 696746e..53bf918 100644 --- a/server/api/services/__tests__/notification.service.test.js +++ b/server/api/services/__tests__/notification.service.test.js @@ -16,9 +16,14 @@ vi.mock('../email-transport.js', () => ({ getEmailTransport: vi.fn(), })); +vi.mock('../project.service.js', () => ({ + default: { getProjectById: vi.fn() }, +})); + const walletContactRepo = (await import('../../repositories/wallet-contact.repository.js')).default; const notificationRepo = (await import('../../repositories/notification.repository.js')).default; const { getEmailTransport } = await import('../email-transport.js'); +const projectService = (await import('../project.service.js')).default; const service = (await import('../notification.service.js')).default; const WALLET = '5Alice'; @@ -130,3 +135,68 @@ describe('NotificationService.notify', () => { expect(second).toBe(sentRow); }); }); + +describe('NotificationService.notifyProjectTeam', () => { + beforeEach(() => { + vi.clearAllMocks(); + getEmailTransport.mockResolvedValue(mockTransport); + notificationRepo.markSent.mockResolvedValue(sentRow); + notificationRepo.markFailed.mockResolvedValue({ ...queuedRow, status: 'failed' }); + }); + + it('calls notify once per non-null team member wallet', async () => { + projectService.getProjectById.mockResolvedValue({ + projectName: 'My Project', + teamMembers: [ + { walletAddress: '5Alice' }, + { walletAddress: null }, + { walletAddress: '5Bob' }, + ], + }); + walletContactRepo.findByWallet.mockResolvedValue(null); + notificationRepo.insertOrGetExisting.mockResolvedValue({ ...queuedRow, status: 'skipped', error: 'no_contact' }); + + const results = await service.notifyProjectTeam('proj-1', 'm2_approved', 'proj-1', {}); + + expect(results).toHaveLength(2); + expect(walletContactRepo.findByWallet).toHaveBeenCalledTimes(2); + expect(walletContactRepo.findByWallet).toHaveBeenCalledWith('5Alice'); + expect(walletContactRepo.findByWallet).toHaveBeenCalledWith('5Bob'); + }); + + it('enriches payload with projectName from the fetched project', async () => { + projectService.getProjectById.mockResolvedValue({ + projectName: 'My Project', + teamMembers: [{ walletAddress: '5Alice' }], + }); + walletContactRepo.findByWallet.mockResolvedValue(null); + notificationRepo.insertOrGetExisting.mockResolvedValue({ ...queuedRow, status: 'skipped', error: 'no_contact' }); + + await service.notifyProjectTeam('proj-1', 'm2_approved', 'proj-1', { extra: 'data' }); + + expect(notificationRepo.insertOrGetExisting).toHaveBeenCalledWith( + expect.objectContaining({ + payload: expect.objectContaining({ projectName: 'My Project', extra: 'data' }), + }), + ); + }); + + it('returns [] when the project has no team members', async () => { + projectService.getProjectById.mockResolvedValue({ projectName: 'My Project', teamMembers: [] }); + const results = await service.notifyProjectTeam('proj-1', 'm2_approved', 'proj-1', {}); + expect(results).toEqual([]); + expect(walletContactRepo.findByWallet).not.toHaveBeenCalled(); + }); + + it('returns [] when getProjectById returns null', async () => { + projectService.getProjectById.mockResolvedValue(null); + const results = await service.notifyProjectTeam('proj-1', 'm2_approved', 'proj-1', {}); + expect(results).toEqual([]); + }); + + it('returns [] and does not throw when getProjectById rejects', async () => { + projectService.getProjectById.mockRejectedValue(new Error('db error')); + const results = await service.notifyProjectTeam('proj-1', 'm2_approved', 'proj-1', {}); + expect(results).toEqual([]); + }); +}); diff --git a/server/api/services/notification.service.js b/server/api/services/notification.service.js index c0e2a44..d672bd2 100644 --- a/server/api/services/notification.service.js +++ b/server/api/services/notification.service.js @@ -2,8 +2,29 @@ import walletContactRepository from '../repositories/wallet-contact.repository.j import notificationRepository from '../repositories/notification.repository.js'; import { getEmailTransport } from './email-transport.js'; import { renderEmail } from './notification-templates/index.js'; +import projectService from './project.service.js'; class NotificationService { + async notifyProjectTeam(projectId, eventType, sourceId, payload) { + try { + const project = await projectService.getProjectById(projectId); + if (!project || !Array.isArray(project.teamMembers) || project.teamMembers.length === 0) { + return []; + } + const enrichedPayload = { projectName: project.projectName, ...payload }; + const results = []; + for (const member of project.teamMembers) { + if (member.walletAddress) { + const row = await this.notify(member.walletAddress, eventType, sourceId, enrichedPayload); + results.push(row); + } + } + return results; + } catch { + return []; + } + } + async notify(walletAddress, eventType, sourceId, payload) { const contact = await walletContactRepository.findByWallet(walletAddress); diff --git a/server/api/services/program-application.service.js b/server/api/services/program-application.service.js index a5c4be0..c496298 100644 --- a/server/api/services/program-application.service.js +++ b/server/api/services/program-application.service.js @@ -1,6 +1,10 @@ import programApplicationRepository from '../repositories/program-application.repository.js'; class ProgramApplicationService { + async getById(id) { + return await programApplicationRepository.getById(id); + } + async listByProgram(programId) { return await programApplicationRepository.listByProgram(programId); }