diff --git a/.changeset/detail-authored-type-narrow-only-editability.md b/.changeset/detail-authored-type-narrow-only-editability.md new file mode 100644 index 000000000..fd21ed48a --- /dev/null +++ b/.changeset/detail-authored-type-narrow-only-editability.md @@ -0,0 +1,17 @@ +--- +'@object-ui/plugin-detail': patch +--- + +Behavior change — **an authored display `type` can NARROW inline editability, but never WIDEN it** (objectui#3355). + +Both detail-surface editability gates (`HeaderHighlight`, the `record:highlights` strip; `DetailSection`, the details body) used to resolve ONE effective type with display precedence — `viewFieldType || objectFieldType`. An authored non-computed `type` therefore ERASED the object's `formula` / `summary` / `rollup` / `auto_number` declaration from the gate's view, and a machine-owned column became inline-editable. + +The gate now reads the two types separately and takes their UNION: a field is non-editable if the authored entry type **or** the object field's type is computed. Renderer/editor selection keeps the old precedence, so nothing about the display changes — only who may write. + +What flips: + +- `{ name: 'supply_share', type: 'number' }` authored over an object field declared `rollup` (or `formula` / `summary` / `auto_number`) — a display override written to fix formatting — no longer offers a pencil / double-click editor. This is the shipped configuration behind objectstack-ai/objectstack#5077: a hook-maintained rollup was overwritten by hand from the header strip and stayed corrupted until an unrelated child-row touch re-fired it (downstream yinlianghui/hotcrm-heimao#61). +- Narrowing is unchanged: an authored `type: 'formula'` still locks a plain object column. +- Fields with no authored computed type over a plain object column stay editable, and the entry-level `readonly` declaration from objectui#3356 is still honored. + +The object schema is authoritative about what is machine-computed; a presentation override has no business granting write access. The rule now lives in ONE shared helper, `isComputedFieldType` in `fieldEnrichment.ts` — beside `enrichDetailField`, the module both hosts already share — with the computed-type set moved there too (still re-exported as `TEXTUAL_REF_FALLBACK_TYPES`), so the strip and the body cannot drift apart again. diff --git a/packages/plugin-detail/src/DetailSection.tsx b/packages/plugin-detail/src/DetailSection.tsx index b97433e18..dd33e857e 100644 --- a/packages/plugin-detail/src/DetailSection.tsx +++ b/packages/plugin-detail/src/DetailSection.tsx @@ -33,8 +33,8 @@ import { useDetailTranslation } from './useDetailTranslation'; import { useSafeFieldLabel } from '@object-ui/react'; import { PermissionFacetLink } from './renderers/PermissionFacetLink'; import { NON_EDITABLE_SYSTEM_FIELDS } from './systemFields'; -import { InlineFieldInput, TEXTUAL_REF_FALLBACK_TYPES } from './InlineFieldInput'; -import { enrichDetailField } from './fieldEnrichment'; +import { InlineFieldInput } from './InlineFieldInput'; +import { enrichDetailField, isComputedFieldType } from './fieldEnrichment'; /** * Section-header icon. `fieldGroups[].icon` declares a Lucide name (spec), @@ -224,8 +224,12 @@ export const DetailSection: React.FC = ({ // fields explicitly flagged `readonly` are never editable. `onEnterInlineEdit` // is only threaded when the record itself is inline-editable, so its presence // carries the object-lifecycle + permission gate. - const inlineEditType = enrichedField.type || field.type; - const isComputedField = TEXTUAL_REF_FALLBACK_TYPES.has(inlineEditType as string); + // Read the authored view type and the object type SEPARATELY — the gate is + // the UNION of the two, so an authored display `type` (which still drives + // renderer/editor selection through `enrichedField.type`) can narrow + // editability but never widen it (objectui#3355). Shared with + // HeaderHighlight via `isComputedFieldType` so the two can't drift. + const isComputedField = isComputedFieldType(field.type, objectDefField?.type); // Honor the OBJECT metadata's read-only flag too — the enrichment above // intentionally doesn't copy `readonly` into enrichedField, so read it // straight off objectDefField (covers formula / non-updateable fields the diff --git a/packages/plugin-detail/src/HeaderHighlight.tsx b/packages/plugin-detail/src/HeaderHighlight.tsx index 5b0852d77..b9f3a8036 100644 --- a/packages/plugin-detail/src/HeaderHighlight.tsx +++ b/packages/plugin-detail/src/HeaderHighlight.tsx @@ -19,8 +19,8 @@ import type { HighlightField } from '@object-ui/types'; import { getCellRenderer, resolveCellRendererType } from '@object-ui/fields'; import { useSafeFieldLabel, useInlineEdit } from '@object-ui/react'; import { Check, X, Pencil } from 'lucide-react'; -import { InlineFieldInput, TEXTUAL_REF_FALLBACK_TYPES } from './InlineFieldInput'; -import { enrichDetailField } from './fieldEnrichment'; +import { InlineFieldInput } from './InlineFieldInput'; +import { enrichDetailField, isComputedFieldType } from './fieldEnrichment'; import { NON_EDITABLE_SYSTEM_FIELDS } from './systemFields'; import { useDetailTranslation } from './useDetailTranslation'; @@ -98,11 +98,17 @@ export const HeaderHighlight: React.FC = ({ const draftVal = inline?.draft?.[field.name]; const value = draftVal !== undefined ? draftVal : rawValue; - // Field-level editability gate — mirrors DetailSection so the strip - // and the body agree on which highlights are editable. Computed - // types (formula/summary/rollup/auto_number), `readonly` (view OR - // object metadata), and immutable system/audit fields never edit. - const isComputed = TEXTUAL_REF_FALLBACK_TYPES.has(resolvedType as string); + // Field-level editability gate — shares `isComputedFieldType` with + // DetailSection so the strip and the body agree on which highlights + // are editable. Computed types (formula/summary/rollup/auto_number), + // `readonly` (view OR object metadata), and immutable system/audit + // fields never edit. + // + // The gate takes the authored type and the object type SEPARATELY, + // not `resolvedType` — an authored display `type` can narrow + // editability but never widen it (objectui#3355). `resolvedType` + // keeps its display precedence for renderer selection just below. + const isComputed = isComputedFieldType(field.type, objectDefField?.type); const isReadonly = field.readonly === true || objectDefField?.readonly === true; const isSystem = NON_EDITABLE_SYSTEM_FIELDS.has(field.name); diff --git a/packages/plugin-detail/src/InlineFieldInput.tsx b/packages/plugin-detail/src/InlineFieldInput.tsx index 8b2cde9e9..38202ab52 100644 --- a/packages/plugin-detail/src/InlineFieldInput.tsx +++ b/packages/plugin-detail/src/InlineFieldInput.tsx @@ -23,16 +23,19 @@ import { coerceToSafeValue, } from '@object-ui/fields'; import { PermissionFacetLink } from './renderers/PermissionFacetLink'; +import { TEXTUAL_REF_FALLBACK_TYPES } from './fieldEnrichment'; /** * Field types that carry a `reference_to` for relational metadata but are NOT * edited via the lookup picker (they have their own dedicated inputs/renderers). - * Used so the inline-edit branch doesn't hijack them into a record picker. + * Used below so the inline-edit branch doesn't hijack them into a record picker. * - * Exported because `DetailSection`'s per-field editability gate keys off the - * same set (a computed field is never editable), so the two must not drift. + * The set itself moved to `fieldEnrichment` (objectui#3355) — the module both + * hosts' editability gates already share — so this renderer fallback and those + * gates read the ONE definition of "machine-computed". Re-exported here to keep + * the package's public name unchanged. */ -export const TEXTUAL_REF_FALLBACK_TYPES = new Set(['formula', 'summary', 'rollup', 'auto_number']); +export { TEXTUAL_REF_FALLBACK_TYPES }; /** * Extract the id a reference widget expects from a value that may already be diff --git a/packages/plugin-detail/src/__tests__/inlineEditTypeNarrowing.test.tsx b/packages/plugin-detail/src/__tests__/inlineEditTypeNarrowing.test.tsx new file mode 100644 index 000000000..5eaf31073 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/inlineEditTypeNarrowing.test.tsx @@ -0,0 +1,240 @@ +/** + * 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. + */ + +/** + * objectui#3355 — an authored display `type` may NARROW inline-edit, never WIDEN it. + * + * Both editability gates used to resolve ONE effective type with display + * precedence (`viewFieldType || objectFieldType`), so an authored non-computed + * type erased the object's `formula` / `summary` / `rollup` / `auto_number` + * declaration from the gate's view and handed the user an editor for a + * machine-owned column. + * + * That is the shipped configuration of the app that reported + * objectstack-ai/objectstack#5077: `{ name: 'supply_share', type: 'number' }` + * — authored purely to fix formatting (objectstack#5066) — over a + * hook-maintained ROLLUP. The rollup was overwritten by hand from the header + * strip and stayed corrupted until an unrelated child-row touch re-fired it + * (downstream yinlianghui/hotcrm-heimao#61). + * + * The gate is now the UNION of the two types (`isComputedFieldType`), shared by + * the highlights strip and the details body so they cannot drift. Renderer + * selection keeps the old precedence — nothing about the display changes. + */ + +import { describe, it, expect, beforeAll, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import * as React from 'react'; +import { RecordContextProvider, InlineEditProvider } from '@object-ui/react'; +import type { DetailViewSection } from '@object-ui/types'; +import { RecordHighlightsRenderer } from '../renderers/record-highlights'; +import { DetailSection } from '../DetailSection'; +import { isComputedFieldType } from '../fieldEnrichment'; + +const COMPUTED_TYPES = ['formula', 'summary', 'rollup', 'auto_number'] as const; + +const data = { supply_share: 28.57, owner: 'Alice' }; + +beforeAll(() => { + // Desktop layout — the mobile branch renders its own read-only row shape. + Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 }); +}); + +// --------------------------------------------------------------------------- +// `record:highlights` — the header strip +// --------------------------------------------------------------------------- + +const renderStrip = (fields: unknown[], objectSchema: Record) => + render( + + + + + , + ); + +/** + * The chip's inline-edit affordance: the hover pencil button plus the + * `cursor-pointer` double-click target — both rendered iff the chip is + * editable, so either one answers the question. + */ +const hasInlineEditAffordance = (container: HTMLElement) => + screen.queryAllByRole('button').length > 0 || + container.innerHTML.includes('cursor-pointer'); + +describe('record:highlights — an authored `type` cannot widen editability (#3355)', () => { + it("locks the reporter's exact config: `{ name: 'supply_share', type: 'number' }` over a rollup", () => { + const { container } = renderStrip([{ name: 'supply_share', type: 'number' }], { + fields: { supply_share: { type: 'rollup' } }, + }); + expect(hasInlineEditAffordance(container)).toBe(false); + }); + + it('still renders the value through the authored type — the lock is not a redaction', () => { + renderStrip([{ name: 'supply_share', type: 'number' }], { + fields: { supply_share: { type: 'rollup' } }, + }); + expect(screen.getByText(/28\.57/)).toBeInTheDocument(); + }); + + for (const objectType of COMPUTED_TYPES) { + it(`locks an authored \`type: 'number'\` over an object \`${objectType}\` field`, () => { + const { container } = renderStrip([{ name: 'supply_share', type: 'number' }], { + fields: { supply_share: { type: objectType } }, + }); + expect(hasInlineEditAffordance(container)).toBe(false); + }); + } + + it("narrowing still works — authored `type: 'formula'` locks a plain object field", () => { + const { container } = renderStrip([{ name: 'supply_share', type: 'formula' }], { + fields: { supply_share: { type: 'number' } }, + }); + expect(hasInlineEditAffordance(container)).toBe(false); + }); + + it('leaves an authored non-computed type over a plain field editable (control)', () => { + const { container } = renderStrip([{ name: 'supply_share', type: 'number' }], { + fields: { supply_share: { type: 'number' } }, + }); + expect(hasInlineEditAffordance(container)).toBe(true); + }); + + it('leaves a bare entry over a plain field editable (control)', () => { + const { container } = renderStrip(['owner'], { fields: { owner: { type: 'text' } } }); + expect(hasInlineEditAffordance(container)).toBe(true); + }); + + it('still honours an entry-level `readonly: true` (#3356 regression guard)', () => { + const { container } = renderStrip( + [{ name: 'supply_share', type: 'number', readonly: true }], + { fields: { supply_share: { type: 'number' } } }, + ); + expect(hasInlineEditAffordance(container)).toBe(false); + }); + + it('locks only the offending chip — a sibling plain field stays editable', () => { + renderStrip( + [ + { name: 'supply_share', type: 'number' }, + { name: 'owner' }, + ], + { fields: { supply_share: { type: 'rollup' }, owner: { type: 'text' } } }, + ); + expect(screen.queryAllByRole('button')).toHaveLength(1); + }); +}); + +// --------------------------------------------------------------------------- +// `DetailSection` — the details body (same gate, second surface) +// --------------------------------------------------------------------------- + +const renderSection = ( + field: Record, + objectSchema: Record, + { isEditing = false }: { isEditing?: boolean } = {}, +) => + render( + , + ); + +/** Read mode: the hover pencil is rendered iff the row is inline-editable. */ +const hasPencil = () => screen.queryAllByLabelText('Double-click to edit').length > 0; + +describe('DetailSection — an authored `type` cannot widen editability (#3355)', () => { + it("locks the reporter's exact config: `{ name: 'supply_share', type: 'number' }` over a rollup", () => { + renderSection({ name: 'supply_share', type: 'number' }, { + fields: { supply_share: { type: 'rollup' } }, + }); + expect(hasPencil()).toBe(false); + }); + + it('renders no editor for that field even with the section in edit mode', () => { + renderSection( + { name: 'supply_share', type: 'number' }, + { fields: { supply_share: { type: 'rollup' } } }, + { isEditing: true }, + ); + expect(screen.queryByRole('spinbutton')).toBeNull(); + // The value is still shown, formatted by the authored `number` type. + expect(screen.getByText(/28\.57/)).toBeInTheDocument(); + }); + + for (const objectType of COMPUTED_TYPES) { + it(`locks an authored \`type: 'number'\` over an object \`${objectType}\` field`, () => { + renderSection( + { name: 'supply_share', type: 'number' }, + { fields: { supply_share: { type: objectType } } }, + { isEditing: true }, + ); + expect(screen.queryByRole('spinbutton')).toBeNull(); + }); + } + + it("narrowing still works — authored `type: 'formula'` locks a plain object field", () => { + renderSection( + { name: 'owner', type: 'formula' }, + { fields: { owner: { type: 'text' } } }, + { isEditing: true }, + ); + expect(screen.queryByDisplayValue('Alice')).toBeNull(); + expect(hasPencil()).toBe(false); + }); + + it('leaves an authored non-computed type over a plain field editable (control)', () => { + renderSection( + { name: 'supply_share', type: 'number' }, + { fields: { supply_share: { type: 'number' } } }, + { isEditing: true }, + ); + expect(screen.getByRole('spinbutton')).toHaveValue(28.57); + }); + + it('leaves a plain field with no authored type editable (control)', () => { + renderSection({ name: 'owner' }, { fields: { owner: { type: 'text' } } }); + expect(hasPencil()).toBe(true); + }); + + it('still honours `readonly` on the view field and on the object metadata', () => { + renderSection({ name: 'owner', readonly: true }, { fields: { owner: { type: 'text' } } }); + expect(hasPencil()).toBe(false); + + renderSection({ name: 'owner' }, { fields: { owner: { type: 'text', readonly: true } } }); + expect(hasPencil()).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// The shared helper itself — one definition both gates call. +// --------------------------------------------------------------------------- + +describe('isComputedFieldType — union, not precedence (#3355)', () => { + it('is true when EITHER side is computed', () => { + expect(isComputedFieldType('number', 'rollup')).toBe(true); + expect(isComputedFieldType('formula', 'number')).toBe(true); + expect(isComputedFieldType(undefined, 'auto_number')).toBe(true); + expect(isComputedFieldType('summary', undefined)).toBe(true); + }); + + it('is false when neither side is computed', () => { + expect(isComputedFieldType('number', 'number')).toBe(false); + expect(isComputedFieldType(undefined, undefined)).toBe(false); + expect(isComputedFieldType('text', null)).toBe(false); + }); +}); diff --git a/packages/plugin-detail/src/fieldEnrichment.ts b/packages/plugin-detail/src/fieldEnrichment.ts index cd9f8178c..cef38ec8f 100644 --- a/packages/plugin-detail/src/fieldEnrichment.ts +++ b/packages/plugin-detail/src/fieldEnrichment.ts @@ -6,6 +6,60 @@ * LICENSE file in the root directory of this source tree. */ +/** + * Field types the PLATFORM computes — the value is machine-owned and no user + * write is legitimate. They carry a `reference_to` for relational metadata but + * have no editor of their own. + * + * One set, three readers, so they cannot drift: `InlineFieldInput` renders them + * as text instead of hijacking them into a record picker, and both editability + * gates (`DetailSection`, `HeaderHighlight`) treat them as never inline-editable. + * It lives beside `enrichDetailField` — deciding whether a column is + * machine-computed is part of resolving a detail field's shape out of the view + * entry plus the object schema, not the private business of any one component. + * Re-exported from `InlineFieldInput` under its historical name. + */ +export const TEXTUAL_REF_FALLBACK_TYPES = new Set([ + 'formula', + 'summary', + 'rollup', + 'auto_number', +]); + +/** + * Is this field machine-computed, given the type authored on the view entry and + * the type declared on the object schema? + * + * **Narrow-only — the answer is the UNION of the two.** Either type being + * computed locks the field. An authored `type` is a DISPLAY override: it picks + * the cell renderer and the editor, and that selection keeps its usual + * precedence (`viewFieldType || objectFieldType`) everywhere else. It can + * therefore only ever *narrow* editability, never widen it. + * + * Before objectui#3355 both gates resolved a single effective type with that + * same display precedence, so an authored non-computed type ERASED the object's + * computed declaration from the gate's view. The app that reported + * objectstack-ai/objectstack#5077 ships `{ name: 'supply_share', type: 'number' }` + * — a display override worked around objectstack#5066 — over a hook-maintained + * ROLLUP, and the header chip became inline-editable: a user overwrote the + * computed value by hand and it stayed corrupted until an unrelated child-row + * touch re-fired the rollup (downstream yinlianghui/hotcrm-heimao#61). + * + * The object schema is authoritative about what is machine-owned; a + * presentation override has no business granting write access. Narrowing still + * works — an authored `type: 'formula'` locks a plain object column, which is + * how a view declares "this column is computed elsewhere". + */ +export function isComputedFieldType( + viewFieldType: unknown, + objectFieldType: unknown, +): boolean { + return ( + TEXTUAL_REF_FALLBACK_TYPES.has(viewFieldType as string) || + TEXTUAL_REF_FALLBACK_TYPES.has(objectFieldType as string) + ); +} + /** * Object-metadata keys carried onto a detail-view field so the read cell and * the inline editor see the SAME resolved field shape the object form does.