diff --git a/.changeset/default-list-view-identity-3770.md b/.changeset/default-list-view-identity-3770.md new file mode 100644 index 000000000..ef7d3fe8f --- /dev/null +++ b/.changeset/default-list-view-identity-3770.md @@ -0,0 +1,12 @@ +--- +"@object-ui/app-shell": patch +--- + +Ask the view composer for a container's view identities instead of deriving `list.name || 'list'`, so the default list view's translated label resolves + +A `defineView` container declares its default list under the `list` key. That key is a slot in the authoring document, not the view's identity: `expandViewContainer` — the same composer the framework's loader and the i18n extractor call — registers an unnamed default list as `.default`. This renderer derived `list.name || 'list'` instead, a third spelling no producer emits, so a default-list-only object probed `objects.._views.list.label`, missed the published `_views.default.label` key (objectstack#5164 ruling A, migrated in objectstack#6124) and fell back to the English metadata label — for the view's description and empty state too. + +- `MetadataProvider.mergeViewsIntoObjects` now expands a stack-packaged container through `expandViewContainer` and routes the result through the same code path as first-class ViewItems. Both authoring gates therefore key `listViews` / `formViews` by the canonical `.` identity, and the container inherits the composer's folding (a `listViews` entry that merely restates `list` collapses into one view) and collision renaming instead of restating them locally. +- `ObjectView` resolves the primary view's id through the new `defaultListViewId` helper — one derivation shared by the view-override lookup and the view-switcher promotion, with no literal fallback. + +The renamed id is also the key a view override is persisted under (`updateViewConfig(object, viewId, …)` writes a `view` metadata record named by the id). Nothing is orphaned: the retired `'list'` spelling is not a representable view identity at all — `ViewItemNameSchema` requires a dotted `.` name — while the record-gate path, which real backends serve, already used the qualified id. Stale `/view/list` links fall back to the object's default view, which is the same view they named. diff --git a/packages/app-shell/src/providers/MetadataProvider.merge.test.ts b/packages/app-shell/src/providers/MetadataProvider.merge.test.ts index 421717a92..fa4c604fd 100644 --- a/packages/app-shell/src/providers/MetadataProvider.merge.test.ts +++ b/packages/app-shell/src/providers/MetadataProvider.merge.test.ts @@ -68,22 +68,85 @@ describe('mergeViewsIntoObjects', () => { formViews: { default: { type: 'simple' } }, }; const [obj] = mergeViewsIntoObjects(objects, [listAll, listCalendar, formDefault, container]); - // Only the canonical `.` ids — the container's short `all`/`list` - // keys must NOT also appear (no double-listing). + // Only the two list ViewItems. Both gates key by the same canonical + // `.` identity now (objectui#3770), so the container can no longer + // double-list them under short keys — but it still MUST be skipped: its own + // `list` is structurally distinct from `listViews.all`, so expanding it would + // add a third `crm_activity.default` list tab (and force the container's + // `formViews.default` into a `_2` rename), and the ViewItem rows are the + // authoritative ones the runtime heals personalization onto. expect(Object.keys(obj.listViews).sort()).toEqual(['crm_activity.all', 'crm_activity.calendar']); }); - it('still consumes the legacy container for objects without ViewItems', () => { - const container = { - name: 'crm_activity', - list: { name: 'list', type: 'grid' }, - listViews: { all: { type: 'grid' } }, - formViews: { default: { type: 'simple' } }, - }; - const [obj] = mergeViewsIntoObjects(objects, [container]); - expect(Object.keys(obj.listViews).sort()).toEqual(['all', 'list']); - expect(obj.formViews.default).toBeTruthy(); - expect(obj.formViews.list).toBeUndefined(); + /** + * Container gate (objectui#3770). A stack-packaged container is served + * UNEXPANDED, so this merge asks `expandViewContainer` for each view's runtime + * identity instead of deriving one. The previous derivation was + * `list.name || 'list'` — a spelling no producer emits, which is why the + * default list's translation key never resolved (the composer, the framework + * loader and the i18n extractor all say `.default`). + */ + describe('aggregated container — identities come from the view composer', () => { + it('keys an unnamed default list by the composer identity `.default`', () => { + const container = { + name: 'crm_activity', + // No `name` — the default `list` implicitly claims `.default`. + list: { label: 'All Activities', type: 'grid', columns: [{ field: 'subject' }] }, + listViews: { calendar: { type: 'calendar' } }, + }; + const [obj] = mergeViewsIntoObjects(objects, [container]); + expect(Object.keys(obj.listViews).sort()).toEqual([ + 'crm_activity.calendar', + 'crm_activity.default', + ]); + // The retired dialect must be gone, not merely joined by the new key. + expect(obj.listViews.list).toBeUndefined(); + // `name` is stamped so ObjectView's `view.name || view.id` — the argument + // `viewLabel` translates by — carries the composer identity. + expect(obj.list.name).toBe('crm_activity.default'); + expect(obj.listViews['crm_activity.default'].isDefault).toBe(true); + expect(obj.listViews['crm_activity.default'].columns).toEqual([{ field: 'subject' }]); + }); + + it('honors an author-supplied `list.name` as the key', () => { + const container = { + name: 'crm_activity', + list: { name: 'my_list', type: 'grid', columns: [{ field: 'subject' }] }, + }; + const [obj] = mergeViewsIntoObjects(objects, [container]); + expect(Object.keys(obj.listViews)).toEqual(['crm_activity.my_list']); + expect(obj.list.name).toBe('crm_activity.my_list'); + }); + + it('folds a `listViews` entry that merely restates `list` into ONE view', () => { + // The composer dedups by structural signature: `listViews.all` restating + // the default `list` collapses into `crm_activity.all`, which is then the + // default. Deriving the id locally could not know that — it would emit a + // second tab for the same view. + const restated = { type: 'grid', label: 'All', columns: [{ field: 'subject' }] }; + const container = { + name: 'crm_activity', + list: restated, + listViews: { all: { ...restated } }, + }; + const [obj] = mergeViewsIntoObjects(objects, [container]); + expect(Object.keys(obj.listViews)).toEqual(['crm_activity.all']); + expect(obj.list.name).toBe('crm_activity.all'); + expect(obj.listViews['crm_activity.all'].isDefault).toBe(true); + }); + + it('routes the container form family into formViews only', () => { + const container = { + name: 'crm_activity', + list: { type: 'grid', columns: [{ field: 'subject' }] }, + formViews: { compact: { type: 'simple' } }, + }; + const [obj] = mergeViewsIntoObjects(objects, [container]); + expect(obj.formViews['crm_activity.compact']).toBeTruthy(); + expect(obj.listViews['crm_activity.compact']).toBeUndefined(); + // …and the list family is untouched by the form entry. + expect(Object.keys(obj.listViews)).toEqual(['crm_activity.default']); + }); }); }); diff --git a/packages/app-shell/src/providers/MetadataProvider.tsx b/packages/app-shell/src/providers/MetadataProvider.tsx index 742e91dc8..397aded8d 100644 --- a/packages/app-shell/src/providers/MetadataProvider.tsx +++ b/packages/app-shell/src/providers/MetadataProvider.tsx @@ -6,6 +6,7 @@ import { useMemo, type ReactNode, } from 'react'; +import { expandViewContainer } from '@objectstack/spec/ui'; import { type ObjectStackAdapter } from '@object-ui/data-objectstack'; import { normalizeSchemaReferenceKeys } from '@object-ui/core'; import { resolveInlineMode } from '@object-ui/plugin-form'; @@ -141,10 +142,18 @@ function isNamedItem(item: unknown): item is { name: string } { * (`isAggregatedViewContainer` / `expandViewContainer` in * `@objectstack/spec/ui`), so it is NOT legacy and this branch is NOT dead * code — delete it and stack-packaged views stop reaching the renderer. - * When an object already has expanded ViewItems the container is skipped - * for THAT object, since it restates the same views (and keying both would - * list every view twice — once under its short key, once under its - * canonical `.` name). + * A container is served UNEXPANDED on purpose (the platform's write + * chokepoint states it: "container bodies are left untouched — + * `expandViewContainer` derives identity itself"), so this branch asks the + * composer for each view's identity rather than deriving one of its own + * (objectui#3770). Both gates therefore produce the same canonical + * `.` ids — including the default `list`'s implicit + * `.default` — and the container inherits the composer's folding and + * collision-renaming rules for free. + * When an object already has expanded ViewItems the container is skipped for + * THAT object: it restates the same identities, and the ViewItem rows are + * the authoritative ones (the runtime heals personalization overlays onto + * them). * * Existing `obj.listViews` / `obj.list_views` win to preserve overrides. */ @@ -160,6 +169,37 @@ function isViewItem(view: any): boolean { return !!view && typeof view === 'object' && !!view.viewKind && !!view.object; } +/** + * Route ONE view identity into its object's bucket — shared by both gates: the + * record gate's stored ViewItems and the views `expandViewContainer` materialises + * out of a stack-packaged container. Both shapes are + * `{ name, object, viewKind, label?, isDefault?, config }`, so both are keyed by + * the canonical `.` name the composer owns. + * + * The `config` body is flattened to the legacy NamedListView/FormView shape the + * renderer consumes (type/data/columns/sections at top level); the item-level + * label/isDefault ride along and `name` is stamped with the id so primary-view + * promotion (which matches on `list.name`) finds this entry by its listViews key. + * FORM-family views land in `formViews` only, never in the list-view switcher. + */ +function applyViewItem(bucket: ViewBucket, view: any): void { + const key = view.name || `${view.object}.${view.viewKind}`; + const body = view.config && typeof view.config === 'object' ? view.config : {}; + const entry = { + ...body, + name: key, + label: view.label ?? (body as any).label, + isDefault: !!view.isDefault, + }; + if (view.viewKind === 'form') { + bucket.formViews[key] = entry; + if (view.isDefault || !bucket.form) bucket.form = entry; + } else { + bucket.listViews[key] = entry; + if (view.isDefault) bucket.primary = entry; + } +} + export function mergeViewsIntoObjects(objects: any[], views: any[]): any[] { if (!objects.length || !views.length) return objects; const byObject: Record = {}; @@ -173,23 +213,9 @@ export function mergeViewsIntoObjects(objects: any[], views: any[]): any[] { for (const view of views) { // ── Record gate: independent ViewItem ({ name, object, viewKind, config }) ── if (isViewItem(view)) { - const bucket = (byObject[view.object] ||= { listViews: {}, formViews: {} }); - // Canonical `.` name doubles as the view id, so `/view/` - // URLs resolve directly against the switcher tab ids. - const key = view.name || `${view.object}.${view.viewKind}`; - const body = view.config && typeof view.config === 'object' ? view.config : {}; - // Flatten `config` to the legacy NamedListView/FormView shape the - // renderer consumes (type/data/columns/sections at top level); carry the - // item-level label/isDefault and stamp `name` so primary-view promotion - // (which matches on `list.name`) finds this entry by its listViews key. - const entry = { ...body, name: key, label: view.label ?? (body as any).label, isDefault: !!view.isDefault }; - if (view.viewKind === 'form') { - bucket.formViews[key] = entry; - if (view.isDefault || !bucket.form) bucket.form = entry; - } else { - bucket.listViews[key] = entry; - if (view.isDefault) bucket.primary = entry; - } + // The canonical `.` name doubles as the view id, so + // `/view/` URLs resolve directly against the switcher tab ids. + applyViewItem((byObject[view.object] ||= { listViews: {}, formViews: {} }), view); continue; } // ── Stack gate: aggregated container ({ list, form, listViews, formViews }) ── @@ -198,27 +224,15 @@ export function mergeViewsIntoObjects(objects: any[], views: any[]): any[] { // Expanded ViewItems supersede the bare container for this object. if (hasViewItems.has(objName)) continue; const bucket = (byObject[objName] ||= { listViews: {}, formViews: {} }); - if (view.list) { - // Preserve the primary list view as `obj.list` per @objectstack/spec - // ViewSchema. Also mirror it into `listViews` under its name so legacy - // consumers (that only iterate `listViews`) still see it. Consumers - // honoring `obj.list` (e.g. ObjectView) should dedup by id. - bucket.primary = view.list; - const k = view.list.name || 'list'; - bucket.listViews[k] = view.list; - } - if (view.form) { - bucket.form = view.form; - } - if (view.listViews && typeof view.listViews === 'object') { - for (const [k, v] of Object.entries(view.listViews as Record)) { - bucket.listViews[k] = v; - } - } - if (view.formViews && typeof view.formViews === 'object') { - for (const [k, v] of Object.entries(view.formViews as Record)) { - bucket.formViews[k] = v; - } + // Ask the composer which views this container declares and what each one's + // runtime identity is (objectui#3770) — the default `list` implicitly claims + // `.default`, and a `listViews` entry that merely restates it folds + // into that entry's own name. The primary list keeps arriving on `obj.list` + // per @objectstack/spec ViewSchema (below) AND is mirrored into `listViews` + // under that identity, so consumers that only iterate `listViews` still see + // it and consumers honoring `obj.list` dedup by the same id. + for (const item of expandViewContainer(objName, view)) { + applyViewItem(bucket, item); } } return objects.map(obj => { diff --git a/packages/app-shell/src/utils/viewIdentity.ts b/packages/app-shell/src/utils/viewIdentity.ts new file mode 100644 index 000000000..ebd76b648 --- /dev/null +++ b/packages/app-shell/src/utils/viewIdentity.ts @@ -0,0 +1,48 @@ +/** + * 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. + */ + +import { expandViewContainer } from '@objectstack/spec/ui'; + +/** + * Runtime identity of an aggregated container's DEFAULT `list` view — asked of + * the spec's view composer, never spelled out here (objectui#3770). + * + * A `defineView` container declares its default list under the `list` key. That + * key is a SLOT in the authoring document, not the view's identity: the composer + * (`expandViewContainer`, the same function the framework's loader and the i18n + * extractor call) registers an unnamed default list as `.default`, and a + * named one as `.`. The renderer used to derive `list.name || + * 'list'` instead — a third spelling that no producer emits, so the default + * list's translation key (`objects.._views.default.label`, canonical per + * objectstack#5164 ruling A) could never be reached and the label fell back to + * English. Deriving the identity from the composer keeps this consumer on the one + * spelling and inherits its rules (implicit `default`, author-supplied `name`, + * collision renaming) instead of restating them. + * + * Call sites that hold the whole container (`MetadataProvider`) call + * `expandViewContainer` directly and get every view's identity, folding + * included. This helper is for the call sites that hold only the default list + * body (`ObjectView`, reading `objectDef.list`). + * + * @param objectName - The bound object's name, as the runtime presents it. + * @param list - The container's default `list` body, or a merged entry derived + * from it (already carrying the composer's qualified `name`). + * @returns The qualified `.` view id, or `undefined` when `list` is + * not a view body. + */ +export function defaultListViewId(objectName: string, list: unknown): string | undefined { + if (!list || typeof list !== 'object') return undefined; + const declared = (list as { name?: unknown }).name; + // Already the composer's qualified identity — `MetadataProvider` stamps it + // onto every merged entry, and re-expanding it would double the prefix + // (`crm_lead.crm_lead.default`). + if (typeof declared === 'string' && declared.startsWith(`${objectName}.`)) { + return declared; + } + return expandViewContainer(objectName, { list })[0]?.name; +} diff --git a/packages/app-shell/src/views/ObjectView.defaultViewIdentity.test.tsx b/packages/app-shell/src/views/ObjectView.defaultViewIdentity.test.tsx new file mode 100644 index 000000000..52bd0c650 --- /dev/null +++ b/packages/app-shell/src/views/ObjectView.defaultViewIdentity.test.tsx @@ -0,0 +1,277 @@ +/** + * 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 default list view's translation key resolves end-to-end (objectui#3770). + * + * objectstack#5164 ruling A settled ONE spelling for `_views` translation keys: + * the runtime view identity's bare name. The i18n extractor derives it from the + * view composer (objectstack#6124) and `packages/lint` enforces it + * (objectstack#6038), so a container's default `list` — declared without a + * `name` — is published as `objects.._views.default.label`. + * + * This renderer derived `list.name || 'list'` instead: a third spelling no + * producer emits. The default list's tab label therefore probed + * `_views.list.label`, missed, and fell back to the English metadata label, + * while every named view resolved fine. Neither repo had a test over this seam, + * which is why the rename landed green on both sides. + * + * What is pinned here is the whole chain a default-list-only object travels: + * + * server TranslationData (keyed `_views.default`) + * → transformSpecTranslations (the console's loadLanguage transform) + * defineView container + * → mergeViewsIntoObjects (the id + `name` the renderer carries) + * → defaultListViewId (the id ObjectView promotes / persists by) + * → useObjectLabel().viewLabel(objectName, view.name || view.id, fallback) + * + * The last link is the one-line expression ObjectView passes at its two call + * sites (`view.name || view.id`); everything before it is the real function. + * + * REVERSE VERIFICATION — both directions predicted before running, and observed: + * restoring the retired derivation (`primary.name || 'list'` in ObjectView plus + * `bucket.listViews[view.list.name || 'list']` in MetadataProvider) turns the + * `_views.default` chain RED (`expected 'list' to be 'showcase_contact.default'`) + * — and turns the negative assertion red in the OPPOSITE direction: the dead key + * starts resolving (`expected '不该命中' to be 'All Contacts'`). The record-gate + * case below stays GREEN under that revert, which is the point — that gate was + * already correct and this change must not move it. Asserting both directions + * means a partial revert of either call site cannot stay green. + */ + +import { describe, it, expect } from 'vitest'; +import * as React from 'react'; +import { renderHook } from '@testing-library/react'; +import { + I18nProvider, + useObjectTranslation, + useObjectLabel, + isSpecTranslationData, + transformSpecTranslations, +} from '@object-ui/i18n'; +import { ViewItemNameSchema } from '@objectstack/spec/ui'; +import { mergeViewsIntoObjects } from '../providers/MetadataProvider'; +import { defaultListViewId } from '../utils/viewIdentity'; + +/** A showcase-shaped object whose ONLY list view is the container's default. */ +const OBJECT = { name: 'showcase_contact', label: 'Contact', fields: { name: { type: 'text' } } }; + +/** + * The container as `defineView` emits it (examples/app-showcase + * `ui/views/contact.view.ts`): a default `list` with no `name`, plus one named + * secondary view so the "named views are unaffected" leg rides the same fixture. + */ +const CONTAINER = { + name: 'showcase_contact', + list: { + label: 'All Contacts', + type: 'grid', + data: { provider: 'object', object: 'showcase_contact' }, + columns: [{ field: 'name' }], + }, + listViews: { + recent: { label: 'Recently Added', type: 'grid', columns: [{ field: 'name' }] }, + }, +}; + +/** + * The zh-CN payload as `/api/v1/i18n/translations/:locale` serves it after + * objectstack#6124 migrated the bundle — the default list keyed `default`. + */ +const ZH_PAYLOAD = { + objects: { + showcase_contact: { + label: '联系人', + fields: { name: { label: '姓名' } }, + _views: { + default: { + label: '联系人', + description: '全部联系人记录', + emptyState: { title: '暂无联系人', message: '新建一个联系人开始。' }, + }, + recent: { label: '最近添加' }, + }, + }, + }, +}; + +/** Same payload authored against the retired spelling — must NOT resolve. */ +const ZH_PAYLOAD_RETIRED_KEY = { + objects: { + showcase_contact: { + fields: { name: { label: '姓名' } }, + _views: { list: { label: '不该命中' } }, + }, + }, +}; + +const wrapper = ({ children }: { children: React.ReactNode }) => + React.createElement( + I18nProvider, + { config: { defaultLanguage: 'zh', detectBrowserLanguage: false } }, + children, + ); + +/** Mount the real resolver over a server payload, exactly as the console does. */ +function withServerBundle(payload: Record) { + expect(isSpecTranslationData(payload)).toBe(true); + const { result } = renderHook( + () => ({ labels: useObjectLabel(), i18n: useObjectTranslation().i18n }), + { wrapper }, + ); + result.current.i18n.addResourceBundle( + 'zh', + 'translation', + transformSpecTranslations(payload as never), + true, + true, + ); + return result.current.labels; +} + +/** The merged object definition an ObjectView receives for this container. */ +function mergedObject(container: Record) { + const [obj] = mergeViewsIntoObjects([{ ...OBJECT }], [container]); + return obj as any; +} + +/** + * One switcher tab, built the way ObjectView's `views` memo builds it: + * `{ id: , ...entry }` (the spread cannot clobber `id`, and an + * entry with no `name` of its own therefore falls back to the key). + */ +const viewTab = (obj: any, id: string) => ({ id, ...obj.listViews[id] }); + +/** Exactly what ObjectView hands `viewLabel` for a tab. */ +const translationArg = (view: any): string => view.name || view.id; + +describe('default list view identity → _views translation key (objectui#3770)', () => { + it('resolves the default list label/description/emptyState under `_views.default`', () => { + const labels = withServerBundle(ZH_PAYLOAD); + const obj = mergedObject(CONTAINER); + const primaryId = defaultListViewId(obj.name, obj.list)!; + + expect(primaryId).toBe('showcase_contact.default'); + expect(obj.listViews[primaryId]).toBeTruthy(); + const entry = viewTab(obj, primaryId); + + expect(labels.viewLabel(obj.name, translationArg(entry), entry.label)).toBe('联系人'); + expect(labels.viewDescription(obj.name, translationArg(entry), undefined)).toBe( + '全部联系人记录', + ); + expect( + labels.viewEmptyState(obj.name, translationArg(entry), { + title: 'No contacts', + message: 'Create one to begin.', + }), + ).toMatchObject({ title: '暂无联系人', message: '新建一个联系人开始。' }); + }); + + it('does NOT resolve the retired `_views.list` spelling', () => { + // The dialect this issue removed. A bundle authored against it must miss and + // fall back to the metadata label — same as any other unknown key — so the + // authoring mistake stays visible instead of half-working in this one client. + const labels = withServerBundle(ZH_PAYLOAD_RETIRED_KEY); + const obj = mergedObject(CONTAINER); + const entry = viewTab(obj, defaultListViewId(obj.name, obj.list)!); + expect(labels.viewLabel(obj.name, translationArg(entry), entry.label)).toBe('All Contacts'); + }); + + it('leaves named views resolving by their own bare key', () => { + const labels = withServerBundle(ZH_PAYLOAD); + const obj = mergedObject(CONTAINER); + expect(obj.listViews['showcase_contact.recent']).toBeTruthy(); + const recent = viewTab(obj, 'showcase_contact.recent'); + expect(labels.viewLabel(obj.name, translationArg(recent), recent.label)).toBe('最近添加'); + }); + + it('keeps a named default list on its author-supplied key', () => { + const labels = withServerBundle({ + objects: { + showcase_contact: { + fields: { name: { label: '姓名' } }, + _views: { my_list: { label: '我的列表' } }, + }, + }, + }); + const obj = mergedObject({ + name: 'showcase_contact', + list: { name: 'my_list', label: 'My List', type: 'grid', columns: [{ field: 'name' }] }, + }); + const primaryId = defaultListViewId(obj.name, obj.list)!; + expect(primaryId).toBe('showcase_contact.my_list'); + expect( + labels.viewLabel(obj.name, translationArg(viewTab(obj, primaryId)), 'My List'), + ).toBe('我的列表'); + }); + + it('resolves the same key for the record gate, whose ids were already correct', () => { + // The framework registers a container AND its expansion, so this shape is + // what a real backend serves. It resolved before this fix too — asserted so + // the two gates are pinned to ONE spelling rather than drifting apart again. + const labels = withServerBundle(ZH_PAYLOAD); + const [obj] = mergeViewsIntoObjects([{ ...OBJECT }], [ + { + name: 'showcase_contact.default', + object: 'showcase_contact', + viewKind: 'list', + isDefault: true, + label: 'All Contacts', + config: { type: 'grid', columns: [{ field: 'name' }] }, + }, + ]); + const entry = viewTab(obj, 'showcase_contact.default'); + expect(labels.viewLabel(obj.name, translationArg(entry), entry.label)).toBe('联系人'); + }); +}); + +/** + * Stored-data surface, measured before the behaviour change and pinned here. + * + * `ObjectView.persistViewPatch` writes a view override by VIEW ID: + * `dataSource.updateViewConfig(objectName, view.id, cfg)` → + * `client.meta.saveItem('view', view.id, { …cfg, object, name: view.id })`, i.e. + * one `sys_metadata` row of type `view` NAMED by the id. The id therefore doubles + * as the persistence key, and `ViewItemNameSchema` is the spec's judgment on + * which strings are legal view identities: + * + * /^[a-z][a-z0-9_]*(\.[a-z][a-z0-9_]*)+$/ + * + * The retired `'list'` derivation is not one of them — bare, no object prefix — + * so it never had a legitimate stored row to migrate to the new id; the composer + * identity is representable. (`defaultViewId` is derived per render from + * `views.find(v => v.isDefault)` and never persisted, so it holds no stored + * state of its own.) + */ +describe('the derived id is a representable view identity (persistence key)', () => { + const legal = (id: string) => ViewItemNameSchema.safeParse(id).success; + + it('accepts the composer identity and rejects the retired `list` spelling', () => { + const obj = mergedObject(CONTAINER); + expect(legal(defaultListViewId(obj.name, obj.list)!)).toBe(true); + expect(legal('list')).toBe(false); + }); + + it('is stable under re-derivation, so a saved override is never re-keyed', () => { + // ObjectView derives the id on every render (overrides effect + view list), + // and the merged entry it re-reads already carries the qualified name. + // Feeding that back in must yield the SAME id — otherwise each pass would + // persist under a fresh name and orphan the previous one. + const obj = mergedObject(CONTAINER); + const first = defaultListViewId(obj.name, obj.list)!; + expect(defaultListViewId(obj.name, obj.listViews[first])).toBe(first); + expect(defaultListViewId(obj.name, { name: first })).toBe(first); + }); + + it('returns undefined when the object declares no default list', () => { + // No `objectDef.list` → no primary promotion and no override id, instead of + // a synthesized one that would collide with a real view. + expect(defaultListViewId('showcase_contact', undefined)).toBeUndefined(); + expect(defaultListViewId('showcase_contact', null)).toBeUndefined(); + }); +}); diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index dc8f15b5a..436f56295 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -63,6 +63,7 @@ import { importTargetFields } from './importTargetFields'; import { useExpressionContext } from '../providers/ExpressionProvider'; import { resolveManagedByEmptyState } from '../utils/managedByEmptyState'; import { resolveViewId } from '../utils/resolveViewId'; +import { defaultListViewId } from '../utils/viewIdentity'; import { warnSuppressedListNav } from '../utils/warnSuppressedListNav'; import { useObjectActions } from '../hooks/useObjectActions'; import { useObjectTranslation, useObjectLabel } from '@object-ui/i18n'; @@ -556,12 +557,11 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an } const definedViews = (objectDef.listViews || objectDef.list_views || {}) as Record; const ids = Object.keys(definedViews); - // Include the primary view id so overrides apply to it too. - const primary = (objectDef as any).list; - if (primary && typeof primary === 'object') { - const primaryId = primary.name || 'list'; - if (!ids.includes(primaryId)) ids.unshift(primaryId); - } + // Include the primary view id so overrides apply to it too. Its identity + // comes from the view composer (objectui#3770) — the same id the override + // was persisted under, since `persistViewPatch` writes by view id. + const primaryId = defaultListViewId(objectDef.name, (objectDef as any).list); + if (primaryId && !ids.includes(primaryId)) ids.unshift(primaryId); if (ids.length === 0) { setViewOverrides({}); return; @@ -635,9 +635,14 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an // ViewSchema). MetadataProvider mirrors it into `listViews` so it's // already in `viewList` above; promote it to the front and mark it as // the default so `defaultViewId` picks it over secondary listViews. + // + // Its id is the composer's runtime identity (objectui#3770), which is + // what the tab label / description / emptyState are translated under: + // `viewLabel` reads `objects.._views..label`, and for a + // default list declared without a `name` that bare key is `default`. const primary = (objectDef as any).list; - if (primary && typeof primary === 'object') { - const primaryId = primary.name || 'list'; + const primaryId = defaultListViewId(objectDef.name, primary); + if (primaryId) { const idx = viewList.findIndex(v => v.id === primaryId); if (idx >= 0) { const [entry] = viewList.splice(idx, 1);