diff --git a/.changeset/lead-conversion-normalized-account-match.md b/.changeset/lead-conversion-normalized-account-match.md new file mode 100644 index 00000000..2a348d49 --- /dev/null +++ b/.changeset/lead-conversion-normalized-account-match.md @@ -0,0 +1,50 @@ +--- +'hotcrm': patch +--- + +Lead conversion now reuses an existing account when the company name differs only in capitalisation or spacing — converting a lead for `ACME Corp` attaches it to your `Acme Corp` account instead of creating a second one. + +The flow used to dedupe on the raw `crm_account.name`, so every case or spacing +variant produced its own account and the account list slowly filled with +near-duplicates of the same company. Matching is now **normalize-then-exact**: +both sides are lower-cased, trimmed and have runs of internal whitespace +collapsed before comparison. It is deliberately not fuzzy — `Acme Corp` and +`Acme Corporation` remain two different companies — because ranking candidate +matches needs a human review step this app does not have. + +The comparison happens on two new derived columns, `crm_account.name_normalized` +and `crm_lead.company_normalized`, maintained by the `account_protection` and +`lead_duplicate_check` hooks. Both are read-only and hidden: nobody authors +them, and the display values (`name`, `company`) are untouched, so the account +created from a lead still carries the company name exactly as it was typed. + +Storing the keys is forced rather than preferred, and each alternative was +measured against 17.0.0-rc.1 (the measurements are pinned in +`test/account-name-normalized-match.test.ts`, so a platform upgrade that changes +any of them fails loudly instead of leaving stale reasoning in a comment): + +- a flow template cannot fold a string — the automation engine's token resolver + understands only `NOW()` / `TODAY()`, so `{LOWER(x)}`, `{TRIM(x)}` and + `{x.toLowerCase()}` all resolve to nothing; +- a formula field has no physical column, so nothing can filter on it; +- `$regex` is not a case-insensitive equality on SQL at all — it compiles to a + substring `LIKE`, which also matches `Not Acme Corp Ltd`, cannot collapse + whitespace, and cannot use an index. + +`name_normalized` carries a plain index, **not** a unique one. Account-name +uniqueness already lives, per organization, on `name` (#625); a unique +normalized column would subsume that constraint and re-open a decision made one +release earlier, for a guarantee this change does not need. It would also be +impossible to add to any deployment that already holds both spellings, since +creating a unique index fails on existing duplicates. + +**Upgrading an existing deployment.** Both columns start empty on rows written +before this version. An account with no key is invisible to the lookup, so +conversion would create *more* duplicates than before, not fewer; a lead with no +key stops its conversion outright, because the automation engine refuses to run +a query whose filter resolved to nothing rather than widening it. A one-time +backfill (re-save each account and open lead; the hooks derive the keys) is +documented in `docs/MAINTENANCE.md` §3.3. Fresh installs need nothing: seed +writes run lifecycle hooks, so every row is stamped as it is created. + +Fixes #626. diff --git a/content/docs/sales/accounts.mdx b/content/docs/sales/accounts.mdx index e080ece7..7f65a505 100644 --- a/content/docs/sales/accounts.mdx +++ b/content/docs/sales/accounts.mdx @@ -59,7 +59,7 @@ The hierarchy gives you: ## Built-in rules -- The account **name must be unique within your organization**. You'll get an error if you try to create a second account with a name you already use. Another organization on the same platform is free to have its own "Acme Corp" — the rule is scoped to your data, not the whole platform. Names are compared exactly as typed, so `Acme Corp` and `ACME Corp` currently count as two different accounts. +- The account **name must be unique within your organization**. You'll get an error if you try to create a second account with a name you already use. Another organization on the same platform is free to have its own "Acme Corp" — the rule is scoped to your data, not the whole platform. The uniqueness check compares names exactly as typed, so `Acme Corp` and `ACME Corp` can both exist as separate accounts. Lead conversion is more forgiving: it ignores capitalisation and extra spaces when looking for an account to reuse (see [Leads](./leads)), so converting a lead for `ACME Corp` attaches it to your existing `Acme Corp` rather than creating a near-duplicate. - **Annual revenue must be zero or positive** — no negative numbers allowed. - You **cannot delete** an account that has open opportunities or active contracts. Set it to *Inactive* instead (`is_active = false`). diff --git a/content/docs/sales/accounts.zh-Hans.mdx b/content/docs/sales/accounts.zh-Hans.mdx index 595a5cc2..3e0420ca 100644 --- a/content/docs/sales/accounts.zh-Hans.mdx +++ b/content/docs/sales/accounts.zh-Hans.mdx @@ -59,7 +59,7 @@ Acme Corp Global ## 内置规则 -- 客户**名称在你所在的组织内必须唯一**。如果你尝试用一个已存在的名称再建一个客户,会收到错误。同一平台上的其他组织可以有自己的 "Acme Corp" —— 这条规则只作用于你自己的数据,而不是整个平台。名称按原样精确比较,因此 `Acme Corp` 与 `ACME Corp` 目前算作两个不同的客户。 +- 客户**名称在你所在的组织内必须唯一**。如果你尝试用一个已存在的名称再建一个客户,会收到错误。同一平台上的其他组织可以有自己的 "Acme Corp" —— 这条规则只作用于你自己的数据,而不是整个平台。唯一性校验按原样精确比较名称,因此 `Acme Corp` 与 `ACME Corp` 可以作为两个独立的客户同时存在。线索转换则更宽松:它在寻找可复用的客户时会忽略大小写和多余空格(见[线索](./leads)),因此转换公司名为 `ACME Corp` 的线索时,会挂到你已有的 `Acme Corp` 上,而不是新建一个近似重复的客户。 - **年营收必须为零或正数**——不允许负数。 - 你**无法删除**一个有开放商机或活跃合同的客户。改为将其设为*非活跃*(`is_active = false`)。 diff --git a/content/docs/sales/accounts.zh-Hant.mdx b/content/docs/sales/accounts.zh-Hant.mdx index 3e0c3c8a..a2277b07 100644 --- a/content/docs/sales/accounts.zh-Hant.mdx +++ b/content/docs/sales/accounts.zh-Hant.mdx @@ -59,7 +59,7 @@ Acme Corp Global ## 內建規則 -- 客戶**名稱在你所屬的組織內必須唯一**。如果你嘗試用一個已存在的名稱再建一個客戶,會收到錯誤。同一平台上的其他組織可以有自己的 "Acme Corp" —— 這條規則只作用於你自己的資料,而不是整個平台。名稱按原樣精確比較,因此 `Acme Corp` 與 `ACME Corp` 目前算作兩個不同的客戶。 +- 客戶**名稱在你所屬的組織內必須唯一**。如果你嘗試用一個已存在的名稱再建一個客戶,會收到錯誤。同一平台上的其他組織可以有自己的 "Acme Corp" —— 這條規則只作用於你自己的資料,而不是整個平台。唯一性檢查按原樣精確比較名稱,因此 `Acme Corp` 與 `ACME Corp` 可以作為兩個獨立的客戶同時存在。潛在客戶轉換則更寬鬆:它在尋找可重用的客戶時會忽略大小寫與多餘空格(見[潛在客戶](./leads)),因此轉換公司名為 `ACME Corp` 的潛在客戶時,會掛到你既有的 `Acme Corp` 上,而不是新建一個近似重複的客戶。 - **年營收必須為零或正數**——不允許負數。 - 你**無法刪除**一個有開放商機或活躍合約的客戶。改為將其設為*非活躍*(`is_active = false`)。 diff --git a/content/docs/sales/leads.mdx b/content/docs/sales/leads.mdx index 73eed1ea..0976f211 100644 --- a/content/docs/sales/leads.mdx +++ b/content/docs/sales/leads.mdx @@ -94,7 +94,7 @@ When a lead is qualified, click **Convert**. A small form pops up asking: The system then, in a single step: -1. Creates an **Account** from the lead's company information. +1. Creates an **Account** from the lead's company information — or reuses the one you already have. The match ignores capitalisation and extra spaces, so a lead for `ACME Corp` lands on your existing `Acme Corp` instead of creating a second account. It is not a fuzzy match: `Acme Corp` and `Acme Corporation` are still two different companies. 2. Creates a **Contact** from the lead's person information, attached to that account, marked as primary. 3. *(Optional)* Creates an **Opportunity** in the *Prospecting* stage with a 90-day close date. 4. Marks the lead as **Converted** and locks it. diff --git a/content/docs/sales/leads.zh-Hans.mdx b/content/docs/sales/leads.zh-Hans.mdx index 6897f952..32aa6fc8 100644 --- a/content/docs/sales/leads.zh-Hans.mdx +++ b/content/docs/sales/leads.zh-Hans.mdx @@ -94,7 +94,7 @@ description: 在入站咨询成为真正交易之前对其进行捕获、评分 然后系统会一步完成: -1. 根据线索的公司信息创建一个**客户**。 +1. 根据线索的公司信息创建一个**客户**——如果已经有同名客户,则直接复用。匹配时忽略大小写和多余空格,因此公司名为 `ACME Corp` 的线索会落到你已有的 `Acme Corp` 上,而不是再建一个客户。这不是模糊匹配:`Acme Corp` 与 `Acme Corporation` 仍然是两家不同的公司。 2. 根据线索的个人信息创建一个**联系人**,附加到该客户上,并标记为主要联系人。 3. *(可选)* 在*开发期*阶段创建一个**商机**,成交日期设为 90 天后。 4. 将线索标记为**已转换**并锁定它。 diff --git a/content/docs/sales/leads.zh-Hant.mdx b/content/docs/sales/leads.zh-Hant.mdx index eac4638a..eeaf76cd 100644 --- a/content/docs/sales/leads.zh-Hant.mdx +++ b/content/docs/sales/leads.zh-Hant.mdx @@ -94,7 +94,7 @@ description: 在入站詢問成為真正交易之前對其進行擷取、評分 然後系統會一步完成: -1. 根據潛在客戶的公司資訊建立一個**客戶**。 +1. 根據潛在客戶的公司資訊建立一個**客戶**——若已有同名客戶,則直接重用。比對時忽略大小寫與多餘空格,因此公司名為 `ACME Corp` 的潛在客戶會落到你既有的 `Acme Corp` 上,而不是再建一個客戶。這不是模糊比對:`Acme Corp` 與 `Acme Corporation` 仍然是兩家不同的公司。 2. 根據潛在客戶的個人資訊建立一個**聯絡人**,附加到該客戶上,並標記為主要聯絡人。 3. *(可選)* 在*開發期*階段建立一個**商機**,成交日期設為 90 天後。 4. 將潛在客戶標記為**已轉換**並鎖定它。 diff --git a/docs/MAINTENANCE.md b/docs/MAINTENANCE.md index 39d10a59..4a910a9f 100644 --- a/docs/MAINTENANCE.md +++ b/docs/MAINTENANCE.md @@ -172,6 +172,74 @@ orphans (the `__search` class above is excluded until #3955 lands): `--allow-destructive` drops columns irreversibly — never run it without the backup from step 2, and never against a database whose plan you have not read. +### 3.3 Backfilling a hook-derived column + +Some HotCRM columns are **derived**: no one authors them, a lifecycle hook +computes them from another field on every write. Two of them are match keys the +lead-conversion flow reads (#626): + +| Column | Derived from | Writer | +| --- | --- | --- | +| `crm_account.name_normalized` | `crm_account.name` | `account_protection` | +| `crm_lead.company_normalized` | `crm_lead.company` | `lead_duplicate_check` | + +A **fresh install needs nothing here.** Seed writes run lifecycle hooks +(`skipTriggers` suppresses record-change automation, not hooks — measured in +#617), so every seeded and every subsequently created row gets its key stamped +on insert. + +An **in-place upgrade does**. A row written before the column existed holds +`NULL`, and the two objects then fail in two different ways — both measured, and +worth knowing apart when triaging: + +- **An account with no key is invisible to the match.** The conversion finds + nothing and creates a *second* account for a company that already has one — + silently. This is the failure that makes the backfill non-optional: it is + more duplicates than the behaviour the change replaced. +- **A lead with no key stops the conversion.** The filter value resolves to + nothing, and `get_record` refuses to run rather than widen the query: + *"refusing to run — 1 filter condition(s) resolved to nothing and were dropped + from the query: `{leadRecord.company_normalized}` (at name_normalized)"*. The + run is recorded failed and the lead stays unconverted. Loud, and the message + names the missing key — re-save that lead and convert again. + +> [!NOTE] +> **This section is a contingency, not a step in any current upgrade.** HotCRM's +> deployment shape today is **fresh installs only**, which is the whole reason +> the procedure below is documented rather than automated, and the reason +> `name_normalized` carries no unique index (see `src/objects/account.object.ts`). +> Both conclusions are conditional on that premise. If HotCRM ever acquires +> long-lived installs that upgrade in place, re-read this section and the index +> decision together — neither is a universal judgement. + +The backfill is a **re-save**: write a row's own `name` / `company` back to it +and the hook derives the key. Nothing else about the row changes, and re-saving +a row that already has a key is a no-op — so the pass is idempotent and safe to +repeat or to run over every row rather than hunting for the empty ones. + +1. **Take a database backup.** This rewrites every account and lead row. +2. **Read the rows.** Any path you already use is fine — a Console export, a + `GET` against the record API, or a direct read replica query. You need only + `id` plus the source field (`name` for accounts, `company` for leads). +3. **Write each row back to itself**, one `PATCH` per record, against the + record endpoint `PATCH {basePath}/data/:object/:id` (`basePath` is + `/api/v1`): + + ```bash + curl -s -X PATCH "$HOTCRM/api/v1/data/crm_account/$ID" \ + -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' \ + -d '{"name": "Acme Corp"}' # the row's OWN current name + ``` + + Do **not** send `name_normalized` itself: it is `readonly`, so an incoming + value is stripped, and the hook would overwrite it anyway. Repeat for + `crm_lead` with `{"company": "…"}`; converted leads can be skipped + (`is_converted = true`) — nothing converts them again. +4. **Verify.** No row should be left with an empty key, and the end-to-end + check is the one that matters: convert a lead whose company differs from an + existing account only in case or spacing, and confirm it **reuses** that + account instead of creating a second one. + ## 4. Seed-data staleness — the #1 HotCRM pitfall Stale seed data is the most common cause of "Studio shows a red diff --git a/src/flows/lead-conversion.flow.ts b/src/flows/lead-conversion.flow.ts index c0d25411..97da868c 100644 --- a/src/flows/lead-conversion.flow.ts +++ b/src/flows/lead-conversion.flow.ts @@ -76,10 +76,47 @@ export const LeadConversionFlow: Flow = { }, { // Account dedupe: before creating a new account, look for an existing one - // with the same company name. Exact-name match (case/whitespace sensitive) - // — the standard lightweight dedupe; fuzzy matching is out of scope here. + // with the same company name — NORMALIZED (#626). This used to compare + // `crm_account.name` against the raw `{leadRecord.company}`, so + // "Acme Corp" and "ACME Corp" produced two accounts. + // + // Both sides of this comparison are stored, hook-maintained columns, and + // that is forced rather than chosen: a flow template cannot normalize + // ANYTHING. `service-automation`'s `resolveToken` recognises exactly one + // function form — `NOW()` / `TODAY()` — and every bare identifier in the + // expression fallback is substituted before evaluation, so no string + // method is reachable either: `{LOWER(x)}`, `{TRIM(x)}` and + // `{x.toLowerCase()}` all resolve to `undefined`, and an unwrapped + // `LOWER({x})` interpolates literally to "LOWER(Acme Corp)". A formula + // field is no help either — it has no physical column to filter on. So + // the producer canonicalizes (`account_protection`, + // `lead_duplicate_check`) and this node does a plain, indexed equality + // match. `test/account-name-normalized-match.test.ts` re-measures all of + // that rather than trusting this paragraph. + // + // Normalize-then-EXACT only: lower + trim + collapse internal + // whitespace. Fuzzy matching stays out of scope, as before. + // + // If the lead carries NO `company_normalized`, this node does not fall + // back and does not match everything — `get_record` REFUSES TO RUN: + // + // get_record: refusing to run — 1 filter condition(s) resolved to + // nothing and were dropped from the query: `{leadRecord.company_ + // normalized}` (at name_normalized). An absent condition does not + // narrow a query, it widens it … + // + // (measured on 17.0.0-rc.1; pinned in the test file). That is the right + // failure: the only way to reach it is a lead row written before this + // change, which is what the backfill in docs/MAINTENANCE.md §3.3 exists + // for, and a conversion that stops with that message is far cheaper to + // diagnose than one that quietly creates a duplicate account. + // + // Deliberately NOT papered over with a second, case-sensitive lookup on + // the raw `name`: a missing key means the producer did not run, and a + // tolerant consumer path would hide that while restoring the exact bug + // this node exists to fix. id: 'find_account', type: 'get_record', label: 'Find Existing Account', - config: { objectName: 'crm_account', filter: { name: '{leadRecord.company}' }, outputVariable: 'matchedAccount' }, + config: { objectName: 'crm_account', filter: { name_normalized: '{leadRecord.company_normalized}' }, outputVariable: 'matchedAccount' }, }, { id: 'decision_account', type: 'decision', label: 'Account Already Exists?', @@ -89,6 +126,12 @@ export const LeadConversionFlow: Flow = { // NEW-account branch. outputVariable is `createdAccount`; the assignment // below normalizes both branches onto a single `accountId` id string so // downstream nodes don't need to know which path ran. + // + // `name` carries the lead's company VERBATIM — the display value. The + // match key `name_normalized` is deliberately absent: it is readonly and + // hook-owned, and `account_protection` derives it from the `name` written + // here, so an account created by this node is immediately findable by the + // next conversion. id: 'create_account', type: 'create_record', label: 'Create Account', config: { objectName: 'crm_account', diff --git a/src/objects/account.hook.ts b/src/objects/account.hook.ts index 2a4f552b..2040c08e 100644 --- a/src/objects/account.hook.ts +++ b/src/objects/account.hook.ts @@ -9,6 +9,8 @@ import type { HookApi } from './_hook-api'; * - Validates `website` format and `annual_revenue` non-negative. * - Projects `billing_address.country` onto the flat `billing_country` column * the territory sharing rules filter on (#621). + * - Folds `name` into the `name_normalized` column lead conversion matches + * accounts on (#626). * - Refuses to delete a `customer` account that still has open opportunities. */ const accountHook: Hook = { @@ -67,6 +69,41 @@ const accountHook: Hook = { const normalized = typeof country === 'string' ? country.trim().toUpperCase() : ''; input.billing_country = normalized === '' ? null : normalized; } + + // ─── Account-name match key (#626) ───────────────────────────────── + // + // `name_normalized` is the column `lead_conversion` matches accounts on, + // and this block is its only writer. It exists because the flow that + // reads it cannot compute it: `service-automation`'s template resolver + // knows one function form (`NOW()` / `TODAY()`), so `{LOWER(x)}`, + // `{TRIM(x)}` and `{x.toLowerCase()}` all resolve to `undefined` — and a + // formula field has no physical column to filter on + // (`fieldHasColumn(formula) === false`). The canonical form therefore has + // to be established HERE, by the producer, exactly as `crm_lead.email` + // and `crm_contact.email` are. + // + // lower + trim + collapse internal whitespace, so "Acme Corp", + // "ACME Corp" and " acme corp " all land on `acme corp`. That is the + // whole transform: normalize-then-EXACT. Fuzzy matching is out of scope + // (see the field's doc comment on `crm_account`). + // + // Recompute ONLY when the write carries the name — a partial update that + // never mentions `name` must leave the key alone, or every unrelated edit + // would blank it and make the account invisible to conversion. `name` is + // required + notNull, so on insert it is always present; the null branch + // is for a whitespace-only value the validator would reject anyway. + // + // Written inline, not as a module-scope helper, for the same reason as + // the block above: hook bodies lower to metadata-only (no free + // identifiers), which `test/action-sandbox.test.ts` enforces. + if ('name' in input) { + const rawName = input.name; + const normalizedName = + typeof rawName === 'string' + ? rawName.trim().toLowerCase().replace(/\s+/g, ' ') + : ''; + input.name_normalized = normalizedName === '' ? null : normalizedName; + } } // Stamp last_activity_date when ownership or type changes (migrated from the diff --git a/src/objects/account.object.ts b/src/objects/account.object.ts index 74bd4464..401cdc46 100644 --- a/src/objects/account.object.ts +++ b/src/objects/account.object.ts @@ -67,6 +67,64 @@ export const Account = ObjectSchema.create({ group: 'basic', }), + /** + * Case- and whitespace-folded copy of `name` — the key lead conversion + * matches accounts on (#626). + * + * ### Why a stored column, and not any of the cheaper shapes + * + * `lead_conversion` used to dedupe on the raw `name`, so `"Acme Corp"` and + * `"ACME Corp"` produced two accounts. Three cheaper fixes were measured + * against 17.0.0-rc.1 and all three are dead ends — `test/account-name- + * normalized-match.test.ts` re-measures each one so this comment cannot + * quietly go stale: + * + * - **The flow cannot normalize.** `service-automation`'s `resolveToken` + * understands exactly one function form, `/^(NOW|TODAY)\s*\(\s*\)…/`. + * `{LOWER(x)}`, `{TRIM(x)}` and `{x.toLowerCase()}` all resolve to + * `undefined` (every bare identifier is substituted before the expression + * is evaluated, so no string method is reachable), and an unwrapped + * `LOWER({x})` interpolates literally to `"LOWER(Acme Corp)"`. + * - **A formula field cannot be the match key.** `driver-sql`'s + * `fieldHasColumn` returns false for `type: 'formula'`, so a computed + * value has no physical column and nothing can filter on it. + * - **`$regex` is not an answer.** On `driver-sql` it does not even run as a + * regex: it compiles to `LIKE '%value%'`, a SUBSTRING match, so + * `"Acme Corp"` would also match `"Not Acme Corp Ltd"`. It cannot collapse + * internal whitespace, the leading wildcard defeats the index, and on the + * in-memory driver it does compile user-controlled text into a `RegExp`. + * + * So the canonical form is established by the PRODUCER at write time — the + * same doctrine `crm_lead.email` / `crm_contact.email` already follow — and + * the reader does a plain, indexed equality match. The other side of that + * comparison is `crm_lead.company_normalized`, which exists for the same + * reason: a flow can compare two stored columns, it cannot compute either. + * + * ### What it holds + * + * `name`, trimmed, lower-cased, with runs of internal whitespace collapsed + * to one space. Nothing else — normalize-then-EXACT. Fuzzy matching + * (punctuation folding, legal-suffix stripping, edit distance) is + * deliberately out of scope; it turns a lookup into a ranking problem and + * needs a human review affordance this app does not have. + * + * Derived, never authored: `account.hook.ts` recomputes it on every write + * that carries `name`, and leaves it alone on every write that does not. + * `readonly` keeps user/API writes off it (INSERT is exempt from the strip + * and UPDATE strips only CALLER-supplied keys, so the hook's own write + * always survives); `hidden` keeps a machine-owned column out of forms and + * pickers — the same pairing `crm_forecast.seed_key` uses. + */ + name_normalized: Field.text({ + label: 'Account Name (Normalized)', + description: + 'Match key for lead conversion: Account Name lower-cased, trimmed, with internal whitespace collapsed. Maintained by the account_protection hook — never edit directly.', + readonly: true, + hidden: true, + maxLength: 255, + group: 'system', + }), + // ADR-0079 record title (was titleFormat '{account_number} - {name}'). display_title: Field.formula({ label: 'Display Title', @@ -292,6 +350,35 @@ export const Account = ObjectSchema.create({ // The territory sharing rules filter on this column, so it is read on // every account query a territory recipient makes (#621). { fields: ['billing_country'] }, + // Lead conversion filters on this column on every single conversion, and + // the whole point of the column is to replace an unindexed `$regex` scan + // (#626). Plain index — NOT `unique: true`, deliberately: + // + // 1. Uniqueness of account names is already declared, per tenant, on the + // `name` field (#625). A unique `name_normalized` SUBSUMES it (equal + // names normalize equally), so adding one would force a re-decision of + // the constraint #625 landed one round earlier — for a guarantee this + // issue does not need. + // 2. This column is a MATCH key, not a policy. The defect was "the flow + // fails to reuse an existing account"; the fix is for the flow to find + // it. Rejecting a near-duplicate name outright is a separate, + // data-quality decision — and this repo has already chosen the soft + // shape for exactly that question once (#598 removed the hard unique on + // `crm_lead.email` because refusing a repeat enquiry is worse than + // recording it). + // 3. Measured hazard: `create_index` FAILS on any deployment that already + // holds `Acme Corp` and `ACME Corp` separately, so a unique index could + // not be an upgrade step without a merge pass first. That hazard is + // bounded today ONLY because this repo's deployment shape is fresh + // installs — see docs/MAINTENANCE.md §3.3. If that premise ever changes, + // this conclusion changes with it; it is conditional, not universal. + // + // Consequence, recorded rather than hidden: two accounts CAN still hold the + // same normalized name if something outside the flow creates them, and + // `get_record` has no `sort` option, so the conversion would reuse an + // arbitrary one of them. Reusing one of N is still strictly better than + // creating the N+1th, which is what happens today. + { fields: ['name_normalized'] }, ], // API surface + capabilities. `trash` / `mru` were removed in @objectstack 12 diff --git a/src/objects/lead.hook.ts b/src/objects/lead.hook.ts index add8ada8..df131fe4 100644 --- a/src/objects/lead.hook.ts +++ b/src/objects/lead.hook.ts @@ -247,7 +247,7 @@ const leadHook: Hook = { * answered a returning visitor with a save error. That constraint is gone; a * re-captured address is now recorded as a fact instead of refused. * - * Two jobs, both `before`-phase so the values land in the same write: + * Three jobs, all `before`-phase so the values land in the same write: * * 1. **Normalize `email`** (trim + lowercase), exactly as `contact_integrity` * does on `crm_contact`. This is what makes the dedupe lookup below a plain @@ -258,6 +258,13 @@ const leadHook: Hook = { * every reader. Also on update: an edit that reintroduced mixed case would * silently make the record invisible to every later dedupe. * + * 1b. **Fold `company` into `company_normalized`** (#626) — a SEPARATE column, + * not an in-place rewrite, because `company` is the display value the + * conversion flow copies onto the account it creates. It is the lead half of + * the pair `crm_account.name_normalized` completes, and it exists for the + * same reason as the email fold: the reader (a flow template) can compare + * two stored columns but cannot compute either one. + * * 2. **Link a repeated address to the record it repeats** — on insert only. * An existing `crm_contact` wins over an open lead: the prospect who already * became a contact is the further-along record, and it is the one the @@ -285,7 +292,7 @@ const leadDuplicateCheckHook: Hook = { events: ['beforeInsert', 'beforeUpdate'], priority: 300, description: - 'Normalize lead email; flag a re-captured address as a suspected duplicate of the record it repeats.', + 'Normalize lead email and company match key; flag a re-captured address as a suspected duplicate of the record it repeats.', handler: async (ctx: HookContext) => { const { event, input } = ctx; @@ -304,6 +311,28 @@ const leadDuplicateCheckHook: Hook = { } const email = typeof input.email === 'string' ? input.email : ''; + // 1b. Canonical COMPANY, into its own column (#626). Same doctrine as the + // email above — the canonical form is established by the producer at + // write time — but it cannot be done in place: `company` is the display + // value and is copied verbatim onto the account `lead_conversion` + // creates, so folding it would ship "acme corp" as the account name. + // `crm_account.name_normalized` is the other half of the pair; the flow + // compares the two columns because it can compute neither + // (`resolveToken` knows only `NOW()` / `TODAY()`). + // + // Runs before the insert-only early return below, so an edit that + // rewrites `company` re-folds the key instead of leaving a stale one. + // Absent key ⇒ untouched: a partial update that never mentions + // `company` must not blank the match key. + if ('company' in input) { + const rawCompany = input.company; + const normalizedCompany = + typeof rawCompany === 'string' + ? rawCompany.trim().toLowerCase().replace(/\s+/g, ' ') + : ''; + input.company_normalized = normalizedCompany === '' ? null : normalizedCompany; + } + if (event !== 'beforeInsert' || !email) return; // 2. Never overwrite an opinion the record already carries. The diff --git a/src/objects/lead.object.ts b/src/objects/lead.object.ts index 0f868917..823677eb 100644 --- a/src/objects/lead.object.ts +++ b/src/objects/lead.object.ts @@ -79,6 +79,43 @@ export const Lead = ObjectSchema.create({ group: 'company_info', }), + /** + * Case- and whitespace-folded copy of `company` — the value lead conversion + * compares against `crm_account.name_normalized` (#626). + * + * ### Why the LEAD needs one too + * + * A normalized column on `crm_account` alone does not fix anything: the + * conversion flow would then be comparing a raw company string against a + * folded account name, and `"ACME Corp"` still would not match + * `acme corp`. Both sides of the comparison have to be canonical, and the + * flow cannot canonicalize either one — measured on 17.0.0-rc.1, + * `service-automation`'s `resolveToken` accepts exactly one function form + * (`NOW()` / `TODAY()`), so `{LOWER(x)}`, `{TRIM(x)}` and + * `{x.toLowerCase()}` all resolve to `undefined`. See the field comment on + * `crm_account.name_normalized` for the full measurement, and + * `test/account-name-normalized-match.test.ts`, which re-runs it. + * + * `company` itself is NOT folded in place: it is the display value, it is + * copied verbatim onto the account the conversion creates, and lower-casing + * it would ship `acme corp` as a customer-visible account name. That is the + * one way this differs from `email`, which has no meaningful case and so is + * canonicalized in place. + * + * Derived, never authored: `lead_duplicate_check` recomputes it on every + * write that carries `company`. Not indexed — nothing filters on it; it is + * read off the lead record the flow already fetched. + */ + company_normalized: Field.text({ + label: 'Company (Normalized)', + description: + 'Match key for lead conversion: Company lower-cased, trimmed, with internal whitespace collapsed. Maintained by the lead_duplicate_check hook — never edit directly.', + readonly: true, + hidden: true, + maxLength: 255, + group: 'company_info', + }), + title: Field.text({ label: 'Job Title', group: 'company_info', diff --git a/test/account-name-normalized-match.test.ts b/test/account-name-normalized-match.test.ts new file mode 100644 index 00000000..52550903 --- /dev/null +++ b/test/account-name-normalized-match.test.ts @@ -0,0 +1,511 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { AutomationEngine, installBuiltinNodes } from '@objectstack/service-automation'; +import { fieldHasColumn, expectedIndexes } from '@objectstack/driver-sql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { applySystemFields } from '@objectstack/objectql'; +import stack from '../objectstack.config'; +import accountHooks from '../src/objects/account.hook'; +import leadHooks from '../src/objects/lead.hook'; +import { LeadConversionFlow } from '../src/flows/lead-conversion.flow'; +import { makeCtx, hookNamed, type Rec } from './helpers/hook-harness'; +import { makeFlowHarness, silentLogger } from './helpers/flow-harness'; +import { runHookBody } from './helpers/action-sandbox'; + +/** + * Case-insensitive account matching in lead conversion (#626). + * + * Converting a lead whose company is `"ACME Corp"` used to create a SECOND + * account next to the `"Acme Corp"` one, because `lead_conversion` matched on + * the raw `crm_account.name`. The fix is a pair of stored, hook-maintained + * match keys — `crm_account.name_normalized` and `crm_lead.company_normalized` + * — that the flow compares with a plain equality filter. + * + * That shape is FORCED, not preferred, and the first three describes below + * measure the three constraints that force it instead of restating them from a + * comment. If any of them ever stops holding, these fail and the design should + * be reconsidered — which is the only way a design premise stays honest across + * a platform upgrade. + */ + +type AnyRec = Record; + +const objects: AnyRec[] = (stack as any).objects ?? []; +const account = objects.find((o) => o.name === 'crm_account') as AnyRec; +const lead = objects.find((o) => o.name === 'crm_lead') as AnyRec; + +// ══════════════════════════════════ premise 1: a flow cannot normalize ══ + +/** + * `service-automation`'s `resolveToken` recognises exactly ONE function form, + * `NOW()` / `TODAY()`. Everything else falls through to an expression + * evaluator that substitutes every bare identifier with its value BEFORE + * evaluating, so a string method is never reachable — `{x.toLowerCase()}` + * resolves the whole dotted path `x.company.toLowerCase` to `undefined` and + * then tries to call `null`. + * + * Run through the REAL engine, because the interesting failure is silent: an + * unrecognised token does not raise, it interpolates to `""` or to itself. + */ +describe('premise: a flow template cannot fold a string', () => { + const CANDIDATES: Record = { + unwrapped_lower: 'LOWER({leadRecord.company})', + fn_lower: '{LOWER(leadRecord.company)}', + fn_trim: '{TRIM(leadRecord.company)}', + method_lower: '{leadRecord.company.toLowerCase()}', + parenthesised_method: '{(leadRecord.company).toLowerCase()}', + passthrough: '{leadRecord.company}', + }; + + let out: Rec = {}; + + beforeAll(async () => { + const store: Record = { crm_lead: [{ id: 'lead_1', company: 'ACME Corp' }] }; + const data: AnyRec = { + async findOne(object: string, opts: AnyRec = {}) { + const where = opts.where ?? opts.filter ?? {}; + return ( + store[object]?.find((r) => Object.entries(where).every(([k, v]) => r[k] === v)) ?? null + ); + }, + async find(object: string) { + return store[object] ?? []; + }, + async insert(object: string, doc: Rec) { + const rec = { id: `${object}_1`, ...doc }; + (store[object] ??= []).push(rec); + return rec; + }, + async update() { + return { modified: 0 }; + }, + }; + + const engine = new AutomationEngine(silentLogger); + installBuiltinNodes(engine, { + logger: silentLogger, + getService: (n: string) => (n === 'data' || n === 'objectql' ? data : undefined), + } as never); + engine.registerFlow('probe_normalize', { + name: 'probe_normalize', + label: 'probe', + type: 'autolaunched', + status: 'active', + variables: [{ name: 'recordId', type: 'text', isInput: true, isOutput: false }], + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: {} }, + { + id: 'get_lead', + type: 'get_record', + label: 'Get Lead', + config: { + objectName: 'crm_lead', + filter: { id: '{recordId}' }, + outputVariable: 'leadRecord', + }, + }, + { + id: 'probe', + type: 'create_record', + label: 'Probe', + config: { objectName: 'probe_out', fields: CANDIDATES, outputVariable: 'probeOut' }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'get_lead', type: 'default' }, + { id: 'e2', source: 'get_lead', target: 'probe', type: 'default' }, + { id: 'e3', source: 'probe', target: 'end', type: 'default' }, + ], + } as never); + + await engine.execute('probe_normalize', { + params: { recordId: 'lead_1' }, + userId: 'user_1', + event: 'manual', + } as never); + out = store.probe_out?.[0] ?? {}; + }); + + it('passes the raw value through unchanged — the only thing that works', () => { + expect(out.passthrough).toBe('ACME Corp'); + }); + + it.each(['fn_lower', 'fn_trim', 'method_lower', 'parenthesised_method'])( + 'resolves %s to undefined — no LOWER/TRIM/string method exists', + (key) => { + expect(out[key]).toBeUndefined(); + }, + ); + + it('interpolates an unwrapped LOWER(...) literally, silently', () => { + // The worst shape of all: it looks like it worked and would be written to + // the database as the literal text. + expect(out.unwrapped_lower).toBe('LOWER(ACME Corp)'); + }); +}); + +// ═════════════════════════ premise 2: a formula field has no column ══ + +describe('premise: a formula field cannot be the match key', () => { + it('driver-sql materializes no column for type: formula', () => { + expect(fieldHasColumn({ type: 'formula' } as never)).toBe(false); + expect(fieldHasColumn({ type: 'text' } as never)).toBe(true); + }); + + it('so the stored match key is a text field, not a formula', () => { + expect(account.fields.name_normalized.type).toBe('text'); + expect(lead.fields.company_normalized.type).toBe('text'); + }); +}); + +// ═══════════════════════════════ premise 3: `$regex` is not an answer ══ + +/** + * The measurement is sharper than "unindexed": on `driver-sql`, `$regex` is not + * evaluated as a regular expression at all. It compiles to `LIKE '%value%'`, a + * LIKE-escaped SUBSTRING match — so it matches strings the caller did not mean, + * and a real pattern matches nothing. + */ +describe('premise: `$regex` cannot express normalize-then-exact on SQL', () => { + let driver: SqliteWasmDriver; + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.connect(); + await driver.initObjects([ + { name: 'probe_account', fields: { name: { type: 'text' } }, indexes: [] } as never, + ]); + await driver.create('probe_account', { name: 'Acme Corp' }); + await driver.create('probe_account', { name: 'Not Acme Corp Ltd' }); + await driver.create('probe_account', { name: 'ACME Corp' }); + }, 60_000); + + afterAll(async () => { + await driver?.disconnect(); + }); + + const names = async (where: Rec): Promise => { + const rows = (await driver.find('probe_account', { + object: 'probe_account', + where, + } as never)) as AnyRec[]; + return rows.map((r) => String(r.name)).sort(); + }; + + it('matches a SUPERSTRING — it is a substring LIKE, not an exact match', async () => { + expect(await names({ name: { $regex: 'Acme Corp' } })).toEqual([ + 'Acme Corp', + 'Not Acme Corp Ltd', + ]); + }); + + it('cannot collapse internal whitespace — a real pattern matches nothing', async () => { + expect(await names({ name: { $regex: '^acme\\s+corp$' } })).toEqual([]); + }); +}); + +// ══════════════════════════════════════════ the columns the fix adds ══ + +describe('crm_account.name_normalized is a machine-owned match key', () => { + const field = () => account.fields.name_normalized as AnyRec; + + it('is stored, readonly and hidden', () => { + expect(field().readonly).toBe(true); + // Hidden keeps a derived column out of forms and pickers — the pairing + // `crm_forecast.seed_key` established. It is also why the field needs no + // locale entry: nothing renders its label. + expect(field().hidden).toBe(true); + }); + + it('is indexed — every conversion filters on it', () => { + const declared = ((account.indexes ?? []) as AnyRec[]).map((i) => (i.fields ?? []).join(',')); + expect(declared).toContain('name_normalized'); + }); + + it('does NOT carry a unique index, and leaves #625 exactly one', () => { + // The decision recorded in `account.object.ts`: uniqueness of account names + // already lives, per tenant, on `name`. A unique `name_normalized` subsumes + // it and would re-open the constraint #625 landed; `create_index` also + // FAILS on any deployment already holding both spellings. This assertion is + // what makes that a decision rather than an omission. + const orgScoped = applySystemFields(account as never, { multiTenant: true }) as AnyRec; + const physicalColumns = new Set([ + 'id', + ...Object.entries(orgScoped.fields as Record) + .filter(([, f]) => (f?.type ?? 'string') !== 'formula') + .map(([name]) => name), + ]); + const indexes = expectedIndexes({ + table: 'crm_account', + fields: orgScoped.fields as Record, + tenantField: 'organization_id', + declaredIndexes: (orgScoped.indexes ?? []) as AnyRec[], + physicalColumns, + }); + expect(indexes.filter((i) => i.unique).map((i) => i.columns)).toEqual([ + ['organization_id', 'name'], + ]); + expect(indexes.some((i) => i.columns.includes('name_normalized'))).toBe(true); + }); +}); + +describe('crm_lead.company_normalized is the other half of the pair', () => { + it('is stored, readonly and hidden', () => { + expect(lead.fields.company_normalized.readonly).toBe(true); + expect(lead.fields.company_normalized.hidden).toBe(true); + }); + + it('leaves `company` itself untouched — it is the display value', () => { + // Folding `company` in place (the way `email` is folded) would ship + // "acme corp" as the name of the account conversion creates. + expect(lead.fields.company.readonly).not.toBe(true); + expect(lead.fields.company.hidden).not.toBe(true); + expect(lead.fields.company.required).toBe(true); + }); +}); + +// ═══════════════════════════════════════ the producers, as real handlers ══ + +const FOLDING_CASES: Array<[string, unknown, unknown]> = [ + ['a mixed-case name', 'Acme Corp', 'acme corp'], + ['the shout-case double-space spelling', 'ACME Corp', 'acme corp'], + ['leading and trailing space', ' Acme Corp ', 'acme corp'], + ['a tab between words', 'Acme\tCorp', 'acme corp'], + ['a newline between words', 'Acme\nCorp', 'acme corp'], + ['an already-canonical value', 'acme corp', 'acme corp'], + ['a whitespace-only value', ' ', null], + ['a non-string value', 42, null], +]; + +describe('account_protection folds name into name_normalized', () => { + const hook = hookNamed(accountHooks, 'account_protection'); + + it.each(FOLDING_CASES)('folds %s', async (_label, name, expected) => { + const input: Rec = { name }; + await hook.handler(makeCtx({ event: 'beforeInsert', input, user: { id: 'user_1' } })); + expect(input.name_normalized).toBe(expected); + }); + + it('re-folds on an update that rewrites the name', async () => { + const input: Rec = { name: 'ACME Corporation' }; + await hook.handler( + makeCtx({ + event: 'beforeUpdate', + input, + previous: { name: 'Acme Corp', name_normalized: 'acme corp' }, + user: { id: 'user_1' }, + }), + ); + expect(input.name_normalized).toBe('acme corporation'); + }); + + it('leaves the key alone when the write does not carry the name', async () => { + // An unrelated partial edit must not blank the match key — that would evict + // the account from every future conversion lookup. + const input: Rec = { phone: '+1-512-555-0100' }; + await hook.handler( + makeCtx({ + event: 'beforeUpdate', + input, + previous: { name: 'Acme Corp', name_normalized: 'acme corp' }, + user: { id: 'user_1' }, + }), + ); + expect('name_normalized' in input).toBe(false); + }); + + it('still projects billing_country in the same write — #621 is untouched', async () => { + // Both derivations live in one handler, so a regression in either is easy + // to introduce while editing the other. One write, both columns. + const input: Rec = { name: 'ACME Corp', billing_address: { city: 'Munich', country: ' de ' } }; + await hook.handler(makeCtx({ event: 'beforeInsert', input, user: { id: 'user_1' } })); + expect(input.name_normalized).toBe('acme corp'); + expect(input.billing_country).toBe('DE'); + }); +}); + +describe('lead_duplicate_check folds company into company_normalized', () => { + const hook = hookNamed(leadHooks, 'lead_duplicate_check'); + + it.each(FOLDING_CASES)('folds %s', async (_label, company, expected) => { + const input: Rec = { company }; + await hook.handler(makeCtx({ event: 'beforeInsert', input })); + expect(input.company_normalized).toBe(expected); + }); + + it('folds on update too, before the insert-only dedupe returns', async () => { + const input: Rec = { company: 'ACME Corp' }; + await hook.handler( + makeCtx({ event: 'beforeUpdate', input, previous: { company: 'Globex' } }), + ); + expect(input.company_normalized).toBe('acme corp'); + }); + + it('leaves the key alone when the write does not carry the company', async () => { + const input: Rec = { phone: '555' }; + await hook.handler(makeCtx({ event: 'beforeUpdate', input, previous: { company: 'Acme' } })); + expect('company_normalized' in input).toBe(false); + }); + + it('does not disturb the email fold it shares the handler with', async () => { + const input: Rec = { company: 'ACME Corp', email: ' Joe@Example.COM ' }; + await hook.handler(makeCtx({ event: 'beforeUpdate', input })); + expect(input.email).toBe('joe@example.com'); + expect(input.company_normalized).toBe('acme corp'); + }); +}); + +// ═══════════════════════════════ the producers, inside the real sandbox ══ + +/** + * Both folds are written INLINE, with no module-scope helper, so the handlers + * still lower to a metadata-only body — which is what the runtime evaluates. + * `action-sandbox.test.ts` proves they lower; only running them in QuickJS + * proves the inlined code uses nothing the sandbox lacks (a `replace` with a + * regex literal, here). + */ +describe('both folds run inside the QuickJS sandbox', () => { + it('account_protection folds name in the VM', async () => { + const hook = hookNamed(accountHooks, 'account_protection'); + const { input } = await runHookBody(hook, { + event: 'beforeInsert', + input: { name: ' ACME Corp ' }, + }); + expect(input.name_normalized).toBe('acme corp'); + }); + + it('lead_duplicate_check folds company in the VM', async () => { + const hook = hookNamed(leadHooks, 'lead_duplicate_check'); + const { input } = await runHookBody(hook, { + event: 'beforeInsert', + input: { company: ' ACME Corp ', email: 'joe@example.com' }, + }); + expect(input.company_normalized).toBe('acme corp'); + }); +}); + +// ════════════════════════════════════════════ the acceptance criteria ══ + +/** + * End-to-end through the REAL flow and the REAL hooks: records are written via + * the hooked data engine, never hand-stamped, so the test proves the producer + * chain (hook → column → flow filter) rather than its own fixtures. + */ +const CONVERSION_HOOKS = [ + hookNamed(leadHooks, 'lead_automation'), + hookNamed(leadHooks, 'lead_duplicate_check'), + hookNamed(accountHooks, 'account_protection'), +] as never[]; + +const makeConversion = () => + makeFlowHarness({ lead_conversion: LeadConversionFlow }, {}, { hooks: CONVERSION_HOOKS }); + +async function convert(harness: ReturnType, leadId: string) { + const runId = await harness.run('lead_conversion', { recordId: leadId }); + await harness.resume(runId!, { createOpportunity: false }); +} + +const leadPayload = (id: string, company: string): Rec => ({ + id, + company, + email: `${id}@example.com`, + first_name: 'Joe', + last_name: 'Green', + status: 'qualified', + is_converted: false, +}); + +describe('acceptance: a case/whitespace variant reuses the same account', () => { + it('converting "ACME Corp" reuses the account created from "Acme Corp"', async () => { + const h = makeConversion(); + await h.data.insert('crm_lead', leadPayload('lead_1', 'Acme Corp')); + await h.data.insert('crm_lead', leadPayload('lead_2', 'ACME Corp')); + + await convert(h, 'lead_1'); + expect(h.store.crm_account).toHaveLength(1); + const created = h.store.crm_account[0]; + expect(created.name, 'the display name is the lead value, verbatim').toBe('Acme Corp'); + expect(created.name_normalized, 'the hook derived the match key').toBe('acme corp'); + + await convert(h, 'lead_2'); + expect(h.store.crm_account, 'no duplicate account').toHaveLength(1); + expect(h.store.crm_lead.find((l) => l.id === 'lead_2')!.converted_account).toBe(created.id); + }); + + it('filters on the normalized column with the folded value', async () => { + // The outcome above could also be produced by matching on something else. + // This pins WHICH query the flow issues. + const h = makeConversion(); + await h.data.insert('crm_lead', leadPayload('lead_1', 'ACME Corp')); + await convert(h, 'lead_1'); + + const lookup = h.queries.find((q) => q.object === 'crm_account'); + expect(lookup?.where).toEqual({ name_normalized: 'acme corp' }); + }); + + it('still creates an account when nothing matches', async () => { + const h = makeConversion(); + await h.data.insert('crm_account', { id: 'acc_1', name: 'Globex Industries' }); + await h.data.insert('crm_lead', leadPayload('lead_1', 'Initech')); + await convert(h, 'lead_1'); + + expect(h.store.crm_account).toHaveLength(2); + expect(h.store.crm_account[1].name).toBe('Initech'); + expect(h.store.crm_account[1].name_normalized).toBe('initech'); + }); + + it('a lead with no match key stops the conversion instead of guessing', async () => { + // The un-backfilled LEAD case (docs/MAINTENANCE.md §3.3). The filter value + // resolves to nothing and `get_record` refuses to run rather than widen the + // query to every account — measured here rather than assumed, because the + // alternative (a silent match-all) would attach the conversion to an + // arbitrary account, which is far worse than failing. + const h = makeConversion(); + h.store.crm_lead = [{ ...leadPayload('lead_legacy', 'Acme Corp'), company_normalized: undefined }]; + const runId = await h.run('lead_conversion', { recordId: 'lead_legacy' }); + const done = (await h.resume(runId!, { createOpportunity: false })) as AnyRec; + + expect(String(done?.error ?? '')).toMatch(/resolved to nothing|refusing to run/); + expect(h.store.crm_account ?? [], 'no account was invented').toHaveLength(0); + expect(h.store.crm_lead[0].is_converted, 'the lead is untouched').not.toBe(true); + }); + + it('an account with no match key is invisible — the reason the backfill is not optional', async () => { + // The un-backfilled ACCOUNT case: this one is SILENT, which is exactly why + // it is documented as the failure that makes the backfill mandatory. + const h = makeConversion(); + h.store.crm_account = [{ id: 'acc_legacy', name: 'Acme Corp', is_active: true }]; + await h.data.insert('crm_lead', leadPayload('lead_1', 'Acme Corp')); + await convert(h, 'lead_1'); + + expect(h.store.crm_account, 'a duplicate account, as documented').toHaveLength(2); + }); + + it('a fresh install and a backfilled install behave identically', async () => { + // "Fresh": the account was created by a conversion, so the hook stamped the + // key on insert. "Backfilled": the account predates the column and the + // operator re-saved it (docs/MAINTENANCE.md §3.3) — modelled as an UPDATE + // carrying `name`, which is exactly what the backfill issues. Both must + // then be found by the same variant-spelling lead. + const fresh = makeConversion(); + await fresh.data.insert('crm_lead', leadPayload('lead_seed', 'Acme Corp')); + await convert(fresh, 'lead_seed'); + + const backfilled = makeConversion(); + // A row written before the column existed: no match key at all. + backfilled.store.crm_account = [{ id: 'acc_legacy', name: 'Acme Corp', is_active: true }]; + await backfilled.data.update('crm_account', { name: 'Acme Corp' }, { where: { id: 'acc_legacy' } }); + + expect(backfilled.store.crm_account[0].name_normalized).toBe( + fresh.store.crm_account[0].name_normalized, + ); + + for (const h of [fresh, backfilled]) { + await h.data.insert('crm_lead', leadPayload('lead_variant', 'ACME Corp')); + await convert(h, 'lead_variant'); + expect(h.store.crm_account, 'reused the existing account').toHaveLength(1); + } + }); +}); diff --git a/test/flow-conversion.test.ts b/test/flow-conversion.test.ts index a46061ac..dcf86c0e 100644 --- a/test/flow-conversion.test.ts +++ b/test/flow-conversion.test.ts @@ -78,9 +78,23 @@ async function runConversion( return { started, resumed }; } +/** + * Fold a name the way the producer hooks do (#626). + * + * This harness has no hooks — it writes rows straight into the store — so the + * fixtures have to carry the derived match keys that `lead_duplicate_check` and + * `account_protection` stamp on every real write. Without them the flow's + * account lookup has no value to filter on and `get_record` refuses to run. + * The hook-owned columns themselves are exercised in + * `test/account-name-normalized-match.test.ts`, which runs the real handlers. + */ +const fold = (value: string): string => value.trim().toLowerCase().replace(/\s+/g, ' '); + const seedLead = (data: ReturnType, over: Rec = {}) => { + const company = (over.company as string) ?? 'Globex Industries'; const lead = { - id: 'lead_1', company: 'Globex Industries', email: 'joe@globex.example.com', + id: 'lead_1', company, company_normalized: fold(company), + email: 'joe@globex.example.com', first_name: 'Joe', last_name: 'Green', phone: '555', title: 'Buyer', lead_source: 'web', is_converted: false, status: 'qualified', ...over, }; @@ -111,7 +125,10 @@ describe('lead_conversion flow — runtime', () => { it('REUSES an existing account with the same company (no duplicate)', async () => { const data = makeDataEngine(); seedLead(data); - data.store.crm_account = [{ id: 'acc_existing', name: 'Globex Industries', is_active: true }]; + data.store.crm_account = [{ + id: 'acc_existing', name: 'Globex Industries', + name_normalized: fold('Globex Industries'), is_active: true, + }]; const engine = buildEngine(data); await runConversion(engine, 'lead_1'); @@ -125,7 +142,10 @@ describe('lead_conversion flow — runtime', () => { it('REUSES an existing contact (same email in the account) — no duplicate', async () => { const data = makeDataEngine(); seedLead(data); - data.store.crm_account = [{ id: 'acc_existing', name: 'Globex Industries', is_active: true }]; + data.store.crm_account = [{ + id: 'acc_existing', name: 'Globex Industries', + name_normalized: fold('Globex Industries'), is_active: true, + }]; data.store.crm_contact = [{ id: 'con_existing', email: 'joe@globex.example.com', crm_account: 'acc_existing', first_name: 'Joe', last_name: 'Green', diff --git a/test/flow-variable-conditions.test.ts b/test/flow-variable-conditions.test.ts index 13030d25..d868deba 100644 --- a/test/flow-variable-conditions.test.ts +++ b/test/flow-variable-conditions.test.ts @@ -733,7 +733,13 @@ describe('the two defects, reproduced end-to-end', () => { try { const api = b.ql.createContext({ isSystem: true }); const lead = await api.object('crm_lead').insert({ + // `company_normalized` is authored here because this harness registers + // FLOWS but no hooks — in production `lead_duplicate_check` derives it + // from `company` on the insert (#626). Without it the conversion's + // account lookup has no filter value and `get_record` refuses to run, + // which is a fixture gap, not the defect this test is about. first_name: 'Sam', last_name: 'Doe', company: 'Globex', + company_normalized: 'globex', status: 'qualified', email: 'sam@globex.com', }); const started = await b.engine.execute('lead_conversion', asUser({ recordId: lead.id })); @@ -757,7 +763,10 @@ describe('the two defects, reproduced end-to-end', () => { try { const api = b.ql.createContext({ isSystem: true }); const lead = await api.object('crm_lead').insert({ + // See the note on the previous test: hook-derived key, hand-authored + // because this harness has no hooks (#626). first_name: 'Jo', last_name: 'Smith', company: 'Acme', + company_normalized: 'acme', status: 'qualified', email: 'jo@acme.com', }); const started = await b.engine.execute('lead_conversion', asUser({ recordId: lead.id }));