Skip to content

feat(agreements): emit milestone events from updateMilestone (issue #6)#33

Open
iyanumajekodunmi756 wants to merge 1 commit into
Thalos-Infrastructure:mainfrom
iyanumajekodunmi756:feat/issue-6-emit-milestone-events
Open

feat(agreements): emit milestone events from updateMilestone (issue #6)#33
iyanumajekodunmi756 wants to merge 1 commit into
Thalos-Infrastructure:mainfrom
iyanumajekodunmi756:feat/issue-6-emit-milestone-events

Conversation

@iyanumajekodunmi756

Copy link
Copy Markdown
Contributor

Implements GrantsFox OSS issue #6 (Closes: #6).

Domain code in AgreementsService.updateMilestone() now emits typed events on the in-process bus (from #3) using the shared constants in src/events/agreement-events.ts. NotificationsService listens via @onevent so emails fire end-to-end for both flows.

Behavior:

  • status='approved' -> emits AgreementEventName.MilestoneApproved with the agreement title/amount/asset, milestone index and description enriched from the row (acceptance criterion: approving a milestone notifies participants with the correct amount/description).
  • evidence_description and/or evidence_url set on the DTO -> emits AgreementEventName.EvidenceSubmitted (acceptance criterion: submitting evidence notifies the other participants).
  • Both events may fire on the same call when both conditions hold.

DTO changes (UpdateMilestoneDto):

  • Added optional evidence_description (1..2000 chars), evidence_url (validated by IsUrl), submitter_name (1..120 chars). All backward compatible: existing callers that only send milestone_index/status/actor_wallet keep working.

Error isolation (DoD):

  • AgreementsService.safeEmit() wraps the sync EventEmitter2.emit so a throwing listener never bubbles back into updateMilestone.
  • @onevent handlers in NotificationsService also wrap their await call in try/catch for the async path.
  • safeEmit returns boolean with a warn-level log (not error) so observability distinguishes 'no listeners' from 'sync throw'.

Visual evidence:

  • docs/EMAIL_NOTIFICATIONS_PLAN.md explains how to reproduce locally including the failure-isolation case (RESEND_API_KEY=re_invalid_key).
  • docs/samples/milestone-events.http has ready-to-run curl snippets for both flows.

closes #6

…halos-Infrastructure#6)

Implements GrantsFox OSS issue Thalos-Infrastructure#6 (Closes: Thalos-Infrastructure#6).

Domain code in AgreementsService.updateMilestone() now emits typed events
on the in-process bus (from Thalos-Infrastructure#3) using the shared constants in
src/events/agreement-events.ts. NotificationsService listens via @onevent
so emails fire end-to-end for both flows.

Behavior:
- status='approved' -> emits AgreementEventName.MilestoneApproved with
  the agreement title/amount/asset, milestone index and description
  enriched from the row (acceptance criterion: approving a milestone
  notifies participants with the correct amount/description).
- evidence_description and/or evidence_url set on the DTO -> emits
  AgreementEventName.EvidenceSubmitted (acceptance criterion: submitting
  evidence notifies the other participants).
- Both events may fire on the same call when both conditions hold.

DTO changes (UpdateMilestoneDto):
- Added optional evidence_description (1..2000 chars),
  evidence_url (validated by IsUrl), submitter_name (1..120 chars).
  All backward compatible: existing callers that only send
  milestone_index/status/actor_wallet keep working.

Error isolation (DoD):
- AgreementsService.safeEmit() wraps the sync EventEmitter2.emit so a
  throwing listener never bubbles back into updateMilestone.
- @onevent handlers in NotificationsService also wrap their await call
  in try/catch for the async path.
- safeEmit returns boolean with a warn-level log (not error) so
  observability distinguishes 'no listeners' from 'sync throw'.

Visual evidence:
- docs/EMAIL_NOTIFICATIONS_PLAN.md explains how to reproduce locally
  including the failure-isolation case (RESEND_API_KEY=re_invalid_key).
- docs/samples/milestone-events.http has ready-to-run curl snippets
  for both flows.
@vercel

vercel Bot commented Jun 21, 2026

Copy link
Copy Markdown

@iyanumajekodunmi756 is attempting to deploy a commit to the ManuelJG's projects Team on Vercel.

A member of the Team first needs to authorize it.

@leandromasotti

leandromasotti commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Hello @iyanumajekodunmi756, your PR looks good. Could you please resolve the conflicts? Thank you for your colaboration

@ManuelJG1999

Copy link
Copy Markdown
Collaborator

@iyanumajekodunmi756

Could you please resolve the conflicts?

@ManuelJG1999

Copy link
Copy Markdown
Collaborator

@iyanumajekodunmi756

Could you please resolve the conflicts?
please share an update ASAP, otherwise, we might have to re-assign the issue.

Looking forward to your PR's resolution.

Thank you.

@Kalchaqui

Copy link
Copy Markdown
Collaborator

Hi @iyanumajekodunmi756 — solid work on #6 and thanks for the docs/samples. @leandromasotti’s approval still stands on the approach, but this PR cannot merge as-is.

Blockers

  1. Rebase required — branch is far behind main with conflicts in ~5 files (agreements.service.ts, update-milestone.dto.ts, notifications.service.ts, templates/types). ManuelJG pinged 3×; please rebase ASAP.

  2. Partial overlap with merged PR Issue 6 milestone events #50main already has milestone evidence DTO fields and @OnEvent listeners, but updateMilestone() still does not emit milestone.approved / evidence.submitted. Your PR adds the missing emission logic (+ safeEmit), which is what we need — but please rebase and avoid duplicating listener code already on main.

  3. DTO alignmentmain uses evidence_urls?: string[]; this PR uses single evidence_url. Pick one shape (prefer keeping main’s array unless you document a breaking change) and update emitMilestoneEvents() + email template accordingly.

After rebase, keep:

  • emitMilestoneEvents() + safeEmit() in AgreementsService
  • Evidence persistence on milestone JSON
  • Docs / .http samples (great proof)

Drop / reconcile on rebase:

  • Duplicate @OnEvent handlers if already present on main
  • Conflicting event constant imports — use the canonical path on main

DoD checklist post-rebase:

  • Approve milestone → participants get email with correct amount/description
  • Submit evidence → other participants notified
  • RESEND_API_KEY=re_invalid_key → API still returns success (failure isolation)
  • pnpm run build passes
  • Closes #6 remains in PR body

Please push the rebased branch and we can merge quickly. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Email Notifications - 04 - Emit milestone events

4 participants