diff --git a/app/components/agreements/SendAgreementModal.test.tsx b/app/components/agreements/SendAgreementModal.test.tsx new file mode 100644 index 00000000..1a3e8fad --- /dev/null +++ b/app/components/agreements/SendAgreementModal.test.tsx @@ -0,0 +1,32 @@ +// @vitest-environment happy-dom +/** + * DELIV-01 — the API now refuses to send an agreement that still carries + * placeholders. That refusal used to land in a page-level line underneath the + * modal's scrim, so Send looked like it did nothing. It must read inside the + * dialog the inspector is looking at. + */ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; + +import { SendAgreementModal } from "./SendAgreementModal"; + +const base = { + open: true, + onSend: () => {}, + onClose: () => {}, + initialSigners: [{ name: "Pat Rivera", email: "pat@example.com", role: "client" as const }], +}; + +describe("SendAgreementModal serverError", () => { + it("shows the API's refusal as an alert inside the dialog", () => { + render(); + const alert = screen.getByRole("alert"); + expect(alert.textContent).toContain("cannot be sent yet"); + expect(alert.closest('[role="dialog"]')).not.toBeNull(); + }); + + it("shows nothing when there is no refusal", () => { + render(); + expect(screen.queryByRole("alert")).toBeNull(); + }); +}); diff --git a/app/components/agreements/SendAgreementModal.tsx b/app/components/agreements/SendAgreementModal.tsx index 9e0ffe24..97189af8 100644 --- a/app/components/agreements/SendAgreementModal.tsx +++ b/app/components/agreements/SendAgreementModal.tsx @@ -101,6 +101,7 @@ export function SendAgreementModal({ busy, initialSigners, templates, + serverError, }: { open: boolean; onSend: (payload: SendAgreementPayload) => void; @@ -109,6 +110,8 @@ export function SendAgreementModal({ initialSigners?: SignerDraft[]; /** Agreement templates to choose from. Omit to hide the picker entirely. */ templates?: AgreementTemplateOption[]; + /** The API's refusal of the last send. Shown inside the dialog: a page-level line sits under the modal's scrim and reads as "Send did nothing". */ + serverError?: string | null; }) { const seed = (): SignerDraftRow[] => { if (initialSigners && initialSigners.length > 0) { @@ -272,7 +275,7 @@ export function SendAgreementModal({ - {error &&

{error}

} + {(error ?? serverError) &&

{error ?? serverError}

} ); } diff --git a/app/routes/inspector-portal.tsx b/app/routes/inspector-portal.tsx index 81ee9e89..344a51ca 100644 --- a/app/routes/inspector-portal.tsx +++ b/app/routes/inspector-portal.tsx @@ -1274,12 +1274,10 @@ export default function InspectionHubPage() { : undefined } busy={agreementModal.busy} + serverError={agreementModal.error} onSend={submitSendAgreement} onClose={() => agreementModal.setOpen(false)} /> - {agreementModal.error && ( -

{agreementModal.error}

- )} {/* Inspector pre-sign — the envelope-level signature an inspector applies before the client sees it. Moved off the Library page with the rest of diff --git a/docs/develop/fork-log.md b/docs/develop/fork-log.md index c88e8f4f..9f740821 100644 --- a/docs/develop/fork-log.md +++ b/docs/develop/fork-log.md @@ -331,3 +331,4 @@ deliberate departure rather than work upstream simply has not done yet. - Record skips as explicitly as takes. "We looked and said no" and "we never looked" are the same silence otherwise. - Update the snapshot when you fetch, not when you remember. +| `agreement` merge fields filled at send, placeholders block the send (2026-10-13 QA sweep DELIV-01, Blocker; no upstream fix) | `AgreementService.findOrCreate` (`server/services/agreement/envelope-legacy.ts`) runs the template through `fillAgreementTokens` (`server/lib/agreement-tokens.ts`) before it freezes `content_snapshot`: `[INSPECTOR_NAME]` (assigned inspector, else the company's legal or company name), `[CUSTOMER_NAME]`, `[PROPERTY_ADDRESS]`, `[INSPECTION_DATE]` (long civil day) and `$[AMOUNT]` (`getEffectivePriceCents`: invoice, then service lines, then `inspections.price`); `[CLIENT_SIGNATURE_BLOCK]` and `[SIGN_DATE]` become "(signature captured below)" and "(recorded at signing)", values HTML-escaped. If any other bracketed ALL-CAPS token remains, or the starter's "Review before sending to real customers" notice is still in the text, it throws 422 naming them and creates no envelope. `SendAgreementModal` takes `serverError` and shows it inside the dialog (the page-level line it replaces sat under the scrim, so Send looked dead). Specs: `tests/unit/agreements/agreement-send-tokens.spec.ts`, `app/components/agreements/SendAgreementModal.test.tsx` | The finding: a fresh company's only template could be sent and signed with every merge field still in brackets and the admin caveat in the permanent record. `git log main..upstream/main` by keyword (placeholder, merge field, starter agreement, agreement) and on the fixture file finds nothing; the upstream plan lists it under its PR 5 but upstream has not landed it. The starter text is unchanged, so an untouched starter is refused (notice and the liability clause both stay) until the company edits it in Settings; that is the intended state of a legal document, not a gap. **Known ceilings:** the match is `[A-Z][A-Z0-9_]{2,}`-style brackets only, so a custom template with lowercase or `{{x}}` fields is sent as written; the check runs only when an envelope is created (reusing a live envelope does not re-check); an inspection with no price blocks on `[AMOUNT]` rather than printing $0.00; the signing page itself still shows what the snapshot holds, so envelopes sent before this change keep their placeholders | diff --git a/server/lib/agreement-tokens.ts b/server/lib/agreement-tokens.ts new file mode 100644 index 00000000..85af2d3b --- /dev/null +++ b/server/lib/agreement-tokens.ts @@ -0,0 +1,61 @@ +/** + * Send-time merge fields for an agreement template. + * + * The starter template ships `[INSPECTOR_NAME]`, `[CUSTOMER_NAME]`, + * `[PROPERTY_ADDRESS]`, `[INSPECTION_DATE]` and `$[AMOUNT]`. They are filled + * from the inspection when the envelope is created, because the envelope's + * `content_snapshot` is the permanent signed record: whatever is still in + * brackets here is what the client signs and what a dispute produces. + * + * Anything bracketed that is NOT filled, and the starter's "review before + * sending" notice, block the send instead (`blockingAgreementIssues`). + */ + +export interface AgreementTokenValues { + inspector?: string | null | undefined; + client?: string | null | undefined; + address?: string | null; + /** Civil day, `YYYY-MM-DD` (an `T…Z` suffix is ignored). */ + date?: string | null; + amountCents?: number | null; +} + +/** Substring of the starter template's admin-only caveat paragraph. */ +const STARTER_NOTICE_MARK = 'Review before sending to real customers'; + +const esc = (s: string): string => + s.replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"'); + +const longDay = (d: string): string | null => { + const t = new Date(`${d.slice(0, 10)}T12:00:00Z`); + return Number.isNaN(t.getTime()) ? null : t.toLocaleDateString('en-US', { timeZone: 'UTC', dateStyle: 'long' }); // i18n-lint-ok: the agreement body is English-only and frozen into content_snapshot +}; + +export function fillAgreementTokens(html: string, v: AgreementTokenValues): string { + const day = v.date ? longDay(v.date) : null; + const map: Record = { + INSPECTOR_NAME: v.inspector?.trim() || null, + CUSTOMER_NAME: v.client?.trim() || null, + PROPERTY_ADDRESS: v.address?.trim() || null, + INSPECTION_DATE: day, + AMOUNT: v.amountCents != null && v.amountCents > 0 ? (v.amountCents / 100).toFixed(2) : null, + // Not known at send time. The signature itself and its timestamp are + // recorded with the signer row; say so rather than leave a bracket. + CLIENT_SIGNATURE_BLOCK: '(signature captured below)', + SIGN_DATE: '(recorded at signing)', + }; + return html.replace(/\[([A-Z_]+)\]/g, (whole, key: string) => { + const val = map[key]; + return val ? esc(val) : whole; + }); +} + +/** Human-readable reasons this body must not be sent as it stands. */ +export function blockingAgreementIssues(html: string): string[] { + const issues: string[] = []; + const tokens = [...new Set(html.match(/\[[A-Z][A-Z0-9_]{2,}\b[^\]]*\]/g) ?? [])] + .map((t) => (t.length > 40 ? `${t.slice(0, 37)}...]` : t)); + if (tokens.length > 0) issues.push(`unfilled placeholders: ${tokens.join(', ')}`); + if (html.includes(STARTER_NOTICE_MARK)) issues.push('the "review before sending" notice is still in the text'); + return issues; +} diff --git a/server/services/agreement/envelope-legacy.ts b/server/services/agreement/envelope-legacy.ts index c7ade5a3..2f90e7c1 100644 --- a/server/services/agreement/envelope-legacy.ts +++ b/server/services/agreement/envelope-legacy.ts @@ -1,5 +1,7 @@ import { eq, and, inArray, lt, sql, desc, asc } from 'drizzle-orm'; -import { agreements, agreementRequests, agreementSigners, inspections } from '../../lib/db/schema'; +import { agreements, agreementRequests, agreementSigners, inspections, inspectionServices, invoices, users } from '../../lib/db/schema'; +import { getEffectivePriceCents } from '../../lib/effective-price'; +import { fillAgreementTokens, blockingAgreementIssues } from '../../lib/agreement-tokens'; import { Errors } from '../../lib/errors'; import { logger } from '../../lib/logger'; import { mintToken, hashToken } from '../../lib/token-hash'; @@ -131,7 +133,10 @@ export function EnvelopeLegacyMixin 0) { + throw Errors.UnprocessableEntity( + `This agreement cannot be sent yet: ${issues.join('; ')}. Edit the template in Settings, then send again.`, + ); + } const contentHash = await sha256Hex(contentSnapshot); // Spec 2026-08-04 section 3 — freeze the contracting identity onto // the envelope. Renaming the company must not retroactively rewrite // which entity a past agreement was signed with. `legalName` arrives // ALREADY resolved from getBrand; no fallback is re-applied here. - const brand = await new BrandingService(this.db).getBrand(tenantId); const newRow = { id: requestId, diff --git a/tests/unit/agreements/agreement-send-tokens.spec.ts b/tests/unit/agreements/agreement-send-tokens.spec.ts new file mode 100644 index 00000000..7d59d7cd --- /dev/null +++ b/tests/unit/agreements/agreement-send-tokens.spec.ts @@ -0,0 +1,97 @@ +/** + * DELIV-01 — the starter agreement used to be sendable and signable with every + * merge field still in brackets and the admin-only "review before sending" + * notice inside the permanent signed snapshot. Fields are now filled at send + * time and a body that still carries placeholders or the notice is refused. + */ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { eq } from 'drizzle-orm'; +import { AgreementService } from '../../../server/services/agreement.service'; +import { PeopleService } from '../../../server/services/people.service'; +import { seedRoleProfiles } from '../../../server/services/seed/seed-role-profiles'; +import { AGREEMENT_TEMPLATE } from '../../../server/services/starter-content/fixtures/agreement-template'; +import { fillAgreementTokens, blockingAgreementIssues } from '../../../server/lib/agreement-tokens'; +import { createTestDb, setupSchema } from '../db'; +import { asD1Db } from '../helpers/test-db'; +import * as schema from '../../../server/lib/db/schema'; +import type { BetterSQLite3Database } from 'drizzle-orm/better-sqlite3'; + +vi.mock('drizzle-orm/d1', () => ({ drizzle: vi.fn() })); +import { drizzle as mockDrizzle } from 'drizzle-orm/d1'; + +const TENANT = '00000000-0000-0000-0000-000000000001'; +const CLIENT = 'contact-client-1'; +const INSP_ID = '550e8400-e29b-41d4-a716-446655440000'; +const AGR_ID = '00000000-0000-0000-0000-000000000020'; +const USER = 'user-inspector-1'; + +const withoutNotice = AGREEMENT_TEMPLATE.content.replace(/^

⚠️.*?<\/p>/, ''); + +describe('fillAgreementTokens / blockingAgreementIssues', () => { + it('fills the starter fields and leaves nothing blocking', () => { + const out = fillAgreementTokens(withoutNotice.replace(/

4\..*?<\/p>/, ''), { + inspector: 'Quinn ', client: 'Pat Rivera', address: '742 Ivy Lane', date: '2026-10-20T09:00:00Z', amountCents: 40000, + }); + expect(out).toContain('Quinn <Assurance> ("Inspector")'); + expect(out).toContain('Pat Rivera ("Client")'); + expect(out).toContain('October 20, 2026'); + expect(out).toContain('Inspection fee: $400.00.'); + expect(blockingAgreementIssues(out)).toEqual([]); + }); + + it('a missing value stays bracketed and blocks', () => { + const out = fillAgreementTokens('

[CUSTOMER_NAME] pays $[AMOUNT]

', { client: 'Pat', amountCents: 0 }); + expect(out).toBe('

Pat pays $[AMOUNT]

'); + expect(blockingAgreementIssues(out)[0]).toContain('[AMOUNT]'); + }); +}); + +describe('AgreementService.findOrCreate — starter agreement merge fields (DELIV-01)', () => { + let db: BetterSQLite3Database; + let svc: AgreementService; + + const seedAgreement = (content: string) => + db.update(schema.agreements).set({ content }).where(eq(schema.agreements.id, AGR_ID)); + + beforeEach(async () => { + const fixture = createTestDb(); + db = fixture.db; + await setupSchema(fixture.sqlite); + (mockDrizzle as unknown as ReturnType).mockReturnValue(db); + await db.insert(schema.tenants).values({ + id: TENANT, slug: 'acme', status: 'active', deploymentMode: 'shared', tier: 'free', createdAt: new Date(), + }); + await db.insert(schema.users).values({ + id: USER, tenantId: TENANT, email: 'q@example.com', passwordHash: 'x', name: 'Quinn Assurance', role: 'owner', createdAt: new Date(), + } as typeof schema.users.$inferInsert); + await db.insert(schema.agreements).values({ + id: AGR_ID, tenantId: TENANT, name: AGREEMENT_TEMPLATE.name, content: AGREEMENT_TEMPLATE.content, version: 1, createdAt: new Date(), + }); + await seedRoleProfiles(asD1Db(db), TENANT, new Date(1)); + await db.insert(schema.contacts).values({ + id: CLIENT, tenantId: TENANT, type: 'client', name: 'Pat Rivera', email: 'pat@example.com', phone: '+15551234567', createdAt: new Date(), + }); + await db.insert(schema.inspections).values({ + id: INSP_ID, tenantId: TENANT, inspectorId: USER, propertyAddress: '742 Ivy Lane, Charlotte, NC', date: '2026-10-20', + status: 'requested', paymentStatus: 'unpaid', price: 40000, agreementRequired: true, paymentRequired: false, createdAt: new Date(), + }); + await new PeopleService({ DB: {} as D1Database }).addPerson(TENANT, INSP_ID, CLIENT, `crp_${TENANT}_client`); + svc = new AgreementService({} as D1Database, { jwtSecret: 'test-secret' }); + }); + + it('the unedited starter template is refused and creates no envelope', async () => { + await expect(svc.findOrCreate(TENANT, INSP_ID)).rejects.toMatchObject({ status: 422 }); + expect(await db.select().from(schema.agreementRequests).all()).toHaveLength(0); + }); + + it('once the notice and liability placeholder are edited out, the snapshot carries the real values', async () => { + await seedAgreement(withoutNotice.replace(/

4\..*?<\/p>/, '')); + const r = await svc.findOrCreate(TENANT, INSP_ID); + const env = await db.select().from(schema.agreementRequests).where(eq(schema.agreementRequests.id, r.requestId)).get(); + expect(env?.contentSnapshot).toContain('Quinn Assurance ("Inspector")'); + expect(env?.contentSnapshot).toContain('Pat Rivera ("Client")'); + expect(env?.contentSnapshot).toContain('742 Ivy Lane, Charlotte, NC on October 20, 2026'); + expect(env?.contentSnapshot).toContain('$400.00'); + expect(env?.contentSnapshot).not.toMatch(/\[[A-Z_]{3,}/); + }); +});