diff --git a/.changeset/highlights-entry-readonly.md b/.changeset/highlights-entry-readonly.md new file mode 100644 index 000000000..987ae0ad2 --- /dev/null +++ b/.changeset/highlights-entry-readonly.md @@ -0,0 +1,6 @@ +--- +'@object-ui/plugin-detail': patch +'@object-ui/types': patch +--- + +`record:highlights` now honours a `readonly: true` on an authored field entry, so a header chip for a platform-owned column no longer offers inline edit. `HeaderHighlight`'s editability gate already consulted `field.readonly`, but the renderer rebuilt each entry from a fixed `{name,label,icon,type}` list and dropped `readonly` one layer before that check, so the gate could never fire from authored metadata — a hook-maintained rollup or approval-written grade could be overwritten by hand from the detail-page header strip and stayed wrong until an unrelated write re-fired the computation. `readonly` is now a declared key on `HighlightField` and on the `RecordHighlightsComponentProps.fields[]` entry union, mirroring `DetailViewField.readonly` (objectstack#5077). diff --git a/packages/plugin-detail/src/HeaderHighlight.tsx b/packages/plugin-detail/src/HeaderHighlight.tsx index 0ed394d65..5b0852d77 100644 --- a/packages/plugin-detail/src/HeaderHighlight.tsx +++ b/packages/plugin-detail/src/HeaderHighlight.tsx @@ -104,7 +104,7 @@ export const HeaderHighlight: React.FC = ({ // object metadata), and immutable system/audit fields never edit. const isComputed = TEXTUAL_REF_FALLBACK_TYPES.has(resolvedType as string); const isReadonly = - (field as any).readonly === true || objectDefField?.readonly === true; + field.readonly === true || objectDefField?.readonly === true; const isSystem = NON_EDITABLE_SYSTEM_FIELDS.has(field.name); const fieldEditable = !isComputed && !isReadonly && !isSystem; const canInlineEditField = canEdit && fieldEditable; diff --git a/packages/plugin-detail/src/__tests__/RecordHighlightsRenderer.readonly.test.tsx b/packages/plugin-detail/src/__tests__/RecordHighlightsRenderer.readonly.test.tsx new file mode 100644 index 000000000..82f490a5e --- /dev/null +++ b/packages/plugin-detail/src/__tests__/RecordHighlightsRenderer.readonly.test.tsx @@ -0,0 +1,135 @@ +/** + * 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. + */ + +/** + * objectstack#5077 — a `record:highlights` chip must not offer inline edit for a + * column the platform owns. + * + * Downstream (yinlianghui/hotcrm-heimao#61) a hook-maintained rollup was + * overwritten by hand from the header strip and stayed corrupted until an + * unrelated child-row touch re-fired the rollup. Such a column cannot be marked + * `readonly` on the OBJECT — `stripReadonlyFields` would drop the hook's own + * write-back too — so the declaration has to live on the highlight entry. + * + * `HeaderHighlight`'s gate always read `field.readonly`; the renderer's entry + * normalization rebuilt each entry from `{name,label,icon,type}` and dropped + * `readonly` one layer earlier, so the gate could never fire from authored + * metadata. These tests pin the whole authored-metadata → chip path, not just + * the gate in isolation. + */ + +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import * as React from 'react'; +import { RecordContextProvider, InlineEditProvider } from '@object-ui/react'; +import { RecordHighlightsRenderer } from '../renderers/record-highlights'; + +const objectSchema = { + fields: { + // Hook-maintained rollup. Declared a plain `number` on the object on + // purpose: the object CANNOT carry `readonly: true` without killing the + // cross-object hook that writes it. + supply_share: { type: 'number' }, + owner: { type: 'text' }, + }, +}; + +const data = { supply_share: 28.57, owner: 'Alice' }; + +const renderStrip = (fields: unknown[]) => + render( + + + + + , + ); + +/** + * The chip's inline-edit affordance: a hover pencil button plus the + * `cursor-pointer` double-click target. Both are rendered iff + * `canInlineEditField`, so either one answers "is this chip editable". + */ +const hasInlineEditAffordance = (container: HTMLElement) => + screen.queryAllByRole('button').length > 0 || + container.innerHTML.includes('cursor-pointer'); + +describe('record:highlights — authored `readonly` reaches the editability gate (#5077)', () => { + it('renders a bare string entry as editable (control)', () => { + const { container } = renderStrip(['owner']); + expect(screen.getByText('Alice')).toBeInTheDocument(); + expect(hasInlineEditAffordance(container)).toBe(true); + }); + + it('renders an object entry WITHOUT `readonly` as editable (control)', () => { + const { container } = renderStrip([{ name: 'supply_share', label: 'Supply Share' }]); + expect(hasInlineEditAffordance(container)).toBe(true); + }); + + it('renders `{ name, readonly: true }` as NON-editable', () => { + const { container } = renderStrip([{ name: 'supply_share', readonly: true }]); + // Value still displayed — the point of a highlight is being seen; this is a + // lock, not a redaction (`redactFields` is the remove-the-chip knob). + expect(screen.getByText(/28\.57/)).toBeInTheDocument(); + expect(hasInlineEditAffordance(container)).toBe(false); + }); + + it('keeps `readonly` per-entry — one locked chip does not lock its siblings', () => { + const { container } = renderStrip([ + { name: 'supply_share', readonly: true }, + { name: 'owner' }, + ]); + // The editable sibling still has exactly one pencil. + expect(screen.queryAllByRole('button')).toHaveLength(1); + expect(container.innerHTML).toContain('cursor-pointer'); + }); + + it('treats only `readonly: true` as a lock — a falsy value stays editable', () => { + const { container } = renderStrip([{ name: 'supply_share', readonly: false }]); + expect(hasInlineEditAffordance(container)).toBe(true); + }); +}); + +/** + * The second half of #5077: an authored computed `type` must reach the SAME + * gate the display-renderer selection reads. `TEXTUAL_REF_FALLBACK_TYPES` + * (formula / summary / rollup / auto_number) is the computed-type set. + * + * This already held at HEAD — `resolvedType = field.type || objectDefField.type` + * feeds both consumers — so these are pin tests: they fail loudly if the gate + * and the renderer selection are ever given separate type resolutions again. + */ +describe('record:highlights — authored computed `type` disables inline edit (#5077)', () => { + for (const type of ['formula', 'summary', 'rollup', 'auto_number']) { + it(`renders an authored \`type: '${type}'\` entry as NON-editable`, () => { + const { container } = renderStrip([{ name: 'supply_share', type }]); + expect(hasInlineEditAffordance(container)).toBe(false); + }); + } + + it('still honours a computed type declared only on the object', () => { + const { container } = render( + + + + + , + ); + expect(hasInlineEditAffordance(container)).toBe(false); + }); +}); diff --git a/packages/plugin-detail/src/renderers/record-highlights.tsx b/packages/plugin-detail/src/renderers/record-highlights.tsx index 8c58ea335..cc37ea61e 100644 --- a/packages/plugin-detail/src/renderers/record-highlights.tsx +++ b/packages/plugin-detail/src/renderers/record-highlights.tsx @@ -50,9 +50,25 @@ export const RecordHighlightsRenderer: React.FC = required.every((p) => perms.can(objectName, p as any)); const rawFields: any[] = Array.isArray(schema.fields) ? schema.fields : []; - // Normalize: spec accepts either bare strings or { name, label?, icon?, type? } + // Normalize: accepts either bare strings or { name, label?, icon?, type?, readonly? }. + // + // `readonly` is copied through deliberately: HeaderHighlight's editability + // gate has always consulted `field.readonly`, but this map used to rebuild + // each entry from a fixed four-key list, so an authored `readonly: true` was + // dropped one layer BEFORE the check that would honour it and the gate could + // never fire from authored metadata (objectstack#5077). Rebuilding key-by-key + // rather than spreading keeps the entry shape closed — an undeclared key is + // still not silently forwarded to the strip. const normalized = rawFields.map((f) => - typeof f === 'string' ? { name: f } : { name: f?.name, label: f?.label, icon: f?.icon, type: f?.type }, + typeof f === 'string' + ? { name: f } + : { + name: f?.name, + label: f?.label, + icon: f?.icon, + type: f?.type, + readonly: f?.readonly === true, + }, ).filter((f) => typeof f.name === 'string' && f.name.length > 0); const enforceFLS = (schema as any).enforceFieldSecurity === true; diff --git a/packages/types/src/record-components.ts b/packages/types/src/record-components.ts index d1eaae9f1..9cea3936e 100644 --- a/packages/types/src/record-components.ts +++ b/packages/types/src/record-components.ts @@ -66,8 +66,18 @@ export interface RecordDetailsComponentProps { * Aligned with @objectstack/spec RecordHighlightsProps. */ export interface RecordHighlightsComponentProps { - /** Fields to display as highlights — bare names or {name,label?,icon?,type?} for inline overrides */ - fields: Array; + /** + * Fields to display as highlights — bare names or + * `{name,label?,icon?,type?,readonly?}` for inline overrides. + * + * `readonly: true` suppresses the chip's inline-edit affordance + * (objectstack#5077) without touching the object field, which is what + * hook-maintained columns need: marking the object field `readonly` would + * also strip the hook's own write-back. + */ + fields: Array< + string | { name: string; label?: string; icon?: string; type?: string; readonly?: boolean } + >; /** Layout mode for highlights display */ layout?: 'horizontal' | 'vertical' | 'grid'; /** ARIA accessibility attributes */ diff --git a/packages/types/src/views.ts b/packages/types/src/views.ts index 116baf27a..c36821cc3 100644 --- a/packages/types/src/views.ts +++ b/packages/types/src/views.ts @@ -134,6 +134,20 @@ export interface HighlightField { type?: DetailViewField['type']; /** Optional icon */ icon?: string; + /** + * Whether the chip is read-only — no inline-edit affordance, ever. + * + * Mirrors `DetailViewField.readonly` so the highlights strip and the details + * body take the same declaration. The strip's editability gate has always + * read this key; it is declared here so it is a typed part of the surface + * rather than an `any` cast (objectstack#5077). + * + * Use it for columns whose value is owned by the platform rather than the + * user — hook-maintained rollups, approval-written grades — where marking + * the OBJECT field `readonly` is not an option because that would also strip + * the hook's own write-back. + */ + readonly?: boolean; } /**