From b250e7fbfeca52a72c08a7cb935ff5d3b433643c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= Date: Tue, 28 Jul 2026 07:01:19 -0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(fields,core,detail):=20make=20the=20sha?= =?UTF-8?q?ring-rule=20dialog=20usable=20=E2=80=94=20i18n,=20a=20picker=20?= =?UTF-8?q?that=20lists=20people,=20and=20permission-aware=20CTAs=20(objec?= =?UTF-8?q?tstack#3821)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Console's "New sharing rule" dialog could not produce a working rule. **The recipient picker listed nothing, ever.** `QueryParams['$orderby']` was typed as `Record | string[] | SortObject[]`, so `queryParamsToRecord` fed any non-array value to `Object.entries`. Handed the clause string `'name asc'` — which callers do build by hand — it walked the string index by index and emitted `$orderby=0 n,1 a,2 m,…`. The server sorted by columns that don't exist and every row was filtered out, so `recipient_id` showed "No matches" for every recipient type. `ObjectGrid` builds the same shape from a schema-level `sort` in three places, so grids with a string sort silently showed an empty table. A string `$orderby` is now passed through verbatim; the type admits `string`, and `RecipientPickerField` switched to the structured form so it can't regress. **The three sharing-rule widgets never had translations.** `ObjectRefField`, `RecipientPickerField` and `FilterConditionField` hardcoded their copy, so a Chinese Console showed "Select an object", "Select a user", "No matches". They now go through `useFieldTranslation`, with keys in all ten locales. The recipient placeholder was the interesting one: it interpolated the enum value into an English sentence (`Select a ${type.replace(/_/g,' ')}`) — a shape no locale can translate — and is now a per-type key. **Editing a rule silently dropped its recipient.** The picker resets the stored id when `recipient_type` changes; it treated the edit form's `'' → 'user'` hydration as such a change, so opening a saved rule blanked the recipient and saving persisted the blank. Only a non-empty predecessor counts now. **Building a filter submitted the surrounding form.** No `FilterBuilder` control declared `type="button"`, and a bare + , + ); +} + +const CONDITION = { + id: 'c1', + logic: 'and', + conditions: [{ id: 'c1a', field: 'title', operator: 'equals', value: 'x' }], +}; + +describe('FilterBuilder never submits its surrounding form', () => { + it('adding a condition does not submit', () => { + const onSubmit = vi.fn((e: any) => e.preventDefault()); + renderInForm(onSubmit); + fireEvent.click(screen.getByRole('button', { name: /add filter|添加筛选/i })); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it('removing a condition does not submit', () => { + const onSubmit = vi.fn((e: any) => e.preventDefault()); + renderInForm(onSubmit, CONDITION); + fireEvent.click(screen.getByRole('button', { name: /remove condition|移除条件|删除条件/i })); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it('clear-all does not submit', () => { + const onSubmit = vi.fn((e: any) => e.preventDefault()); + renderInForm(onSubmit, CONDITION); + fireEvent.click(screen.getByRole('button', { name: /clear all|清除全部/i })); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it('every FilterBuilder control declares an explicit button type', () => { + const { container } = renderInForm(vi.fn(), CONDITION); + const implicitSubmit = Array.from(container.querySelectorAll('button')).filter( + (b) => !b.getAttribute('type') && b.textContent !== 'save', + ); + expect(implicitSubmit.map((b) => b.textContent)).toEqual([]); + }); +}); diff --git a/packages/components/src/__tests__/form-write-error-message.test.tsx b/packages/components/src/__tests__/form-write-error-message.test.tsx new file mode 100644 index 0000000000..53449eb6f2 --- /dev/null +++ b/packages/components/src/__tests__/form-write-error-message.test.tsx @@ -0,0 +1,86 @@ +/** + * 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#3821 — a rejected write is user-facing copy, not server + * diagnostics. + * + * The form used to render `error.message` verbatim, so a sharing/RLS denial + * surfaced in the dialog and the toast as + * `FORBIDDEN: insufficient privileges to update showcase_private_note + * pi-TgoJ4_DM55Fqz` — untranslated, and leaking the object's machine name and + * the record id to whoever hit it. + */ +import { describe, it, expect, vi, beforeEach, beforeAll } from 'vitest'; +import React from 'react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { ComponentRegistry } from '@object-ui/core'; + +beforeAll(async () => { + await import('../renderers'); +}, 30000); + +const SCHEMA = { + type: 'form', + fields: [{ name: 'title', label: 'Title', type: 'text' }], + submitText: 'Save', +} as any; + +function forbiddenError() { + const err: any = new Error( + 'FORBIDDEN: insufficient privileges to update showcase_private_note pi-TgoJ4_DM55Fqz', + ); + err.code = 'FORBIDDEN'; + err.status = 403; + return err; +} + +function renderForm(onAction: () => Promise) { + const Form = ComponentRegistry.get('form')!; + // `onAction` is a PROP on the renderer, not part of the schema. + return render(
); +} + +describe('form write errors are user-facing copy', () => { + beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + it('shows a permission message instead of the raw FORBIDDEN text', async () => { + renderForm(async () => { throw forbiddenError(); }); + fireEvent.click(screen.getByRole("button", { name: /submit/i })); + + const alert = await screen.findByText(/permission to save/i); + expect(alert).toBeInTheDocument(); + }); + + it('never leaks the object machine name or the record id', async () => { + const { container } = renderForm(async () => { throw forbiddenError(); }); + fireEvent.click(screen.getByRole("button", { name: /submit/i })); + + await screen.findByText(/permission to save/i); + expect(container.textContent).not.toMatch(/showcase_private_note/); + expect(container.textContent).not.toMatch(/pi-TgoJ4_DM55Fqz/); + expect(container.textContent).not.toMatch(/FORBIDDEN/); + }); + + it('keeps the server text for non-permission failures — it is the useful part', async () => { + renderForm(async () => { throw new Error('Title must be unique'); }); + fireEvent.click(screen.getByRole("button", { name: /submit/i })); + + expect(await screen.findByText(/title must be unique/i)).toBeInTheDocument(); + }); + + it('falls back to a generic localized message when the error carries no text', async () => { + renderForm(async () => { throw {}; }); + fireEvent.click(screen.getByRole("button", { name: /submit/i })); + + expect(await screen.findByText(/could not save/i)).toBeInTheDocument(); + }); +}); diff --git a/packages/components/src/custom/filter-builder.tsx b/packages/components/src/custom/filter-builder.tsx index 88e4946649..9aa1dc4f80 100644 --- a/packages/components/src/custom/filter-builder.tsx +++ b/packages/components/src/custom/filter-builder.tsx @@ -390,6 +390,7 @@ function FilterBuilder({ {t('filterBuilder.where')} {filterGroup.conditions.length > 1 && ( ); diff --git a/packages/fields/src/widgets/ObjectRefField.tsx b/packages/fields/src/widgets/ObjectRefField.tsx index 2f4d25eacb..1b071efdd9 100644 --- a/packages/fields/src/widgets/ObjectRefField.tsx +++ b/packages/fields/src/widgets/ObjectRefField.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { Combobox, EmptyValue, cn } from '@object-ui/components'; import { SchemaRendererContext } from '@object-ui/react'; import type { FieldWidgetProps } from './types'; +import { useFieldTranslation } from './useFieldTranslation'; /** * ObjectRefField — object-name picker for form fields that store the machine @@ -33,6 +34,7 @@ export function ObjectRefField({ ...props }: FieldWidgetProps) { const ctx = React.useContext(SchemaRendererContext); + const { t } = useFieldTranslation(); const dataSource: any = (props as any).dataSource ?? (ctx as any)?.dataSource ?? null; const disabled = (props as any).disabled as boolean | undefined; @@ -94,9 +96,9 @@ export function ObjectRefField({ options={options} value={value ?? ''} onValueChange={(v) => onChange(v as any)} - placeholder={objects === null ? 'Loading objects…' : 'Select an object'} - searchPlaceholder="Search objects…" - emptyText={objects === null ? 'Loading…' : 'No objects found'} + placeholder={objects === null ? t('fields.objectRef.loading') : t('fields.objectRef.placeholder')} + searchPlaceholder={t('fields.objectRef.search')} + emptyText={objects === null ? t('common.loading') : t('fields.objectRef.empty')} disabled={disabled} className={cn(className)} /> diff --git a/packages/fields/src/widgets/RecipientPickerField.test.tsx b/packages/fields/src/widgets/RecipientPickerField.test.tsx new file mode 100644 index 0000000000..17d532f8bb --- /dev/null +++ b/packages/fields/src/widgets/RecipientPickerField.test.tsx @@ -0,0 +1,157 @@ +/** + * 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. + */ + +/** + * RecipientPickerField — the sharing-rule recipient picker (objectstack#3821). + * + * Two regressions are pinned here: + * 1. the candidate query must use the STRUCTURED `$orderby` form. It used to + * pass the clause string `'name asc'`, which ApiDataSource walked + * character by character into `0 n,1 a,2 m,…`; the server then sorted by + * columns that don't exist and every recipient list came back empty, so + * the control read "No matches" forever and no rule could be saved. + * 2. the placeholder must come from a per-type i18n key, not from + * interpolating the enum value into an English sentence. + */ +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { RecipientPickerField } from './RecipientPickerField'; + +const USERS = [ + { id: 'usr_1', name: 'Ada Auditor' }, + { id: 'usr_2', name: 'Bob Builder' }, +]; + +function mockDataSource(rows: any[] = USERS) { + return { find: vi.fn().mockResolvedValue({ data: rows }) } as any; +} + +function renderPicker(recipientType: string, dataSource: any, value = '') { + return render( + , + ); +} + +describe('RecipientPickerField — candidate query', () => { + it('orders candidates with the structured $orderby form, never a clause string', async () => { + const ds = mockDataSource(); + renderPicker('user', ds); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + const [object, params] = ds.find.mock.calls[0]; + expect(object).toBe('sys_user'); + expect(params.$orderby).toEqual({ name: 'asc' }); + // A bare string is what regressed; assert the shape explicitly. + expect(typeof params.$orderby).not.toBe('string'); + }); + + it('queries the object mapped to each recipient type', async () => { + for (const [type, object] of [ + ['user', 'sys_user'], + ['team', 'sys_team'], + ['business_unit', 'sys_business_unit'], + ['unit_and_subordinates', 'sys_business_unit'], + ['position', 'sys_position'], + ] as const) { + const ds = mockDataSource([]); + const { unmount } = renderPicker(type, ds); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(ds.find.mock.calls[0][0]).toBe(object); + unmount(); + } + }); + + it('shows the loaded records as options once the list is opened', async () => { + const ds = mockDataSource(); + renderPicker('user', ds); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + + fireEvent.click(screen.getByRole('combobox')); + expect(await screen.findByText('Ada Auditor')).toBeInTheDocument(); + expect(screen.getByText('Bob Builder')).toBeInTheDocument(); + }); +}); + +describe('RecipientPickerField — keeping the stored recipient', () => { + function renderControlled(initialType: string, value: string, onChange: any) { + const ds = mockDataSource(); + const view = render( + , + ); + const rerenderWith = (type: string) => + view.rerender( + , + ); + return { ...view, rerenderWith }; + } + + it('keeps the saved recipient when the type arrives late (edit form hydrating)', async () => { + // The edit dialog mounts the widget before the record lands, so + // recipient_type is '' on the first render and 'user' on the next. That is + // hydration, not the admin changing the type — clearing there wiped the + // saved recipient and the blank got persisted on save. + const onChange = vi.fn(); + const { rerenderWith } = renderControlled('', 'usr_1', onChange); + rerenderWith('user'); + await waitFor(() => expect(onChange).not.toHaveBeenCalled()); + }); + + it('clears the recipient when the admin actually switches type', async () => { + const onChange = vi.fn(); + const { rerenderWith } = renderControlled('user', 'usr_1', onChange); + rerenderWith('team'); + await waitFor(() => expect(onChange).toHaveBeenCalledWith('')); + }); + + it('does not clear when the type is unset back to empty', async () => { + const onChange = vi.fn(); + const { rerenderWith } = renderControlled('user', 'usr_1', onChange); + rerenderWith(''); + await waitFor(() => expect(onChange).not.toHaveBeenCalled()); + }); +}); + +describe('RecipientPickerField — localizable copy', () => { + it('takes the placeholder from a per-type key rather than the raw enum value', async () => { + // With no I18nProvider the widget falls back to the English defaults, so + // the assertion is on the *wording*: 'Select a business unit' can only come + // from the keyed string — interpolating the enum produced + // 'Select a business unit' only by accident and 'Select a unit and + // subordinates' for the next type over. + const ds = mockDataSource([]); + renderPicker('unit_and_subordinates', ds); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(await screen.findByText('Select a business unit')).toBeInTheDocument(); + expect(screen.queryByText(/unit and subordinates/i)).not.toBeInTheDocument(); + }); + + it('prompts for a recipient type before one is chosen', () => { + renderPicker('', mockDataSource([])); + expect(screen.getByText('Select a recipient type first.')).toBeInTheDocument(); + }); +}); diff --git a/packages/fields/src/widgets/RecipientPickerField.tsx b/packages/fields/src/widgets/RecipientPickerField.tsx index dca7e2807b..e2162cc79d 100644 --- a/packages/fields/src/widgets/RecipientPickerField.tsx +++ b/packages/fields/src/widgets/RecipientPickerField.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { Combobox, EmptyValue, cn } from '@object-ui/components'; import { SchemaRendererContext } from '@object-ui/react'; import type { FieldWidgetProps } from './types'; +import { useFieldTranslation } from './useFieldTranslation'; /** * RecipientPickerField — dependent record picker for a polymorphic recipient @@ -31,14 +32,20 @@ interface RecipientMapping { storeField: 'id' | 'name'; /** Candidate display-label fields, in preference order. */ labelFields: string[]; + /** + * i18n key for the "choose one" placeholder. Keyed per type rather than + * interpolating the enum value into an English sentence — "Select a + * business unit" and "选择业务单元" share no structure (objectstack#3821). + */ + placeholderKey: string; } const TYPE_TO_OBJECT: Record = { - user: { object: 'sys_user', storeField: 'id', labelFields: ['name', 'full_name', 'email'] }, - team: { object: 'sys_team', storeField: 'id', labelFields: ['name', 'label'] }, - business_unit: { object: 'sys_business_unit', storeField: 'id', labelFields: ['name', 'label'] }, - unit_and_subordinates: { object: 'sys_business_unit', storeField: 'id', labelFields: ['name', 'label'] }, - position: { object: 'sys_position', storeField: 'name', labelFields: ['label', 'name'] }, + user: { object: 'sys_user', storeField: 'id', labelFields: ['name', 'full_name', 'email'], placeholderKey: 'fields.recipient.selectUser' }, + team: { object: 'sys_team', storeField: 'id', labelFields: ['name', 'label'], placeholderKey: 'fields.recipient.selectTeam' }, + business_unit: { object: 'sys_business_unit', storeField: 'id', labelFields: ['name', 'label'], placeholderKey: 'fields.recipient.selectBusinessUnit' }, + unit_and_subordinates: { object: 'sys_business_unit', storeField: 'id', labelFields: ['name', 'label'], placeholderKey: 'fields.recipient.selectUnitAndSubordinates' }, + position: { object: 'sys_position', storeField: 'name', labelFields: ['label', 'name'], placeholderKey: 'fields.recipient.selectPosition' }, }; export function RecipientPickerField({ @@ -49,6 +56,7 @@ export function RecipientPickerField({ ...props }: FieldWidgetProps) { const ctx = React.useContext(SchemaRendererContext); + const { t } = useFieldTranslation(); const dataSource: any = (props as any).dataSource ?? (ctx as any)?.dataSource ?? null; const disabled = (props as any).disabled as boolean | undefined; const dependentValues: Record = (props as any).dependentValues ?? {}; @@ -57,12 +65,18 @@ export function RecipientPickerField({ const [records, setRecords] = React.useState(null); - // Reset the stored recipient when the type changes AFTER mount (an id for a - // user is not a valid team/business-unit id). The ref starts null so the - // initial render of an existing rule never clears its value. - const prevType = React.useRef(null); + // Reset the stored recipient when the admin PICKS a different type (an id for + // a user is not a valid team/business-unit id). + // + // "Different type" means one non-empty type replacing another. The empty + // string is not a type — it is the edit form before the record has hydrated, + // and treating `'' → 'user'` as a change wiped the saved recipient the moment + // an existing rule was opened for editing (objectstack#3821): the picker went + // blank, and saving persisted the blank. Only a non-empty predecessor can + // invalidate the stored id. + const prevType = React.useRef(''); React.useEffect(() => { - if (prevType.current !== null && prevType.current !== recipientType && value) { + if (prevType.current && recipientType && prevType.current !== recipientType && value) { onChange('' as any); } prevType.current = recipientType; @@ -75,7 +89,11 @@ export function RecipientPickerField({ let cancelled = false; (async () => { try { - const res = await dataSource.find(mapping.object, { $top: 500, $orderby: 'name asc' }); + // Object form, not the 'name asc' clause string: the clause string is + // supported again (objectstack#3821 fixed ApiDataSource walking it + // character by character), but the structured form can't regress that + // way for any data source. + const res = await dataSource.find(mapping.object, { $top: 500, $orderby: { name: 'asc' } }); const list: any[] = res?.data ?? res?.records ?? (Array.isArray(res) ? res : []); if (!cancelled) setRecords(Array.isArray(list) ? list : []); } catch { @@ -105,7 +123,7 @@ export function RecipientPickerField({ if (!recipientType) { return (

- Select a recipient type first. + {t('fields.recipient.selectTypeFirst')}

); } @@ -137,10 +155,12 @@ export function RecipientPickerField({ value={value ?? ''} onValueChange={(v) => onChange(v as any)} placeholder={ - records === null ? 'Loading…' : `Select a ${recipientType.replace(/_/g, ' ')}` + records === null + ? t('fields.recipient.loading') + : t(mapping.placeholderKey ?? 'fields.recipient.select') } - searchPlaceholder="Search…" - emptyText={records === null ? 'Loading…' : 'No matches'} + searchPlaceholder={t('fields.recipient.search')} + emptyText={records === null ? t('fields.recipient.loading') : t('fields.recipient.empty')} disabled={disabled} className={className} /> diff --git a/packages/fields/src/widgets/useFieldTranslation.ts b/packages/fields/src/widgets/useFieldTranslation.ts index 2f753ab02f..2227c74d38 100644 --- a/packages/fields/src/widgets/useFieldTranslation.ts +++ b/packages/fields/src/widgets/useFieldTranslation.ts @@ -39,6 +39,30 @@ const FIELD_DEFAULTS: Record = { 'lookup.nextPage': 'Next page', 'lookup.jumpToPage': 'Jump to page', 'lookup.retry': 'Retry', + // objectstack#3821 — sharing-rule authoring widgets (object-ref / + // recipient-picker / filter-condition). The recipient placeholder is keyed + // PER TYPE rather than interpolating the enum value into an English + // sentence, which no locale could translate. + 'fields.objectRef.loading': 'Loading objects…', + 'fields.objectRef.placeholder': 'Select an object', + 'fields.objectRef.search': 'Search objects…', + 'fields.objectRef.empty': 'No objects found', + 'fields.recipient.selectTypeFirst': 'Select a recipient type first.', + 'fields.recipient.loading': 'Loading…', + 'fields.recipient.search': 'Search…', + 'fields.recipient.empty': 'No matches', + 'fields.recipient.select': 'Select a recipient', + 'fields.recipient.selectUser': 'Select a user', + 'fields.recipient.selectTeam': 'Select a team', + 'fields.recipient.selectBusinessUnit': 'Select a business unit', + 'fields.recipient.selectPosition': 'Select a position', + 'fields.recipient.selectUnitAndSubordinates': 'Select a business unit', + 'fields.filterCondition.selectObjectFirst': 'Select an object first.', + 'fields.filterCondition.allRecords': 'All records', + 'fields.filterCondition.invalidJson': 'Invalid JSON — the rule will match no records until fixed.', + 'fields.filterCondition.jsonOnly': 'This criteria can only be edited as JSON', + 'fields.filterCondition.editAsJson': 'Edit as JSON', + 'fields.filterCondition.useVisualBuilder': 'Use visual builder', // objectui#2600 B5 — capability picker scope group headers. 'capability.group.platform': 'Platform', 'capability.group.org': 'Organization', diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index 52de220dad..1d06841b6b 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -98,6 +98,8 @@ const ar = { type: "{{field}} يجب أن يكون {{type}} صالحاً", }, form: { + noPermissionToSave: "ليس لديك إذن لحفظ هذا السجل.", + submitFailed: "تعذّر الحفظ. يرجى المحاولة مرة أخرى.", addItem: "إضافة عنصر", removeItem: "إزالة عنصر", fieldRequired: "هذا الحقل مطلوب", @@ -142,6 +144,32 @@ const ar = { basicEditorHint: "محرر النص الغني (أساسي)", placeholder: "اكتب شيئاً...", }, + objectRef: { + loading: "جارٍ تحميل الكائنات…", + placeholder: "اختر كائناً", + search: "بحث في الكائنات…", + empty: "لم يتم العثور على كائنات", + }, + recipient: { + selectTypeFirst: "اختر نوع المستلم أولاً.", + loading: "جارٍ التحميل…", + search: "بحث…", + empty: "لا توجد نتائج مطابقة", + select: "اختر مستلماً", + selectUser: "اختر مستخدماً", + selectTeam: "اختر فريقاً", + selectBusinessUnit: "اختر وحدة عمل", + selectPosition: "اختر منصباً", + selectUnitAndSubordinates: "اختر وحدة عمل", + }, + filterCondition: { + selectObjectFirst: "اختر كائناً أولاً.", + allRecords: "كل السجلات", + invalidJson: "JSON غير صالح — لن تطابق القاعدة أي سجل حتى يتم إصلاحه.", + jsonOnly: "لا يمكن تحرير هذا المعيار إلا بصيغة JSON", + editAsJson: "التحرير بصيغة JSON", + useVisualBuilder: "استخدام المُنشئ المرئي", + }, }, table: { rowsPerPage: "صفوف في الصفحة", diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index c9ae22a99e..66fd5113b1 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -98,6 +98,8 @@ const de = { type: "{{field}} muss ein gültiger {{type}} sein", }, form: { + noPermissionToSave: "Sie haben keine Berechtigung, diesen Datensatz zu speichern.", + submitFailed: "Speichern fehlgeschlagen. Bitte versuchen Sie es erneut.", addItem: "Element hinzufügen", removeItem: "Element entfernen", fieldRequired: "Dieses Feld ist erforderlich", @@ -142,6 +144,32 @@ const de = { basicEditorHint: "Rich-Text-Editor (einfach)", placeholder: "Text eingeben...", }, + objectRef: { + loading: "Objekte werden geladen…", + placeholder: "Objekt auswählen", + search: "Objekte suchen…", + empty: "Keine Objekte gefunden", + }, + recipient: { + selectTypeFirst: "Wählen Sie zuerst einen Empfängertyp.", + loading: "Wird geladen…", + search: "Suchen…", + empty: "Keine Treffer", + select: "Empfänger auswählen", + selectUser: "Benutzer auswählen", + selectTeam: "Team auswählen", + selectBusinessUnit: "Geschäftseinheit auswählen", + selectPosition: "Position auswählen", + selectUnitAndSubordinates: "Geschäftseinheit auswählen", + }, + filterCondition: { + selectObjectFirst: "Wählen Sie zuerst ein Objekt.", + allRecords: "Alle Datensätze", + invalidJson: "Ungültiges JSON — die Regel trifft auf keinen Datensatz zu, bis dies behoben ist.", + jsonOnly: "Dieses Kriterium kann nur als JSON bearbeitet werden", + editAsJson: "Als JSON bearbeiten", + useVisualBuilder: "Visuellen Builder verwenden", + }, }, table: { rowsPerPage: "Zeilen pro Seite", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 5ee32606a6..61c57b4ede 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -113,6 +113,8 @@ const en = { type: '{{field}} must be a valid {{type}}', }, form: { + noPermissionToSave: "You don't have permission to save this record.", + submitFailed: 'Could not save. Please try again.', addItem: 'Add item', removeItem: 'Remove item', fieldRequired: 'This field is required', @@ -174,6 +176,32 @@ const en = { basicEditorHint: 'Rich text editor (basic)', placeholder: 'Enter text...', }, + objectRef: { + loading: 'Loading objects…', + placeholder: 'Select an object', + search: 'Search objects…', + empty: 'No objects found', + }, + recipient: { + selectTypeFirst: 'Select a recipient type first.', + loading: 'Loading…', + search: 'Search…', + empty: 'No matches', + select: 'Select a recipient', + selectUser: 'Select a user', + selectTeam: 'Select a team', + selectBusinessUnit: 'Select a business unit', + selectPosition: 'Select a position', + selectUnitAndSubordinates: 'Select a business unit', + }, + filterCondition: { + selectObjectFirst: 'Select an object first.', + allRecords: 'All records', + invalidJson: 'Invalid JSON — the rule will match no records until fixed.', + jsonOnly: 'This criteria can only be edited as JSON', + editAsJson: 'Edit as JSON', + useVisualBuilder: 'Use visual builder', + }, }, table: { rowsPerPage: 'Rows per page', diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index f13bfc5e1b..5d0f70a8f8 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -98,6 +98,8 @@ const es = { type: "{{field}} debe ser un {{type}} válido", }, form: { + noPermissionToSave: "No tienes permiso para guardar este registro.", + submitFailed: "No se pudo guardar. Inténtalo de nuevo.", addItem: "Añadir elemento", removeItem: "Eliminar elemento", fieldRequired: "Este campo es obligatorio", @@ -142,6 +144,32 @@ const es = { basicEditorHint: "Editor de texto enriquecido (básico)", placeholder: "Escribe algo...", }, + objectRef: { + loading: "Cargando objetos…", + placeholder: "Seleccionar un objeto", + search: "Buscar objetos…", + empty: "No se encontraron objetos", + }, + recipient: { + selectTypeFirst: "Selecciona primero un tipo de destinatario.", + loading: "Cargando…", + search: "Buscar…", + empty: "Sin coincidencias", + select: "Seleccionar un destinatario", + selectUser: "Seleccionar un usuario", + selectTeam: "Seleccionar un equipo", + selectBusinessUnit: "Seleccionar una unidad de negocio", + selectPosition: "Seleccionar un puesto", + selectUnitAndSubordinates: "Seleccionar una unidad de negocio", + }, + filterCondition: { + selectObjectFirst: "Selecciona primero un objeto.", + allRecords: "Todos los registros", + invalidJson: "JSON no válido: la regla no coincidirá con ningún registro hasta que se corrija.", + jsonOnly: "Este criterio solo se puede editar como JSON", + editAsJson: "Editar como JSON", + useVisualBuilder: "Usar el constructor visual", + }, }, table: { rowsPerPage: "Filas por página", diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index f756233b83..7997186fa4 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -98,6 +98,8 @@ const fr = { type: "{{field}} doit être un {{type}} valide", }, form: { + noPermissionToSave: "Vous n'avez pas l'autorisation d'enregistrer cet enregistrement.", + submitFailed: "Échec de l'enregistrement. Veuillez réessayer.", addItem: "Ajouter un élément", removeItem: "Supprimer un élément", fieldRequired: "Ce champ est obligatoire", @@ -142,6 +144,32 @@ const fr = { basicEditorHint: "Éditeur de texte enrichi (basique)", placeholder: "Écrivez quelque chose...", }, + objectRef: { + loading: "Chargement des objets…", + placeholder: "Sélectionner un objet", + search: "Rechercher des objets…", + empty: "Aucun objet trouvé", + }, + recipient: { + selectTypeFirst: "Sélectionnez d'abord un type de destinataire.", + loading: "Chargement…", + search: "Rechercher…", + empty: "Aucune correspondance", + select: "Sélectionner un destinataire", + selectUser: "Sélectionner un utilisateur", + selectTeam: "Sélectionner une équipe", + selectBusinessUnit: "Sélectionner une unité opérationnelle", + selectPosition: "Sélectionner un poste", + selectUnitAndSubordinates: "Sélectionner une unité opérationnelle", + }, + filterCondition: { + selectObjectFirst: "Sélectionnez d'abord un objet.", + allRecords: "Tous les enregistrements", + invalidJson: "JSON invalide — la règle ne correspondra à aucun enregistrement tant qu'elle n'est pas corrigée.", + jsonOnly: "Ce critère ne peut être modifié qu'en JSON", + editAsJson: "Modifier en JSON", + useVisualBuilder: "Utiliser le constructeur visuel", + }, }, table: { rowsPerPage: "Lignes par page", diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index 27d17b1f51..ba85843423 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -98,6 +98,8 @@ const ja = { type: "{{field}}は有効な{{type}}である必要があります", }, form: { + noPermissionToSave: "このレコードを保存する権限がありません。", + submitFailed: "保存できませんでした。もう一度お試しください。", addItem: "項目を追加", removeItem: "項目を削除", fieldRequired: "この項目は必須です", @@ -142,6 +144,32 @@ const ja = { basicEditorHint: "リッチテキストエディター（基本）", placeholder: "テキストを入力...", }, + objectRef: { + loading: "オブジェクトを読み込み中…", + placeholder: "オブジェクトを選択", + search: "オブジェクトを検索…", + empty: "オブジェクトが見つかりません", + }, + recipient: { + selectTypeFirst: "先に受信者タイプを選択してください。", + loading: "読み込み中…", + search: "検索…", + empty: "一致する項目がありません", + select: "受信者を選択", + selectUser: "ユーザーを選択", + selectTeam: "チームを選択", + selectBusinessUnit: "事業単位を選択", + selectPosition: "役職を選択", + selectUnitAndSubordinates: "事業単位を選択", + }, + filterCondition: { + selectObjectFirst: "先にオブジェクトを選択してください。", + allRecords: "すべてのレコード", + invalidJson: "JSON が不正です — 修正するまでこのルールはどのレコードにも一致しません。", + jsonOnly: "この条件は JSON でのみ編集できます", + editAsJson: "JSON で編集", + useVisualBuilder: "ビジュアルビルダーを使用", + }, }, table: { rowsPerPage: "1ページの行数", diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index 45bf11676e..b01ff83b94 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -98,6 +98,8 @@ const ko = { type: "{{field}}은(는) 유효한 {{type}}이어야 합니다", }, form: { + noPermissionToSave: "이 레코드를 저장할 권한이 없습니다.", + submitFailed: "저장하지 못했습니다. 다시 시도해 주세요.", addItem: "항목 추가", removeItem: "항목 제거", fieldRequired: "이 필드는 필수입니다", @@ -142,6 +144,32 @@ const ko = { basicEditorHint: "서식 있는 텍스트 편집기 (기본)", placeholder: "내용을 입력하세요...", }, + objectRef: { + loading: "객체를 불러오는 중…", + placeholder: "객체 선택", + search: "객체 검색…", + empty: "객체를 찾을 수 없습니다", + }, + recipient: { + selectTypeFirst: "먼저 수신자 유형을 선택하세요.", + loading: "불러오는 중…", + search: "검색…", + empty: "일치하는 항목 없음", + select: "수신자 선택", + selectUser: "사용자 선택", + selectTeam: "팀 선택", + selectBusinessUnit: "사업 단위 선택", + selectPosition: "직위 선택", + selectUnitAndSubordinates: "사업 단위 선택", + }, + filterCondition: { + selectObjectFirst: "먼저 객체를 선택하세요.", + allRecords: "모든 레코드", + invalidJson: "잘못된 JSON — 수정하기 전까지 이 규칙은 어떤 레코드와도 일치하지 않습니다.", + jsonOnly: "이 조건은 JSON으로만 편집할 수 있습니다", + editAsJson: "JSON으로 편집", + useVisualBuilder: "비주얼 빌더 사용", + }, }, table: { rowsPerPage: "페이지당 행 수", diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index b97eb17c26..033b01e1e3 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -98,6 +98,8 @@ const pt = { type: "{{field}} deve ser um {{type}} válido", }, form: { + noPermissionToSave: "Você não tem permissão para salvar este registro.", + submitFailed: "Não foi possível salvar. Tente novamente.", addItem: "Adicionar item", removeItem: "Remover item", fieldRequired: "Este campo é obrigatório", @@ -142,6 +144,32 @@ const pt = { basicEditorHint: "Editor de texto rico (básico)", placeholder: "Digite algo...", }, + objectRef: { + loading: "Carregando objetos…", + placeholder: "Selecionar um objeto", + search: "Pesquisar objetos…", + empty: "Nenhum objeto encontrado", + }, + recipient: { + selectTypeFirst: "Selecione primeiro um tipo de destinatário.", + loading: "Carregando…", + search: "Pesquisar…", + empty: "Nenhuma correspondência", + select: "Selecionar um destinatário", + selectUser: "Selecionar um usuário", + selectTeam: "Selecionar uma equipe", + selectBusinessUnit: "Selecionar uma unidade de negócio", + selectPosition: "Selecionar um cargo", + selectUnitAndSubordinates: "Selecionar uma unidade de negócio", + }, + filterCondition: { + selectObjectFirst: "Selecione primeiro um objeto.", + allRecords: "Todos os registros", + invalidJson: "JSON inválido — a regra não corresponderá a nenhum registro até ser corrigida.", + jsonOnly: "Este critério só pode ser editado como JSON", + editAsJson: "Editar como JSON", + useVisualBuilder: "Usar o construtor visual", + }, }, table: { rowsPerPage: "Linhas por página", diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index 29ed37fdec..cf9a033de8 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -98,6 +98,8 @@ const ru = { type: "{{field}} должно быть допустимым {{type}}", }, form: { + noPermissionToSave: "У вас нет прав на сохранение этой записи.", + submitFailed: "Не удалось сохранить. Попробуйте ещё раз.", addItem: "Добавить элемент", removeItem: "Удалить элемент", fieldRequired: "Это поле обязательно", @@ -142,6 +144,32 @@ const ru = { basicEditorHint: "Редактор форматированного текста (базовый)", placeholder: "Введите текст...", }, + objectRef: { + loading: "Загрузка объектов…", + placeholder: "Выберите объект", + search: "Поиск объектов…", + empty: "Объекты не найдены", + }, + recipient: { + selectTypeFirst: "Сначала выберите тип получателя.", + loading: "Загрузка…", + search: "Поиск…", + empty: "Совпадений нет", + select: "Выберите получателя", + selectUser: "Выберите пользователя", + selectTeam: "Выберите команду", + selectBusinessUnit: "Выберите бизнес-подразделение", + selectPosition: "Выберите должность", + selectUnitAndSubordinates: "Выберите бизнес-подразделение", + }, + filterCondition: { + selectObjectFirst: "Сначала выберите объект.", + allRecords: "Все записи", + invalidJson: "Некорректный JSON — правило не будет соответствовать ни одной записи, пока это не исправлено.", + jsonOnly: "Этот критерий можно редактировать только как JSON", + editAsJson: "Редактировать как JSON", + useVisualBuilder: "Использовать визуальный конструктор", + }, }, table: { rowsPerPage: "Строк на странице", diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index f3dc8286e7..830fbd41c2 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -112,6 +112,8 @@ const zh = { type: '{{field}}必须是有效的{{type}}', }, form: { + noPermissionToSave: '您没有权限保存这条记录。', + submitFailed: '保存失败，请重试。', addItem: '添加项目', removeItem: '移除项目', fieldRequired: '此字段为必填项', @@ -173,6 +175,32 @@ const zh = { basicEditorHint: '富文本编辑器（基础）', placeholder: '请输入文字...', }, + objectRef: { + loading: '正在加载对象…', + placeholder: '请选择对象', + search: '搜索对象…', + empty: '未找到对象', + }, + recipient: { + selectTypeFirst: '请先选择接收方类型。', + loading: '加载中…', + search: '搜索…', + empty: '无匹配项', + select: '请选择接收方', + selectUser: '请选择用户', + selectTeam: '请选择团队', + selectBusinessUnit: '请选择业务单元', + selectPosition: '请选择岗位', + selectUnitAndSubordinates: '请选择业务单元', + }, + filterCondition: { + selectObjectFirst: '请先选择对象。', + allRecords: '全部记录', + invalidJson: 'JSON 无效 —— 修正之前该规则不会匹配任何记录。', + jsonOnly: '该条件只能以 JSON 方式编辑', + editAsJson: '以 JSON 编辑', + useVisualBuilder: '使用可视化构建器', + }, }, table: { rowsPerPage: '每页行数', diff --git a/packages/permissions/src/evaluator.ts b/packages/permissions/src/evaluator.ts index f642a2b46f..ea0276fc5d 100644 --- a/packages/permissions/src/evaluator.ts +++ b/packages/permissions/src/evaluator.ts @@ -57,11 +57,17 @@ export function evaluatePermission({ const roleConfig = objectConfig.roles[roleName]; if (!roleConfig) continue; - if (roleConfig.actions.includes(action)) { + // `actions` is optional in practice: a role config may carry only + // `fieldPermissions` (the field-level gate is the whole point of the + // entry). Reading `.includes` off it unguarded threw a TypeError that + // propagated out of `check()` and took the whole view down with it — + // a permission *check* must never be able to crash a render + // (objectstack#3821). + if (roleConfig.actions?.includes(action)) { // Check row-level permissions if record is provided if (record && roleConfig.rowPermissions?.length) { const rowAllowed = roleConfig.rowPermissions.some( - (rp) => rp.actions.includes(action), + (rp) => rp.actions?.includes(action), ); if (!rowAllowed) continue; } diff --git a/packages/plugin-detail/src/DetailView.tsx b/packages/plugin-detail/src/DetailView.tsx index e81583221a..0c7aaa0481 100644 --- a/packages/plugin-detail/src/DetailView.tsx +++ b/packages/plugin-detail/src/DetailView.tsx @@ -48,6 +48,7 @@ import { usePermissions } from '@object-ui/permissions'; import { useLocalization, resolveFieldCurrency } from '@object-ui/i18n'; import type { DetailViewSchema, DataSource, ActionSchema, SchemaNode } from '@object-ui/types'; import { useDetailTranslation } from './useDetailTranslation'; +import { useRecordEditable } from './useRecordEditable'; /** Default page size for related lists in the detail view */ const DEFAULT_RELATED_PAGE_SIZE = 5; @@ -261,7 +262,50 @@ export const DetailView: React.FC = ({ summaryFields: filterFields(rawSchema.summaryFields as any[]) as any, }; }, [rawSchema, perms]); - const schema = gatedSchema; + + /** + * Record-level write gate (objectstack#3821). Object-level permissions say + * whether the user may edit `showcase_private_note` at all; they cannot say + * whether they may edit THIS one. A read-only sharing grant sits exactly in + * that gap, so the header used to offer "Edit", open the form, and let the + * user retype a field before the server rejected it with a 403. Ask the + * explain engine for the row-level verdict and reflect it on the CTA. + * + * Only consulted when the object-level check already passes — when it + * doesn't, the buttons are hidden anyway and the request would be waste. + * Fails open (see `useRecordEditable`); the server remains the authority. + */ + const objectAllowsUpdate = React.useMemo( + () => !perms?.isLoaded || !rawSchema.objectName || perms.can(rawSchema.objectName, 'update'), + [perms, rawSchema.objectName], + ); + const objectAllowsDelete = React.useMemo( + () => !perms?.isLoaded || !rawSchema.objectName || perms.can(rawSchema.objectName, 'delete'), + [perms, rawSchema.objectName], + ); + const recordId = + rawSchema.resourceId != null ? String(rawSchema.resourceId) : undefined; + const canEditRecord = useRecordEditable( + rawSchema.objectName, + recordId, + 'update', + objectAllowsUpdate, + ); + const canDeleteRecord = useRecordEditable( + rawSchema.objectName, + recordId, + 'delete', + objectAllowsDelete, + ); + + const schema = React.useMemo( + () => ({ + ...gatedSchema, + showEdit: gatedSchema.showEdit && objectAllowsUpdate && canEditRecord, + showDelete: gatedSchema.showDelete && objectAllowsDelete && canDeleteRecord, + }), + [gatedSchema, objectAllowsUpdate, canEditRecord, objectAllowsDelete, canDeleteRecord], + ); // Fire onDataLoaded whenever the record changes so hosts can publish it diff --git a/packages/plugin-detail/src/index.tsx b/packages/plugin-detail/src/index.tsx index 2949908e94..ed0d08811c 100644 --- a/packages/plugin-detail/src/index.tsx +++ b/packages/plugin-detail/src/index.tsx @@ -47,6 +47,8 @@ export type { ConcurrentUpdateConflict, ConcurrentUpdateDialogProps, } from './ConcurrentUpdateDialog'; +export { useRecordEditable, __clearRecordEditableCache } from './useRecordEditable'; +export type { RecordOperation } from './useRecordEditable'; export { SectionGroup } from './SectionGroup'; export { HeaderHighlight } from './HeaderHighlight'; export { InlineFieldInput, extractLookupId, TEXTUAL_REF_FALLBACK_TYPES } from './InlineFieldInput'; diff --git a/packages/plugin-detail/src/useRecordEditable.test.tsx b/packages/plugin-detail/src/useRecordEditable.test.tsx new file mode 100644 index 0000000000..51832330b6 --- /dev/null +++ b/packages/plugin-detail/src/useRecordEditable.test.tsx @@ -0,0 +1,103 @@ +/** + * 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#3821 — the record-level write gate behind the detail header's + * Edit / Delete CTAs. + * + * A record shared READ-ONLY lives inside an object the user may otherwise edit, + * so object-level permissions said "yes" and the header offered Edit; the user + * only found out at save time, via a 403. The gate asks the explain engine for + * the row-level verdict instead. + * + * Every uncertainty must fail OPEN — a courtesy hint may never be the reason a + * permitted user cannot act. The server is the authority (ADR-0057 D10). + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook, waitFor } from '@testing-library/react'; +import { useRecordEditable, __clearRecordEditableCache } from './useRecordEditable'; + +function mockExplain(body: unknown, ok = true) { + return vi.fn(async () => ({ ok, json: async () => body })) as any; +} + +describe('useRecordEditable', () => { + beforeEach(() => { + __clearRecordEditableCache(); + }); + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('disables the action when the record-level verdict says no', async () => { + vi.stubGlobal('fetch', mockExplain({ allowed: true, record: { recordId: 'r1', visible: false } })); + const { result } = renderHook(() => useRecordEditable('note', 'r1')); + await waitFor(() => expect(result.current).toBe(false)); + }); + + it('keeps the action when the record-level verdict says yes', async () => { + vi.stubGlobal('fetch', mockExplain({ allowed: true, record: { recordId: 'r1', visible: true } })); + const { result } = renderHook(() => useRecordEditable('note', 'r1')); + await waitFor(() => expect(result.current).toBe(true)); + }); + + it('asks about the requested operation and record', async () => { + const fetchMock = mockExplain({ record: { recordId: 'r1', visible: true } }); + vi.stubGlobal('fetch', fetchMock); + renderHook(() => useRecordEditable('note', 'r1', 'delete')); + await waitFor(() => expect(fetchMock).toHaveBeenCalled()); + const body = JSON.parse(fetchMock.mock.calls[0][1].body); + expect(body).toEqual({ object: 'note', operation: 'delete', recordId: 'r1' }); + }); + + it('fails open on a non-OK response (401 / 403 / 501 deployments)', async () => { + vi.stubGlobal('fetch', mockExplain({}, false)); + const { result } = renderHook(() => useRecordEditable('note', 'r1')); + await waitFor(() => expect(result.current).toBe(true)); + }); + + it('fails open when the network throws', async () => { + vi.stubGlobal('fetch', vi.fn(async () => { throw new Error('offline'); }) as any); + const { result } = renderHook(() => useRecordEditable('note', 'r1')); + await waitFor(() => expect(result.current).toBe(true)); + }); + + it('fails open when the report carries no record verdict', async () => { + vi.stubGlobal('fetch', mockExplain({ allowed: false })); + const { result } = renderHook(() => useRecordEditable('note', 'r1')); + await waitFor(() => expect(result.current).toBe(true)); + }); + + it('does not call explain when disabled — the object-level gate already decided', async () => { + const fetchMock = mockExplain({ record: { recordId: 'r1', visible: false } }); + vi.stubGlobal('fetch', fetchMock); + const { result } = renderHook(() => useRecordEditable('note', 'r1', 'update', false)); + expect(result.current).toBe(true); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('does not call explain without an object or a record id', async () => { + const fetchMock = mockExplain({}); + vi.stubGlobal('fetch', fetchMock); + renderHook(() => useRecordEditable(undefined, 'r1')); + renderHook(() => useRecordEditable('note', undefined)); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('memoises the verdict so revisiting a record costs nothing', async () => { + const fetchMock = mockExplain({ record: { recordId: 'r1', visible: false } }); + vi.stubGlobal('fetch', fetchMock); + + const first = renderHook(() => useRecordEditable('note', 'r1')); + await waitFor(() => expect(first.result.current).toBe(false)); + + const second = renderHook(() => useRecordEditable('note', 'r1')); + expect(second.result.current).toBe(false); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/plugin-detail/src/useRecordEditable.ts b/packages/plugin-detail/src/useRecordEditable.ts new file mode 100644 index 0000000000..7a027fde0e --- /dev/null +++ b/packages/plugin-detail/src/useRecordEditable.ts @@ -0,0 +1,88 @@ +/** + * 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. + */ + +/** + * Is THIS record editable by the current user? (objectstack#3821) + * + * Object-level permissions can't answer that. A record shared read-only via a + * sharing rule sits inside an object the user may otherwise create and edit + * freely — so the header offered a primary "Edit" CTA that opened the form, let + * the user retype a field, and only then bounced with a 403. The server is the + * authority (ADR-0057 D10) and stays so; this is the courtesy check that stops + * the UI from inviting a write it knows will fail. + * + * The answer comes from the explain engine's record-grained verdict + * (`POST /api/v1/security/explain` with a `recordId`, ADR-0090 D6 / ADR-0095 + * C2) — the same pipeline the enforcement middleware runs, so the button and + * the server can't disagree. Explaining ONESELF needs no special permission; + * only explaining another principal does. + * + * **Fail-open on every uncertainty** — no answer yet, a non-OK response, a + * deployment without `@objectstack/plugin-security` (501), a split SPA origin + * where the cookie doesn't reach the API: the button stays enabled and the + * server does its job. A permission hint must never be the reason a permitted + * user cannot act. + */ +import * as React from 'react'; + +/** Cache keyed by `object:recordId:operation` so revisiting a record is free. */ +const verdictCache = new Map(); + +export type RecordOperation = 'update' | 'delete'; + +export function useRecordEditable( + objectName: string | undefined, + recordId: string | undefined, + operation: RecordOperation = 'update', + enabled = true, +): boolean { + const key = objectName && recordId ? `${objectName}:${recordId}:${operation}` : ''; + const [allowed, setAllowed] = React.useState(() => + key && verdictCache.has(key) ? verdictCache.get(key)! : true, + ); + + React.useEffect(() => { + if (!enabled || !key) { + setAllowed(true); + return; + } + if (verdictCache.has(key)) { + setAllowed(verdictCache.get(key)!); + return; + } + let cancelled = false; + (async () => { + try { + const res = await fetch('/api/v1/security/explain', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + credentials: 'include', + body: JSON.stringify({ object: objectName, operation, recordId }), + }); + if (!res.ok) return; // 401 / 403 / 501 → fail open + const decision = await res.json(); + const verdict = decision?.record?.visible; + if (typeof verdict !== 'boolean') return; // no record verdict → fail open + verdictCache.set(key, verdict); + if (!cancelled) setAllowed(verdict); + } catch { + /* network/parse failure → fail open */ + } + })(); + return () => { + cancelled = true; + }; + }, [key, objectName, recordId, operation, enabled]); + + return allowed; +} + +/** Test seam — drops the memoised verdicts. */ +export function __clearRecordEditableCache(): void { + verdictCache.clear(); +} diff --git a/packages/types/src/data.ts b/packages/types/src/data.ts index bd067353a2..2d0d31654e 100644 --- a/packages/types/src/data.ts +++ b/packages/types/src/data.ts @@ -35,11 +35,13 @@ export interface QueryParams { /** * Sort order - * Can be a Map { field: 'asc' }, an Array of strings ['field', '-field'], or Array of sort objects + * Can be an OData clause string 'field asc, other desc', a Map { field: 'asc' }, + * an Array of strings ['field', '-field'], or an Array of sort objects. + * @example 'name asc' * @example { createdAt: 'desc', name: 'asc' } * @example ['name', '-createdAt'] */ - $orderby?: Record | string[] | Array<{ field: string; order?: 'asc' | 'desc' }>; + $orderby?: string | Record | string[] | Array<{ field: string; order?: 'asc' | 'desc' }>; /** * Number of records to skip (for pagination) From 3d18a888d5289e5c96792ab3966cdf2dabb3a6a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= Date: Tue, 28 Jul 2026 08:02:32 -0700 Subject: [PATCH 2/2] fix(components,fields): a long combobox option truncates instead of running past the border (objectstack#3821) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sharing-rule object picker rendered "成员 (showcase_project_membership)" straight through the control's right edge and into the field beside it. Two causes, both mine from the picker work in this branch: - `Combobox`'s trigger keeps the component's `w-[200px]` default, so the object picker sat at a third of the form column while `名称` next to it ran full width — and it is the field holding the longest content on the form. - The selected label was a bare text child of a flex button. Flex items need `truncate` AND `min-w-0` to clip; it had neither, so the text simply drew outside the box instead of ellipsizing. The label now lives in a truncating span, the trigger may shrink, and the dropdown takes `--radix-popover-trigger-width` rather than its own hardcoded 200px — a combobox widened by its consumer used to clip its own options. The two sharing-rule pickers pass `w-full` so they line up with every other input; `w-[200px]` stays as the component default and consumers keep overriding it, so no other call site changes. Pinned by tests that assert the classes, since none of this is visible in an accessibility tree. Browser-measured both ways: at 1280px the trigger is 362px and the 247px label fits with room to spare; narrowed to 700px the label clips at 210px with `text-overflow: ellipsis` and stays inside the button. Co-Authored-By: Claude Opus 5 --- .../sharing-rule-widgets-i18n-and-orderby.md | 10 ++++ .../__tests__/combobox-long-label.test.tsx | 54 +++++++++++++++++++ packages/components/src/custom/combobox.tsx | 18 +++++-- .../fields/src/widgets/ObjectRefField.tsx | 5 +- .../src/widgets/RecipientPickerField.tsx | 2 +- 5 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 packages/components/src/__tests__/combobox-long-label.test.tsx diff --git a/.changeset/sharing-rule-widgets-i18n-and-orderby.md b/.changeset/sharing-rule-widgets-i18n-and-orderby.md index 5774eea5ae..bc461eedfc 100644 --- a/.changeset/sharing-rule-widgets-i18n-and-orderby.md +++ b/.changeset/sharing-rule-widgets-i18n-and-orderby.md @@ -76,6 +76,16 @@ says no, and **fails open** on every uncertainty — an unanswered hint must nev be the reason a permitted user cannot act; the server stays the authority (ADR-0057 D10). +**A long option rendered straight past the combobox border.** `Combobox`'s +trigger pinned itself to the component's `w-[200px]` default while the fields +around it ran the full form column, and the selected label was a bare text child +of a flex button — flex items need `truncate` AND `min-w-0` to clip, and it had +neither. So "成员 (showcase_project_membership)" in the object picker overflowed +the control and collided with the field beside it. The label now truncates, the +trigger can shrink, the dropdown matches the trigger's width instead of a +hardcoded 200px (a widened combobox used to clip its own options), and the two +sharing-rule pickers ask for `w-full` so they line up with every other input. + Hardens `evaluatePermission` while there: a role config carrying only `fieldPermissions` (no `actions`) made `check()` throw a TypeError that propagated out of the render. A permission check must not be able to crash a diff --git a/packages/components/src/__tests__/combobox-long-label.test.tsx b/packages/components/src/__tests__/combobox-long-label.test.tsx new file mode 100644 index 0000000000..7ab07c5c86 --- /dev/null +++ b/packages/components/src/__tests__/combobox-long-label.test.tsx @@ -0,0 +1,54 @@ +/** + * 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#3821 — a long option must ellipsize inside the trigger, not + * render past its border. + * + * The selected label was a bare text child of a flex button, so + * "成员 (showcase_project_membership)" in the sharing-rule object picker + * overflowed the control and collided with the field beside it. Flex children + * need `truncate` AND `min-w-0` to clip; either one alone does nothing. + */ +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { Combobox } from '../custom/combobox'; + +const LONG = '成员 (showcase_project_membership)'; +const OPTIONS = [{ value: 'showcase_project_membership', label: LONG }]; + +describe('Combobox trigger with a long label', () => { + it('renders the label in a truncating element', () => { + render(); + const label = screen.getByText(LONG); + expect(label.className).toContain('truncate'); + }); + + it('lets the trigger shrink so truncation can engage', () => { + render(); + // `truncate` is inert inside a flex item that refuses to shrink below its + // content, which is what `min-w-0` on the trigger unlocks. + expect(screen.getByRole('combobox').className).toContain('min-w-0'); + }); + + it('lets a consumer widen the trigger — the 200px default must not win', () => { + render( + , + ); + const cls = screen.getByRole('combobox').className; + expect(cls).toContain('w-full'); + expect(cls).not.toContain('w-[200px]'); + }); + + it('truncates the placeholder too', () => { + render(); + expect(screen.getByText(LONG).className).toContain('truncate'); + }); +}); diff --git a/packages/components/src/custom/combobox.tsx b/packages/components/src/custom/combobox.tsx index 63565f91e4..a1e73a2ee8 100644 --- a/packages/components/src/custom/combobox.tsx +++ b/packages/components/src/custom/combobox.tsx @@ -62,16 +62,24 @@ export function Combobox({ variant="outline" role="combobox" aria-expanded={open} - className={cn("w-[200px] justify-between", className)} + className={cn("w-[200px] min-w-0 justify-between", className)} disabled={disabled} > - {value - ? options.find((option) => option.value === value)?.label - : placeholder} + {/* The label is a flex child, so it needs `truncate` + `min-w-0` of + its own — without them a long option ("成员 + (showcase_project_membership)") rendered straight past the + button's border instead of ellipsizing (objectstack#3821). */} + + {value + ? options.find((option) => option.value === value)?.label + : placeholder} + - + {/* Match the trigger instead of a hardcoded 200px: a combobox widened by + its consumer had its own options clipped in the dropdown. */} + diff --git a/packages/fields/src/widgets/ObjectRefField.tsx b/packages/fields/src/widgets/ObjectRefField.tsx index 1b071efdd9..22d4ec2b9a 100644 --- a/packages/fields/src/widgets/ObjectRefField.tsx +++ b/packages/fields/src/widgets/ObjectRefField.tsx @@ -100,7 +100,10 @@ export function ObjectRefField({ searchPlaceholder={t('fields.objectRef.search')} emptyText={objects === null ? t('common.loading') : t('fields.objectRef.empty')} disabled={disabled} - className={cn(className)} + // Fill the form column like every other input. Combobox's own + // `w-[200px]` is a component default that left this control stranded at + // a third of the row while `名称` beside it ran full width. + className={cn('w-full', className)} /> ); } diff --git a/packages/fields/src/widgets/RecipientPickerField.tsx b/packages/fields/src/widgets/RecipientPickerField.tsx index e2162cc79d..8801123eba 100644 --- a/packages/fields/src/widgets/RecipientPickerField.tsx +++ b/packages/fields/src/widgets/RecipientPickerField.tsx @@ -162,7 +162,7 @@ export function RecipientPickerField({ searchPlaceholder={t('fields.recipient.search')} emptyText={records === null ? t('fields.recipient.loading') : t('fields.recipient.empty')} disabled={disabled} - className={className} + className={cn('w-full', className)} /> ); }