diff --git a/.changeset/account-name-unique-per-tenant.md b/.changeset/account-name-unique-per-tenant.md new file mode 100644 index 00000000..3153be1d --- /dev/null +++ b/.changeset/account-name-unique-per-tenant.md @@ -0,0 +1,41 @@ +--- +'hotcrm': patch +--- + +Account names are now unique **per organization** instead of platform-wide, so two organizations can each have an "Acme Corp". + +`crm_account` was the last core object still spelling uniqueness as a table-level +declared index — `indexes: [{ fields: ['name'], unique: true }]`. A declared +index is materialized over exactly its `fields`, i.e. platform-wide, while +field-level `unique: true` has been tenant-scoped since framework #3696. The +physical constraint was therefore `UNIQUE (name)`, and the SECOND organization to +create an account called "Acme Corp" was refused by the database. Account name is +also the seed data's external-id / upsert key, so this bit on the very first +multi-tenant install, before anyone had typed a record. + +The declaration moves onto the field, matching `crm_contact.email` and +`crm_product.sku`, and the table-level entry is **removed** rather than kept +alongside it: declaring both leaves the platform-wide index enforcing the old +behaviour and the per-tenant composite unreachable (framework#3991 +`unique/double-declaration`) — the fix would have looked applied and done +nothing. A freshly migrated database now carries +`uniq_crm_account_organization_id_name (organization_id, name)`, which also +indexes the column for `searchableFields` and the seed upsert. + +Uniqueness within one organization is unchanged: a second "Acme Corp" in the same +org is still rejected. + +**Upgrading an existing deployment.** Every HotCRM install today is a fresh one, +where the database is built from current metadata and the platform-wide +`uniq_crm_account_name` index is simply never created. If an installation with an +existing populated database ever needs this version, the old index must be +dropped with one `os migrate apply --allow-destructive` run: it is strictly +tighter than the new composite, so while it survives it keeps enforcing the +platform-wide rule and **this fix silently does nothing**. The boot-time +reconciler creates the new index (`create_index`, `safe`) but skips the drop +(`drop_index`, `destructive`) — see `docs/MAINTENANCE.md` §3.1. + +Note that account names are matched exactly, so `Acme Corp` and `ACME Corp` are +two different accounts; the user documentation no longer claims otherwise. + +Fixes #625. diff --git a/content/docs/sales/accounts.mdx b/content/docs/sales/accounts.mdx index 574dfbba..e080ece7 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** across the system (case-insensitive). You'll get an error if you try to create a duplicate. +- 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. - **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 b73e526f..595a5cc2 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` 目前算作两个不同的客户。 - **年营收必须为零或正数**——不允许负数。 - 你**无法删除**一个有开放商机或活跃合同的客户。改为将其设为*非活跃*（`is_active = false`）。 diff --git a/content/docs/sales/accounts.zh-Hant.mdx b/content/docs/sales/accounts.zh-Hant.mdx index b7ceb2ea..3e0c3c8a 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` 目前算作兩個不同的客戶。 - **年營收必須為零或正數**——不允許負數。 - 你**無法刪除**一個有開放商機或活躍合約的客戶。改為將其設為*非活躍*（`is_active = false`）。 diff --git a/src/objects/account.object.ts b/src/objects/account.object.ts index 6e1a7c7e..74bd4464 100644 --- a/src/objects/account.object.ts +++ b/src/objects/account.object.ts @@ -44,11 +44,25 @@ export const Account = ObjectSchema.create({ }), // Basic Information + // + // `unique: true` is declared HERE, on the field, and deliberately NOT as a + // `{ fields: ['name'], unique: true }` entry in `indexes[]` below (#625). + // Since framework #3696 the field-level form is tenant-scoped — it + // materializes as `(organization_id, name)`, unique WITHIN an organization — + // while a declared index is taken verbatim, i.e. platform-wide. The table + // form is what this object used to carry, and it meant the SECOND + // organization to create an "Acme Corp" was rejected by the database. + // Account name is also the seed data's external-id / upsert key + // (`src/data/sales.seed.ts`), so that bit the very first multi-tenant + // install. The composite also indexes the column, so no separate + // `{ fields: ['name'] }` entry is needed for the `searchableFields` / + // seed-upsert read paths. name: Field.text({ label: 'Account Name', required: true, storage: { notNull: true }, searchable: true, + unique: true, maxLength: 255, group: 'basic', }), @@ -264,11 +278,15 @@ export const Account = ObjectSchema.create({ }, // Database indexes for performance + // + // No `{ fields: ['name'], unique: true }` here (#625). Account-name + // uniqueness is declared on the `name` field itself, which since framework + // #3696 builds the tenant composite `(organization_id, name)`. Declaring the + // single-column index too makes the platform-wide constraint win and leaves + // the per-tenant one unreachable (framework#3991 `unique/double-declaration`) + // — the same trap `crm_contact`, `crm_lead` and `crm_product` document. Two + // organizations must be able to each have their own "Acme Corp". indexes: [ - // Account name is the external id / upsert key (see src/data) and must be - // unique. In 7.6 uniqueness is expressed as a unique index — the standalone - // `type: 'unique'` validation rule was removed (ADR-0032 validation union). - { fields: ['name'], unique: true }, { fields: ['owner'] }, { fields: ['type', 'is_active'] }, // The territory sharing rules filter on this column, so it is read on @@ -294,7 +312,8 @@ export const Account = ObjectSchema.create({ // This object declares none. Two entries used to live here: // // - `account_name_unique` (type: 'unique') was removed in 7.6 — uniqueness - // now lives on the `name` index above (unique: true). + // now lives on the `name` field above (`unique: true`), which the driver + // materializes as the per-tenant `(organization_id, name)` index (#625). // - `revenue_positive` was removed in #514 (item 7) as a duplicate. It // restated a check `account.hook.ts` already performs on beforeInsert / // beforeUpdate, and the two disagreed in wording: the validation said diff --git a/test/account-name-tenant-scope.test.ts b/test/account-name-tenant-scope.test.ts new file mode 100644 index 00000000..db87c713 --- /dev/null +++ b/test/account-name-tenant-scope.test.ts @@ -0,0 +1,158 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { applySystemFields } from '@objectstack/objectql'; +import { expectedIndexes } from '@objectstack/driver-sql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import stack from '../objectstack.config'; + +/** + * Account-name uniqueness is PER ORGANIZATION, not platform-wide (#625). + * + * `crm_account` used to spell the constraint as a table-level declared index, + * `indexes: [{ fields: ['name'], unique: true }]`. `driver-sql` keeps a + * declared index's columns verbatim — only FIELD-level `unique: true` gets the + * tenant composite `(organization_id, ...)` (framework#3696). So the physical + * index was `UNIQUE (name)` and the SECOND organization to create an account + * called "Acme Corp" was rejected by the database. Account name is also the + * seed data's external-id / upsert key, so it bit the first multi-tenant + * install. `crm_contact.email`, `crm_lead.email` and `crm_product.sku` already + * carry the field-level spelling and document the trap; `crm_account` did not. + * + * The two acceptance criteria are exercised against a REAL SQLite database + * driven by the REAL `crm_account` metadata, not against the metadata alone: a + * declaration is not a constraint until the driver turns it into DDL, and the + * whole defect was a mismatch between the two. The metadata assertions below + * exist to name WHICH spelling is required, so a future edit that reverts to + * the table form fails with the reason rather than a bare SQL error. + */ + +type AnyRec = Record; + +const objects: AnyRec[] = (stack as any).objects ?? []; +const account = objects.find((o) => o.name === 'crm_account') as AnyRec; + +/** What the runtime actually hands the driver on a multi-org stack. */ +const orgScoped = applySystemFields(account as any, { multiTenant: true }) as AnyRec; + +/** Columns that get materialized — formula fields never do (`fieldHasColumn`). */ +const physicalColumns = new Set([ + 'id', + ...Object.entries(orgScoped.fields as Record) + .filter(([, f]) => (f?.type ?? 'string') !== 'formula') + .map(([name]) => name), +]); + +// ───────────────────────────────────── the shape the rule is declared in ── + +describe('crm_account declares name uniqueness on the FIELD', () => { + it('carries field-level `unique: true` on name', () => { + expect(account.fields.name.unique).toBe(true); + }); + + it('declares NO single-column unique index on name', () => { + // framework#3991 `unique/double-declaration`: declaring both makes the + // platform-wide index win and leaves the per-tenant composite unreachable, + // so the field-level declaration above would silently do nothing. + const uniqueIndexes = ((account.indexes ?? []) as AnyRec[]).filter((i) => i.unique === true); + expect( + uniqueIndexes, + 'a declared unique index on crm_account re-imposes the platform-wide constraint #625 removed', + ).toEqual([]); + }); + + it('matches how crm_contact / crm_product spell the same intent', () => { + const contact = objects.find((o) => o.name === 'crm_contact') as AnyRec; + const product = objects.find((o) => o.name === 'crm_product') as AnyRec; + expect(contact.fields.email.unique).toBe(true); + expect(product.fields.sku.unique).toBe(true); + }); +}); + +// ─────────────────────────────── the index the driver derives from it ── + +describe('the physical index set is tenant-scoped', () => { + const indexes = expectedIndexes({ + table: 'crm_account', + fields: orgScoped.fields as Record, + tenantField: 'organization_id', + declaredIndexes: (orgScoped.indexes ?? []) as AnyRec[], + physicalColumns, + }); + + it('expects exactly one unique index, on (organization_id, name)', () => { + const unique = indexes.filter((i) => i.unique); + expect(unique).toEqual([ + { + name: 'uniq_crm_account_organization_id_name', + columns: ['organization_id', 'name'], + unique: true, + }, + ]); + }); + + it('expects no platform-wide uniq_crm_account_name', () => { + expect(indexes.map((i) => i.name)).not.toContain('uniq_crm_account_name'); + }); + + it('still indexes name — the seed upsert key and $search read it', () => { + // `searchableFields: ['name', ...]` promises a real indexed column, and + // `src/data/sales.seed.ts` upserts accounts on `externalId: 'name'` (a + // per-organization lookup). The tenant composite leads with + // organization_id, so `WHERE organization_id = ? AND name = ?` is a prefix + // match and no separate `{ fields: ['name'] }` entry is needed. + const covering = indexes.filter((i) => i.columns.includes('name')); + expect(covering.length).toBeGreaterThan(0); + }); +}); + +// ───────────────────────────────── the constraint a real database enforces ── + +describe('two organizations can each have an "Acme Corp" (real SQLite)', () => { + let driver: SqliteWasmDriver; + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.connect(); + // The exact call the runtime makes at boot: name + fields + declared + // indexes. This is what turns the metadata above into DDL. + await driver.initObjects([ + { + name: 'crm_account', + fields: orgScoped.fields as Record, + indexes: orgScoped.indexes, + } as never, + ]); + }, 60_000); + + afterAll(async () => { + await driver?.disconnect(); + }); + + it('materialized the per-tenant unique index and nothing platform-wide', async () => { + const rows = (await driver + .getKnex() + .raw("select name from sqlite_master where type = 'index' and tbl_name = 'crm_account'")) as Array<{ + name: string; + }>; + const names = rows.map((r) => r.name); + expect(names).toContain('uniq_crm_account_organization_id_name'); + expect(names).not.toContain('uniq_crm_account_name'); + }); + + it('accepts the same account name in two different organizations', async () => { + const first = await driver.create('crm_account', { name: 'Acme Corp' }, { tenantId: 'org_a' }); + const second = await driver.create('crm_account', { name: 'Acme Corp' }, { tenantId: 'org_b' }); + expect(first?.id).toBeTruthy(); + expect(second?.id).toBeTruthy(); + expect(second.id).not.toBe(first.id); + expect(second.organization_id).toBe('org_b'); + }); + + it('still rejects a duplicate account name WITHIN one organization', async () => { + await driver.create('crm_account', { name: 'Globex' }, { tenantId: 'org_a' }); + await expect( + driver.create('crm_account', { name: 'Globex' }, { tenantId: 'org_a' }), + ).rejects.toThrow(/UNIQUE constraint failed: crm_account\.organization_id, crm_account\.name/); + }); +});