fix(plugin-view): the grid's row and bulk Delete delete on the registered object-view path

.changeset/10383-object-view-grid-delete-deletes.md

@@ -0,0 +1,15 @@
+---
+'@object-ui/plugin-view': minor
+---
+
+The grid's row Delete and bulk Delete now delete the record when `ObjectView` renders the grid itself — the registered `object-view` renderer, with no host list view (objectui#10383).
+
+`ObjectGrid` hands the clicked row, or the selection, to its `onDelete` / `onBulkDelete` consumer and leaves the delete to it. `ObjectView`'s two handlers ignored the record and only refreshed, so a Delete the grid offers by default asked no question, called `dataSource.delete` zero times, and the row was still there after the refresh.
+
+They now bind to the same record-delete core as the console's own object list (`recordDelete` from `@object-ui/core`), so the two paths behave the same:
+
+- a row Delete asks "Are you sure you want to delete this record?". A package-owned permission set (a `sys_permission_set` row whose `managed_by` is `'package'`) is asked the reset question instead, because deleting it resets it to its shipped baseline rather than removing it. A bulk Delete asks once for the whole selection ("Delete N selected records? This cannot be undone.");
+- Continue calls `dataSource.delete(objectName, id)` for each record, then refreshes the grid, so the deleted rows are gone. Cancel deletes nothing;
+- the outcome is reported with the console's toasts: "LABEL deleted successfully" (or the reset message for a package-owned permission set) / "Deleted N LABEL records", or "Failed to delete LABEL" (with the error message, and no refresh) / "N deleted, M failed".
+
+The confirm dialog is `ObjectView`'s own, in the console's dialog shape. Its texts reuse translation keys the console already resolves, so they read the same in every shipped language. Whether Delete is offered at all is unchanged. It is still the grid's own verdict, the same one the console list uses: `operations.delete`, the principal's delete grant, the object's lifecycle and `userActions`, its API operations and the per-record verdict.

.changeset/10383-record-delete-core.md

@@ -0,0 +1,11 @@
+---
+'@object-ui/core': minor
+---
+
+New export `recordDelete` — the one record-delete core that list hosts bind to, so a Delete behaves the same wherever it is offered (objectui#10383).
+
+`recordDelete.confirmText({ objectName, t }, record?)` returns the question to ask before deleting one record. `recordDelete.run(deps, request)` performs the delete and reports it: one record, or several deleted one call each and settled together, then a refresh and the success, failure or "N deleted, M failed" toast. `deps` carries the host's `objectName`, `label`, `t`, `toast`, `dataSource` and optional `onRefresh`, so `@object-ui/core` takes no i18n or toast dependency. `request` is the action runner's `delete` shape (`params.records`, or `params.recordId` with an optional `params.record`), and the result is what a runner `delete` handler returns: the toasts are the feedback, so no path returns an `error` except a request with no record id.
+
+ADR-0094 lives here: a `sys_permission_set` row whose `managed_by` is `'package'` is reset to its shipped baseline, not removed, so it is asked the reset question and gets the reset toast. The row passed in is read first, with a best-effort `findOne` when only the id is known.
+
+The console's object list (`@object-ui/app-shell` `useObjectActions`) and the registered `object-view` grid (`@object-ui/plugin-view`) both call it. The console's behaviour is unchanged: its handler code moved here with no behaviour change, including the returns that keep the runner from showing a second toast.

packages/app-shell/src/hooks/useObjectActions.ts

@@ -16,7 +16,7 @@ import { useNavigate, useParams } from 'react-router-dom';
 import { useActionRunner } from '@object-ui/react';
 import { useObjectTranslation } from '@object-ui/i18n';
 import { toast } from 'sonner';
-import type { ActionDef, ActionResult } from '@object-ui/core';
+import { recordDelete, type ActionDef, type ActionResult } from '@object-ui/core';
 
 interface ObjectActionConfig {
   objectName: string;
@@ -83,102 +83,21 @@ export function useObjectActions({
       return { success: true };
     });
 
-    // Handler: delete
-    runner.registerHandler('delete', async (action: any) => {
-      // Accept several param shapes used across call sites:
-      //   { params: { recordId } }       — toolbar / programmatic deletes
-      //   { params: { record } }         — ObjectGrid row dropdown
-      //   { params: { records: [...] } } — bulk delete (multi-row)
-      //   { recordId } (legacy)          — pre-params shape
-      const records = Array.isArray(action.params?.records)
-        ? action.params.records.filter((r: any) => r?.id != null)
-        : null;
-
-      // Bulk path — delete every record in parallel and report a summary.
-      if (records && records.length > 1) {
-        const results = await Promise.allSettled(
-          records.map((r: any) => dataSource.delete(objectName, r.id)),
-        );
-        const failed = results.filter(r => r.status === 'rejected').length;
-        const succeeded = results.length - failed;
-        onRefresh?.();
-        if (failed === 0) {
-          toast.success(
-            t('objectActions.bulkDeleteSuccess', {
-              count: succeeded,
-              label: objectLabel || objectName,
-              defaultValue: `Deleted ${succeeded} ${objectLabel || objectName} records`,
-            }),
-          );
-          // `silent`: the handler already toasted the localized summary above;
-          // without this the runner's post-execution hook adds a second, generic
-          // "Action completed successfully" toast (double success toast).
-          return { success: true, reload: true, silent: true };
-        }
-        toast.error(
-          t('objectActions.bulkDeletePartial', {
-            succeeded,
-            failed,
-            defaultValue: `${succeeded} deleted, ${failed} failed`,
-          }),
-        );
-        // The toast above is the authoritative feedback (it carries the
-        // succeeded/failed summary the runner can't reconstruct). Return
-        // WITHOUT `error` so the ActionRunner post-execution hook — this runner
-        // has a toastHandler (onToast) — doesn't fire a second, duplicate toast.
-        return { success: false };
-      }
-
-      const recordId =
-        action.params?.recordId ??
-        action.params?.record?.id ??
-        records?.[0]?.id ??
-        action.recordId;
-      if (!recordId) return { success: false, error: t('objectActions.noRecordId') };
-
-      // [ADR-0094] "Deleting" a PACKAGE-OWNED permission set doesn't remove
-      // the row — the backend drops the environment overlay and RESETS the
-      // set to its shipped baseline. Detect it BEFORE the delete (the row
-      // still exists) so the success toast tells the truth; a caller that
-      // passed the row spares the lookup.
-      let packagedSetReset = false;
-      if (objectName === 'sys_permission_set') {
-        try {
-          const row =
-            action.params?.record?.managed_by !== undefined
-              ? action.params.record
-              : await dataSource.findOne(objectName, recordId);
-          packagedSetReset = row?.managed_by === 'package';
-        } catch { /* best-effort — fall back to the generic delete copy */ }
-      }
-
-      try {
-        await dataSource.delete(objectName, recordId);
-        onRefresh?.();
-        toast.success(
-          packagedSetReset
-            // No `label` argument, unlike the `deleteSuccess` branch below: this
-            // sentence names the permission set by kind rather than by label, so
-            // none of the ten packs has a `{{label}}` hole and i18next dropped
-            // the argument in silence (objectui#3845).
-            ? t('objectActions.resetPackageSetSuccess', {
-                defaultValue: 'Permission set reset to its shipped baseline',
-              })
-            : t('objectActions.deleteSuccess', { label: objectLabel || objectName }),
-        );
-        // `silent`: handler owns the localized success toast above — suppress the
-        // runner's generic duplicate (see the bulk branch).
-        return { success: true, reload: true, silent: true };
-      } catch (err: any) {
-        toast.error(t('objectActions.deleteFailed', { label: objectLabel || objectName }), {
-          description: err.message,
-        });
-        // Keep the richer toast above (label + error description) and return
-        // WITHOUT `error` so the ActionRunner post-execution hook doesn't toast
-        // the raw message a second time. See the bulk branch for the rationale.
-        return { success: false };
-      }
-    });
+    // Handler: delete — the shared record-delete core (objectui#10383), the
+    // same one `plugin-view`'s grid Delete binds to. Its param shapes:
+    //   { params: { recordId } }       — toolbar / programmatic deletes
+    //   { params: { record } }         — ObjectGrid row dropdown
+    //   { params: { records: [...] } } — bulk delete (multi-row)
+    //   { recordId } (legacy)          — pre-params shape
+    // It owns the toasts and returns without `error` (and `silent` on
+    // success), so the runner's post-execution hook adds no second toast; the
+    // ADR-0094 package-owned permission-set reset copy lives there too.
+    runner.registerHandler('delete', (action: any) =>
+      recordDelete.run(
+        { objectName, label: objectLabel || objectName, dataSource, t, toast, onRefresh },
+        action,
+      ),
+    );
 
     // Handler: navigate
     runner.registerHandler('navigate', async (action: any) => {
@@ -202,21 +121,13 @@ export function useObjectActions({
 
   const deleteRecord = useCallback(
     async (recordId: string, record?: Record<string, unknown>) => {
-      // [ADR-0094] A package-owned permission set is never removed by the data
-      // door — the backend drops the environment overlay and RESETS the set to
-      // its shipped baseline. Ask the honest question instead of promising an
-      // irreversible delete the user can see doesn't happen (the row stays).
-      const packagedSetReset =
-        objectName === 'sys_permission_set' && (record as any)?.managed_by === 'package';
+      // The question comes from the shared record-delete core (objectui#10383):
+      // for a package-owned permission set it is the honest RESET question
+      // (ADR-0094), not a promise of an irreversible delete the user can see
+      // doesn't happen (the row stays).
       return execute({
         type: 'delete',
-        confirmText: packagedSetReset
-          ? t('objectActions.resetPackageSetConfirm', {
-              defaultValue:
-                'This permission set ships with an installed package and cannot be removed. ' +
-                'Deleting resets it to the shipped baseline and discards your environment customization. Continue?',
-            })
-          : t('objectActions.deleteConfirm'),
+        confirmText: recordDelete.confirmText({ objectName, t }, record),
         params: record ? { recordId, record } : { recordId },
       });
     },

packages/core/src/actions/__tests__/recordDelete.test.ts

@@ -0,0 +1,213 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * The shared record-delete core (objectui#10383, ADR-0094), pinned at the unit
+ * level: the question, the single delete, the bulk delete, the partial failure
+ * and the package-owned permission-set reset — with the runner-facing result
+ * shape each path returns, because the console registers `run` as its action
+ * runner's `delete` handler and those shapes are what keep the runner from
+ * toasting a second time.
+ *
+ * The translator echoes the key (plus `|defaultValue` when one is passed) so
+ * each assertion names the copy it expects without depending on a pack.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { recordDelete } from '../recordDelete';
+
+const t = (key: string, options?: Record<string, unknown>) => {
+  const vars = { ...(options ?? {}) };
+  delete vars.defaultValue;
+  const suffix = Object.keys(vars).length ? ` ${JSON.stringify(vars)}` : '';
+  return `${key}${suffix}`;
+};
+
+function harness(opts: {
+  objectName?: string;
+  failIds?: string[];
+  findOne?: (resource: string, id: string) => Promise<unknown>;
+} = {}) {
+  const failIds = opts.failIds ?? [];
+  const dataSource = {
+    delete: vi.fn(async (_resource: string, id: string) => {
+      if (failIds.includes(id)) throw new Error(`refused ${id}`);
+      return true;
+    }),
+    findOne: vi.fn(opts.findOne ?? (async () => null)),
+  };
+  const toast = { success: vi.fn(), error: vi.fn() };
+  const onRefresh = vi.fn();
+  const deps = {
+    objectName: opts.objectName ?? 'crm_lead',
+    label: 'Lead',
+    t,
+    dataSource,
+    toast,
+    onRefresh,
+  };
+  return { deps, dataSource, toast, onRefresh };
+}
+
+describe('recordDelete.confirmText', () => {
+  it('asks the plain delete question for an ordinary record', () => {
+    expect(recordDelete.confirmText({ objectName: 'crm_lead', t }, { id: 'l1' })).toBe(
+      'objectActions.deleteConfirm',
+    );
+  });
+
+  it('asks the RESET question for a package-owned permission set (ADR-0094)', () => {
+    expect(
+      recordDelete.confirmText(
+        { objectName: 'sys_permission_set', t },
+        { id: 'ps1', managed_by: 'package' },
+      ),
+    ).toBe('objectActions.resetPackageSetConfirm');
+  });
+
+  it('keeps the plain question for an environment-owned permission set, or with no row', () => {
+    expect(
+      recordDelete.confirmText({ objectName: 'sys_permission_set', t }, { id: 'ps1', managed_by: 'user' }),
+    ).toBe('objectActions.deleteConfirm');
+    expect(recordDelete.confirmText({ objectName: 'sys_permission_set', t })).toBe(
+      'objectActions.deleteConfirm',
+    );
+  });
+});
+
+describe('recordDelete.run — one record', () => {
+  it('deletes, refreshes, toasts success, and returns the silent success the runner expects', async () => {
+    const { deps, dataSource, toast, onRefresh } = harness();
+    const res = await recordDelete.run(deps, { params: { recordId: 'l1' } });
+
+    expect(dataSource.delete).toHaveBeenCalledTimes(1);
+    expect(dataSource.delete).toHaveBeenCalledWith('crm_lead', 'l1');
+    expect(onRefresh).toHaveBeenCalledTimes(1);
+    expect(toast.success).toHaveBeenCalledWith('objectActions.deleteSuccess {"label":"Lead"}');
+    expect(toast.error).not.toHaveBeenCalled();
+    expect(res).toEqual({ success: true, reload: true, silent: true });
+  });
+
+  it('a refused delete toasts the failure with its message, does not refresh, and returns no `error`', async () => {
+    const { deps, toast, onRefresh } = harness({ failIds: ['l1'] });
+    const res = await recordDelete.run(deps, { params: { recordId: 'l1' } });
+
+    expect(toast.error).toHaveBeenCalledTimes(1);
+    expect(toast.error).toHaveBeenCalledWith('objectActions.deleteFailed {"label":"Lead"}', {
+      description: 'refused l1',
+    });
+    expect(onRefresh).not.toHaveBeenCalled();
+    expect(res).toEqual({ success: false });
+  });
+
+  it('reads the id off `params.record`, then a single `params.records` entry, then the legacy top-level `recordId`', async () => {
+    const a = harness();
+    await recordDelete.run(a.deps, { params: { record: { id: 'from-record' } } });
+    expect(a.dataSource.delete).toHaveBeenCalledWith('crm_lead', 'from-record');
+
+    const b = harness();
+    await recordDelete.run(b.deps, { params: { records: [{ id: 'only-one' }] } });
+    expect(b.dataSource.delete).toHaveBeenCalledWith('crm_lead', 'only-one');
+
+    const c = harness();
+    await recordDelete.run(c.deps, { recordId: 'legacy' });
+    expect(c.dataSource.delete).toHaveBeenCalledWith('crm_lead', 'legacy');
+  });
+
+  it('with no id at all, deletes nothing and returns the `error` for the runner to report', async () => {
+    const { deps, dataSource, toast } = harness();
+    const res = await recordDelete.run(deps, { params: {} });
+
+    expect(dataSource.delete).not.toHaveBeenCalled();
+    expect(toast.success).not.toHaveBeenCalled();
+    expect(toast.error).not.toHaveBeenCalled();
+    expect(res).toEqual({ success: false, error: 'objectActions.noRecordId' });
+  });
+});
+
+describe('recordDelete.run — several records', () => {
+  it('deletes each record, refreshes once, and toasts ONE summary', async () => {
+    const { deps, dataSource, toast, onRefresh } = harness();
+    const res = await recordDelete.run(deps, {
+      params: { records: [{ id: 'a' }, { id: 'b' }, { name: 'no id — skipped' }] },
+    });
+
+    expect(dataSource.delete).toHaveBeenCalledTimes(2);
+    expect(dataSource.delete).toHaveBeenCalledWith('crm_lead', 'a');
+    expect(dataSource.delete).toHaveBeenCalledWith('crm_lead', 'b');
+    expect(onRefresh).toHaveBeenCalledTimes(1);
+    expect(toast.success).toHaveBeenCalledTimes(1);
+    expect(toast.success).toHaveBeenCalledWith(
+      'objectActions.bulkDeleteSuccess {"count":2,"label":"Lead"}',
+    );
+    expect(res).toEqual({ success: true, reload: true, silent: true });
+  });
+
+  it('a partial failure toasts "N deleted, M failed" once, still refreshes, and returns no `error`', async () => {
+    const { deps, dataSource, toast, onRefresh } = harness({ failIds: ['b'] });
+    const res = await recordDelete.run(deps, { params: { records: [{ id: 'a' }, { id: 'b' }] } });
+
+    expect(dataSource.delete).toHaveBeenCalledTimes(2);
+    expect(onRefresh).toHaveBeenCalledTimes(1);
+    expect(toast.error).toHaveBeenCalledTimes(1);
+    expect(toast.error).toHaveBeenCalledWith(
+      'objectActions.bulkDeletePartial {"succeeded":1,"failed":1}',
+    );
+    expect(toast.success).not.toHaveBeenCalled();
+    expect(res).toEqual({ success: false });
+  });
+});
+
+describe('recordDelete.run — ADR-0094 package-owned permission set', () => {
+  it('toasts the RESET when the row passed in is package-owned, without a lookup', async () => {
+    const { deps, dataSource, toast } = harness({ objectName: 'sys_permission_set' });
+    await recordDelete.run(deps, {
+      params: { recordId: 'ps1', record: { id: 'ps1', managed_by: 'package' } },
+    });
+
+    expect(dataSource.delete).toHaveBeenCalledWith('sys_permission_set', 'ps1');
+    expect(dataSource.findOne).not.toHaveBeenCalled();
+    expect(toast.success).toHaveBeenCalledWith('objectActions.resetPackageSetSuccess');
+  });
+
+  it('looks the row up with findOne when only the id is known, and toasts the reset', async () => {
+    const { deps, dataSource, toast } = harness({
+      objectName: 'sys_permission_set',
+      findOne: async () => ({ id: 'ps1', managed_by: 'package' }),
+    });
+    await recordDelete.run(deps, { params: { recordId: 'ps1' } });
+
+    expect(dataSource.findOne).toHaveBeenCalledWith('sys_permission_set', 'ps1');
+    expect(toast.success).toHaveBeenCalledWith('objectActions.resetPackageSetSuccess');
+  });
+
+  it('an environment-owned set, or a failed lookup, keeps the plain delete copy (control)', async () => {
+    const env = harness({ objectName: 'sys_permission_set' });
+    await recordDelete.run(env.deps, {
+      params: { recordId: 'ps1', record: { id: 'ps1', managed_by: 'user' } },
+    });
+    expect(env.toast.success).toHaveBeenCalledWith('objectActions.deleteSuccess {"label":"Lead"}');
+
+    const lost = harness({
+      objectName: 'sys_permission_set',
+      findOne: async () => {
+        throw new Error('lookup failed');
+      },
+    });
+    await recordDelete.run(lost.deps, { params: { recordId: 'ps1' } });
+    // Best-effort: the lookup failing does not block the delete.
+    expect(lost.dataSource.delete).toHaveBeenCalledWith('sys_permission_set', 'ps1');
+    expect(lost.toast.success).toHaveBeenCalledWith('objectActions.deleteSuccess {"label":"Lead"}');
+  });
+
+  it('an ordinary object is never looked up', async () => {
+    const { deps, dataSource } = harness();
+    await recordDelete.run(deps, { params: { recordId: 'l1' } });
+    expect(dataSource.findOne).not.toHaveBeenCalled();
+  });
+});