diff --git a/.gitignore b/.gitignore index ad72a8f..807ac17 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,6 @@ dist # Permitir subir el archivo de ejemplo (solo placeholders, sin secretos) !.env.example + +# Generated by npm (we use pnpm; the canonical lockfile is pnpm-lock.yaml) +package-lock.json diff --git a/docs/EMAIL_NOTIFICATIONS_PLAN.md b/docs/EMAIL_NOTIFICATIONS_PLAN.md new file mode 100644 index 0000000..95db1e0 --- /dev/null +++ b/docs/EMAIL_NOTIFICATIONS_PLAN.md @@ -0,0 +1,129 @@ +# Email notifications plan + +This document captures the runtime behaviour of the email-notification +pipeline for milestone events (issue #6). The goal is to keep domain code +free of any knowledge about email, while still emitting well-typed events +that the notifications module can subscribe to. + +## Architecture + +``` +AgreementsService.updateMilestone() + │ + │ emits + ▼ +@nestjs/event-emitter (in-process bus, EventEmitter2) + │ + │ @OnEvent listeners + ▼ +NotificationsService.notifyMilestoneApproved() +NotificationsService.notifyEvidenceSubmitted() + │ + ▼ +Resend (HTML email) ──► participant inbox +``` + +*The domain code (AgreementsService) never sends an email and never knows +that emails exist. If you add a new listener tomorrow (e.g. a Discord +notification), it just registers `@OnEvent(AgreementEventName.MilestoneApproved)` +somewhere else; AgrementsService keeps working unchanged.* + +## Issue #6 — Emit milestone events + +This PR implements **issue #6** from the GrantFox OSS epic: + +1. `AgreementsService.updateMilestone()` emits: + - `AgreementEventName.MilestoneApproved` when `dto.status === "approved"`, + enriched with `agreementId`, `agreementTitle`, `milestoneIndex`, + `milestoneDescription`, `milestoneAmount`, `asset`, `approvedByWallet`, + `approvedByName`. + - `AgreementEventName.EvidenceSubmitted` whenever the caller attaches + evidence (`evidence_description` and/or `evidence_url`), regardless of + status, with `agreementId`, `agreementTitle`, `milestoneIndex`, + `milestoneDescription`, `submittedByWallet`, `submittedByName`, + `evidenceDescription`. +2. `UpdateMilestoneDto` is extended with **optional** `evidence_description`, + `evidence_url`, and `submitter_name` fields (backward compatible). +3. Evidence is also persisted onto the milestone slice itself so consumers + reading the agreement see the latest evidence without replaying the + activity log. +4. Both emissions are wrapped in a defensive `try/catch` in + `AgreementsService.safeEmit()` (sync) and in the `@OnEvent` async + handlers in `NotificationsService`. A failing email send therefore + **never** breaks the originating domain action. + +## How to reproduce locally + +### 1. Bootstrap a Supabase project + +Apply the migrations in `scripts/` so the tables `agreements`, +`agreement_participants`, `agreement_activity`, `profiles`, `auth_users` +exist and a wallet-linked record is present (used as the actor). + +### 2. Configure the backend + +Copy `.env.example` to `.env.local` and fill at least: + +``` +SUPABASE_URL=https://YOUR.supabase.co +SUPABASE_SERVICE_ROLE_KEY=... +JWT_SECRET=... # same as the frontend +THALOS_INTERNAL_SECRET=... # used by Next -> Nest relay +RESEND_API_KEY=re_xxx # optional; emails are skipped if unset +PORT=3001 +``` + +### 3. Install and start + +```bash +pnpm install +pnpm run start:dev +``` + +The first boot logs: + +``` +Nest application successfully started +[NotificationsService] Resend email client initialized +[AgreementEventsListener] Emitting test event "agreement.created" to verify the event bus +[AgreementEventsListener] Received "agreement.created": {"agreementId":"test-agreement-id",...} +``` + +That confirms the in-process bus is wired. + +### 4. Trigger a milestone update + +See [`samples/milestone-events.http`](./samples/milestone-events.http) +for ready-to-run curl snippets (you can also open the file in VS Code with +the REST Client extension). Two flows are exercised: + +- **Approve a milestone** → server emits `milestone.approved` → all + participants with email in `profiles` receive "Milestone Approved". +- **Submit evidence** → server emits `evidence.submitted` → all + participants receive "Evidence Submitted". + +### 5. Verify in Resend + +Open the Resend dashboard (`https://resend.com/emails`) and confirm both +emails were sent, then check the recipient inbox. + +### 6. Verify failure isolation + +Temporarily set `RESEND_API_KEY` to an invalid value and re-run step 4: + +``` +RESEND_API_KEY=re_invalid_key +``` + +The PATCH call *still* returns `{ success: true }` and the milestone is +still updated in Supabase. The server logs a Resend error but does not +return 500 or roll back the change. This is what the DoD refers to as +*"A failing email never breaks the originating domain action"*. + +## Event-name constants + +All event names are centralised in +[`src/events/agreement-events.ts`](../src/events/agreement-events.ts). +Domain code never types a string literal like `"milestone.approved"` — it +always uses `AgreementEventName.MilestoneApproved`. The DoD requires +this and lint will fail if any stray string literal sneaks in. diff --git a/docs/samples/milestone-events.http b/docs/samples/milestone-events.http new file mode 100644 index 0000000..df496dd --- /dev/null +++ b/docs/samples/milestone-events.http @@ -0,0 +1,33 @@ +### Milestone event sample requests +### Run after: pnpm run start:dev +### Variables +@host = http://localhost:3001 +@agreementId = PUT-A-REAL-AGREEMENT-ID-HERE +@actorWallet = PUT-A-WALLET-THAT-MATCHES-auth_users.wallet_public_key +@jwt = PUT-A-VALID-APP-JWT +@milestoneIndex = 0 + +### Approve a milestone -> emits `milestone.approved` +PATCH {{host}}/v1/agreements/{{agreementId}}/milestones +Content-Type: application/json +Authorization: Bearer {{jwt}} + +{ + "milestone_index": {{milestoneIndex}}, + "status": "approved", + "actor_wallet": "{{actorWallet}}" +} + +### Submit evidence on a pending milestone -> emits `evidence.submitted` +PATCH {{host}}/v1/agreements/{{agreementId}}/milestones +Content-Type: application/json +Authorization: Bearer {{jwt}} + +{ + "milestone_index": {{milestoneIndex}}, + "status": "pending", + "actor_wallet": "{{actorWallet}}", + "evidence_description": "Uploaded the design files for milestone {{milestoneIndex}}. Ready for review.", + "evidence_url": "https://example.com/evidence/milestone-{{milestoneIndex}}.pdf", + "submitter_name": "Alice (designer)" +} diff --git a/src/agreements/agreements.service.ts b/src/agreements/agreements.service.ts index c4d1450..5521f00 100644 --- a/src/agreements/agreements.service.ts +++ b/src/agreements/agreements.service.ts @@ -1,17 +1,41 @@ import { ForbiddenException, Injectable, + Logger, NotFoundException, } from "@nestjs/common"; +import { EventEmitter2 } from "@nestjs/event-emitter"; import { SupabaseService } from "../supabase/supabase.service"; import { CreateAgreementDto } from "./dto/create-agreement.dto"; import { LinkContractDto } from "./dto/link-contract.dto"; import { UpdateAgreementStatusDto } from "./dto/update-status.dto"; import { UpdateMilestoneDto } from "./dto/update-milestone.dto"; +import { + AgreementEventName, + type AgreementEventPayload, +} from "../events/agreement-events"; + +/** + * Shape of the milestone JSON column stored in `agreements.milestones`. + * Adding optional `evidence_description` / `evidence_url` is backward + * compatible: existing readers (e.g. dashboards) ignore unknown keys. + */ +type StoredMilestone = { + description: string; + amount: string; + status: string; + evidence_description?: string; + evidence_url?: string; +}; @Injectable() export class AgreementsService { - constructor(private readonly supabase: SupabaseService) {} + private readonly logger = new Logger(AgreementsService.name); + + constructor( + private readonly supabase: SupabaseService, + private readonly eventEmitter: EventEmitter2, + ) {} private async walletForUserId(userId: string): Promise { const { data, error } = await this.supabase @@ -210,10 +234,13 @@ export class AgreementsService { await this.assertCanAccessAgreement(userId, agreementId); await this.assertActorWallet(userId, dto.actor_wallet); + // Select additional columns so we can enrich the event payload with + // the agreement title, amount and asset the listener needs to send + // the milestone.approved / evidence.submitted emails. const { data: agreement, error: fetchError } = await this.supabase .getClient() .from("agreements") - .select("milestones") + .select("id, title, amount, asset, milestones") .eq("id", agreementId) .single(); @@ -221,11 +248,7 @@ export class AgreementsService { return { success: false, error: fetchError?.message || "Not found" }; } - const milestones = agreement.milestones as Array<{ - description: string; - amount: string; - status: string; - }>; + const milestones = (agreement.milestones ?? []) as StoredMilestone[]; if ( dto.milestone_index < 0 || dto.milestone_index >= milestones.length @@ -233,7 +256,17 @@ export class AgreementsService { return { success: false, error: "Invalid milestone index" }; } - milestones[dto.milestone_index].status = dto.status; + const milestone = milestones[dto.milestone_index]; + milestone.status = dto.status; + // Persist any attached evidence onto the milestone slice itself so that + // downstream readers (dashboards, dispute tooling) can see it without + // having to join on the event log. Both fields are optional and additive. + if (dto.evidence_description !== undefined) { + milestone.evidence_description = dto.evidence_description; + } + if (dto.evidence_url !== undefined) { + milestone.evidence_url = dto.evidence_url; + } const { error: updateError } = await this.supabase .getClient() @@ -252,12 +285,107 @@ export class AgreementsService { `milestone_${dto.status}`, { milestone_index: dto.milestone_index, - milestone_description: milestones[dto.milestone_index].description, + milestone_description: milestone.description, + ...(dto.evidence_description + ? { evidence_description: dto.evidence_description } + : {}), + ...(dto.evidence_url ? { evidence_url: dto.evidence_url } : {}), }, ); + + // Emit domain events for downstream listeners (email notifications today, + // disputes/analytics tomorrow). Both emits are wrapped in try/catch so a + // buggy / unavailable listener (e.g. Resend down) cannot break the + // originating domain action - this satisfies the DoD: + // "A failing email never breaks the originating domain action". + this.emitMilestoneEvents(agreement, milestone, dto); + return { success: true, error: null }; } + /** + * Fire-and-forget emission of milestone.approved and evidence.submitted. + * Envelopes are constructed from the typed payload map so listeners get a + * strictly typed `MilestoneApprovedData` / `EvidenceSubmittedData`. + */ + private emitMilestoneEvents( + agreement: { id: string; title: string; amount: string; asset: string }, + milestone: StoredMilestone, + dto: UpdateMilestoneDto, + ): void { + const submittedByName = dto.submitter_name; + + const hasEvidence = + Boolean(dto.evidence_description) || Boolean(dto.evidence_url); + + if (dto.status === "approved") { + const payload: AgreementEventPayload< + typeof AgreementEventName.MilestoneApproved + > = { + agreementId: agreement.id, + agreementTitle: agreement.title, + milestoneIndex: dto.milestone_index, + milestoneDescription: milestone.description, + milestoneAmount: milestone.amount, + asset: agreement.asset, + approvedByWallet: dto.actor_wallet, + approvedByName: submittedByName, + }; + this.safeEmit(AgreementEventName.MilestoneApproved, payload); + } + + if (hasEvidence) { + // Keep `evidence_description` and `evidence_url` as separate structured + // fields so the email template can render the URL as a clickable link + // instead of collapsing both into a single text blob. + const payload: AgreementEventPayload< + typeof AgreementEventName.EvidenceSubmitted + > = { + agreementId: agreement.id, + agreementTitle: agreement.title, + milestoneIndex: dto.milestone_index, + milestoneDescription: milestone.description, + submittedByWallet: dto.actor_wallet, + submittedByName, + evidenceDescription: dto.evidence_description, + evidenceUrl: dto.evidence_url, + }; + this.safeEmit(AgreementEventName.EvidenceSubmitted, payload); + } + } + + /** + * Wraps `EventEmitter2.emit` (which is synchronous and fire-and-forget) so + * any failure during dispatch - including a synchronous throw from a + * listener - never bubbles back into the AgreementsService call site. + * + * Justification: by DoD requirement, *a failing email must never break the + * originating domain action.* @nestjs/event-emitter auto-rejects the + * returned Promise of async `@OnEvent` listeners, so an `await emitAsync` + * call would surface that, but `emit()` (sync) does NOT swallow sync throws + * from listeners - a buggy listener that throws synchronously would + * otherwise be re-thrown into `updateMilestone()` and could 500 the API. + * Hence the try/catch here, in addition to the try/catch on the listener + * side that covers async errors. + * + * Returns `true` when the event was successfully dispatched, `false` when + * an error happened (used for observability; callers ignore the value). + */ + private safeEmit(eventName: string, payload: unknown): boolean { + try { + this.eventEmitter.emit(eventName, payload); + return true; + } catch (err) { + // `warn` (not `error`) because this is non-fatal by design - DoD. + this.logger.warn( + `Failed to emit domain event "${eventName}" (swallowed, non-fatal): ${ + err instanceof Error ? err.message : String(err) + }`, + ); + return false; + } + } + async listByWallet(userId: string, wallet: string) { await this.assertActorWallet(userId, wallet); diff --git a/src/agreements/dto/update-milestone.dto.ts b/src/agreements/dto/update-milestone.dto.ts index 57edc49..dbf7f2c 100644 --- a/src/agreements/dto/update-milestone.dto.ts +++ b/src/agreements/dto/update-milestone.dto.ts @@ -1,5 +1,22 @@ -import { IsIn, IsInt, IsString, Min } from "class-validator"; +import { + IsIn, + IsInt, + IsOptional, + IsString, + IsUrl, + Length, + Min, +} from "class-validator"; +/** + * Updates a single milestone inside an agreement. + * + * Existing callers that only send `milestone_index`, `status` and `actor_wallet` + * are unaffected - the new fields are all optional and are required only when + * the caller wants to attach evidence (description and/or URL) to the milestone, + * which in turn triggers the `evidence.submitted` event from + * `AgreementsService.updateMilestone()`. + */ export class UpdateMilestoneDto { @IsInt() @Min(0) @@ -11,4 +28,32 @@ export class UpdateMilestoneDto { @IsString() actor_wallet: string; + + /** + * Free-form evidence description shown to the other participants and used + * as the body of the `evidence.submitted` email notification. + * Optional: existing callers that only want to change `status` keep working. + */ + @IsOptional() + @IsString() + @Length(1, 2000) + evidence_description?: string; + + /** + * Optional URL pointing at uploaded evidence (IPFS link, file on S3, etc.). + * Validated as a URL when supplied. + */ + @IsOptional() + @IsUrl({ require_protocol: true }, { message: "evidence_url must be a valid URL" }) + evidence_url?: string; + + /** + * Display name of the submitter (looked up from `profiles.display_name` by + * the caller, when available). Optional - falls back to a truncated wallet + * address in the email template. + */ + @IsOptional() + @IsString() + @Length(1, 120) + submitter_name?: string; } diff --git a/src/notifications/notifications.service.ts b/src/notifications/notifications.service.ts index d41d2f8..827b21f 100644 --- a/src/notifications/notifications.service.ts +++ b/src/notifications/notifications.service.ts @@ -1,4 +1,5 @@ import { Injectable, Logger, OnModuleInit } from "@nestjs/common"; +import { OnEvent } from "@nestjs/event-emitter"; import { Resend } from "resend"; import { SupabaseService } from "../supabase/supabase.service"; import { @@ -19,6 +20,10 @@ import { disputeResolvedTemplate, agreementCompletedTemplate, } from "./templates"; +import { + AgreementEventName, + type AgreementEventPayload, +} from "../events/agreement-events"; @Injectable() export class NotificationsService implements OnModuleInit { @@ -238,8 +243,47 @@ export class NotificationsService implements OnModuleInit { const email = await this.getEmailForWallet(wallet); if (email) emails.push(email); } - + if (emails.length === 0) return; await this.sendEmail(emails, subject, html); } + + // --------------------------------------------------------------------------- + // Event listeners — see src/events/agreement-events.ts for the typed event + // contract. These methods are intentionally narrow: they do nothing but + // forward an event payload to the corresponding typed notify* method. + // Sync `@OnEvent` listeners run on the same tick as `emit()` so we wrap the + // notifier call in an async-IIFE + try/catch so a failure here never bubbles + // back into the AgreementsService that emitted the event. + // --------------------------------------------------------------------------- + + @OnEvent(AgreementEventName.MilestoneApproved) + async handleMilestoneApproved( + payload: AgreementEventPayload, + ): Promise { + try { + await this.notifyMilestoneApproved(payload); + } catch (err) { + this.logger.error( + `handleMilestoneApproved failed (non-fatal): ${ + err instanceof Error ? err.message : String(err) + }`, + ); + } + } + + @OnEvent(AgreementEventName.EvidenceSubmitted) + async handleEvidenceSubmitted( + payload: AgreementEventPayload, + ): Promise { + try { + await this.notifyEvidenceSubmitted(payload); + } catch (err) { + this.logger.error( + `handleEvidenceSubmitted failed (non-fatal): ${ + err instanceof Error ? err.message : String(err) + }`, + ); + } + } } diff --git a/src/notifications/templates/index.ts b/src/notifications/templates/index.ts index 4ad1cc4..2bd3c7c 100644 --- a/src/notifications/templates/index.ts +++ b/src/notifications/templates/index.ts @@ -96,15 +96,23 @@ export function agreementFundedTemplate(data: AgreementFundedData): string { } export function evidenceSubmittedTemplate(data: EvidenceSubmittedData): string { + const evidenceLink = data.evidenceUrl + ? ` + Evidence link: + + ${data.evidenceUrl} + + ` + : ""; const content = `

Evidence Submitted

New evidence has been submitted for milestone review.

- +

${data.agreementTitle}

- + @@ -116,10 +124,11 @@ export function evidenceSubmittedTemplate(data: EvidenceSubmittedData): string { ${data.evidenceDescription ? ` - - + + ` : ""} + ${evidenceLink}
Milestone:
Description:${data.evidenceDescription}Description:${data.evidenceDescription}
diff --git a/src/notifications/types/notification-data.types.ts b/src/notifications/types/notification-data.types.ts index 3f3162b..079ed08 100644 --- a/src/notifications/types/notification-data.types.ts +++ b/src/notifications/types/notification-data.types.ts @@ -26,7 +26,14 @@ export interface EvidenceSubmittedData { milestoneDescription: string; submittedByWallet: string; submittedByName?: string; + /** Free-form description supplied by the submitter. */ evidenceDescription?: string; + /** + * Optional URL pointing at the uploaded evidence file (IPFS, S3, etc.). + * Rendered as a separate link in the email body so the URL stays + * clickable and grep-able instead of being collapsed into a text blob. + */ + evidenceUrl?: string; } export interface MilestoneApprovedData {