Skip to content

crm_quote.crm_contact 要不要自 presented 起 requiredWhen——缺 contact 的报价永远起草不出合同,失败只出现在服务器日志里(源自 #714) #1017

Closed
#1068
Closed
crm_quote.crm_contact 要不要自 presented 起 requiredWhen——缺 contact 的报价永远起草不出合同,失败只出现在服务器日志里(源自 #714)#1017
#1068
Assignees
huangyiirene
Labels
metadataDeclarative metadata — schema, security posture, UI surfacespm:dispatchedDispatched to a dev agent by /pm-dispatch

Description

@yinlianghui
yinlianghui
opened on Aug 7, 2026

源自 #714 / PR #1013 的验收遗留(机械缺陷已修完:false 不再进 lookup、失败已变诚实、close-won 不再被连累)。悬而未决的是一个产品语义分叉,按纪律交维护者拍板。

事实(已实测,证据在 PR #1013 正文末节)

  • crm_quote.crm_contact 有意可选;crm_contract.crm_contact 必填(required + notNull,缺键与 null 均报 Primary Contact is required,warn-first / strict 两种 posture 一致);
  • 因此缺 contact 的报价被接受后,合同永远起草不出——修复后失败诚实(报 required 而非 received boolean,且不吞 close-won),但仍然只出现在服务器日志里,rep 在界面上没有当场可见的信号;
  • quote schema 自己的注释写着 "Recipient is nailed down by the time a quote is presented"——这个意图没有任何机制在执行:没有规则拦住一张没有 contact 的报价走到 presented/accepted;
  • content/docs/sales/quotes.mdx 已写给销售的口径是 "what the quote does not carry, acceptance cannot pass on",与现状一致。

选项

  • A. 维持现状:靠文档提醒。最省,但缺 contact 时用户仍无当场信号,失败继续留在日志里。
  • B. 给 crm_quote.crm_contact 加自 presented 起的 requiredWhen:把失败前移到同步、可修复、有人在场的时刻;契合「declared = enforced」与 schema 注释里已写死的意图。代价:改变报价何时可被呈现,并触及存量数据(已 presented 而无 contact 的报价)。
  • C. 放松 crm_contract.crm_contact 为可选:CPQ 链恒成立,但削弱合同数据模型(法律文件不指名对手方),与 Contracts/Quotes 两页文档冲突,属典型 consumer-side 宽容。

推荐

B(#714 的 dev 推荐,PM 同意):意图已在 schema 注释里写死、只是从未被执行;B 把错误搬到有人在场的时刻。但 B 改变呈现门槛并触及存量数据,需维护者拍板。拍板后一单可派。

Refs #714 · PR #1013(正文末节含三选项完整论证)

Activity

  1. yinlianghui
    added
    metadataDeclarative metadata — schema, security posture, UI surfaces
    needs-user-decisionNeeds the maintainer's call before work proceeds
    on Aug 7, 2026
  2. huangyiirene
    removed
    needs-user-decisionNeeds the maintainer's call before work proceeds
    on Aug 11, 2026

  3. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    huangyiirene
    on Aug 11, 2026 – with Claude
    Collaborator

    Ruling (maintainer, 2026-08-11, PM chat, verbatim: 「接受你的全部建议」): Option B — add requiredWhen on crm_quote.crm_contact from presented onward. The schema comment "Recipient is nailed down by the time a quote is presented" finally gets a mechanism; the contract-drafting failure moves from an async server log to a synchronous, someone-is-watching moment (declared = enforced). Scope includes handling stock presented-without-contact quotes (enumerate and report them in the PR; the requiredWhen gate applies on write, so stock rows need a stated disposition, not silent breakage).

    Queued (S). Serial constraint: same schema file as #599's ceiling validation (quote.object.ts) — this card dispatches first, #599 follows in a later round.

    Generated by Claude Code

  4. huangyiirene
    added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    on Aug 11, 2026
  5. huangyiirene
    self-assigned this
    on Aug 11, 2026

  6. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    huangyiirene
    on Aug 11, 2026 – with Claude
    Collaborator

    Claim: PM loop round 3 (hotcrm whole-repo seat)
    Session: session_01NM6o28jmBgsyTRQHutn7LC
    Branch: claude/issue-1017-quote-contact-requiredwhen
    Worktree: dedicated cloud session (fresh clone)
    Domain: hotcrm whole-repo seat
    File surface: src/objects/quote.object.ts (the crm_contact field block only), quote tests, content/docs/sales/quotes.mdx if the口径 needs updating
    Container & model: S (contract-touching, not mechanical), mode:cloud (dedicated session), model: claude-opus-5
    Serial constraints cleared: #599's ceiling validation is held for a later round (same file); in-flight #1066 and same-batch #595 are both disjoint from quote.object.ts.

    Premise re-verified on origin/main before dispatch: crm_quote.crm_contact is optional at quote.object.ts:81 and still carries the comment "Recipient is nailed down by the time a quote is presented" with no mechanism behind it; crm_contract.crm_contact is required: true + storage.notNull at contract.object.ts:66-67. In-repo requiredWhen precedent for the dev to follow: src/objects/lead.object.ts:372,378 (ADR-0113, P\...`` predicate form).

    Generated by Claude Code

  7. huangyiirene
    added a commit that references this issue on Aug 11, 2026

    test(quote): pin what the write-time gate does NOT reach (#1017)

    cdfb49a

  8. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    huangyiirene
    on Aug 11, 2026 – with Claude
    Collaborator
    {
  "issue": 1017,
  "status": "in_progress",
  "branch": "claude/issue-1017-quote-contact-requiredwhen",
  "pr": "https://github.com/objectstack-ai/hotcrm/pull/1068",
  "premise_still_valid": true,
  "summary": "Option B implemented as ruled. `crm_quote.crm_contact` now carries `requiredWhen: has(record.status) && (record.status == \"presented\" || record.status == \"accepted\")`, giving the schema comment 'Recipient is nailed down by the time a quote is presented' its first mechanism; the contract-drafting refusal moves from an async server log to a synchronous 400 VALIDATION_FAILED against the field. Gated states determined from the object's own `quote_status_progression` vocabulary, not guessed: `presented` + `accepted` only. `expired` is excluded because the nightly `quote_expiration` flow expires never-sent drafts (gating it would fail a scheduled system write on exactly the quotes that have no recipient); `rejected` is excluded because it is legal straight out of `in_review`, i.e. a quote killed before anyone saw it. Draft/in_review still need no contact, so `quote_generation` can still quote a contact-less opportunity. STOCK DATA: enumerated all 5 seeded quotes in src/data/revenue.seed.ts — count of gated-state-without-contact rows is 0 (accepted/Acme and presented/Globex both name a contact; the contact-less one is `rejected`, which is not gated). demo-bootstrap, quote-generation, scripts/** and e2e/** create no gated quotes. Disposition: nothing to migrate, and measured on both drivers (memory + real SQLite) a pre-existing presented-without-contact row is NOT bricked — it reads, ordinary edits land, and filling the contact in is an ordinary edit; the residue is that such a legacy row can still be walked to `accepted`, where it meets the pre-#1017 behaviour. Zero such rows ship here. All three behaviours are pinned as tests and written into the quotes doc's admin tips. Diff confined to the crm_contact field block in quote.object.ts so #599 inherits no conflict; package.json/lockfiles, case.hook.ts, service.seed.ts and e2e/** untouched.",
  "tests": "New test/quote-contact-required-when.test.ts (22 cases) drives real ObjectQL on BOTH driver shapes — InMemoryDriver (sparse rows, the shape that makes an unguarded predicate abort) and a real SQLite database (column-complete NULLs). Each refusal is asserted by substance: envelope code VALIDATION_FAILED (which @objectstack/runtime's validation-failure.ts answers with VALIDATION_FAILED_STATUS = 400 + fields[]; the test mirrors its discriminator since neither is exported), per-field code `required`, the message naming the field, AND the record not moving. Refused: draft→presented, draft→accepted, INSERT born presented, blanking the contact on a presented quote. Positive cases green: presenting AND accepting a quote WITH a contact, expiring a never-sent contact-less quote, rejecting one killed in review, unrelated edits, later edits to an already-presented quote. REVERSE VERIFICATION (run once, direction predicted first): deleting the single requiredWhen line flips 7 of 19 to red — all five enforcement pins report 'the write was ADMITTED — no refusal to inspect', the 12 that stay green are the positive/ungated/seed paths that should not change. One existing fixture needed the contact (test/cascade-guard-messages.test.ts drove a contact-less quote to accepted); fixed. Gates against main at c5bc586 (rc.5 — #1066's rc.6 bump had NOT landed at merge time; re-merge warranted if it lands first): pnpm validate 0, typecheck 0, lint 0 (116 warnings all pre-existing, none naming crm_contact), hygiene 0, build 0, test 81 files / 1896 passed / 1 skipped, test:coverage branches 84.82% vs threshold 78. Changeset added. CI not yet reported — PR opened as draft.",
  "open_questions": [],
  "out_of_scope_findings": [
    "Filed hotcrm#1069 (new, unassigned, dedup-searched): `requiredWhen` is a TRANSITION gate, not an invariant — a record already in the required state is never re-validated. Measured on both drivers: a legacy presented-without-contact quote goes presented→accepted unrefused, while draft→presented is refused. Same hole applies to crm_opportunity.win_reason/loss_reason (#593) and crm_lead's duplicate lookups (ADR-0113): a deal already at closed_won with no win_reason keeps taking edits forever. Half of this is load-bearing (it is why adding a gate does not brick stock rows, which #1017's ruling asked for), so the card poses it as a product choice rather than asserting a platform bug."
  ]
}

    Generated by Claude Code

  9. huangyiirene
    closed this as completedin #1068on Aug 11, 2026

  10. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    huangyiirene
    on Aug 11, 2026 – with Claude
    Collaborator

    ACCEPT — PR #1068 reviewed and MERGED (8a90449). Option B implemented as ruled; the sentence "Recipient is nailed down by the time a quote is presented" now has a mechanism.

    Review notes:

    • The gate is requiredWhen: has(record.status) && (status == "presented" || status == "accepted"), with the has() guard carrying real load — the recorded trap (a predicate that fails to evaluate is skipped with a warning, leaving a rule that reads as enforced and requires nothing) is pinned by its own test.
    • Which states are gated was read off quote_status_progression, not assumed: expired (the nightly sweep over never-sent drafts) and rejected (killed in internal review) are deliberately left open, and the test asserts their absence from the predicate.
    • Refusals are asserted by substance — envelope code = VALIDATION_FAILED, HTTP 400, per-field code = required, and the record did not move — driven through a real ObjectQL on both driver shapes (in-memory sparse records and a real SQLite database with column-complete NULLs). Reverse verification ran: deleting the one line flips exactly the five enforcement pins to "the write was ADMITTED", the positive paths stay green.
    • The ruling's stock-data sub-clause is answered by measurement, not assertion: all 5 seeded quotes carry a contact; a legacy presented-without-contact row (built by inserting through an ungated schema and re-opening the store gated) stays readable and editable, is repaired by an ordinary edit, and the one residue — such a row can still be walked to accepted — is stated in the PR, pinned as a test, and written into the doc's admin tips rather than left for a user to discover.

    Serial note: #599's discount-ceiling validation lands in this same file next round; this diff stayed confined to the crm_contact block so that card inherits no conflict.

    Generated by Claude Code

  11. os-sales
    added a commit that references this issue on Sep 3, 2026

    test(i18n): extend the Chinese term guard to status names and field l…

    d46ab56
  12. os-zhuang
    added a commit that references this issue on Oct 8, 2026

    test: run the sandboxed bodies through the verify handle's real doors

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

Metadata

Metadata

Assignees

Labels

metadataDeclarative metadata — schema, security posture, UI surfacespm:dispatchedDispatched to a dev agent by /pm-dispatch

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions