Repository navigation
fix(revenue): a closed deal's line items are frozen with it - #2113
Conversation
A closed-won or closed-lost opportunity froze its own fields, but a user could still add a line to it, change one of its lines, or move a line onto or off it. `opportunity_amount_rollup` rightly leaves a closed deal's amount alone, so its lines stopped summing to it (measured on 18.0.0-next.1: a rep's 3 x 500 line on their own closed-won deal was admitted, the deal kept 1,000, the lines summed to 1,500). `opportunity_line_item_freeze` refuses a user's line insert or update whose old or new parent deal is closed, with the deal freeze's own envelope (RECORD_LOCKED / 409) and the deal named by its name. "Closed" is the test `opportunity_lifecycle` uses (closed_won / closed_lost); only user writes are judged, and an update is judged by what it changes, so the line-number back-fill touch still numbers a closed deal's lines. It sits beside `crm_opportunity_line_item` in src/revenue rather than in `opportunity_lifecycle`: a hook may not attach to another package's object (ADR-0130 R4, enforced here by co-location). Delete is not guarded; that half is its own decision card. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P8mZuzuyZAHSrgQKmv6ASv
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
席位核对复核人:repo:hotcrm 执行席(#1353)· 判决 ACCEPT · 钉 head 形态:一卡一 PR, 前提实测:diff 逐行读过。 门禁 check-runs(钉 head):Build and Test (22.x) ✓ · Quality Checks ✓ · Check Changeset ✓ · Playwright ✓ · link-check ✓ · CodeQL ✓ · Analyze Code (javascript) ✓ · Label Pull Request ✓ · Vercel Preview Comments ✓ — 9/9 success,mergeable_state 接受的偏差:守卫没有放进卡里说的 范围外去向:删除那一半 → 决策卡 #2078(已存在)。「hook 与对象同包」目前没有机械检查——仅记录(AGENTS.md §3,不加门禁)。 落地条款:席位经中继 |
Fixes #2077
Clause-②: no
What changes
A closed-won or closed-lost opportunity froze its own fields, but its line items did not freeze with it. A rep could add a line to a closed deal, change one of its lines, or move a line onto or off it, and the write went through.
opportunity_amount_rolluprightly leaves a closed deal's amount alone, so the lines stopped summing to the deal's Amount.opportunity_line_item_freeze, a new hook beside the object insrc/revenue/objects/opportunity_line_item.hook.ts, now refuses a user's insert or update of a line whose old or new parent deal is closed. It answers with the deal freeze's own envelope,RECORD_LOCKED/ 409, and a business sentence that names the deal by its name, never by its id:Scope: this is the insert / update half of the card. The delete half is decision card #2078, which remains open. Delete is not guarded here.
The premise, measured on the unchanged base
Base
9e064b3d,@objectstack/*18.0.0-next.1. A sales rep wrote through the engine write door (the pathPOST /api/v1/data/crm_opportunity_line_itemtakes), using a temporary probe file that was deleted before the commit:closed_wondeal: admitted. The deal's amount stayed at 1,000 and its one line summed to 1,500.closed_lostdeal's line: admitted (quantity 9 stored).closed_wondeal: admitted (the stored parent is now the closed deal).Where the guard lives: beside the line, not in
opportunity_lifecycleThe card asked for "one freeze, one envelope, in the existing lifecycle hook".
opportunity_lifecyclelives insrc/sales, andcrm_opportunity_line_itembelongs tosrc/revenue.opportunity_lifecyclename['crm_opportunity', 'crm_opportunity_line_item']keepstest/package-ownership.test.tsgreen (24/24). Today the app package registers both directories, so nothing mechanical refuses it. The edit was restored, and the blob was confirmed equal toHEAD.src/revenue/is a package. Decision 5 ofdocs/architecture/module-split-plan.mdmakes itapp.objectstack.hotcrm.revenue, and there R4 is a loud author-time refusal. The two campaign hooks set the precedent: they moved intosrc/salesto sit beside the objects they hook.So the line freeze is its own hook, beside its object. It keeps the deal freeze's terms, which means one envelope and one definition of "closed":
stageisclosed_wonorclosed_lost. That is the testopportunity_lifecycleapplies, andopportunity_amount_rollupuses the same one. No second definition is introduced.ctx.user?.id), the boundary the deal freeze draws. A write with no user (the seed, a migration) passes.quote_workflowjudges quote lines.id,updated_at,updated_byandline_numberdo not count, so the 4.0.0 line-number back-fill's{ line_number: null }touch still numbers a closed deal's line.opportunity_lifecyclegives for its own ordering.controlled_by_parentwrite gate still applies to that write.Tests
test/opportunity-line-freeze.test.tsis the deal-side twin oftest/quote-line-freeze.test.ts:code+status(RECORD_LOCKED/ 409), the sentence, and the absence of the deal id: an insert onto aclosed_wondeal and onto aclosed_lostdeal (nothing stored), a change to a closed deal's line (stored line unchanged), and a move off a closed deal and onto one (both lines stay put).runShippedHook) and givesSandboxErrorcarryingcode/status/userMessage.Two existing tests were also edited.
test/refusal-envelope.test.ts: the hand-maintained call-site count goes from 24 to 25, with a history line.test/runtime-coverage.test.ts: the new file is listed as a runtime test file.Ablation (one run, after the fix was committed).
scripts/ablation-replace.mjsreplaced the closed-stage predicate with a stage that never matches. The tests load source throughobjectstack.config.ts, so nodist/is involved. Injected marker count was 1 and the anchor went from 1 to 0.test/opportunity-line-freeze.test.tsthen had 5 failed, 3 passed: all five refusal cases went red and the three controls stayed green. The file was restored with blob ==HEAD(192c970e) and an emptygit diff HEAD. A first attempt aborted on a script variable error before any mutation was written, so that attempt was not an ablation.Through the real REST door (dev server on port 4177,
pnpm demo:staff, signed in asna.rep@objectos.ai):POST /api/v1/data/crm_opportunity_line_itemonto the rep's closed-won deal: 409,code: "RECORD_LOCKED", withuserMessageset to the sentence above and no record id. The deal kept 3 lines before and after.PATCHof one of that deal's lines: 409.POSTonto the rep's open deal: 201.Gates (local, at
bf9bb7a6)pnpm validate(lock)✓ Validation passedpnpm typechecktsc --noEmitclean;--listFilesincludes both changed.tsfilespnpm lint(lock)18 suggestion(s), unchanged in kind (approval-approver advisories)pnpm lint:i18n-gate✓ i18n lint gate: 0 i18n/missing-* issuespnpm hygiene✓ source hygiene cleanpnpm hygiene:tokens✓ source token ratchet cleanpnpm build(lock)✓ Build complete; the hook ships as a loweredjsbody (1,039 chars) at priority 90pnpm test(lock)Test Files 180 passed (180)·Tests 3671 passed / 1 skipped (3672)Playwright e2e runs in CI.
Token ratchet (ceilings RULED x1.5):
src/revenuebusiness semantics: ~15,918 before, ~16,313 after (ceiling ~24,000).src/revenueauthored total: ~19,058 before, ~19,453 after (ceiling ~28,500).src/sales: unchanged at ~103,157 authored total (ceiling ~160,500).origin/main8dfed905(feat(picklists): the shared option lists becomepicklistmetadata —*.picklist.tsandField.select({ picklist })replace_picklists.ts(acceptance of objectstack#18164) #2000 landed after this branch was cut):src/revenueauthored total ~19,557 / 28,500, still clean. There are no conflicting files.Acceptance notes
opportunity_lifecycle; it sits beside the line item, for the R4 / co-location reason above. It still has one envelope and one "closed" test, but it is a second hook body.test/package-ownership.test.tsjudges by artifact package, andsrc/revenueis still registered by the app package, so asrc/saleshook naming asrc/revenueobject passes every gate until revenue is packaged. This is an observation, not filed (AGENTS.md §3: no gate farm here).content/docs/sales/opportunities.mdx"Closed deals are locked" now also holds for adding and changing lines. Whether removing a line is part of that lock is [Decision] Removing a line item from an accepted quote or a closed deal: keep allowing it and say so (A), or refuse a hand delete once the platform tells a hook "this delete is a parent's cascade" (C)? #2078's question.Generated by Claude Code