diff --git a/.changeset/2161-accepted-quote-frees-contact.md b/.changeset/2161-accepted-quote-frees-contact.md new file mode 100644 index 000000000..1962294ec --- /dev/null +++ b/.changeset/2161-accepted-quote-frees-contact.md @@ -0,0 +1,27 @@ +--- +'hotcrm': patch +--- + +A contact or account whose only quote is accepted can now be deleted + +Handling a data-deletion request used to stop at the first quote the customer had accepted. Deleting +the contact, or the prospect account above it, was refused with *"… 1 active quote(s) … Close or +reassign the opportunities and quotes first"*, and neither could be done: an accepted quote is +settled and frozen. + +Now both deletes go through. **The accepted quote stays, with its Contact cleared** and everything +else on it unchanged, the way a closed deal already keeps its record when its primary contact is +deleted. When the accepted quote sits under the account being deleted, it goes with that account, as +every quote under a deleted account does. + +What still holds a contact is open work: an open opportunity or open quote naming them, or a +contract with them as its primary contact. The refusal now lists only what holds the contact, with +real plurals (*"1 open opportunity and 2 open quotes"*, not *"1 open opportunity(ies), 2 active +quote(s), 0 active contract(s)"*), and names only steps that free the contact: close the opportunity +or give it another primary contact, give the quote another contact or reject it, give the contract +another primary contact. + +A quote's Contact is still required on the save that moves the quote to Presented or Accepted, and +cannot be emptied while the quote is Presented, so a contract is never drafted without a recipient. +The Contact field's help text now says that. *Import & Export › Data deletion requests* and *Quotes* +describe the new behaviour. diff --git a/content/docs/guides/import-and-export.mdx b/content/docs/guides/import-and-export.mdx index f933b8b4d..468a7e3a3 100644 --- a/content/docs/guides/import-and-export.mdx +++ b/content/docs/guides/import-and-export.mdx @@ -151,8 +151,8 @@ None of it exists. There is no Privacy group in Setup, no data-subject-request o What you can do today is delete or edit the records directly: -- **Delete the contact.** The event-attendee rows naming that person go with it; the meetings themselves stay, so the team's history does not develop holes. -- **Delete the account** and its contacts cascade with it. Check what else hangs off it first — that cascade is worth planning rather than discovering. +- **Delete the contact.** The event-attendee rows naming that person go with it; the meetings themselves stay, so the team's history does not develop holes. Settled records stay too, with the person cleared from them: a deal already won or lost, and a quote the customer accepted, rejected or let expire, keep everything but the contact. The delete is refused while an open opportunity or open quote names the person, or while a contract has them as its primary contact, and the message says what to change first. +- **Delete the account** and its contacts cascade with it, as do the quotes under it, accepted ones included. Check what else hangs off it first — that cascade is worth planning rather than discovering. A record that would hold one of its contacts, as above, refuses the account delete too. - **Anonymise by hand** by clearing the identifying fields on the record you keep. There is no one-click equivalent, and no certificate comes out the other end. ## Backups & disaster recovery diff --git a/content/docs/guides/import-and-export.zh-Hans.mdx b/content/docs/guides/import-and-export.zh-Hans.mdx index 29590d484..4631a33bc 100644 --- a/content/docs/guides/import-and-export.zh-Hans.mdx +++ b/content/docs/guides/import-and-export.zh-Hans.mdx @@ -151,8 +151,8 @@ description: 数据进出 HotCRM 真正落地的那部分——列表视图上 今天你能做的，是直接删除或编辑记录： -- **删除该联系人。** 指名这个人的参会人行会随之删除；会议本身保留，因此团队的历史不会因此出现空洞。 -- **删除该客户**，它的联系人会级联删除。先看清还有什么挂在它下面——这条级联值得事先规划，而不是事后发现。 +- **删除该联系人。** 指名这个人的参会人行会随之删除；会议本身保留，因此团队的历史不会因此出现空洞。已经了结的记录也会保留，只是把这个人从上面清掉：已经赢单或丢单的商机，以及客户已接受、已拒绝或已过期的报价，除联系人以外一切照旧。只要还有未结的商机或报价指名这个人，或者还有合同以他为主要联系人，删除就会被拒绝，报错会说明先要改什么。 +- **删除该客户**，它的联系人会级联删除，挂在它下面的报价也一样，已接受的也不例外。先看清还有什么挂在它下面——这条级联值得事先规划，而不是事后发现。上面那些会挡住联系人的记录，同样会挡住客户的删除。 - **手工匿名化**：在你要保留的那条记录上清空可识别字段。没有一键等价物，末尾也不会产出任何证书。 ## 备份与灾难恢复 diff --git a/content/docs/guides/import-and-export.zh-Hant.mdx b/content/docs/guides/import-and-export.zh-Hant.mdx index bb3443a70..922a1f510 100644 --- a/content/docs/guides/import-and-export.zh-Hant.mdx +++ b/content/docs/guides/import-and-export.zh-Hant.mdx @@ -153,8 +153,8 @@ description: 資料進出 HotCRM 真正落地的那部分——清單檢視上 今天你能做的，是直接刪除或編輯記錄： -- **刪除該聯絡人。** 指名這個人的與會人列會隨之刪除；會議本身保留，因此團隊的歷史不會因此出現空洞。 -- **刪除該客戶**，它的聯絡人會級聯刪除。先看清還有什麼掛在它下面——這條級聯值得事先規劃，而不是事後發現。 +- **刪除該聯絡人。** 指名這個人的與會人列會隨之刪除；會議本身保留，因此團隊的歷史不會因此出現空洞。已經了結的記錄也會保留，只是把這個人從上面清掉：已經贏單或丟單的商機，以及客戶已接受、已拒絕或已過期的報價，除聯絡人以外一切照舊。只要還有未結的商機或報價指名這個人，或者還有合約以他為主要聯絡人，刪除就會被拒絕，報錯會說明先要改什麼。 +- **刪除該客戶**，它的聯絡人會級聯刪除，掛在它下面的報價也一樣，已接受的也不例外。先看清還有什麼掛在它下面——這條級聯值得事先規劃，而不是事後發現。上面那些會擋住聯絡人的記錄，同樣會擋住客戶的刪除。 - **手動匿名化**：在你要保留的那筆記錄上清空可識別欄位。沒有一鍵等價物，末尾也不會產出任何憑證。 ## 備份與災難復原 diff --git a/content/docs/sales/quotes.mdx b/content/docs/sales/quotes.mdx index f34fac900..7a8d903a8 100644 --- a/content/docs/sales/quotes.mdx +++ b/content/docs/sales/quotes.mdx @@ -67,7 +67,7 @@ On the quote's *Related* tab, the line items panel shows each line's product, qu - **Expiration date must be after the quote date** — no expired-on-creation quotes. - **Discount cannot exceed 60%.** This is a hard ceiling, not a warning: a quote whose **Discount %** is above 60 is refused on save, with *"Discount cannot exceed 60%"*. There is no approval that unblocks it — see [Discount ceiling](#discount-ceiling) below for what to do instead. (A discount above 100% was never possible; the field itself only accepts 0–100.) - **No single line may exceed 60% either.** The same ceiling, the same 60, applied to each line item's own **Discount %**: a line above it is refused on save with *"Line discount cannot exceed 60%"*. Both numbers are needed, because a quote's total runs them one after the other — each line is discounted first, then the quote's percentage comes off the sum — so a quote sitting innocently at 0% with every line at 90% off used to price 90% below list and clear the quote rule outright. -- **A quote cannot be Presented or Accepted without a contact.** A *Draft* or *In Review* quote needs no recipient — you often price a deal before knowing who signs it. But the write that moves it to *Presented* (or straight to *Accepted*) is refused unless **Contact** is filled in, and the message points at that field. Quotes that are never sent are unaffected: *Rejected* straight out of review, and *Expired* (which the nightly expiry sweep sets where it runs), both still go through without a contact. +- **A quote cannot be Presented or Accepted without a contact.** A *Draft* or *In Review* quote needs no recipient — you often price a deal before knowing who signs it. But the write that moves it to *Presented* (or straight to *Accepted*) is refused unless **Contact** is filled in, and the message points at that field; while a quote is *Presented*, Contact cannot be emptied either. Once a quote is *Accepted* the requirement has done its job — the contract was drafted from that save — so deleting the person later, for a data-deletion request say, leaves the accepted quote in place with **Contact** cleared. Quotes that are never sent are unaffected: *Rejected* straight out of review, and *Expired* (which the nightly expiry sweep sets where it runs), both still go through without a contact. - **A quote cannot be Accepted while an approval holds its opportunity.** Accepting a quote closes its deal as *Closed Won*, so the save that sets *Accepted* is refused while that deal is waiting on its large-deal approval (**Approval Status** is *Pending*) — or, where your administrator has switched them on, while its status-change or qualification approval is *Pending* or *Rejected* (**Status Change Approval**, **Qualification Approval**). The message names the approval that holds the deal. Nothing else moves: the quote stays *Presented*, the deal keeps its stage and no contract is drafted. Record the acceptance once that approval has been granted. ## What happens automatically @@ -117,7 +117,7 @@ To write the covering email inside HotCRM, use **Send Email** on the customer's There are no accept or reject buttons either. The customer's decision is recorded the same way: **Edit** the quote, set **Status**, click **Update**. -- **Accepted** — the save that makes a quote *Accepted* is what drafts the contract and close-wins the deal, as described in [What happens automatically](#what-happens-automatically). While an approval holds the deal, that save is refused instead (see [Built-in rules](#built-in-rules)). From then on the quote is frozen: a change to any field except **Internal Notes** is refused, Status included. Its line items are frozen with it: a line can no longer be added, changed or removed, and the same holds for an *Expired* quote. Deleting the whole quote is not affected, and its lines go with it. +- **Accepted** — the save that makes a quote *Accepted* is what drafts the contract and close-wins the deal, as described in [What happens automatically](#what-happens-automatically). While an approval holds the deal, that save is refused instead (see [Built-in rules](#built-in-rules)). From then on the quote is frozen: a change to any field except **Internal Notes** is refused, Status included. The one change it still takes is a link being emptied: when the contact or opportunity it names is deleted, the quote stays and that field is cleared. Its line items are frozen with it: a line can no longer be added, changed or removed, and the same holds for an *Expired* quote. Deleting the whole quote is not affected, and its lines go with it. - **Rejected** — nothing else happens. No contract is drafted and the opportunity keeps its stage, so you can quote again: set the rejected quote back to *Draft*, revise it and present it again, or click **Generate Quote** on the opportunity for a fresh one. The row of statuses across the top of the quote shows where it stands. It is a display, not a set of buttons. @@ -164,7 +164,7 @@ A brand-new org has no quotes at all — they are generated from opportunities, - The daily expiration sweep runs at 1 AM, and only on a deployment that has switched scheduled work on: a default install boots with the `quote_expiration` flow unbound, and the switch is described in [Administration › Automation](/docs/administration/automation#scheduled-automation). The time lives in source, not in Setup: it is the `schedule: '0 1 * * *'` on the start node of the `quote_expiration` flow (`src/revenue/flows/quote-expiration.flow.ts`). Changing it is a code change plus a redeploy — there is no setting to adjust on the quote object. - There is no quote PDF template to configure. HotCRM does not produce the quote document (the PDF a rep sends is made outside the app), and the one quote email template it ships is *Quote Created*, the notification a rep receives after **Generate Quote**. -- The Contact requirement is checked **when a quote moves into Presented or Accepted**, not on every save. A quote that was already sitting in *Presented* without a contact before this rule shipped keeps working normally — you can read and edit it, and filling the Contact in is an ordinary edit. It is the only case that can still reach *Accepted* without a recipient, and there it behaves as it always did: the contract cannot be drafted, and the reason lands in the server log. HotCRM's own demo data contains no such quote. +- The Contact requirement is checked **when a quote moves into Presented or Accepted** (and, while it is Presented, on any save that would empty it), not on every save. A quote that was already sitting in *Presented* without a contact before this rule shipped keeps working normally — you can read and edit it, and filling the Contact in is an ordinary edit. It is the only case that can still reach *Accepted* without a recipient, and there it behaves as it always did: the contract cannot be drafted, and the reason lands in the server log. HotCRM's own demo data contains no such quote. - The Discount ceiling is checked on **every save**, not only on a status change — the opposite boundary to the Contact requirement above, and deliberately so. "The discount may never exceed 60%" is an invariant; "a quote must have a recipient by the time it is presented" is a condition on a transition. The consequence worth knowing: a quote stored above the ceiling before this rule shipped would be frozen to every edit except lowering its discount, which is always allowed. HotCRM's own demo data contains no such quote — the deepest discount it seeds is 20%. - The line-item ceiling behaves the same way and reaches the same legacy case: a line stored above 60 before the rule shipped is refused on its next edit until its discount comes down. One boundary is worth knowing: a validation is evaluated against the record being saved, so an over-ceiling *line* freezes that line, not its parent quote — the quote can still change status. Nothing HotCRM ships is in that state (the deepest line discount it seeds is 20%), and after this rule a line above the ceiling cannot be written at all. - The ceiling value lives in source, not in Setup: it is `QUOTE_DISCOUNT_CEILING` in `src/sales/objects/_thresholds.ts`, interpolated into the condition and the message of *both* rules — the one on the quote object and the one on the quote line item. There is only ever one number to change, and changing it is a code change plus a redeploy. diff --git a/content/docs/sales/quotes.zh-Hans.mdx b/content/docs/sales/quotes.zh-Hans.mdx index e1e681bce..23d5ff2a8 100644 --- a/content/docs/sales/quotes.zh-Hans.mdx +++ b/content/docs/sales/quotes.zh-Hans.mdx @@ -114,7 +114,7 @@ HotCRM 不会替你发送报价。报价记录上没有发送按钮，应用既 接受和拒绝也都没有按钮。客户的决定用同样的方式记录：**编辑**报价，修改**状态**，点击**更新**。 -- **已接受**——把报价改为*已接受*的那次保存，就是起草合同并把商机推进为成交的那个动作，详见上文的[自动发生的事](#自动发生的事)。商机被审批卡住时，这次保存会被拒绝（见[内置规则](#内置规则)）。此后这份报价被冻结：除**内部备注**以外，任何字段的修改都会被拒绝，状态也不例外。它的行项目也随之冻结：不能再新增、修改或删除任何一行，*已过期*的报价同样如此。删除整份报价不受影响，它的行项目会随之一起删除。 +- **已接受**——把报价改为*已接受*的那次保存，就是起草合同并把商机推进为成交的那个动作，详见上文的[自动发生的事](#自动发生的事)。商机被审批卡住时，这次保存会被拒绝（见[内置规则](#内置规则)）。此后这份报价被冻结：除**内部备注**以外，任何字段的修改都会被拒绝，状态也不例外。唯一还能落地的改动是清空一条链接：它指名的联系人或商机被删除时，报价保留，那一栏被清空。它的行项目也随之冻结：不能再新增、修改或删除任何一行，*已过期*的报价同样如此。删除整份报价不受影响，它的行项目会随之一起删除。 - **已拒绝**——不会引发任何其他变化。不会起草合同，商机保持原阶段，所以你可以重新报价：把这份被拒绝的报价改回*草稿*，修改之后再次提交；或者在商机上点击**生成报价单**，另生成一份新的。 报价顶部那一排状态只显示这份报价当前所处的位置，它是一个显示，不是一组按钮。 diff --git a/content/docs/sales/quotes.zh-Hant.mdx b/content/docs/sales/quotes.zh-Hant.mdx index cf43231aa..4acd59294 100644 --- a/content/docs/sales/quotes.zh-Hant.mdx +++ b/content/docs/sales/quotes.zh-Hant.mdx @@ -116,7 +116,7 @@ HotCRM 不會替你發送報價。報價記錄上沒有發送按鈕，應用既 接受和拒絕也都沒有按鈕。客戶的決定用同樣的方式記錄：在報價上點擊 **Edit**，修改**狀態**，點擊 **Update**。 -- **已接受**——把報價改為*已接受*的那次儲存，就是起草合約並把商機推進為成交的那個動作，詳見上文的[自動發生的事](#自動發生的事)。商機被審批卡住時，這次儲存會被拒絕（見[內建規則](#內建規則)）。此後這份報價被凍結：除**內部備註**以外，任何欄位的修改都會被拒絕，狀態也不例外。它的行項目也隨之凍結：不能再新增、修改或刪除任何一行，*已過期*的報價同樣如此。刪除整份報價不受影響，它的行項目會隨之一起刪除。 +- **已接受**——把報價改為*已接受*的那次儲存，就是起草合約並把商機推進為成交的那個動作，詳見上文的[自動發生的事](#自動發生的事)。商機被審批卡住時，這次儲存會被拒絕（見[內建規則](#內建規則)）。此後這份報價被凍結：除**內部備註**以外，任何欄位的修改都會被拒絕，狀態也不例外。唯一還能落地的改動是清空一條連結：它指名的聯絡人或商機被刪除時，報價保留，那一欄被清空。它的行項目也隨之凍結：不能再新增、修改或刪除任何一行，*已過期*的報價同樣如此。刪除整份報價不受影響，它的行項目會隨之一起刪除。 - **已拒絕**——不會引發任何其他變化。不會起草合約，商機保持原階段，所以你可以重新報價：把這份被拒絕的報價改回*草稿*，修改之後再次提交；或者在商機上點擊**生成報價單**，另生成一份新的。 報價頂部那一排狀態只顯示這份報價目前所處的位置，它是一個顯示，不是一組按鈕。 diff --git a/src/revenue/objects/quote.object.ts b/src/revenue/objects/quote.object.ts index 1b442eefb..a17bd7326 100644 --- a/src/revenue/objects/quote.object.ts +++ b/src/revenue/objects/quote.object.ts @@ -84,7 +84,11 @@ export const Quote = ObjectSchema.create({ crm_contact: Field.lookup('crm_contact', { label: 'Contact', group: 'basic', - // Optional AT DRAFT TIME, required from `presented` on (#1017). + // A TRANSITION GATE, not an invariant (#1017, narrowed by #2161; AGENTS.md + // Metadata semantics rule 7). Optional at draft time. Required on the + // write that takes the quote INTO `presented` or `accepted`, and on every + // write while it is `presented`. Once the quote is accepted, the + // requirement has done its job and the link may be cleared. // // quote_generation maps the opportunity's `primary_contact`, which is // itself optional — a flat `required: true` here meant contact-less @@ -102,6 +106,25 @@ export const Quote = ObjectSchema.create({ // watching moment: the rep is told on the write that turns the quote // outward, against the field, with the quote still editable. // + // WHY THE ACCEPTED HALF STOPS AT THE ACCEPTING WRITE (#2161, the + // maintainer's ruling A of 2026-10-19). The contract is drafted from + // that one write (`quote_accepted_contract_draft` reads the contact off + // it), so that is the write that needs a recipient. After it the quote is + // settled and frozen, and #720 (maintainer, 2026-08-11) holds: "A frozen + // record must not make the people it references undeletable." The engine + // frees a deleted contact by writing `{ crm_contact: null }` on every + // quote that names it (`set_null`), and while this rule also covered an + // ALREADY-accepted quote it refused that write: measured on + // 18.0.0-next.3, `DELETE /data/crm_contact/` answered 400 "Contact is + // required", so the person stayed undeletable, and so did the account + // above them. Reading `previous` keeps the requirement on the write that + // turns a quote accepted and lifts it from every write after, so both + // rulings hold: no contract is drafted without a recipient (#1017), and + // the person who signed can still be erased (#720). The accepted quote + // stays, with its contact cleared — the trade #720 accepted for a closed + // deal's `primary_contact`. `contact_integrity` (contact.hook.ts) no + // longer counts an accepted quote for the same reason. + // // WHY EXACTLY `presented` + `accepted`, and not the other two terminal // states. Both of those are reachable WITHOUT the quote ever being sent // (see the `quote_status_progression` transitions below): @@ -116,21 +139,29 @@ export const Quote = ObjectSchema.create({ // verdict asks for a recipient the quote never had. // Neither state passes anything on to a contract, so neither needs one. // - // ⚠️ `has(record.status)` is load-bearing, not decoration — the same trap - // documented at `lead.object.ts`'s duplicate lookups: a bare + // ⚠️ Every read is guarded (#630), and the guards are load-bearing — the + // same trap documented at `lead.object.ts`'s duplicate lookups. A bare // `record.status == "presented"` aborts with `No such key` on any merged - // record that simply omits the column, and the engine's answer to a - // predicate that fails to evaluate is to SKIP it ("requiredWhen for - // 'crm_contact' failed to evaluate — skipped"). The rule would then read - // as enforced and require nothing at all. + // record that simply omits the column, and since 17.0.0-rc.2 the engine's + // answer to a predicate that fails to evaluate is to REJECT the write + // ("requiredWhen for 'crm_contact' failed to evaluate … — write + // rejected"; before rc.2 it was skipped in silence), so an unguarded + // read would refuse every write to such a row. `previous` is null on an + // insert, so `previous == null` decides an insert (a quote born accepted + // still needs its contact) before `previous.status` is read, and + // `has(previous.status)` covers a prior row stored without it. (Measured + // on 18.0.0-next.5: `has()` on a null `previous` also answers false, so + // the two overlap on an insert. The explicit `previous == null` is kept: + // it is the spelling the engine's own unevaluable-`previous` diagnostic + // asks for, and it does not lean on that answer.) // // @objectstack 12: string[] `referenceFilters` is dead (not read by the // picker); `dependsOn` is the live cascading form — scopes contacts to the // quote's `crm_account` (ADR-0049). dependsOn: ['crm_account'], - requiredWhen: P`has(record.status) && (record.status == "presented" || record.status == "accepted")`, + requiredWhen: P`has(record.status) && (record.status == "presented" || (record.status == "accepted" && (previous == null || !has(previous.status) || previous.status != "accepted")))`, description: - 'Required once the quote is Presented or Accepted — the drafted contract takes its Primary Contact from here.', + 'Required on the save that moves the quote to Presented or Accepted — the drafted contract takes its Primary Contact from here.', }), crm_opportunity: Field.lookup('crm_opportunity', { diff --git a/src/sales/objects/contact.hook.ts b/src/sales/objects/contact.hook.ts index bc3a5d90a..b844d4532 100644 --- a/src/sales/objects/contact.hook.ts +++ b/src/sales/objects/contact.hook.ts @@ -137,8 +137,14 @@ const contactHook: Hook = { api.object('crm_opportunity').count({ where: { primary_contact: id, stage: { $nin: ['closed_won', 'closed_lost'] } }, }), + // An OPEN quote holds the contact; a settled one does not (#2161). An + // accepted quote is settled like a rejected or expired one: its + // contact is required only on the write that accepts it + // (`crm_quote.crm_contact`'s `requiredWhen`, quote.object.ts), so the + // engine clears the link on delete and the accepted quote stays. The + // opportunity count above excludes both settled stages the same way. api.object('crm_quote').count({ - where: { crm_contact: id, status: { $nin: ['rejected', 'expired'] } }, + where: { crm_contact: id, status: { $nin: ['accepted', 'rejected', 'expired'] } }, }), api.object('crm_contract').count({ where: { crm_contact: id, status: 'activated' }, @@ -150,14 +156,15 @@ const contactHook: Hook = { // operation (#693). This hook fires on a direct // `DELETE /crm_contact/` AND as a cascade child of // `DELETE /crm_account/` (`crm_contact.crm_account` is a - // master-detail with `deleteBehavior: 'cascade'`), and the hook context - // carries nothing that distinguishes the two — measured on 17.0.0-rc.2, - // a cascaded `beforeDelete` ctx holds exactly - // {api,event,input,object,previous,provenance,ql,session,transaction,user} - // with no cascade marker. So "Cannot delete contact" told an account - // deleter that they had asked to delete a contact, and sent them - // looking for the wrong record. Naming the contact and both - // consequences is true in either context. + // master-detail with `deleteBehavior: 'cascade'`). Measured on + // 17.0.0-rc.2, a cascaded `beforeDelete` ctx carried no cascade marker; + // since 18.0.0-next.2 the platform declares one, + // `ctx.referentialCascadeDelete` (objectstack#23221, pinned in + // `test/cascade-guard-messages.test.ts`), and this guard does not read + // it. So "Cannot delete contact" told an account deleter that they had + // asked to delete a contact, and sent them looking for the wrong + // record. Naming the contact and both consequences is true in either + // context. const name = [ctx.previous?.first_name, ctx.previous?.last_name] .filter((part) => typeof part === 'string' && part.trim() !== '') .join(' '); @@ -170,7 +177,7 @@ const contactHook: Hook = { // What frees the contact is not the same for the three (#2019). An // opportunity's `primary_contact` and a quote's `crm_contact` are // optional, so once the deal or quote is settled the engine clears the - // link and the delete lands. A contract's `crm_contact` is REQUIRED: a + // link and the delete lands (an accepted quote included since #2161). A contract's `crm_contact` is REQUIRED: a // contract keeps its primary contact for its whole life, ended or not // — it is the account's signed record and is kept, so the engine // refuses the delete while ANY contract names the contact. The old @@ -180,16 +187,46 @@ const contactHook: Hook = { // primary contact on it. // // ⛔ The COUNT stays `activated`, and that is deliberate. This guard also - // runs inside an account delete (the cascade above) and cannot tell the - // two apart, and an account delete takes its draft and ended contracts + // runs inside an account delete (the cascade above) and does not tell + // the two apart, and an account delete takes its draft and ended contracts // with it (#549, `account_protection`). Counting those here would make // this guard refuse that account delete itself — today the engine // already refuses it on the cascade's order (objectstack#22305, pinned // in `test/hooks-runtime-sales.test.ts`), and that is the platform's to // fix. An ended contract on a DIRECT contact delete is refused by the // engine, in its own words naming the contract. + // + // The sentence names only what holds the contact, each count with its + // own plural (#2060 item 12, A-17: "2 open opportunity(ies), 0 active + // quote(s)" listed a zero and left the grammar to the reader), and only + // remedies that FREE it (#2161). For an open quote that is another + // contact or a rejection — ⛔ not acceptance: accepting drafts a + // contract naming the same person, and a contract keeps its primary + // contact for life. "Close or reassign the quotes" was the old remedy, + // and on an accepted quote, which this count used to include, neither + // could be done. + const held: string[] = []; + const remedies: string[] = []; + if (openOpps > 0) { + held.push(`${openOpps} open ${openOpps === 1 ? 'opportunity' : 'opportunities'}`); + remedies.push('close each open opportunity or give it another primary contact'); + } + if (openQuotes > 0) { + held.push(`${openQuotes} open ${openQuotes === 1 ? 'quote' : 'quotes'}`); + remedies.push('give each open quote another contact or reject it'); + } + if (activeContracts > 0) { + held.push(`${activeContracts} active ${activeContracts === 1 ? 'contract' : 'contracts'}`); + remedies.push('give each contract another primary contact: a contract keeps its primary contact even after it ends'); + } + const referents = + held.length > 1 ? `${held.slice(0, -1).join(', ')} and ${held[held.length - 1]}` : held[0]; + const todo = + remedies.length > 1 + ? `${remedies.slice(0, -1).join('; ')}; and ${remedies[remedies.length - 1]}` + : remedies[0]; throw refuse( - `${subject} is still referenced by ${openOpps} open opportunity(ies), ${openQuotes} active quote(s), ${activeContracts} active contract(s), so it cannot be deleted — and neither can its account, because deleting an account deletes its contacts. Close or reassign the opportunities and quotes first, and give each contract another primary contact: a contract keeps its primary contact even after it ends.`, + `${subject} is still referenced by ${referents}, so it cannot be deleted — and neither can its account, because deleting an account deletes its contacts. To free the contact, ${todo}.`, 'DELETE_RESTRICTED', 409, ); diff --git a/src/sales/translations/en/objects.commerce.ts b/src/sales/translations/en/objects.commerce.ts index 5b8708f9f..2bf63b65e 100644 --- a/src/sales/translations/en/objects.commerce.ts +++ b/src/sales/translations/en/objects.commerce.ts @@ -26,7 +26,7 @@ export const commerce: Record = { crm_account: { label: 'Account' }, crm_contact: { label: 'Contact', - help: 'Required once the quote is Presented or Accepted — the drafted contract takes its Primary Contact from here.', + help: 'Required on the save that moves the quote to Presented or Accepted — the drafted contract takes its Primary Contact from here.', }, crm_opportunity: { label: 'Opportunity' }, owner_id: { label: 'Quote Owner' }, diff --git a/src/sales/translations/zh-CN/objects.commerce.ts b/src/sales/translations/zh-CN/objects.commerce.ts index 5403a6b57..68fbe898b 100644 --- a/src/sales/translations/zh-CN/objects.commerce.ts +++ b/src/sales/translations/zh-CN/objects.commerce.ts @@ -50,7 +50,7 @@ export const commerce: Record = { description: { label: '描述' }, crm_contact: { label: '联系人', - help: '报价进入「已提交」或「已接受」后必填 —— 起草的合同从这里取主要联系人。', + help: '把报价改为「已提交」或「已接受」的那次保存必须填写 —— 起草的合同从这里取主要联系人。', }, owner_id: { label: '报价负责人' }, quote_date: { label: '报价日期' }, diff --git a/test/accepted-quote-frees-contact.test.ts b/test/accepted-quote-frees-contact.test.ts new file mode 100644 index 000000000..f86d38300 --- /dev/null +++ b/test/accepted-quote-frees-contact.test.ts @@ -0,0 +1,221 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeAll } from 'vitest'; +import type { VerifyStack } from '@objectstack/verify'; +import { REFUSAL_CODES } from '../src/sales/objects/_refusal'; +import { hotcrmStack, signUpPerson, type Person } from './helpers/verify-stack'; + +/** + * An accepted quote no longer makes its contact undeletable (#2161). + * + * ### The defect + * + * `contact_integrity` counted an accepted quote as "active", so a sales manager + * deleting the contact, or the prospect account above it, was told 409 + * `DELETE_RESTRICTED` "… 1 active quote(s) … Close or reassign the + * opportunities and quotes first". Neither remedy could be followed: accepted + * is a settled state, and `quote_workflow`'s freeze refuses a new contact on + * it. The one-word count fix alone was not enough — measured on 18.0.0-next.3, + * the engine's referential clear `{ crm_contact: null }` on the accepted quote + * was then refused by #1017's `requiredWhen`, 400 "Contact is required". + * + * ### The ruling (maintainer, 2026-10-19, letter A) + * + * `crm_quote.crm_contact` is required on the write that turns a quote INTO + * Presented or Accepted, and while it is Presented; an already-accepted quote + * lets the engine's referential cleanup clear it. `contact_integrity` stops + * counting an accepted quote, and its refusal names only remedies that free + * the contact. Both #720 ("A frozen record must not make the people it + * references undeletable") and #1017 (no contract drafted without a + * recipient) hold. + * + * ### The shapes, and which one each pin covers + * + * - A DIRECT contact delete: the accepted quote survives, `crm_contact` + * cleared, every other business field untouched. + * - An ACCOUNT delete where the accepted quote sits on ANOTHER account: the + * contact goes with its account, and the quote survives the same way. + * - An ACCOUNT delete where the accepted quote sits UNDER the deleted account: + * `crm_quote.crm_account` is a master-detail with `deleteBehavior: + * 'cascade'`, so the engine deletes the quote as the account's child. "The + * quote survives" does not apply to that shape, in any option; what the pin + * holds is that the delete lands. + * - CONTROL: an open (presented) quote still holds its contact, on both doors. + * + * Every request goes through the REST door as a signed-up sales manager — the + * caller the card measured — and each refusal is asserted by its envelope + * (`code` + `status`), never by "it threw". + */ + +type Rec = Record; + +let verify: VerifyStack; +let manager: Person; +let k = 0; + +beforeAll(async () => { + verify = await hotcrmStack(); + manager = await signUpPerson(verify, 'manager@accepted-quote-frees-contact.test', { + name: 'Erasure Manager', positions: ['sales_manager'], permissionSets: ['sales_manager'], + }); +}, 180_000); + +const stored = async (object: string, id: string): Promise => (await verify.rows(object, { id }))[0]; + +/** A REST call as the manager: status and parsed body. */ +const rest = async (method: string, path: string, body?: unknown): Promise<{ status: number; body: Rec }> => { + const res = await verify.apiAs(manager.token, method, path, body); + const text = await res.text(); + let parsed: Rec = {}; + try { parsed = JSON.parse(text); } catch { parsed = { raw: text }; } + return { status: res.status, body: parsed }; +}; + +/** + * A prospect account with one contact, and a quote in `status` naming that + * contact — under the contact's own account, or under a second account when + * `quoteElsewhere`. Seeded as the system: the fixture is not the subject. + */ +const build = async (status: string, quoteElsewhere = false) => { + const n = ++k; + const [account] = await verify.seed('crm_account', [{ name: `Erasure Co ${n}`, type: 'prospect', owner_id: manager.id }]); + const [other] = await verify.seed('crm_account', [{ name: `Erasure Other ${n}`, type: 'prospect', owner_id: manager.id }]); + const [contact] = await verify.seed('crm_contact', [{ + first_name: 'Ada', last_name: `Erasure ${n}`, email: `ada${n}@accepted-quote-frees-contact.test`, + crm_account: account!.id, owner_id: manager.id, + }]); + const [quote] = await verify.seed('crm_quote', [{ + name: `EQ-${n}`, status, crm_account: quoteElsewhere ? other!.id : account!.id, crm_contact: contact!.id, + quote_date: '2026-01-01', expiration_date: '2030-12-31', tax: 100, internal_notes: `signed ${n}`, owner_id: manager.id, + }]); + return { account: account!, other: other!, contact: contact!, quote: (await stored('crm_quote', String(quote!.id)))! }; +}; + +/** The columns a referential clear may move: the link itself and the engine's audit stamp. */ +const MOVED = new Set(['crm_contact', 'updated_at', 'updated_by']); +const unmoved = (row: Rec): Rec => + Object.fromEntries(Object.entries(row).filter(([key]) => !MOVED.has(key))); + +describe('deleting a contact whose only referent is an ACCEPTED quote', () => { + it('direct delete: lands, and the accepted quote survives with only its contact cleared', async () => { + const { contact, quote } = await build('accepted'); + const res = await rest('DELETE', `/data/crm_contact/${contact.id}`); + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(await stored('crm_contact', String(contact.id))).toBeUndefined(); + + const after = await stored('crm_quote', String(quote.id)); + expect(after, 'the accepted quote was destroyed').toBeDefined(); + expect(after!.status).toBe('accepted'); + expect(after!.crm_contact ?? null).toBeNull(); + // Every other column is as it was: the clear is the engine's one write. + expect(unmoved(after!)).toEqual(unmoved(quote)); + }); + + it('account delete, the quote on ANOTHER account: lands, and the quote survives with its contact cleared', async () => { + const { account, other, contact, quote } = await build('accepted', true); + const res = await rest('DELETE', `/data/crm_account/${account.id}`); + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(await stored('crm_account', String(account.id))).toBeUndefined(); + expect(await stored('crm_contact', String(contact.id))).toBeUndefined(); + + const after = await stored('crm_quote', String(quote.id)); + expect(after, 'the accepted quote on the other account was destroyed').toBeDefined(); + expect(after!.crm_account).toBe(other.id); + expect(after!.status).toBe('accepted'); + expect(after!.crm_contact ?? null).toBeNull(); + expect(unmoved(after!)).toEqual(unmoved(quote)); + }); + + it('account delete, the quote UNDER that account: lands, and the quote goes with it as its master-detail child', async () => { + // The card's literal repro. The quote does NOT survive here, and no option + // could make it: `crm_quote.crm_account` cascades. What changed is that + // the delete is no longer refused. + const { account, contact, quote } = await build('accepted'); + const res = await rest('DELETE', `/data/crm_account/${account.id}`); + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(await stored('crm_account', String(account.id))).toBeUndefined(); + expect(await stored('crm_contact', String(contact.id))).toBeUndefined(); + expect(await stored('crm_quote', String(quote.id))).toBeUndefined(); + }); +}); + +describe('CONTROL — an open quote still holds its contact', () => { + const expectHeld = (res: { status: number; body: Rec }) => { + expect(res.status, JSON.stringify(res.body)).toBe(REFUSAL_CODES.delete_restricted.status); + expect(res.body.code).toBe(REFUSAL_CODES.delete_restricted.code); + }; + + it('a PRESENTED quote refuses the direct delete, and nothing moves', async () => { + const { contact, quote } = await build('presented'); + const res = await rest('DELETE', `/data/crm_contact/${contact.id}`); + expectHeld(res); + expect(String(res.body.error)).toContain(`Contact Ada Erasure ${k} is still referenced by 1 open quote, so it cannot be deleted`); + expect(await stored('crm_contact', String(contact.id))).toBeDefined(); + expect((await stored('crm_quote', String(quote.id)))!.crm_contact).toBe(contact.id); + }); + + it('a PRESENTED quote refuses the account delete too, and the chain stays', async () => { + const { account, contact, quote } = await build('presented'); + const res = await rest('DELETE', `/data/crm_account/${account.id}`); + expectHeld(res); + expect(await stored('crm_account', String(account.id))).toBeDefined(); + expect(await stored('crm_contact', String(contact.id))).toBeDefined(); + expect((await stored('crm_quote', String(quote.id)))!.crm_contact).toBe(contact.id); + }); +}); + +describe('the refusal names what holds the contact, and only remedies that free it', () => { + it('counts with real plurals, lists no zero, and offers no remedy that keeps the contact held', async () => { + const { account, contact } = await build('presented'); + await verify.seed('crm_quote', [{ + name: `EQ-${k}-b`, status: 'draft', crm_account: account.id, crm_contact: contact.id, + quote_date: '2026-01-01', expiration_date: '2030-12-31', owner_id: manager.id, + }]); + await verify.seed('crm_opportunity', [{ + name: `Erasure deal ${k}`, crm_account: account.id, primary_contact: contact.id, stage: 'proposal', + amount: 10, close_date: '2030-06-30', owner_id: manager.id, + }]); + const res = await rest('DELETE', `/data/crm_contact/${contact.id}`); + expect(res.status, JSON.stringify(res.body)).toBe(REFUSAL_CODES.delete_restricted.status); + expect(res.body.code).toBe(REFUSAL_CODES.delete_restricted.code); + const message = String(res.body.error); + expect(message).toContain('is still referenced by 1 open opportunity and 2 open quotes, so it cannot be deleted'); + expect(message).toContain( + 'To free the contact, close each open opportunity or give it another primary contact; and give each open quote another contact or reject it.', + ); + // A-17: no "(s)" / "(ies)" shape, and no referent counted at zero. + expect(message).not.toMatch(/\((s|ies)\)/); + expect(message).not.toMatch(/\b0 /); + expect(message).not.toContain('contract'); + // Accepting is not offered for a quote: it drafts a contract naming the + // same person, and a contract keeps its primary contact for life. + expect(message).not.toMatch(/accept/i); + }); +}); + +describe('the gate still stands where #1017 put it', () => { + const expectContactRequired = (res: { status: number; body: Rec }) => { + expect(res.status, JSON.stringify(res.body)).toBe(REFUSAL_CODES.invalid_value.status); + expect(res.body.code).toBe(REFUSAL_CODES.invalid_value.code); + expect((res.body.fields ?? []).map((f: Rec) => `${f.field}:${f.code}`)).toContain('crm_contact:required'); + }; + + it('refuses the write that accepts a presented quote while clearing its contact', async () => { + const { quote } = await build('presented'); + const res = await rest('PATCH', `/data/crm_quote/${quote.id}`, { status: 'accepted', crm_contact: null }); + expectContactRequired(res); + const after = await stored('crm_quote', String(quote.id)); + expect(after!.status).toBe('presented'); + expect(after!.crm_contact).toBe(quote.crm_contact); + }); + + it('refuses a quote born accepted with no contact (an insert has no previous row)', async () => { + const { account } = await build('draft'); + const res = await rest('POST', '/data/crm_quote', { + name: `EQ-${k}-born`, status: 'accepted', crm_account: account.id, + quote_date: '2026-01-01', expiration_date: '2030-12-31', + }); + expectContactRequired(res); + expect(await verify.rows('crm_quote', { name: `EQ-${k}-born` })).toHaveLength(0); + }); +}); diff --git a/test/cascade-guard-messages.test.ts b/test/cascade-guard-messages.test.ts index ffa9123dc..0aefe37f7 100644 --- a/test/cascade-guard-messages.test.ts +++ b/test/cascade-guard-messages.test.ts @@ -220,7 +220,7 @@ describe('deleting an account whose contact is still referenced', () => { // BY, and it stays out of the prose. expect(message).toContain('Contact Ada Lovelace is still referenced by'); expect(message).not.toContain(contact.id); - expect(message).toContain('1 open opportunity(ies)'); + expect(message).toContain('still referenced by 1 open opportunity, so it cannot be deleted'); expect(message).toContain('neither can its account'); // The reported symptom: a caller who addressed an ACCOUNT was told they // could not delete a CONTACT. @@ -241,7 +241,7 @@ describe('deleting an account whose contact is still referenced', () => { const message = await deleteAndCatch('crm_contact', contact.id); expect(message).toContain('Contact Ada Lovelace is still referenced by'); expect(message).not.toContain(contact.id); - expect(message).toContain('1 open opportunity(ies)'); + expect(message).toContain('still referenced by 1 open opportunity, so it cannot be deleted'); }); it('still deletes an account whose contact nothing references', async () => { diff --git a/test/hooks-runtime-sales.test.ts b/test/hooks-runtime-sales.test.ts index 4c9cff1fc..4b99efb3d 100644 --- a/test/hooks-runtime-sales.test.ts +++ b/test/hooks-runtime-sales.test.ts @@ -1008,7 +1008,7 @@ describe('contact_integrity', () => { it('refuses to delete a contact referenced by open work', async () => { const contact = await referenced({ opp: 'proposal', quote: 'draft', contract: 'activated' }); await expect(deleteContact(contact)) - .rejects.toThrow(/still referenced by 1 open opportunity\(ies\), 1 active quote\(s\), 1 active contract\(s\)/); + .rejects.toThrow(/still referenced by 1 open opportunity, 1 open quote and 1 active contract, so it cannot be deleted/); }); /** @@ -1079,7 +1079,7 @@ describe('contact_integrity', () => { const guarded = await deleteContact(contact).then(() => null, (e: Error & Rec) => e); expect(guarded, `the contact under an activated contract was deleted (${ending} leg)`).toBeTruthy(); expect(guarded).toMatchObject({ code: 'DELETE_RESTRICTED', status: 409 }); - expect(guarded!.message, ending).toContain('1 active contract(s)'); + expect(guarded!.message, ending).toContain('still referenced by 1 active contract, so it cannot be deleted'); expect(guarded!.message, ending).toContain('give each contract another primary contact: a contract keeps its primary contact even after it ends.'); expect(guarded!.message, ending).not.toContain('Close or reassign those records'); diff --git a/test/quote-contact-required-when.test.ts b/test/quote-contact-required-when.test.ts index ba9e58718..c38d5c196 100644 --- a/test/quote-contact-required-when.test.ts +++ b/test/quote-contact-required-when.test.ts @@ -102,9 +102,12 @@ const expectContactRequired = (env: ReturnType) => { // ─────────────────────────────────────────── the declared shape of the gate ── describe('the gate is declared where the platform can act on it', () => { - it('requires the contact exactly from `presented` onward', () => { + it('requires the contact on the write INTO `presented` or `accepted`, and while `presented`', () => { + // A gate, not an invariant (#2161, ruling A): an ALREADY-accepted quote is + // past it, so the engine's referential clear can free a deleted contact + // (#720). `test/accepted-quote-frees-contact.test.ts` drives both halves. expect(celSource(quote.fields.crm_contact.requiredWhen)).toBe( - 'has(record.status) && (record.status == "presented" || record.status == "accepted")', + 'has(record.status) && (record.status == "presented" || (record.status == "accepted" && (previous == null || !has(previous.status) || previous.status != "accepted")))', ); });