fix(plugin-designer): editing an app keeps its stored navigation (objectui#10894)

.changeset/10894-app-edit-keeps-navigation.md

@@ -0,0 +1,18 @@
+---
+'@object-ui/plugin-designer': patch
+---
+
+fix(plugin-designer): editing an app keeps its stored navigation (objectui#10894)
+
+**Clause-②: no** — an edit stops destroying stored navigation. No declared type, accepted key or published surface moves.
+
+The console's "edit app" page (`EditAppPage`) loads the stored app into `AppCreationWizard`. Leaving the wizard's Objects step replaced the draft's `navigation` with one generated object entry per selected object, and every edit passes through that step. So any edit, even a title change, saved a stored `[object, separator, group, url]` tree as `[object]`. It dropped every separator, group (with its children) and `url` / `dashboard` / `page` / `report` / `component` / `action` entry, and each object entry's own label, icon and order, with no warning.
+
+- **Leaving the Objects step no longer replaces the navigation.** An empty navigation (the create path) is still filled with one object entry per selected object. A non-empty one is merged:
+  - a newly selected object gets an object entry, appended at the end;
+  - the object entries of a deselected object are dropped, at the top level or inside a group. The group keeps its place and its other children, even when that leaves it with none;
+  - every other entry is kept as stored, in its position. That includes each object entry's label, icon and order, and an object entry for an object the Objects step does not list.
+- **`EditAppPage` counts an object as selected when the stored navigation has an object entry for it at the top level or inside a group, at any depth.** It counted only top-level entries, so a grouped object showed as unselected.
+- The same merge applies on the create path once the author has shaped the navigation. Going back to the Objects step and forward again keeps the added separators, groups and links, and the order.
+
+Pinned in `packages/plugin-designer/src/__tests__/EditAppPage.keepsNavigation-10894.test.tsx`.

packages/plugin-designer/src/AppCreationWizard.tsx

@@ -112,6 +112,78 @@ function generateNavFromObjects(objects: ObjectSelection[]): NavigationItem[] {
     }));
 }
 
+/**
+ * The objects a navigation tree has an object entry for, wherever the entry
+ * sits: at the top level or inside a group (objectui#10894).
+ */
+export function navObjectNames(items: readonly NavigationItem[]): Set<string> {
+  const names = new Set<string>();
+  const walk = (list: readonly NavigationItem[]) => {
+    for (const item of list) {
+      if (item.type === 'object' && item.objectName) names.add(item.objectName);
+      else if (item.type === 'group' && item.children) walk(item.children);
+    }
+  };
+  walk(items);
+  return names;
+}
+
+/**
+ * `items` without the object entries of a `deselected` object, top level and
+ * inside groups. A group keeps its place and its other children, even when
+ * that leaves it with none. Returns `items` itself when nothing was dropped.
+ */
+function dropDeselectedObjectEntries(
+  items: NavigationItem[],
+  deselected: ReadonlySet<string>,
+): NavigationItem[] {
+  let changed = false;
+  const kept: NavigationItem[] = [];
+  for (const item of items) {
+    if (item.type === 'object' && item.objectName && deselected.has(item.objectName)) {
+      changed = true;
+      continue;
+    }
+    if (item.type === 'group' && item.children) {
+      const children = dropDeselectedObjectEntries(item.children, deselected);
+      if (children !== item.children) {
+        changed = true;
+        kept.push({ ...item, children });
+        continue;
+      }
+    }
+    kept.push(item);
+  }
+  return changed ? kept : items;
+}
+
+/**
+ * The navigation the wizard carries out of the Objects step (objectui#10894).
+ *
+ * It never replaces what the draft holds. An EMPTY navigation (the create
+ * path) is filled with one object entry per selected object. A non-empty one
+ * (an edit, or a create the author already shaped) is merged:
+ * - the object entries of an object the Objects step lists as deselected are
+ *   dropped, wherever they sit;
+ * - a selected object with no object entry anywhere in the tree gets one,
+ *   appended at the end;
+ * - every other entry is kept as it is, in its position: separators, groups
+ *   and their children, `url` / `dashboard` / `page` / `report` /
+ *   `component` / `action` entries, each object entry's authored label, icon
+ *   and order, and an object entry for an object the Objects step does not
+ *   list (nothing deselected it).
+ */
+function mergeNavWithObjects(
+  navigation: NavigationItem[],
+  objects: ObjectSelection[],
+): NavigationItem[] {
+  const deselected = new Set(objects.filter((o) => !o.selected).map((o) => o.name));
+  const kept = dropDeselectedObjectEntries(navigation, deselected);
+  const present = navObjectNames(kept);
+  const added = generateNavFromObjects(objects.filter((o) => !present.has(o.name)));
+  return added.length === 0 ? kept : [...kept, ...added];
+}
+
 let navItemCounter = 0;
 
 function createNavId(prefix: string): string {
@@ -781,11 +853,12 @@ export function AppCreationWizard({
 
   const handleNext = useCallback(() => {
     if (isLastStep) return;
-    // Auto-generate navigation when moving from objects → navigation
+    // Leaving objects → navigation: fill an empty navigation from the selected
+    // objects, or merge the selection into the one the draft holds.
     if (currentStep === 1) {
       setDraft((prev) => ({
         ...prev,
-        navigation: generateNavFromObjects(prev.objects),
+        navigation: mergeNavWithObjects(prev.navigation, prev.objects),
       }));
     }
     setCurrentStep((s) => s + 1);
@@ -824,11 +897,11 @@ export function AppCreationWizard({
       if (index <= currentStep) {
         setCurrentStep(index);
       } else if (index === currentStep + 1 && canProceed) {
-        // Auto-generate navigation when jumping from objects → navigation
+        // Jumping objects → navigation: the same fill-or-merge as `handleNext`.
         if (currentStep === 1 && index === 2) {
           setDraft((prev) => ({
             ...prev,
-            navigation: generateNavFromObjects(prev.objects),
+            navigation: mergeNavWithObjects(prev.navigation, prev.objects),
           }));
         }
         setCurrentStep(index);

packages/plugin-designer/src/__tests__/EditAppPage.keepsNavigation-10894.test.tsx

@@ -0,0 +1,211 @@
+/**
+ * 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.
+ */
+
+/**
+ * Editing an app through the wizard keeps its stored navigation
+ * (objectui#10894).
+ *
+ * `EditAppPage` loads the stored app into `AppCreationWizard` and saves what
+ * the wizard completes with through `client.meta.saveItem('app', …)`. Leaving
+ * the Objects step used to REPLACE `draft.navigation` with one generated
+ * object entry per selected object, so every edit, even a title change, saved
+ * a stored `[object, separator, group, url]` tree as `[object]`.
+ *
+ * Leaving the Objects step now fills an EMPTY navigation (the create path, as
+ * before) and otherwise merges: an entry is appended for a newly selected
+ * object, the entries of a deselected object are dropped wherever they sit, and
+ * every other entry is kept as stored, in its position.
+ *
+ * Each case drives the REAL `EditAppPage` / `CreateAppPage` and wizard to the
+ * body `saveItem` receives.
+ */
+
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import React from 'react';
+import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
+import { AppSchema as SpecAppSchema } from '@objectstack/spec/ui';
+
+const saveItem = vi.fn().mockResolvedValue({});
+const apps: Record<string, unknown>[] = [];
+let routeParams: Record<string, string> = {};
+
+vi.mock('react-router-dom', () => ({
+  useParams: () => routeParams,
+  useNavigate: () => vi.fn(),
+}));
+
+const OBJECTS = [
+  { name: 'account', label: 'Account', pluralLabel: 'Accounts', icon: 'Building' },
+  { name: 'contact', label: 'Contact', pluralLabel: 'Contacts', icon: 'User' },
+  { name: 'opportunity', label: 'Opportunity', pluralLabel: 'Opportunities', icon: 'Target' },
+];
+
+vi.mock('@object-ui/react', async (importOriginal) => {
+  const actual = await importOriginal<Record<string, unknown>>();
+  return {
+    ...actual,
+    useAdapter: () => ({ getClient: () => ({ meta: { saveItem } }) }),
+    useMetadata: () => ({ apps, objects: OBJECTS, refresh: () => Promise.resolve() }),
+  };
+});
+
+vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn(), info: vi.fn() } }));
+
+import { CreateAppPage } from '../pages/CreateAppPage';
+import { EditAppPage } from '../pages/EditAppPage';
+
+/** The spec's issues for a document, as `code` plus the path and keys it names. */
+const specIssues = (doc: unknown) => {
+  const r = SpecAppSchema.safeParse(doc);
+  return r.success
+    ? null
+    : r.error.issues.map((i) => ({ code: i.code, path: i.path.join('.'), keys: (i as { keys?: string[] }).keys }));
+};
+
+const next = () => fireEvent.click(screen.getByTestId('wizard-next'));
+
+/** The one body `saveItem` received. */
+async function savedBody(): Promise<Record<string, unknown>> {
+  await waitFor(() => expect(saveItem).toHaveBeenCalledTimes(1));
+  const [type, name, body] = saveItem.mock.calls[0];
+  expect([type, name]).toEqual(['app', 'acme_crm']);
+  return body as Record<string, unknown>;
+}
+
+const ACCOUNTS = { id: 'nav_accounts', type: 'object', label: 'Customer Accounts', icon: 'Briefcase', objectName: 'account' };
+const RULE = { id: 'nav_rule', type: 'separator' };
+const PIPELINE = { id: 'nav_pipeline', type: 'dashboard', label: 'Pipeline', dashboardName: 'sales_pipeline' };
+const SALES = { id: 'nav_sales', type: 'group', label: 'Sales', icon: 'LineChart', children: [PIPELINE] };
+const DOCS = { id: 'nav_docs', type: 'url', label: 'Docs', url: 'https://docs.acme.dev', target: '_blank' };
+
+/**
+ * A stored app carrying every key the wizard maintains, so the whole saved
+ * body can be compared with it, and a navigation of
+ * `[object, separator, group(with a dashboard child), url]`.
+ */
+const STORED = {
+  name: 'acme_crm',
+  label: 'Acme CRM',
+  description: 'Customer relationships',
+  icon: 'Briefcase',
+  branding: { logo: '/acme.svg', primaryColor: '#2563eb', favicon: '/acme.ico' },
+  navigation: [ACCOUNTS, RULE, SALES, DOCS],
+};
+
+const RENAMED = 'Acme CRM (renamed)';
+
+type Leave = 'Next' | 'the step indicator';
+
+/**
+ * Edit `stored` through the real page: rename it on the basic step, run
+ * `onObjects` on the Objects step, leave it by `leave`, and complete. Returns
+ * the saved body and the top-level entry ids the Navigation step listed.
+ */
+async function editThrough(stored: Record<string, unknown>, onObjects?: () => void, leave: Leave = 'Next') {
+  apps.push(stored);
+  routeParams = { editAppName: 'acme_crm' };
+  render(<EditAppPage />);
+  fireEvent.change(screen.getByTestId('app-title-input'), { target: { value: RENAMED } });
+  next(); // → objects
+  onObjects?.();
+  if (leave === 'Next') next();
+  else fireEvent.click(screen.getByTestId('wizard-step-navigation'));
+  const listed = within(screen.getByTestId('wizard-step-navigation-content'))
+    .queryAllByTestId(/^nav-item-/)
+    .map((el) => el.getAttribute('data-testid')!.slice('nav-item-'.length));
+  next(); // → branding
+  fireEvent.click(screen.getByTestId('wizard-complete'));
+  return { body: await savedBody(), listed };
+}
+
+const toggle = (objectName: string) => () => fireEvent.click(screen.getByTestId(`object-card-${objectName}`));
+
+beforeEach(() => {
+  saveItem.mockClear();
+  apps.length = 0;
+  routeParams = {};
+  localStorage.clear();
+});
+afterEach(() => cleanup());
+
+describe('objectui#10894 — an edit keeps the stored navigation tree', () => {
+  it.each<Leave>(['Next', 'the step indicator'])(
+    'the stored tree round-trips byte-equal apart from the edited title, leaving the Objects step by %s',
+    async (leave) => {
+      const { body, listed } = await editThrough(STORED, undefined, leave);
+      expect(listed).toEqual(['nav_accounts', 'nav_rule', 'nav_sales', 'nav_docs']);
+      expect(JSON.stringify(body)).toBe(JSON.stringify({ ...STORED, label: RENAMED }));
+      expect(specIssues(body)).toBeNull();
+    },
+  );
+
+  it('selecting a new object appends exactly its entry and keeps everything else', async () => {
+    const { body } = await editThrough(STORED, toggle('contact'));
+    expect(body.navigation).toEqual([
+      ...STORED.navigation,
+      { id: 'contact', type: 'object', label: 'Contacts', icon: 'User', objectName: 'contact' },
+    ]);
+    expect(specIssues(body)).toBeNull();
+  });
+
+  it('deselecting an object drops only its entry', async () => {
+    const { body } = await editThrough(STORED, toggle('account'));
+    expect(body.navigation).toEqual([RULE, SALES, DOCS]);
+    expect(specIssues(body)).toBeNull();
+  });
+});
+
+describe('objectui#10894 — an object entry the author placed inside a group', () => {
+  const PEOPLE = { id: 'nav_people', type: 'object', label: 'People', icon: 'Users', objectName: 'contact' };
+  const NESTED = { ...STORED, navigation: [ACCOUNTS, RULE, { ...SALES, children: [PIPELINE, PEOPLE] }, DOCS] };
+
+  it('round-trips untouched: the Objects step counts its object as selected', async () => {
+    const { body } = await editThrough(NESTED);
+    expect(JSON.stringify(body.navigation)).toBe(JSON.stringify(NESTED.navigation));
+  });
+
+  it('deselecting its object drops it from inside the group; the group and its other children stay', async () => {
+    const { body } = await editThrough(NESTED, toggle('contact'));
+    expect(body.navigation).toEqual([ACCOUNTS, RULE, SALES, DOCS]);
+    expect(specIssues(body)).toBeNull();
+  });
+
+  it('a group whose only child is dropped stays, with an empty `children`', async () => {
+    const ONLY_CHILD = { ...STORED, navigation: [ACCOUNTS, { ...SALES, children: [PEOPLE] }] };
+    const { body } = await editThrough(ONLY_CHILD, toggle('contact'));
+    expect(body.navigation).toEqual([ACCOUNTS, { ...SALES, children: [] }]);
+    expect(specIssues(body)).toBeNull();
+  });
+
+  it('an object entry whose object the Objects step does not list is kept: nothing deselected it', async () => {
+    const TASKS = { id: 'nav_tasks', type: 'object', label: 'Tasks', objectName: 'task' };
+    const UNLISTED = { ...STORED, navigation: [ACCOUNTS, { ...SALES, children: [PIPELINE, TASKS] }] };
+    const { body } = await editThrough(UNLISTED);
+    expect(JSON.stringify(body.navigation)).toBe(JSON.stringify(UNLISTED.navigation));
+  });
+});
+
+describe('objectui#10894 — the create path still generates navigation for an empty draft', () => {
+  it('CONTROL — one object entry per selected object, in the Objects step order', async () => {
+    render(<CreateAppPage />);
+    fireEvent.change(screen.getByTestId('app-name-input'), { target: { value: 'acme_crm' } });
+    fireEvent.change(screen.getByTestId('app-title-input'), { target: { value: 'Acme CRM' } });
+    next(); // → objects
+    fireEvent.click(screen.getByTestId('object-card-contact'));
+    fireEvent.click(screen.getByTestId('object-card-account'));
+    next(); // → navigation, generated from the selected objects
+    next(); // → branding
+    fireEvent.click(screen.getByTestId('wizard-complete'));
+    const body = await savedBody();
+    expect(body.navigation).toEqual([
+      { id: 'account', type: 'object', label: 'Accounts', icon: 'Building', objectName: 'account' },
+      { id: 'contact', type: 'object', label: 'Contacts', icon: 'User', objectName: 'contact' },
+    ]);
+    expect(specIssues(body)).toBeNull();
+  });
+});

packages/plugin-designer/src/pages/EditAppPage.tsx

@@ -9,7 +9,7 @@
 
 import { useCallback, useMemo } from 'react';
 import { useNavigate, useParams } from 'react-router-dom';
-import { AppCreationWizard } from '../AppCreationWizard';
+import { AppCreationWizard, navObjectNames } from '../AppCreationWizard';
 import { wizardDraftToAppSchema } from '@object-ui/types';
 import type { AppWizardDraft, ObjectSelection } from '@object-ui/types';
 import { AppSchema as SpecAppSchema, AppBrandingSchema as SpecAppBrandingSchema } from '@objectstack/spec/ui';
@@ -56,15 +56,18 @@ export function EditAppPage() {
   // Find the app to edit
   const appToEdit = apps.find((a: any) => a.name === targetAppName);
 
-  // Map metadata objects to ObjectSelection format
+  // Map metadata objects to ObjectSelection format. An object is selected when
+  // the stored navigation has an object entry for it ANYWHERE, a group's
+  // children included (objectui#10894): leaving the Objects step drops the
+  // entries of every object listed as deselected, so a grouped entry read as
+  // unselected would be lost on an edit that never touched it.
+  const storedObjectNames = navObjectNames(appToEdit?.navigation || []);
   const availableObjects: ObjectSelection[] = (objects || []).map((obj: any) => ({
     name: obj.name,
     label: obj.label || obj.name,
     pluralLabel: obj.pluralLabel,
     icon: obj.icon,
-    selected: appToEdit?.navigation?.some(
-      (nav: any) => nav.type === 'object' && nav.objectName === obj.name,
-    ) ?? false,
+    selected: storedObjectNames.has(obj.name),
   }));
 
   // Convert existing app to wizard draft