Skip to content

fix(work-orders): email everyone who has to act; tell builders about terminate and withdraw - #87

Merged
itexpert120 merged 3 commits into
stagingfrom
fix/work-order-emails
Oct 31, 2026
Merged

itexpert120 merged 3 commits into
stagingfrom
fix/work-order-emails

Conversation

@saadiqbal-dev

Copy link
Copy Markdown
Collaborator

What changes

Fixes the gaps found in the email-flow review: moments where someone has to act but got no email.

Emails for people who have to act

  • Documenso: the representative is emailed to sign a new work order first ("Sign Work Order X to send it to …"), with a link to their signing page. Before, nothing went out until they had signed, so an order could stall silently.
  • Builders are emailed when their report is ready to send (with the draft report), when the period ends soon, when the payout account needs confirming, and when the order isn't renewed.
  • Representatives are emailed when a renewal needs a decision (met/missed targets listed) and when the builder declines, in MultiAgency or in Documenso.

Terminate and delete

  • Terminate tells the builder by email and in-app: no new payout requests, and payouts already sent aren't affected.
  • Delete of an offered order tells the builder by email and in-app that it was withdrawn and the earlier links no longer work. The in-app notice links to My work.
  • Both only go to a builder who was actually sent the order: nothing for drafts, or for Documenso orders the representative hadn't signed yet.

Representative fallback

  • Agency emails, new documents and Documenso fall back to the Agency's default representative (name and email) when the work order has none.

Copy

  • The "signed" email says it starts on its own unless something still blocks it. Before, it said it activated when the period started.
  • The "active" email says "starts on " when the period is in the future.
  • In-app notices no longer tell managers to activate it themselves.
  • A representative rejecting their own order in Documenso no longer notifies managers that the builder declined.
  • "Not renewed" emails only the builder, since the representative made that decision.

Review

  • api/src/services/work-orders/emails.ts: new email cases and copy
  • api/src/services/work-orders/service.ts:
    • contactsOf fallback
    • sentToBuilder
    • triggers in offer, syncEnvelope, terminate and remove, on top of staging's safer terminate/delete
  • api/src/services/notifications.ts: work_order_terminated / work_order_withdrawn, activation copy

Testing

  • pnpm typecheck is clean.
  • API tests: 809 passed. UI tests: 297 passed.
  • New tests cover:
    • who gets each email
    • sign-first link
    • signed/active wording
    • the withdrawn email
    • the default-representative fallback
    • terminate/delete notices, and staying quiet when the builder never saw the order
    • Documenso decline by builder vs representative

@multi-review multi-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline findings for e5d9982; see the summary comment for the rest.

Comment thread api/src/services/work-orders/service.ts Outdated
Comment on lines +513 to +514
const representativeEmail =
row.representativeEmail ?? fallback?.defaultRepresentativeEmail ?? null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · correctness: Default-representative fallback puts the wrong person's email on in-app countersignatures and signed PDFs

countersign (l.3878–3887) records the signer's email as contacts.representativeEmail ?? <actor's own email>. Before this change contacts.representativeEmail was just row.representativeEmail, so on a work order with no representative the actual signer's email was stored. Now contactsOf falls back to the Agency's default representative.

Scenario: the Agency's default representative is Dana, and admin Bob countersigns a work order that has no representative set. The signature row gets name "Bob" (typed), userId Bob, but email dana@…. finalizeSigned (l.991–998) then prints that email on the signed PDF's signature page, so the legal signature record names one person with another person's email.

Suggested fix: In countersign, use row.representativeEmail ?? actorEmail instead of the fallback value. Alternatively, make contactsOf expose whether the email came from the fallback and skip the fallback when recording a signature.

@multi-review

multi-review Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Multi Review · 8d71585 (changes since e5d9982)

This commit fixes the three problems raised in earlier reviews, and I found nothing new to flag. contactsOf now uses the Agency's default representative as a whole (name and email together) only when the work order has no representative email. countersign goes back to recording the work order's own representative email, or the signer's own email if there isn't one. sentToBuilder now checks the live Documenso envelope when local records still show the order waiting on the representative, so a builder who already has a signing invite gets the terminate notice.

No new issues found · 3 still open from earlier

What was checked (8 areas)
  • contactsOf and its callers: announce (l.537), l.694, l.906, sendForSignature (l.1428), offer email check (l.3118), in-app accept (l.4519)
  • the three combinations in the new contactsOf: row has an email; no row email but a default exists; no row email and no default
  • countersign signer email: work order's representative email, else the signer's own email
  • sentToBuilder: how it uses signingOf / signingFrom (awaitingRepresentative) and the checkEnvelope fallback when no state is passed
  • terminate: sentToBuilder is called before cancelEnvelope, and what happens when the Documenso check fails
  • remove: passes checked?.state ?? null, and when checkEnvelope returns null (no envelope)
  • checkEnvelope / pendingEnvelope guards
  • new integration tests for default-rep pairing, countersign email, and terminating before/after the representative signs in Documenso

Still open from earlier reviews

  • 🟠 Medium api/src/services/work-orders/service.ts:513 Default-representative fallback puts the wrong person's email on in-app countersignatures and signed PDFs
  • 🟡 Low api/src/services/work-orders/service.ts:518 Representative name from the work order gets paired with the default representative's email
  • 🟡 Low api/src/services/work-orders/service.ts:3936 Terminate skips the builder notice when the representative has signed in Documenso but the sweep hasn't recorded it yet

Comment @multi-review to re-review new commits, or @multi-review full for a full review.

@saadiqbal-dev

Copy link
Copy Markdown
Collaborator Author

Fixed in 8d71585:

  • Countersignature email (medium): an in-app countersignature now records the signer's own email, not the default representative's.
  • Name/email pairing (low): a work order without a representative email now uses the Agency's default representative's name and email together, never one person's name with another's email.
  • Terminate (low): terminate asks Documenso whether the representative already signed before deciding the builder never saw the order, the same check delete does.

Tests added for each. Typecheck is clean and all API tests pass (812).

@multi-review

@jlwaugh

jlwaugh commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Pushed fixes on top of a8ee630 (3b141a7). Typecheck, lint and all tests pass, and each change has a test that fails without it.

  • A builder drafting their own payout request isn't emailed that it's ready.
  • Asking for the payout account of a Documenso offer is refused, since signing confirms it.
  • An envelope cancelled in Documenso notifies managers and emails the representative, instead of going back to draft silently.
  • Terminating an offer the builder never accepted sends "withdrawn", not "terminated". The withdrawn notice now tells the builder to ignore any signing link, instead of saying the links no longer work. This supersedes the description's Terminate and Delete bullets.
  • Whether the period has started is decided in the agency's time zone, and the offer email gives the period's dates.
  • The "signs it first" message names the default representative.

Not in this PR:

  • A representative declining in Documenso notifies nobody.
  • A builder isn't told when the envelope is cancelled in Documenso.
  • Older builder-first orders can stay "accepted" after a decline or cancel. This predates this PR; follow-up PR to come.
  • An in-app countersignature records the order's representative email when one is set, even if a different manager signs.

Nothing needed from you.

@itexpert120
itexpert120 merged commit 3045694 into staging Oct 31, 2026
5 checks passed
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.

3 participants