fix: two CRM end-to-end runtime gaps (delete cascade + dead convert route)

.changeset/crm-e2e-gaps.md

@@ -0,0 +1,10 @@
+---
+"@objectstack/objectql": minor
+"@objectstack/rest": minor
+---
+
+fix: two runtime gaps found by driving the CRM example end-to-end.
+
+**Delete of a parent with a required-FK child no longer fails with a misleading "<field> is required" error.** `cascadeDeleteRelations` defaulted a `lookup` FK to `set_null`; for a *required* FK that issued an UPDATE clearing the column, which the child's validator rejected with a `400 "<field> is required"` naming a field that isn't even on the object being deleted (e.g. deleting a `crm_account` with opportunities → `"account is required"`). A required FK can't be nulled, so a *defaulted* `set_null` now escalates to `restrict`: the delete is refused with a clear `409 DELETE_RESTRICTED` carrying the dependent object + count (`"Cannot delete crm_account (…): 4 dependent crm_opportunity record(s) reference it via account … set deleteBehavior:'cascade'"`). Explicit `cascade`/`restrict` and optional (nullable) lookups are unchanged.
+
+**Removed the hardcoded `POST /data/lead/:id/convert` endpoint + `convertLead` protocol method.** It hardcoded bare object names (`lead`/`account`/`contact`/`opportunity`) and a fixed Salesforce field mapping into the framework runtime, so it was unreachable by any real (namespaced) app — `/data/crm_lead/:id/convert` 404s, and the literal `lead` object doesn't exist. Lead conversion is an app concern modeled correctly as a flow (the CRM ships a `crm_convert_lead_wizard` screen flow); baking a CRM-specific workflow into the framework was false surface. Untested, undocumented, unused by the example. Removed.

packages/objectql/src/engine-cascade-delete.test.ts

@@ -0,0 +1,141 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * Cascade-on-delete behavior for parent→child foreign keys, with a REAL
+ * {@link ObjectQL} engine + in-memory driver.
+ *
+ * Regression: deleting a parent whose child has a *required* lookup FK used to
+ * default to `set_null`, issuing an UPDATE that cleared the required FK — which
+ * the child's validator rejected with a misleading "<field> is required" 400
+ * naming a field that isn't even on the object being deleted (CRM e2e gap).
+ * A required FK can't be nulled, so the defaulted `set_null` now escalates to
+ * `restrict`: the delete is refused with a clear dependent-count message
+ * (`DELETE_RESTRICTED`, 409). Explicit `cascade`/`restrict` and optional
+ * (nullable) lookups are unaffected.
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import { ObjectQL } from './engine.js';
+
+const acct = {
+    name: 'acct',
+    label: 'Account',
+    fields: {
+        id: { name: 'id', type: 'text' as const, primaryKey: true },
+        name: { name: 'name', type: 'text' as const },
+    },
+};
+const oppRequired = {
+    name: 'opp',
+    label: 'Opportunity',
+    fields: {
+        id: { name: 'id', type: 'text' as const, primaryKey: true },
+        name: { name: 'name', type: 'text' as const },
+        // required lookup → can't be nulled
+        account: { name: 'account', type: 'lookup' as const, reference: 'acct', required: true },
+    },
+};
+const noteOptional = {
+    name: 'note',
+    label: 'Note',
+    fields: {
+        id: { name: 'id', type: 'text' as const, primaryKey: true },
+        body: { name: 'body', type: 'text' as const },
+        // optional lookup → default set_null is valid
+        account: { name: 'account', type: 'lookup' as const, reference: 'acct' },
+    },
+};
+const taskCascade = {
+    name: 'task',
+    label: 'Task',
+    fields: {
+        id: { name: 'id', type: 'text' as const, primaryKey: true },
+        title: { name: 'title', type: 'text' as const },
+        // required FK but author explicitly opted into cascade
+        account: { name: 'account', type: 'lookup' as const, reference: 'acct', required: true, deleteBehavior: 'cascade' },
+    },
+};
+
+function makeMemoryDriver() {
+    const stores = new Map<string, Map<string, Record<string, unknown>>>();
+    const storeFor = (o: string) => { let s = stores.get(o); if (!s) { s = new Map(); stores.set(o, s); } return s; };
+    let nextId = 0;
+    const matches = (row: Record<string, unknown>, where: any): boolean => {
+        if (!where || typeof where !== 'object') return true;
+        for (const [k, v] of Object.entries(where)) {
+            if (k.startsWith('$')) continue;
+            const exp = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v;
+            if ((row[k] ?? null) !== (exp ?? null)) return false;
+        }
+        return true;
+    };
+    const driver: any = {
+        name: 'memory', version: '0.0.0', supports: {},
+        async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
+        async find(o: string, ast: any) { return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); },
+        findStream() { throw new Error('ns'); },
+        async findOne(o: string, ast: any) { for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; return null; },
+        async create(o: string, data: Record<string, unknown>) {
+            nextId += 1; const id = (data.id as string) ?? `r_${nextId}`; const row = { ...data, id }; storeFor(o).set(id, row); return row;
+        },
+        async update(o: string, id: string, data: Record<string, unknown>) {
+            const s = storeFor(o); const cur = s.get(id); if (!cur) throw new Error(`nf ${o}/${id}`);
+            const up = { ...cur, ...data, id }; s.set(id, up); return up;
+        },
+        async upsert(o: string, data: Record<string, unknown>) { const id = data.id as string | undefined; return id && storeFor(o).has(id) ? this.update(o, id, data) : this.create(o, data); },
+        async delete(o: string, id: string) { return storeFor(o).delete(id); },
+        async count(o: string, ast: any) { return (await this.find(o, ast)).length; },
+        async bulkCreate(o: string, rows: Record<string, unknown>[]) { return Promise.all(rows.map((r) => this.create(o, r))); },
+        async bulkUpdate() { return []; }, async bulkDelete() {},
+        async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, async commit() {}, async rollback() {},
+    };
+    return { driver, stores };
+}
+
+describe('cascadeDeleteRelations — required FK escalates set_null → restrict', () => {
+    let engine: ObjectQL;
+
+    beforeEach(async () => {
+        engine = new ObjectQL();
+        const { driver } = makeMemoryDriver();
+        engine.registerDriver(driver, true);
+        await engine.init();
+        for (const o of [acct, oppRequired, noteOptional, taskCascade]) engine.registry.registerObject(o as any);
+    });
+
+    it('refuses to delete a parent with a REQUIRED-FK child (DELETE_RESTRICTED, 409) and leaves both rows', async () => {
+        const a = await engine.insert('acct', { name: 'Acme' });
+        await engine.insert('opp', { name: 'Deal', account: a.id });
+
+        await expect(engine.delete('acct', { where: { id: a.id } } as any))
+            .rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: 'opp', dependentCount: 1 });
+
+        // Nothing was deleted or mutated.
+        expect(await engine.findOne('acct', { where: { id: a.id } })).toBeTruthy();
+        expect((await engine.find('opp', {})).length).toBe(1);
+    });
+
+    it('deletes a parent that has no dependents', async () => {
+        const a = await engine.insert('acct', { name: 'Empty' });
+        await engine.delete('acct', { where: { id: a.id } } as any);
+        expect(await engine.findOne('acct', { where: { id: a.id } })).toBeNull();
+    });
+
+    it('nulls the FK for an OPTIONAL (nullable) lookup child and deletes the parent', async () => {
+        const a = await engine.insert('acct', { name: 'Acme' });
+        const n = await engine.insert('note', { body: 'hi', account: a.id });
+        await engine.delete('acct', { where: { id: a.id } } as any);
+        expect(await engine.findOne('acct', { where: { id: a.id } })).toBeNull();
+        const note = await engine.findOne('note', { where: { id: n.id } });
+        expect(note).toBeTruthy();
+        expect((note as any).account).toBeNull();
+    });
+
+    it('honors an explicit deleteBehavior:cascade on a required FK (children removed, no escalation)', async () => {
+        const a = await engine.insert('acct', { name: 'Acme' });
+        const t = await engine.insert('task', { title: 'Follow up', account: a.id });
+        await engine.delete('acct', { where: { id: a.id } } as any);
+        expect(await engine.findOne('acct', { where: { id: a.id } })).toBeNull();
+        expect(await engine.findOne('task', { where: { id: t.id } })).toBeNull();
+    });
+});

packages/objectql/src/engine.ts

@@ -2210,7 +2210,10 @@ export class ObjectQL implements IDataEngine {
    *   - `set_null` → clear the foreign key,
    *   - `restrict` → refuse the delete when dependents exist.
    * `master_detail` defaults to `cascade` (the parent owns the child
-   * lifecycle); `lookup` defaults to `set_null`. Only runs for single-id
+   * lifecycle); `lookup` defaults to `set_null` — except a `set_null` default
+   * on a REQUIRED lookup escalates to `restrict` (you can't null a NOT NULL
+   * FK; restricting with a clear dependent-count message beats a misleading
+   * "<field> is required" 400 from the child). Only runs for single-id
    * deletes — multi/predicate deletes skip cascade (logged).
    */
   private async cascadeDeleteRelations(
@@ -2243,11 +2246,24 @@ export class ObjectQL implements IDataEngine {
         // child FK is typically required, so set_null would be invalid). Only
         // an explicit `restrict` deviates. A plain lookup honors its
         // configured deleteBehavior (default set_null).
-        const behavior: string =
+        let behavior: string =
           fdef.type === 'master_detail'
             ? (fdef.deleteBehavior === 'restrict' ? 'restrict' : 'cascade')
             : (fdef.deleteBehavior || 'set_null');
 
+        // A REQUIRED foreign key cannot be nulled — set_null would issue an
+        // UPDATE clearing the FK, which the child's required-field validator
+        // rejects with a misleading "<field> is required" 400 (the field isn't
+        // even on the object being deleted). That's a contradiction, not the
+        // author's intent: a required FK means the child can't exist detached,
+        // so deleting the parent must be RESTRICTed (SQL's default for a
+        // NOT NULL FK). Authors who want the children gone set
+        // deleteBehavior:'cascade' explicitly. This only escalates the
+        // *defaulted* set_null; an explicit cascade/restrict is untouched.
+        if (behavior === 'set_null' && fdef.required === true) {
+          behavior = 'restrict';
+        }
+
         let dependents: any[];
         try {
           dependents = await this.find(childName, { where: { [fieldName]: id }, context } as any);
@@ -2257,9 +2273,19 @@ export class ObjectQL implements IDataEngine {
         if (!dependents || dependents.length === 0) continue;
 
         if (behavior === 'restrict') {
-          throw new Error(
-            `Cannot delete ${object} (${id}): ${dependents.length} dependent ${childName} record(s) via ${fieldName}`,
+          const reason = fdef.deleteBehavior !== 'restrict' && fdef.required === true
+            ? ` (${fieldName} is required, so it cannot be cleared)`
+            : '';
+          const err: any = new Error(
+            `Cannot delete ${object} (${id}): ${dependents.length} dependent ${childName} record(s) reference it via ${fieldName}${reason}. ` +
+            `Delete or reassign them first, or set deleteBehavior:'cascade' on ${childName}.${fieldName}.`,
           );
+          err.code = 'DELETE_RESTRICTED';
+          err.status = 409;
+          err.object = object;
+          err.dependentObject = childName;
+          err.dependentCount = dependents.length;
+          throw err;
         }
 
         for (const dep of dependents) {