Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions .changeset/sharing-rule-widgets-i18n-and-orderby.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
---
"@object-ui/core": patch
"@object-ui/types": patch
"@object-ui/fields": patch
"@object-ui/i18n": patch
"@object-ui/components": patch
"@object-ui/plugin-detail": patch
"@object-ui/permissions": patch
---

fix(core,fields): a string `$orderby` is a clause, not a character array — and localize the sharing-rule widgets (objectstack#3821)

**The recipient picker listed nothing, ever.** `QueryParams['$orderby']` was
typed as `Record | string[] | SortObject[]`, so `queryParamsToRecord` sent any
non-array value through `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,3 e,4 ,5 a,6 s,7 c`. The server sorted by columns
that don't exist and every row was filtered out, so
`sys_sharing_rule.recipient_id` rendered "No matches" for every recipient type
and no sharing rule could be created from the Console. `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 server's OData
normalizer has always parsed `'name asc'`), and the type admits `string`.
`RecipientPickerField` additionally switched to the structured
`{ name: 'asc' }` form so it can't regress this way against any data source.

**The three sharing-rule authoring widgets never had translations.**
`ObjectRefField`, `RecipientPickerField` and `FilterConditionField` hardcoded
their English copy — a Chinese Console showed "Select an object", "Select a
user", "Search…", "No matches", "Edit as JSON". They now go through
`useFieldTranslation` like every other widget, with keys added under `fields.*`
in all ten locales.

The recipient placeholder was the interesting one: it read
`` `Select a ${recipientType.replace(/_/g,' ')}` ``, interpolating the enum
value into an English sentence — a shape no locale can translate. It is now a
per-type key (`fields.recipient.selectUser`, `…selectBusinessUnit`, …), so
"选择业务单元" and "Select a business unit" no longer have to share a structure.

**Editing a rule silently dropped its recipient.** The picker resets the stored
id when `recipient_type` changes, because an id valid for a user is meaningless
for a team. It treated the edit form's `'' → 'user'` hydration as such a change:
opening any saved rule blanked the recipient, and saving persisted the blank.
Only a non-empty predecessor now counts as a type switch.

**Building a filter submitted the surrounding form.** None of `FilterBuilder`'s
controls declared `type="button"`, and a bare `<button>` inside a `<form>`
defaults to `type="submit"`. Adding, removing or clearing a condition therefore
submitted the sharing-rule dialog — firing validation mid-edit, and on an
already-valid form saving the record before the admin was done.

**A rejected write showed the user raw server diagnostics.** The form rendered
`error.message` verbatim, so a sharing / RLS denial reached 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. Permission failures now render localized copy
(`form.noPermissionToSave`, added in all ten locales), with the server text kept
on the console for debugging; other failures still show the server's message,
which is the useful part, and fall back to `form.submitFailed` when there is
none — replacing the previously hardcoded English "An error occurred during
submission".

**The detail header offered "Edit" on records the user may only read.** Object
permissions can't express "this one record is read-only" — a read-only sharing
grant sits inside an object the user may otherwise edit — so the header showed
the primary Edit CTA, opened the form, and let the user retype a field before
the server rejected the save. `DetailView` now gates Edit / Delete on the
object-level check AND on 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 button and server cannot
disagree). Explaining oneself needs no special permission. The probe is one
cached request per record, skipped entirely when the object-level check already
says no, and **fails open** on every uncertainty — an unanswered hint must never
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
view.

Browser-verified against the framework showcase Console in Chinese: object /
criteria / recipient copy is fully localized, the recipient dropdown lists real
users, business units and positions, a saved rule reopens with its recipient and
criteria intact, editing the filter no longer submits, and a rule created
end-to-end stores a real record id rather than free text. The criteria authored
in the builder is honored by the evaluator: `{"pinned":true}` on an owner-private
object granted the recipient exactly the matching records and nothing else.
31 changes: 28 additions & 3 deletions packages/app-shell/src/views/RecordDetailView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import { useState, useEffect, useCallback, useMemo, useRef } from 'react';
import { useParams, useNavigate, useLocation, useSearchParams, Link } from 'react-router-dom';
import { RecordChatterPanel, InlineEditSaveBar, buildDefaultPageSchema, deriveFieldGroupDetailSections, extractMentions, resolveTitleField } from '@object-ui/plugin-detail';
import { RecordChatterPanel, InlineEditSaveBar, buildDefaultPageSchema, deriveFieldGroupDetailSections, extractMentions, resolveTitleField, useRecordEditable } from '@object-ui/plugin-detail';
import { Empty, EmptyTitle, EmptyDescription } from '@object-ui/components';
import { useAuth, createAuthenticatedFetch } from '@object-ui/auth';
import { usePermissions } from '@object-ui/permissions';
Expand Down Expand Up @@ -941,6 +941,26 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
[objectDef, objects, canOnObject, permissionsLoaded],
);

// [objectstack#3821] RECORD-level write gate. Everything above is object
// scoped — it cannot express "this one record is read-only", which is
// exactly what a read-only sharing grant is. So the header offered Edit on
// a record the user may only read: the form opened, the user retyped a
// field, and the server rejected the save with a 403. Ask the explain
// engine for the row-level verdict (fail-open; the server stays the
// authority per ADR-0057 D10) and fold it into the same affordance gates.
const recordWriteAllowed = useRecordEditable(
objectDef?.name,
pureRecordId,
'update',
!!objectDef?.name && !!pureRecordId,
);
const recordDeleteAllowed = useRecordEditable(
objectDef?.name,
pureRecordId,
'delete',
!!objectDef?.name && !!pureRecordId,
);

// ── Audit history fetch ────────────────────────────────────────────
// Loads recent sys_audit_log entries for this record so the record page can
// render a read-only "History" tab (`record:history`). Gated on three preconditions to keep
Expand Down Expand Up @@ -1776,7 +1796,12 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
// menu permanently — Delete must never surface as an inline red button
// just because an object has few actions.
const synthSystemActions: ActionDef[] = (() => {
const affordances = resolveRecordHeaderActionGates(objectDef, effectiveApiOperations);
const objectAffordances = resolveRecordHeaderActionGates(objectDef, effectiveApiOperations);
// Object-level gate AND the record-level verdict (objectstack#3821).
const affordances = {
edit: objectAffordances.edit && recordWriteAllowed,
delete: objectAffordances.delete && recordDeleteAllowed,
};
const items: ActionDef[] = [];
if (affordances.edit) {
// Single primary Edit CTA → opens the full record form. Inline editing
Expand Down Expand Up @@ -1921,7 +1946,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
on backends that track the lock via approval requests only and
never materialize an `approval_status` field (objectui#2618). */}
<InlineEditProvider
canEdit={resolveRecordHeaderActionGates(objectDef, effectiveApiOperations).edit && !approvalLocked}
canEdit={resolveRecordHeaderActionGates(objectDef, effectiveApiOperations).edit && recordWriteAllowed && !approvalLocked}
locked={approvalLocked}
lockedReason={t('detail.lockedTooltip', {
defaultValue: 'This record has a pending approval request; editing is locked',
Expand Down
54 changes: 54 additions & 0 deletions packages/components/src/__tests__/combobox-long-label.test.tsx
Original file line number Diff line number Diff line change
@@ -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(<Combobox options={OPTIONS} value={OPTIONS[0].value} onValueChange={vi.fn()} />);
const label = screen.getByText(LONG);
expect(label.className).toContain('truncate');
});

it('lets the trigger shrink so truncation can engage', () => {
render(<Combobox options={OPTIONS} value={OPTIONS[0].value} onValueChange={vi.fn()} />);
// `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(
<Combobox options={OPTIONS} value={OPTIONS[0].value} onValueChange={vi.fn()} className="w-full" />,
);
const cls = screen.getByRole('combobox').className;
expect(cls).toContain('w-full');
expect(cls).not.toContain('w-[200px]');
});

it('truncates the placeholder too', () => {
render(<Combobox options={OPTIONS} value="" onValueChange={vi.fn()} placeholder={LONG} />);
expect(screen.getByText(LONG).className).toContain('truncate');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/**
* 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.
*/

/**
* FilterBuilder is embedded inside forms — notably the sharing-rule dialog's
* `criteria_json` field. None of its controls declared `type="button"`, and a
* bare <button> inside a <form> defaults to `type="submit"`, so building a
* filter submitted the surrounding dialog: validation fired mid-edit, and on an
* already-valid form the record saved before the admin was done
* (objectstack#3821).
*/
import { describe, it, expect, vi } from 'vitest';
import React from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import '@testing-library/jest-dom';
import { FilterBuilder } from '../custom/filter-builder';

const FIELDS = [
{ value: 'title', label: 'Title', type: 'text' },
{ value: 'pinned', label: 'Pinned', type: 'boolean' },
];

function renderInForm(onSubmit: () => void, value?: any) {
return render(
<form onSubmit={onSubmit}>
<FilterBuilder
fields={FIELDS as any}
value={value}
onChange={vi.fn()}
showClearAll
/>
<button type="submit">save</button>
</form>,
);
}

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([]);
});
});
Original file line number Diff line number Diff line change
@@ -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<unknown>) {
const Form = ComponentRegistry.get('form')!;
// `onAction` is a PROP on the renderer, not part of the schema.
return render(<Form schema={SCHEMA} onAction={onAction} />);
}

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();
});
});
Loading
Loading