From e4b532b7dd8e3463f96092abfc1a036c8b0e2452 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 21:44:56 +0000 Subject: [PATCH] fix(app-shell): a report's or dataset's Label with no machine name leaves the Name empty (objectui#12789) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The report and dataset designers derived the Name from the Label with `toFieldName`, whose empty result is the placeholder `field`. A Label in a non-Latin script (`收入`), or a cleared Label, therefore named the item `field` silently. Both now derive with `toFieldNameOrEmpty`, the same strict rule with no placeholder, beside `toFieldName` in `previews/object-fields-io.ts`; the Name stays empty, as the object designer's does, and the save path asks for one. `nameFollowsLabel` compares against the same function. `toFieldName` keeps its `field` fallback for its other callers. Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude --- ...-report-dataset-label-leaves-name-empty.md | 9 ++ ...pector.labelLeavesNameEmpty-12789.test.tsx | 113 ++++++++++++++++++ .../inspectors/DatasetDefaultInspector.tsx | 16 ++- ...pector.labelLeavesNameEmpty-12789.test.tsx | 110 +++++++++++++++++ .../inspectors/ReportDefaultInspector.tsx | 14 ++- .../previews/object-fields-io.test.ts | 18 +++ .../previews/object-fields-io.ts | 23 +++- 7 files changed, 294 insertions(+), 9 deletions(-) create mode 100644 .changeset/12789-report-dataset-label-leaves-name-empty.md create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.labelLeavesNameEmpty-12789.test.tsx create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.labelLeavesNameEmpty-12789.test.tsx diff --git a/.changeset/12789-report-dataset-label-leaves-name-empty.md b/.changeset/12789-report-dataset-label-leaves-name-empty.md new file mode 100644 index 0000000000..66f927c29a --- /dev/null +++ b/.changeset/12789-report-dataset-label-leaves-name-empty.md @@ -0,0 +1,9 @@ +--- +'@object-ui/app-shell': patch +--- + +A new report's or dataset's Label in a non-Latin script no longer names it `field` (objectui#12789). The report and dataset designers derive the Name from the Label until you type a Name yourself. When the Label held no Latin letters or digits (for example `收入`), or was cleared, that derivation wrote the placeholder `field`, so the item was silently named `field` and a second one collided with the first. + +Such a Label now leaves the Name empty, as the object designer already does, and saving asks you for a name ("A name is required."). A Name left empty this way still follows a Latin Label typed later. A Label that yields a name derives exactly what it derived before. An existing report's or dataset's name is unchanged and still not editable. + +Nothing is added to or removed from the package entry: no export, prop or type changes. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.labelLeavesNameEmpty-12789.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.labelLeavesNameEmpty-12789.test.tsx new file mode 100644 index 0000000000..5cd532dc76 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.labelLeavesNameEmpty-12789.test.tsx @@ -0,0 +1,113 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#12789 — in create mode, a dataset Label that yields no machine name + * leaves the Name empty, as the object designer does. + * + * The same defect, and the same repair, as the report designer's Label + * (`ReportDefaultInspector.labelLeavesNameEmpty-12789.test.tsx`): the Label + * handler derived the Name with `toFieldName`, whose empty result is the + * placeholder `field`, so a Label in a non-Latin script (`收入`), or a cleared + * Label, named the dataset `field` without a word. It now derives with + * `toFieldNameOrEmpty`, the same strict rule with no placeholder, and the + * comparison that decides whether the Name still follows the Label uses the + * same function. + * + * The inspector is rendered with a host double that merges every patch back + * into the draft, so each keystroke sees the value the last one committed, as + * in the designer. Keystrokes are real (user-event). + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +// The catalog hooks answer empty, so the inspector renders without a +// MetadataClient or a network; the Name and Label boxes need neither. +vi.mock('./useDatasetFields', () => ({ + useObjectOptions: () => ({ options: [], loading: false }), + useDatasetFieldCatalog: () => ({ relationships: [], fieldOptions: [], loading: false }), + useDatasetUsage: () => ({ reports: 0, dashboards: 0, loading: false }), + fieldTypeToDimensionType: (t: string) => (t === 'date' ? 'date' : 'string'), +})); + +import { DatasetDefaultInspector } from './DatasetDefaultInspector'; + +afterEach(cleanup); + +/** The create-mode designer: an empty `name`, and every patch merged back. */ +function CreateHost() { + const [draft, setDraft] = React.useState>({ dimensions: [], measures: [] }); + return ( + <> + setDraft((d) => ({ ...d, ...patch }))} + readOnly={false} + locale="en-US" + /> + {typeof draft.name === 'string' ? draft.name : '(absent)'} + + ); +} + +const nameBox = () => screen.getByLabelText('Name', { exact: true }) as HTMLInputElement; +const labelBox = () => screen.getByLabelText('Label', { exact: true }) as HTMLInputElement; +const draftName = () => screen.getByTestId('draft-name').textContent; + +describe('dataset designer create mode — a Label with no machine name leaves the Name empty (objectui#12789)', () => { + it('the Label 收入 leaves the Name empty, in the box and in the draft', async () => { + const user = userEvent.setup(); + render(); + await user.type(labelBox(), '收入'); + expect(labelBox()).toHaveValue('收入'); + expect(nameBox()).toHaveValue(''); + expect(draftName()).toBe(''); + }); + + it('a cleared Label leaves the Name empty', async () => { + const user = userEvent.setup(); + render(); + await user.type(labelBox(), 'Revenue'); + expect(nameBox()).toHaveValue('revenue'); + await user.clear(labelBox()); + expect(nameBox()).toHaveValue(''); + expect(draftName()).toBe(''); + }); + + it('a Name left empty by a non-Latin Label still follows a Latin Label typed later', async () => { + const user = userEvent.setup(); + render(); + await user.type(labelBox(), '收入'); + expect(nameBox()).toHaveValue(''); + await user.clear(labelBox()); + await user.type(labelBox(), 'Revenue Board'); + expect(nameBox()).toHaveValue('revenue_board'); + }); + + it('a Name the author typed stays theirs while the Label yields no machine name, even when it reads field', async () => { + const user = userEvent.setup(); + render(); + await user.type(labelBox(), '收入'); + await user.clear(nameBox()); + await user.type(nameBox(), 'field'); + await user.clear(labelBox()); + await user.type(labelBox(), 'Revenue'); + expect(nameBox()).toHaveValue('field'); + expect(draftName()).toBe('field'); + }); + + it('a Latin Label still derives the strict form: the f_ prefix, and field from the Label Field', async () => { + const user = userEvent.setup(); + render(); + await user.type(labelBox(), '2024 Revenue'); + expect(nameBox()).toHaveValue('f_2024_revenue'); + await user.clear(labelBox()); + await user.type(labelBox(), 'Field'); + expect(nameBox()).toHaveValue('field'); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx index 2810e6ea3b..0ad28a9d29 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx @@ -32,7 +32,7 @@ import { spliceArray, } from './_shared.js'; import { InspectorComboField, type InspectorComboOption } from './InspectorComboField.js'; -import { toFieldName, toFieldNameLoose } from '../previews/object-fields-io.js'; +import { toFieldNameLoose, toFieldNameOrEmpty } from '../previews/object-fields-io.js'; import { formatMeasure } from '@object-ui/core'; import { useDisplayLocale } from '@object-ui/i18n'; import { conditionToGroup, groupToCondition, isClearedGroup, type BuilderGroup, type FilterCondition } from './datasetFilterCondition.js'; @@ -396,7 +396,11 @@ export function DatasetDefaultInspector({ draft, onPatch, readOnly, name, locale // here was reset whenever the host remounted this panel (a Properties → // Source → Properties round trip does), and the next label keystroke then // replaced the name the author had typed. - const nameFollowsLabel = createMode && (nameValue === '' || nameValue === toFieldName(label)); + // objectui#12789 — the comparison uses the same derivation as the Label + // handler, so a Name left empty by a non-Latin Label still follows a later + // Latin one, and a Name the author typed is never mistaken for the `field` + // placeholder. + const nameFollowsLabel = createMode && (nameValue === '' || nameValue === toFieldNameOrEmpty(label)); const { options: objectOptions, loading: objectsLoading } = useObjectOptions(); const { relationships, fieldOptions, loading: catalogLoading } = useDatasetFieldCatalog(object, include); @@ -517,9 +521,13 @@ export function DatasetDefaultInspector({ draft, onPatch, readOnly, name, locale value={label} onCommit={(v) => { // Live-derive the snake_case name from the label until the author edits - // the Name field directly (create mode only). + // the Name field directly (create mode only). objectui#12789 — a Label + // that yields no machine name (a non-Latin script, or a cleared Label) + // leaves the Name empty, as the object designer does, so the save + // path asks the author for one; it never writes the `field` + // placeholder. const patch: Record = { label: v }; - if (nameFollowsLabel) patch.name = toFieldName(v); + if (nameFollowsLabel) patch.name = toFieldNameOrEmpty(v); onPatch(patch); }} disabled={readOnly} diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.labelLeavesNameEmpty-12789.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.labelLeavesNameEmpty-12789.test.tsx new file mode 100644 index 0000000000..85b3d62e4d --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.labelLeavesNameEmpty-12789.test.tsx @@ -0,0 +1,110 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#12789 — in create mode, a report Label that yields no machine name + * leaves the Name empty, as the object designer does. + * + * The Label handler derived the Name with `toFieldName`, which writes the + * placeholder `field` when its result is empty. So a Label in a non-Latin + * script (`收入`), or a cleared Label, named the report `field` without a word, + * and a second such report collided with the first. The derivation now uses + * `toFieldNameOrEmpty`, the same strict rule with no placeholder: the Name + * stays empty and the save path asks the author for one. The comparison that + * decides whether the Name still follows the Label uses the same function. + * + * The inspector is rendered with a host double that merges every patch back + * into the draft, so each keystroke sees the value the last one committed, as + * in the designer. Keystrokes are real (user-event). + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { ReportDefaultInspector } from './ReportDefaultInspector'; + +afterEach(cleanup); + +// One fetch double at module scope, never torn down: the inspector's own side +// reads must not reach a socket after a test body returns. +vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('null', { status: 404, headers: { 'content-type': 'application/json' } })), +); + +/** The create-mode designer: an empty `name`, and every patch merged back. */ +function CreateHost() { + const [draft, setDraft] = React.useState>({ type: 'summary' }); + return ( + <> + setDraft((d) => ({ ...d, ...patch }))} + readOnly={false} + locale="en-US" + datasetCatalogOverride={[]} + /> + {typeof draft.name === 'string' ? draft.name : '(absent)'} + + ); +} + +const nameBox = () => screen.getByLabelText('Name', { exact: true }) as HTMLInputElement; +const labelBox = () => screen.getByLabelText('Label', { exact: true }) as HTMLInputElement; +const draftName = () => screen.getByTestId('draft-name').textContent; + +describe('report designer create mode — a Label with no machine name leaves the Name empty (objectui#12789)', () => { + it('the Label 收入 leaves the Name empty, in the box and in the draft', async () => { + const user = userEvent.setup(); + render(); + await user.type(labelBox(), '收入'); + expect(labelBox()).toHaveValue('收入'); + expect(nameBox()).toHaveValue(''); + expect(draftName()).toBe(''); + }); + + it('a cleared Label leaves the Name empty', async () => { + const user = userEvent.setup(); + render(); + await user.type(labelBox(), 'Revenue'); + expect(nameBox()).toHaveValue('revenue'); + await user.clear(labelBox()); + expect(nameBox()).toHaveValue(''); + expect(draftName()).toBe(''); + }); + + it('a Name left empty by a non-Latin Label still follows a Latin Label typed later', async () => { + const user = userEvent.setup(); + render(); + await user.type(labelBox(), '收入'); + expect(nameBox()).toHaveValue(''); + await user.clear(labelBox()); + await user.type(labelBox(), 'Revenue Board'); + expect(nameBox()).toHaveValue('revenue_board'); + }); + + it('a Name the author typed stays theirs while the Label yields no machine name, even when it reads field', async () => { + const user = userEvent.setup(); + render(); + await user.type(labelBox(), '收入'); + await user.clear(nameBox()); + await user.type(nameBox(), 'field'); + await user.clear(labelBox()); + await user.type(labelBox(), 'Revenue'); + expect(nameBox()).toHaveValue('field'); + expect(draftName()).toBe('field'); + }); + + it('a Latin Label still derives the strict form: the f_ prefix, and field from the Label Field', async () => { + const user = userEvent.setup(); + render(); + await user.type(labelBox(), '2024 Revenue'); + expect(nameBox()).toHaveValue('f_2024_revenue'); + await user.clear(labelBox()); + await user.type(labelBox(), 'Field'); + expect(nameBox()).toHaveValue('field'); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.tsx index 2b1f5e084f..00bc25f1ae 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.tsx @@ -40,7 +40,7 @@ import { rosterFrom, } from './_shared.js'; import { AddFieldPopover, FieldListRow } from '../previews/ViewColumnPanes.js'; -import { toFieldName, toFieldNameLoose } from '../previews/object-fields-io.js'; +import { toFieldNameLoose, toFieldNameOrEmpty } from '../previews/object-fields-io.js'; import type { MetadataDefaultInspectorProps } from '../default-inspector-registry.js'; import { SchemaForm } from '../SchemaForm.js'; import type { ObjectFieldInfo } from '../previews/useObjectFields.js'; @@ -339,7 +339,11 @@ export function ReportDefaultInspector({ // here was reset whenever the host remounted this panel (a Properties → // Source → Properties round trip does), and the next label keystroke then // replaced the name the author had typed. - const nameFollowsLabel = createMode && (nameValue === '' || nameValue === toFieldName(labelValue)); + // objectui#12789 — the comparison uses the same derivation as the Label + // handler below, so a Name left empty by a non-Latin Label still follows a + // later Latin one, and a Name the author typed is never mistaken for the + // `field` placeholder. + const nameFollowsLabel = createMode && (nameValue === '' || nameValue === toFieldNameOrEmpty(labelValue)); const datasetName = typeof draft.dataset === 'string' ? (draft.dataset as string) : ''; const values = React.useMemo(() => readNames(draft.values), [draft.values]); @@ -507,7 +511,11 @@ export function ReportDefaultInspector({ // Live-derive the snake_case name from the label until the author // edits the Name field directly (create mode only). `v` is the plain // string the author just typed, which is what the slug must read. - if (nameFollowsLabel) patch.name = toFieldName(v); + // objectui#12789 — a Label that yields no machine name (a non-Latin + // script, or a cleared Label) leaves the Name empty, as the object + // designer does, so the save path asks the author for one; it never + // writes the `field` placeholder. + if (nameFollowsLabel) patch.name = toFieldNameOrEmpty(v); onPatch(patch); }} placeholder={tr('engine.inspector.report.labelPlaceholder')} diff --git a/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.test.ts b/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.test.ts index bab8e6eb23..5d923a10cd 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.test.ts +++ b/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.test.ts @@ -8,6 +8,7 @@ import { writeFields, toFieldName, toFieldNameLoose, + toFieldNameOrEmpty, readGroups, genGroupKey, addGroup, @@ -337,6 +338,23 @@ describe('toFieldName (strict — for complete strings)', () => { }); }); +describe('toFieldNameOrEmpty (strict, no placeholder — for a Name derived from a Label) (objectui#12789)', () => { + it('returns "" where toFieldName writes the "field" placeholder', () => { + for (const raw of ['', ' ', '收入', '报修工单', '-- __ --']) { + expect(toFieldName(raw)).toBe('field'); + expect(toFieldNameOrEmpty(raw)).toBe(''); + } + }); + + it('derives exactly what toFieldName derives from every input that yields a machine name', () => { + for (const raw of ['Repair Ticket', 'order-item', 'report_', 'a__b', '1abc', 'Revenue -- Board ', 'Field', '收入 Revenue']) { + expect(toFieldNameOrEmpty(raw)).toBe(toFieldName(raw)); + } + expect(toFieldNameOrEmpty('Field')).toBe('field'); // a real name, not the placeholder + expect(toFieldNameOrEmpty('1abc')).toBe('f_1abc'); + }); +}); + describe('toFieldNameLoose (prefix-stable — for live keystroke input)', () => { it('keeps a trailing underscore so mid-word "_" survives typing', () => { expect(toFieldNameLoose('repair_')).toBe('repair_'); diff --git a/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.ts b/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.ts index c602fa7d83..830296834b 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.ts +++ b/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.ts @@ -259,14 +259,33 @@ export function toLabel(name: string): string { .join(' '); } -/** Normalize an arbitrary string into a valid snake_case field name. */ +/** + * Normalize an arbitrary string into a valid snake_case field name. Input that + * yields no machine name (empty, or only non-Latin characters) becomes the + * placeholder `field`; {@link toFieldNameOrEmpty} is the same rule without it. + */ export function toFieldName(raw: string): string { + return toFieldNameOrEmpty(raw) || 'field'; +} + +/** + * The strict {@link toFieldName} rule with no placeholder: `''` when the input + * yields no machine name — a Label in a non-Latin script, or a cleared one. + * + * For a Name DERIVED from a Label (objectui#12789). The report and dataset + * designers derived with `toFieldName`, so a Label like `收入` silently named + * the item `field`, and a second such item collided with the first. An empty + * derivation leaves the Name empty instead, as the object designer's `slugify` + * does, so the save path asks the author for one. Every other input derives + * exactly what `toFieldName` derives. + */ +export function toFieldNameOrEmpty(raw: string): string { const lower = raw.trim().toLowerCase(); const sanitized = lower .replace(/[^a-z0-9]+/g, '_') .replace(/^_+|_+$/g, '') .replace(/_{2,}/g, '_'); - if (!sanitized) return 'field'; + if (!sanitized) return ''; if (!/^[a-z_]/.test(sanitized)) return `f_${sanitized}`; return sanitized; }