Merge pull request #1176 from objectstack-ai/copilot/optimize-object-field-management

Add MetadataService for object/field CRUD persistence and fix grid action buttons

Author: xuyushun441-sys

CHANGELOG.md

@@ -9,6 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- **Metadata service layer** (`@object-ui/console`): New `MetadataService` class (`services/MetadataService.ts`) that encapsulates object and field CRUD operations against the ObjectStack metadata API (`client.meta.saveItem`). Provides `saveObject()`, `deleteObject()`, and `saveFields()` methods with automatic cache invalidation. Includes static `diffObjects()` and `diffFields()` helpers to detect create/update/delete changes between arrays. Companion `useMetadataService` hook (`hooks/useMetadataService.ts`) provides a memoised service instance from the `useAdapter()` context. 16 new unit tests.
+
+### Fixed
+
+- **Object/field changes now persist to backend** (`@object-ui/console`): Refactored `ObjectManagerPage` so that `handleObjectsChange` and `handleFieldsChange` call the MetadataService API instead of only updating local state. Implements optimistic update pattern — UI updates immediately, rolls back on API failure. After successful persistence, the MetadataProvider is refreshed so changes survive page reloads. Toast messages now accurately reflect the operation performed (create/update/delete) and display error details on failure. Added saving state indicators with a loading spinner during API operations.
+
+- **ObjectManager & FieldDesigner read-only grid fix** (`@object-ui/plugin-designer`): Added `operations: { create: true, update: true, delete: true }` to the `ObjectGridSchema` in both `ObjectManager` and `FieldDesigner` components. The real `ObjectGrid` requires `schema.operations` to render action buttons (add/edit/delete); without it, the grid renders as read-only regardless of the `readOnly` prop or callback handlers. The `operations` property is now conditionally set based on the `readOnly` prop.
+
+- **FieldDesigner modal dialog for add/edit** (`@object-ui/plugin-designer`): Replaced the inline `FieldEditor` panel (rendered below the grid) with a proper `ModalForm` dialog, matching the `ObjectManager` pattern. Add Field and Edit Field now open a modal with all field properties (name, label, type, group, description, toggles for required/unique/readonly/hidden/indexed/externalId/trackHistory, default value, placeholder, referenceTo, formula). This provides a consistent, professional UX across both object and field management.
+
+- **Duplicate action column in ObjectGrid** (`@object-ui/plugin-grid`): Fixed a bug where `ObjectGrid` rendered two action columns when `schema.operations` was set — one via `RowActionMenu` (working dropdown with Edit/Delete) and another via DataTable's built-in `rowActions` (inline buttons calling unset `schema.onRowEdit`/`schema.onRowDelete`). The DataTable's `rowActions` is now only enabled for inline-editable grids, preventing the duplicate column and dead buttons.
+
 - **AI service discovery** (`@object-ui/react`): Added `ai` service type to `DiscoveryInfo.services` interface with `enabled`, `status`, and `route` fields. Added `isAiEnabled` convenience property to `useDiscovery()` hook return value — returns `true` only when `services.ai.enabled === true` and `services.ai.status === 'available'`, defaults to `false` otherwise.
 
 - **Conditional chatbot rendering** (`@object-ui/console`): Console floating chatbot (FAB) now only renders when the AI service is detected as available via `useDiscovery().isAiEnabled`. Previously the chatbot was always visible; now it is hidden when the server has no AI plugin installed.

ROADMAP.md

@@ -924,6 +924,18 @@ Enterprise-grade visual designers for managing object definitions and configurin
 - [x] Read-only mode support
 - [x] 22 unit tests
 
+**Metadata Persistence (Console Integration):**
+- [x] `MetadataService` class wrapping `client.meta.saveItem` for object/field CRUD
+- [x] `useMetadataService` hook for adapter-aware service access
+- [x] Optimistic update pattern with rollback on API failure
+- [x] Object create/update persisted via `saveItem('object', name, data)`
+- [x] Object delete via soft-delete (`enabled: false, _deleted: true`)
+- [x] Field changes persisted as part of parent object metadata
+- [x] MetadataProvider `refresh()` called after successful mutations
+- [x] Saving state indicators with loading spinners
+- [x] Accurate toast messages (create/update/delete action labels, error details)
+- [x] 16 new MetadataService unit tests + 2 ObjectManagerPage API integration tests
+
 **Type Definitions (`@object-ui/types`):**
 - [x] `ObjectDefinition`, `ObjectDefinitionRelationship`, `ObjectManagerSchema`
 - [x] `DesignerFieldType` (27 types), `DesignerFieldOption`, `DesignerValidationRule`

apps/console/src/__tests__/MetadataService.test.ts

@@ -0,0 +1,247 @@
+/**
+ * MetadataService unit tests
+ *
+ * Tests the service layer that wraps ObjectStack metadata API calls
+ * for object and field CRUD operations.
+ */
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { MetadataService } from '../services/MetadataService';
+import type { ObjectDefinition, DesignerFieldDefinition } from '@object-ui/types';
+
+// ---------------------------------------------------------------------------
+// Mock adapter / client
+// ---------------------------------------------------------------------------
+
+function createMockAdapter() {
+  const mockClient = {
+    meta: {
+      saveItem: vi.fn().mockResolvedValue({}),
+      getItem: vi.fn().mockResolvedValue({ item: { name: 'account', fields: [] } }),
+    },
+  };
+
+  const adapter = {
+    getClient: vi.fn().mockReturnValue(mockClient),
+    invalidateCache: vi.fn(),
+  } as any;
+
+  return { adapter, mockClient };
+}
+
+// ---------------------------------------------------------------------------
+// Helpers
+// ---------------------------------------------------------------------------
+
+function makeObject(overrides: Partial<ObjectDefinition> = {}): ObjectDefinition {
+  return {
+    id: 'account',
+    name: 'account',
+    label: 'Accounts',
+    fieldCount: 3,
+    sortOrder: 0,
+    isSystem: false,
+    enabled: true,
+    ...overrides,
+  };
+}
+
+function makeField(overrides: Partial<DesignerFieldDefinition> = {}): DesignerFieldDefinition {
+  return {
+    id: 'name',
+    name: 'name',
+    label: 'Name',
+    type: 'text',
+    sortOrder: 0,
+    required: false,
+    unique: false,
+    readonly: false,
+    hidden: false,
+    externalId: false,
+    trackHistory: false,
+    indexed: false,
+    isSystem: false,
+    ...overrides,
+  };
+}
+
+// ---------------------------------------------------------------------------
+// Tests: saveObject
+// ---------------------------------------------------------------------------
+
+describe('MetadataService', () => {
+  let service: MetadataService;
+  let mockClient: ReturnType<typeof createMockAdapter>['mockClient'];
+  let adapter: ReturnType<typeof createMockAdapter>['adapter'];
+
+  beforeEach(() => {
+    ({ adapter, mockClient } = createMockAdapter());
+    service = new MetadataService(adapter);
+  });
+
+  describe('saveObject', () => {
+    it('should call client.meta.saveItem with the correct payload', async () => {
+      const obj = makeObject();
+      await service.saveObject(obj);
+
+      expect(mockClient.meta.saveItem).toHaveBeenCalledWith(
+        'object',
+        'account',
+        expect.objectContaining({ name: 'account', label: 'Accounts', enabled: true }),
+      );
+    });
+
+    it('should invalidate cache after save', async () => {
+      await service.saveObject(makeObject());
+      expect(adapter.invalidateCache).toHaveBeenCalledWith('object:account');
+    });
+
+    it('should propagate API errors', async () => {
+      mockClient.meta.saveItem.mockRejectedValueOnce(new Error('Network error'));
+      await expect(service.saveObject(makeObject())).rejects.toThrow('Network error');
+    });
+  });
+
+  // -------------------------------------------------------------------------
+  // Tests: deleteObject
+  // -------------------------------------------------------------------------
+
+  describe('deleteObject', () => {
+    it('should save the object as disabled with _deleted flag', async () => {
+      await service.deleteObject('account');
+
+      expect(mockClient.meta.saveItem).toHaveBeenCalledWith(
+        'object',
+        'account',
+        expect.objectContaining({ name: 'account', enabled: false, _deleted: true }),
+      );
+    });
+
+    it('should invalidate cache after delete', async () => {
+      await service.deleteObject('account');
+      expect(adapter.invalidateCache).toHaveBeenCalledWith('object:account');
+    });
+  });
+
+  // -------------------------------------------------------------------------
+  // Tests: saveFields
+  // -------------------------------------------------------------------------
+
+  describe('saveFields', () => {
+    it('should fetch existing object and merge fields', async () => {
+      const fields = [makeField(), makeField({ id: 'email', name: 'email', label: 'Email', type: 'email' })];
+      await service.saveFields('account', fields);
+
+      expect(mockClient.meta.getItem).toHaveBeenCalledWith('object', 'account');
+      expect(mockClient.meta.saveItem).toHaveBeenCalledWith(
+        'object',
+        'account',
+        expect.objectContaining({
+          name: 'account',
+          fields: expect.arrayContaining([
+            expect.objectContaining({ name: 'name', type: 'text' }),
+            expect.objectContaining({ name: 'email', type: 'email' }),
+          ]),
+        }),
+      );
+    });
+
+    it('should proceed even if fetching existing object fails', async () => {
+      mockClient.meta.getItem.mockRejectedValueOnce(new Error('Not found'));
+      const fields = [makeField()];
+      await service.saveFields('new_object', fields);
+
+      expect(mockClient.meta.saveItem).toHaveBeenCalledWith(
+        'object',
+        'new_object',
+        expect.objectContaining({
+          name: 'new_object',
+          fields: [expect.objectContaining({ name: 'name' })],
+        }),
+      );
+    });
+
+    it('should invalidate cache after saving fields', async () => {
+      await service.saveFields('account', [makeField()]);
+      expect(adapter.invalidateCache).toHaveBeenCalledWith('object:account');
+    });
+  });
+
+  // -------------------------------------------------------------------------
+  // Tests: diffObjects (static)
+  // -------------------------------------------------------------------------
+
+  describe('diffObjects', () => {
+    it('should detect a created object', () => {
+      const prev = [makeObject()];
+      const newObj = makeObject({ id: 'contact', name: 'contact', label: 'Contacts' });
+      const next = [...prev, newObj];
+
+      const result = MetadataService.diffObjects(prev, next);
+      expect(result).toEqual({ type: 'create', object: newObj });
+    });
+
+    it('should detect a deleted object', () => {
+      const obj1 = makeObject();
+      const obj2 = makeObject({ id: 'contact', name: 'contact', label: 'Contacts' });
+      const prev = [obj1, obj2];
+      const next = [obj1];
+
+      const result = MetadataService.diffObjects(prev, next);
+      expect(result).toEqual({ type: 'delete', object: obj2 });
+    });
+
+    it('should detect an updated object', () => {
+      const prev = [makeObject()];
+      const updated = makeObject({ label: 'Updated Accounts' });
+      const next = [updated];
+
+      const result = MetadataService.diffObjects(prev, next);
+      expect(result).toEqual({ type: 'update', object: updated });
+    });
+
+    it('should return null when arrays are identical', () => {
+      const objs = [makeObject()];
+      expect(MetadataService.diffObjects(objs, objs)).toBeNull();
+    });
+  });
+
+  // -------------------------------------------------------------------------
+  // Tests: diffFields (static)
+  // -------------------------------------------------------------------------
+
+  describe('diffFields', () => {
+    it('should detect a created field', () => {
+      const prev = [makeField()];
+      const newField = makeField({ id: 'email', name: 'email', label: 'Email' });
+      const next = [...prev, newField];
+
+      const result = MetadataService.diffFields(prev, next);
+      expect(result).toEqual({ type: 'create', field: newField });
+    });
+
+    it('should detect a deleted field', () => {
+      const f1 = makeField();
+      const f2 = makeField({ id: 'email', name: 'email', label: 'Email' });
+      const prev = [f1, f2];
+      const next = [f1];
+
+      const result = MetadataService.diffFields(prev, next);
+      expect(result).toEqual({ type: 'delete', field: f2 });
+    });
+
+    it('should detect an updated field', () => {
+      const prev = [makeField()];
+      const updated = makeField({ label: 'Full Name' });
+      const next = [updated];
+
+      const result = MetadataService.diffFields(prev, next);
+      expect(result).toEqual({ type: 'update', field: updated });
+    });
+
+    it('should return null when arrays are identical', () => {
+      const fields = [makeField()];
+      expect(MetadataService.diffFields(fields, fields)).toBeNull();
+    });
+  });
+});

apps/console/src/__tests__/ObjectManagerPage.test.tsx

@@ -3,14 +3,24 @@
  *
  * Tests for the system administration Object Manager page that integrates
  * ObjectManager and FieldDesigner from @object-ui/plugin-designer.
- * Covers list view, detail view with URL-based navigation, and field management.
+ * Covers list view, detail view with URL-based navigation, field management,
+ * and API integration via MetadataService.
  */
 
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { render, screen, fireEvent, waitFor } from '@testing-library/react';
 import { MemoryRouter, Routes, Route } from 'react-router-dom';
 import { ObjectManagerPage } from '../pages/system/ObjectManagerPage';
 
+// ---------------------------------------------------------------------------
+// Shared mock state
+// ---------------------------------------------------------------------------
+
+const mockRefresh = vi.fn().mockResolvedValue(undefined);
+const mockSaveItem = vi.fn().mockResolvedValue({});
+const mockGetItem = vi.fn().mockResolvedValue({ item: { name: 'account', fields: [] } });
+const mockInvalidateCache = vi.fn();
+
 // Mock MetadataProvider
 vi.mock('../context/MetadataProvider', () => ({
   useMetadata: () => ({
@@ -51,7 +61,20 @@ vi.mock('../context/MetadataProvider', () => ({
         ],
       },
     ],
-    refresh: vi.fn(),
+    refresh: mockRefresh,
+  }),
+}));
+
+// Mock AdapterProvider (useAdapter) – provides adapter to useMetadataService
+vi.mock('../context/AdapterProvider', () => ({
+  useAdapter: () => ({
+    getClient: () => ({
+      meta: {
+        saveItem: mockSaveItem,
+        getItem: mockGetItem,
+      },
+    }),
+    invalidateCache: mockInvalidateCache,
   }),
 }));
 
@@ -172,4 +195,24 @@ describe('ObjectManagerPage', () => {
       });
     });
   });
+
+  // -------------------------------------------------------------------------
+  // API Integration tests
+  // -------------------------------------------------------------------------
+
+  describe('API Integration', () => {
+    it('should have MetadataService available (adapter mock is wired)', () => {
+      // Rendering the page should not crash even though useMetadataService
+      // depends on useAdapter — the mock above provides a valid adapter.
+      renderPage();
+      expect(screen.getByTestId('object-manager-page')).toBeDefined();
+    });
+
+    it('should render detail view with MetadataService props', () => {
+      renderPage('/system/objects/account');
+      expect(screen.getByTestId('object-detail-view')).toBeDefined();
+      // The FieldDesigner should be rendered (service is passed through)
+      expect(screen.getByTestId('field-designer')).toBeDefined();
+    });
+  });
 });

apps/console/src/hooks/useMetadataService.ts

@@ -0,0 +1,20 @@
+/**
+ * useMetadataService
+ *
+ * React hook that creates a memoised `MetadataService` instance from the
+ * current `ObjectStackAdapter` (via `useAdapter`).
+ *
+ * Returns `null` when the adapter is not yet available (e.g. while
+ * the connection is being established).
+ *
+ * @module hooks/useMetadataService
+ */
+
+import { useMemo } from 'react';
+import { useAdapter } from '../context/AdapterProvider';
+import { MetadataService } from '../services/MetadataService';
+
+export function useMetadataService(): MetadataService | null {
+  const adapter = useAdapter();
+  return useMemo(() => (adapter ? new MetadataService(adapter) : null), [adapter]);
+}