Skip to content

feat(quote): a quote may not be presented without a recipient (#1017) - #1068

Merged
huangyiirene merged 2 commits into
mainfrom
claude/issue-1017-quote-contact-requiredwhen
Aug 11, 2026
Merged

feat(quote): a quote may not be presented without a recipient (#1017)#1068
huangyiirene merged 2 commits into
mainfrom
claude/issue-1017-quote-contact-requiredwhen

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #1017

Description

Option B, per the maintainer's ruling on #1017. crm_quote.crm_contact has carried the sentence "Recipient is nailed down by the time a quote is presented" for the field's whole life with nothing enforcing it. That intent now has a mechanism: the field is requiredWhen the quote's status is presented or accepted.

Why it matters, restated from #714 / PR #1013: crm_contract.crm_contact is required + notNull, so a quote accepted without a recipient could never draft its contract. Since #1013 that failure is honest and no longer swallows the close-won leg — but quote_on_accepted is async: true + onError: 'log', so the accepting write still answers 200 and the only evidence is a server log with no human in front of it. The gate moves the same refusal forward to the write that turns the quote outward:

PATCH /api/v1/data/crm_quote/<id> {"status":"presented"}
→ 400 VALIDATION_FAILED   "Contact is required"     (the quote stays draft)

Option A (docs only) and Option C (relax crm_contract.crm_contact) were declined in the ruling and are not revisited here.

Which states are gated, and why not the other two

Read off quote_status_progression in the same file rather than guessed. The gate is presented + accepted. The two states a quote can reach without ever being sent are deliberately left open:

state why it is NOT gated
expired the nightly quote_expiration flow sweeps on expiration_date alone and expires never-sent drafts as readily as presented ones. Gating it would make a scheduled system write fail on exactly the quotes that have no recipient because nobody sent them — and its runAs: 'system' would not exempt it, since the gate applies on write, not on actor.
rejected legal straight from in_review, i.e. a quote killed internally before anyone saw it. Demanding a contact to file that verdict asks for a recipient the quote never had.

Neither passes anything on to a contract, so neither needs one. Drafting is unchanged: draft / in_review still need no recipient, which is what lets quote_generation quote a contact-less opportunity (the original reason the field is optional at all).

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Documentation update

Related Issues

Fixes #1017
Related to #714 · PR #1013

Changes Made

  • src/objects/quote.object.tscrm_contact gains requiredWhen: has(record.status) && (record.status == "presented" || record.status == "accepted") and a field description. The diff is confined to that field block, so Discount governance: ceiling validation + approval routing; one definition of "large deal" #599's discount-ceiling validation on this same file does not inherit a conflict.
  • test/quote-contact-required-when.test.ts — new, 22 cases (see Testing).
  • src/translations/{en,zh-CN,es-ES,ja-JP}.ts — the field now has a description, and test/i18n-references.test.ts requires a matching help string in every locale bundle.
  • content/docs/sales/quotes.mdx — the page's standing 口径 was "Put both on the quote before you mark it accepted — what the quote does not carry, acceptance cannot pass on", i.e. advice. The contact half is now enforced, so that paragraph is rewritten, a Built-in rules bullet is added, and an admin tip records the write-time boundary below. The related opportunity is still genuinely optional and is now called out as such.
  • test/cascade-guard-messages.test.ts — one fixture drove a contact-less quote to accepted to reach the freeze it measures; it now gets a contact. This is the only existing test the gate touched.
  • Changeset added.

Stock data — enumeration and disposition

The ruling's binding sub-clause: the gate applies on write, so existing presented-without-contact quotes need a stated disposition.

Enumerated (count: 0 gated rows without a contact). The only quote records this repo ships are the 5 in src/data/revenue.seed.ts:

seeded quote status crm_contact gated?
Acme Platform Upgrade Quote accepted john.smith@acme.example.com ✅ passes
Globex Manufacturing Suite Proposal presented sarah.j@globex.example.com ✅ passes
Wayne Enterprise License Quote in_review present (not required) not gated
Initech Cloud Migration Estimate draft present (not required) not gated
Stark Medical Pilot Quote rejected none not gated — see the table above

Also swept, and none of them creates a quote: src/flows/demo-bootstrap.flow.ts (backfills owner_id only), src/flows/quote-generation.flow.ts (creates draft), scripts/** (backfill-owner-id.ts writes ownership only), e2e/** (names crm_quote in an object list, creates none). Two test fixtures hold contact-less quotes: test/flow-scheduled.test.ts (stub flow harness, no engine validation — unaffected) and test/cascade-guard-messages.test.ts (fixed, above).

Disposition: nothing to migrate, and no stock row becomes un-editable. This is measured, not inferred, and the measurement changed what the doc says. A row that entered the gated state before the rule existed was built the way an upgrading deployment gets one — insert through an engine whose schema has no requiredWhen, then re-open the same store with the shipped schema — on both drivers:

legacy SQL row: status = presented | crm_contact key present = true | value = null
READ                                  → works
unrelated edit (total_price)          → ADMITTED
internal_notes                        → ADMITTED
move to expired / rejected            → ADMITTED
fill the contact in (the repair)      → ADMITTED

The engine evaluates the requirement on the write that makes the predicate become true, not on every later write while it holds. So pre-existing rows are not bricked, and the repair is an ordinary edit in the same form that reports the error. The cost of that, stated rather than hidden: such a legacy row can still be walked on to accepted, where it meets the pre-#1017 behaviour (contract cannot be drafted, reason in the log). That residue exists only for rows already gated when the rule arrived — a quote created under this schema cannot get there, because draft → presented is refused. This repo ships no such row, so the residue is empty here. All three behaviours are pinned as tests, and the boundary is written into the doc's admin tips.

Testing

  • Unit tests pass (pnpm test) — 81 files, 1896 passed, 1 skipped
  • Linting passes (pnpm lint) — exit 0, 116 warnings / 14 suggestions, all pre-existing (form sections without name, line-item lookups); none names crm_contact
  • Build succeeds (pnpm build)
  • New tests added

Gates run on this branch after git fetch origin main && git merge origin/main (main at c5bc586; #1066's rc.5 → rc.6 bump had not landed at that point, so these numbers are against rc.5 — re-running is warranted if #1066 merges first): pnpm validate ✅ · pnpm typecheck ✅ · pnpm lint ✅ · pnpm hygiene ✅ · pnpm build ✅ · pnpm test ✅ · pnpm test:coverage ✅ (branches 84.82%, threshold 78).

test/quote-contact-required-when.test.ts (22 cases) does not check that a rule is declared — this repo has measured five metadata surfaces that accept a rule and then don't apply it, and requiredWhen has its own version of that (a predicate that fails to evaluate is skipped with a warning, leaving a rule that reads as enforced and requires nothing; hence the has(record.status) guard, following the lead.object.ts precedent under ADR-0113). So refusals are driven through a real ObjectQL on both driver shapes — InMemoryDriver, whose stored rows omit unwritten columns, and a real SQLite database, whose rows are column-complete with NULLs.

Each refusal is asserted by its substance, not by "it threw": envelope code = VALIDATION_FAILED (which @objectstack/runtime's validation-failure.ts answers with VALIDATION_FAILED_STATUS = 400 plus a fields[] payload — the module exports neither, so the test mirrors its discriminator, which is what makes the assertion bite: a refusal raised in some other shape would stop being recognised and come back 500), per-field code = required, message names the field, and the record did not move — a rule that reports a problem while the write lands anyway is the flow-condition failure mode (#633), not enforcement.

Refused: draft → presented; draft → accepted; an INSERT born presented; blanking the contact out of a presented quote. Green and staying green: presenting and accepting a quote that has a contact; expiring a never-sent contact-less quote (the nightly sweep); rejecting one killed in review; unrelated edits; a later edit to an already-presented quote.

Reverse verification (run once, direction predicted first)

Deleting the single requiredWhen line and re-running the file:

× requires the contact exactly from `presented` onward
    → AssertionError: expected '' to be 'has(record.status) && (record.status …'
× guards the status read with has(...) — the difference between enforced and inert
× refuses to PRESENT a quote with no contact, and the quote stays draft
    → AssertionError: the write was ADMITTED — no refusal to inspect
× refuses to ACCEPT a quote with no contact — the state that drafts the contract
× refuses an INSERT that is born presented
× refuses to blank the contact out of a quote that is already presented
× refuses the presentation and leaves the row where it was   (SQLite)
Tests  7 failed | 12 passed (19)

All five enforcement pins flip to "the write was ADMITTED" — the pin fails for the right reason, in the predicted direction. The 12 that stayed green are the positive paths, the ungated states and the seed enumeration: platform behaviour did not change there, so they should be green on both sides. (Reverse run made before the three legacy-row cases were added, hence 19 rather than 22; those measure engine behaviour with the rule present.)

Checklist

  • I have added a changeset
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my feature works
  • New and existing unit tests pass locally with my changes

Additional Notes

Draft: gates are green locally against main at c5bc586, CI is still to report. Untouched by design, per the card: package.json / lockfiles (#1066), src/objects/case.hook.ts, src/data/service.seed.ts (#595), e2e/**.


Generated by Claude Code

claude added 2 commits August 11, 2026 06:14
`crm_quote.crm_contact` carried the sentence "Recipient is nailed down by
the time a quote is presented" for the field's whole life with nothing
enforcing it. `crm_contract.crm_contact` is `required` + `notNull`, so a
quote accepted without a recipient could never draft its contract — since
#714/#1013 that failure is honest and no longer swallows the close-won leg,
but `quote_on_accepted` is `async` + `onError: 'log'`, so the accepting
write still answers 200 and the only evidence is a server log.

Per the maintainer's ruling on #1017 (Option B), the sentence now has a
mechanism: `requiredWhen` on `presented`/`accepted`. The refusal moves to
the synchronous, someone-is-watching moment, reported against the field
with the quote still editable.

Drafting is unchanged — `draft`/`in_review` still need no recipient, which
is what lets `quote_generation` quote a contact-less opportunity. The two
states reachable without the quote ever being sent are deliberately not
gated: `expired`, written by the nightly `quote_expiration` sweep over
never-sent drafts, and `rejected`, legal straight out of `in_review`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013BksPWmRi2gbZjapgUVU4V
Measured, not assumed: the engine evaluates the requirement on the write
that makes the predicate BECOME true, so a quote that was already
`presented` without a contact when the rule landed is not bricked — it
reads, ordinary edits land, and filling the contact in is an ordinary
edit. The cost is that such a row can still be walked on to `accepted`,
where it meets the pre-#1017 behaviour. This repo ships no such row, so
the residue is empty here; it is pinned so the boundary is a recorded
measurement rather than a later surprise, and stated for admins in the
quotes doc.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013BksPWmRi2gbZjapgUVU4V
@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hotcrm Ignored Ignored Aug 11, 2026 6:19am

Request Review

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

Labels

ci/cd CI plumbing and the verification pipeline metadata Declarative metadata — schema, security posture, UI surfaces

Projects

None yet

2 participants