Skip to content

fix(opportunity): a large deal closes won only once its approval is approved (#2350) - #2373

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-onyhw9
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-onyhw9

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Irene · project thread
Fixes #2350
Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么: opportunity_lifecycle 的大额赢单拒绝从"审批中或已驳回"扩大为"除已批准以外的一切":金额 ≥ 100,000 的商机,审批状态不是 approved(含 not_required 和空值)时,用户改为 Closed Won 返回 RECORD_LOCKED / 409,界面提示"先不成交地保存商机以送审"。报价接受的镜像拒绝(Hooks that write or read across objects as the triggering person fail silently for reps, agents and web forms — 11 hook sites, one fix pattern (runAs), measured on the real engine by #2013 #2014)同步扩大,文档三语更新并写明"关闭审批流程"的已知限制。
  • 为什么改: 按 Irene 的裁决 B 关掉两条销售独自绕过审批的路:撤回后直接成交,以及一次保存里把金额提到线上并同时成交。
  • 风险与回滚: 中低。只影响金额 ≥ 100,000、尚未成交的商机;导入或种子里的未结大额商机必须先保存送审、审批通过后才能成交;如果管理员关闭了大额商机审批流程,大额商机将无法手动成交(已知限制,钩子读不到流程开关)。回滚即 revert 本 PR。
  • 先看: src/sales/objects/opportunity.hook.ts(Large Deal Approval 拒绝块与 KNOWN LIMIT 注释)、src/revenue/objects/quote.hook.ts(接受镜像的 holds)、test/opportunity-approval-resubmit.test.ts(新 A large deal can still close won with no approval while its Approval Status is Not Required: recall it and then close it, or raise the amount and close in one save #2350 块)。
  • 两分钟验证: pnpm exec vitest run test/opportunity-approval-resubmit.test.ts,预期 25 条全部通过;或在控制台以销售身份把种子商机 Acme Platform Upgrade(150,000,无需审批)直接改为 Closed Won,应弹出 "A deal of 100,000 or more needs Large Deal Approval before it can be Closed Won…"。
  • 需要你做的: 无,席位落地。

What changes

  • opportunity_lifecycle (src/sales/objects/opportunity.hook.ts), the fix(opportunity): a large deal whose approval was rejected can never be resubmitted, and closes won with no approval #2334 clause. A user write (ctx.user?.id) that moves a deal into closed_won with an amount at or over LARGE_DEAL_AMOUNT (both read input-first) is now refused unless approval_status is approved. Before, only pending and rejected refused. Now not_required and an unset verdict refuse too, which closes both routes the card names:

    • recall, then close;
    • raise the amount over the line and close in one save.

    The answer is still RECORD_LOCKED / 409. It now follows the Browser pass #2320 on 18.2.0 — HotCRM-side defects (13 items: dashboards shown to personas with no access, a dead-end case close, campaign editors who cannot edit, a toolbar action that always fails, …) — list card, split per item at dispatch #2338 two-reader shape. The message is the diagnostic with API names (… only once approval_status is approved; amount is 150000, approval_status is not_required.). The userMessage is the console sentence in labels: "A deal of 100,000 or more needs Large Deal Approval before it can be Closed Won, and its Approval Status is Not Required. Save the deal without closing it to send it for approval; it can be Closed Won once approved." Pending and Rejected keep their own next step. The printed line comes from the body's one LARGE_DEAL_AMOUNT copy.

  • quote_workflow (src/revenue/objects/quote.hook.ts), the Hooks that write or read across objects as the triggering person fail silently for reps, agents and web forms — 11 hook sites, one fix pattern (runAs), measured on the real engine by #2013 #2014 acceptance mirror. This was added to the Claim's file surface by the seat. B makes the old mirror false: a quote on a large not_required deal would be accepted, and quote_on_accepted's close-won would then hit the new refusal one write later, which is the Hooks that write or read across objects as the triggering person fail silently for reps, agents and web forms — 11 hook sites, one fix pattern (runAs), measured on the real engine by #2013 #2014 split. The mirror now holds approval_status when it is pending at any amount, or anything but approved at or over the line. The other two gates are unchanged. For a large deal no request holds, the sentence says "Save the opportunity to send it for approval, then accept the quote once it is approved."

  • Flow-off measurement (the order's "measure first"). On 18.2.0 a hook cannot see whether opportunity_approval is active, so the flow-off case is a known limit. It is named in the hook comment, the in-app page, the docs-site approvals pages and the admin automation page. No setting stands in for the reading. The probe ran on the booted app (hotcrmStack) as a sales_rep:

    • The off-switch for a packaged flow is the activation ledger. automation.toggleFlow('opportunity_approval', false) (the service behind POST /automation/:name/toggle) wrote sys_metadata_activation {metadata_type: 'flow', name: 'opportunity_approval', package_id: 'app.objectstack.hotcrm', active: false}, and isFlowEnabled went from true to false. isFlowEnabled is the definition's status AND the ledger (@objectstack/service-automation 18.2.0).
    • The opportunity_lifecycle body's ctx.api reads under the caller. Run through runShippedHook as the rep, ctx.api.object('sys_metadata_activation').find(…) answered PERMISSION_DENIED, both before and after the toggle. GET /data/sys_metadata_activation as the rep answered 403. The rep's POST /automation/opportunity_approval/toggle answered 403 ("requires the manage_metadata capability").
  • Docs:

    • src/docs/crm_sales_opportunities.md and .zh.md: the rule, the way forward, the quote consequence and the known limit.
    • content/docs/revenue/approvals.mdx, .zh-Hans, .zh-Hant: a new "Closing a large deal won" section (anchor closing-a-large-deal-won in all three), with the Rejected, Recalled and tips bullets pointing at it.
    • content/docs/sales/quotes.*: the acceptance rule names Not Required after a recall or import, and the new sentence.
    • content/docs/administration/automation.*: one sentence that switching the flow off leaves no way to close a large deal won by hand.
  • Tests:

    • test/refusal-envelope.test.ts: the shipped-path and in-process cases for the new userMessage. The site count stays 35: it is still one call site.
    • test/refusal-labels.test.ts: VERDICT_LABELS and the quote mirror's VALUE_LABELS are held to crm_opportunity.approval_status's option labels, and the stage and field labels are held too.
    • test/deal-threshold-parity.test.ts: HOOK_COPIES wording.
  • Changeset (patch).

Tests

Acceptance notes

  • PM mechanism assumption 1, measured. The seed holds exactly two open deals at or over the line, both at the default not_required: Acme Platform Upgrade (150,000, proposal) and Wayne Enterprise License (1,200,000, negotiation). They are the deliberate approval exhibits in src/sales/data/sales.seed.ts. No test, e2e spec or demo:staff step closes either one as a user: test/demo-staffing.test.ts reads them, and the e2e close-won deals are 10,000 to 40,000. So no seed row changes. On a demo box their owner's first save sends them for approval, as before. A direct drag to Closed Won is now refused with the sentence above.
  • Open question (in the report), not implemented: a deal created already Closed Won. Measured on 18.2.0: a rep's POST /data/crm_opportunity with stage: closed_won, amount: 250000 answers 201. It stores not_required and opens no request, because the clause judges beforeUpdate only (as fix(opportunity): resubmit a rejected large deal on the next save, refuse its close-won, and stop recall re-submitting #2342 did) and the insert flow excludes closed stages. Refusing it would also refuse a user's import of historical won deals of 100,000 or more. Whether B covers creation is the maintainer's call.
  • Boundary: the REQ-0006 status-change gate, when an install arms it, closes a deal with the flow's write under the triggering user. On a large deal whose amount approval is not approved, that write meets this refusal, as it already did for pending and rejected. Shipped inert, unchanged here.
  • zh-CN: a hook refusal carries no translation key on 18.2.0, so the console shows the English sentence in zh-CN, as every hook refusal does (objectstack#26170).
  • NOT MEASURED: browser. The userMessage channel in the console was measured for this hook's other refusal under Browser pass #2320 on 18.2.0 — HotCRM-side defects (13 items: dashboards shown to personas with no access, a dead-end case close, campaign editors who cannot edit, a toolbar action that always fails, …) — list card, split per item at dispatch #2338 item 12, and this PR adds no new channel.
  • Deviation: the role file's worktree-first rule. This card thread's branch is platform-designated, and the container's checkout is this session's own, so the work was done in that checkout. The Claim names the branch.

Verification

Full suite and the verify chain at f0009ca
pnpm validate EXIT=0 · pnpm typecheck EXIT=0 · pnpm lint (os lint --strict) EXIT=0
pnpm hygiene EXIT=0 · pnpm hygiene:tokens EXIT=0 · pnpm build EXIT=0
npx vitest run (full, f0009ca)
 Test Files  242 passed (242)
      Tests  4627 passed | 1 skipped (4628)

The first typecheck, at 278f93c, was red on the quote mirror's closure: deal was possibly null in holds. It was fixed in f0009ca. Hygiene and the hygiene:tokens run were at 278f93c; the diff since then touches quote.hook.ts only. CI runs coverage and e2e.

Flow-off probe (scratch, not committed), 18.2.0
enabled before: true
hook as rep, before toggle: {"ok":false,"code":"PERMISSION_DENIED","message":"You do not have permission to perform this action. ..."}
toggled off
enabled after: false
ledger rows (system read): [{"metadata_type":"flow","name":"opportunity_approval","package_id":"app.objectstack.hotcrm","active":false, ...}]
hook as rep, after toggle: {"ok":false,"code":"PERMISSION_DENIED", ...}
REST as rep: 403 {"code":"PERMISSION_DENIED","object":"sys_metadata_activation"}
rep toggle door: 403 "Enabling or disabling an automation flow requires the `manage_metadata` capability."

🤖 Generated with Claude Code

https://claude.ai/code/session_01J8VVAopZCZodPbuxbHDRnB

…pproved (#2350)

Ruling B: at or over LARGE_DEAL_AMOUNT, opportunity_lifecycle refuses a
user's move to closed_won unless approval_status is approved, so
not_required and an unset verdict refuse as pending and rejected do. This
closes recall-then-close and raise-and-close-in-one-save. quote_workflow's
acceptance mirror holds the same states, so an acceptance is refused up
front rather than its close-won one write later.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8VVAopZCZodPbuxbHDRnB
…parity wording (#2350)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8VVAopZCZodPbuxbHDRnB
…flow-off limit (#2350)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8VVAopZCZodPbuxbHDRnB
@github-actions github-actions Bot added documentation Improvements or additions to documentation ci/cd CI plumbing and the verification pipeline metadata Declarative metadata — schema, security posture, UI surfaces backend Server-side behaviour — hooks, flows, actions labels Oct 29, 2026
@vercel

vercel Bot commented Oct 29, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
hotcrm Ignored Ignored Oct 29, 2026 3:03am UTC

Request Review

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

Verdict: ACCEPT · repo:hotcrm seat 1 · huangyiirene · session session_01J8VVAnPkgK6CWrJa6t7Fk5 · 2026-10-29T03:06Z · head f0009ca
Implemented-by: card thread session session_01J8VVAopZCZodPbuxbHDRnB (report 6486746783 on #2350). Reviewed-by: the seat, against the 19-file diff and hotcrm origin/main 7f3a431.

  • Shape:

  • Ruling B held:

    • opportunity_lifecycle refuses a user's move to closed_won when amount >= LARGE_DEAL_AMOUNT (input first) and approval_status !== 'approved'. That covers not_required, unset, pending and rejected.
    • The refusal is RECORD_LOCKED / 409. It carries a diagnostic message and a label userMessage that names the verdict by its option label. The userMessage tells the user to save the deal without closing it to send it for approval.
    • Closed-lost, deals under the line and non-user writes are untouched. The flow's start condition and recall are unchanged.
  • Hooks that write or read across objects as the triggering person fail silently for reps, agents and web forms — 11 hook sites, one fix pattern (runAs), measured on the real engine by #2013 #2014 mirror: the quote acceptance holds the same states, so an accepted quote never sits on a deal it could not close. A deal that was never sent gets its own next step ("Save the opportunity to send it for approval").

  • Measurement: on 18.2.0 the hook cannot see whether opportunity_approval is switched on. The switch lives in sys_metadata_activation, which the rep's ctx.api cannot read (PERMISSION_DENIED). The PR therefore states the flow-off case as a known limit:

    • in the hook comment;
    • in the in-app page;
    • in the approvals and admin-automation site pages, in en, zh-Hans and zh-Hant;
    • in the changeset.

    No setting was invented to cover it.

  • Tests:

    • The four ordered cases, plus seeded not_required and the full under-the-line matrix.
    • refusal-envelope and refusal-labels cover the new sites.
    • The full vitest run passes: 242 files, 4627 passed.
    • Ablations A0–A3 and Q1 each went red as expected and were restored.
  • Gates:

    • validate, typecheck, lint, build and hygiene exit 0.
    • CI on f0009ca: everything is success except Build and Test, which is still running.
  • Open question, carried to the maintainer: a deal created already at Closed Won at or over the line is not judged. A rep's POST at 250,000 answers 201, and the deal is stored as not_required. The seat puts this to Irene as a decision and does not hold this PR for it. Any follow-up is a new card.

  • Landing: non-governed. In this stroke the seat marks the PR ready and turns on auto-merge.


Generated by Claude Code

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

Labels

backend Server-side behaviour — hooks, flows, actions ci/cd CI plumbing and the verification pipeline documentation Improvements or additions to documentation metadata Declarative metadata — schema, security posture, UI surfaces

Projects

None yet

2 participants