Skip to content

Commit 64e5efd

Browse files
fix(ui): draw a refused list or related-list read as no-access, not a retryable failure (#27856)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgDFH4S5pR1C8YvYCWWsZfXx)_ ## 维护者速读 - 改了什么:列表读取被拒(403)时不再显示「重试」按钮;记录页相关列表读取被拒时改画一行中性的「无权访问」提示,不再是红框加「你无权执行此操作」。 - 为什么改:成员只是没有读权限,重试不可能变好;红色的动作拒绝句让只来查看记录的人以为自己做错了事。 - 风险与回滚:只动两个组件的拒绝态分支,5xx、网络失败、api-disabled 的画法不变;回滚即还原本 PR 两个源文件。 - 先看:`ui/packages/plugin-detail/src/RelatedList.tsx` 的 `RelatedListLoadFailed`;`ui/packages/plugin-list/src/ListView.tsx` 的 `list-error-retry` 条件。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/plugin-list/src/__tests__/ListView.forbiddenNoRetry.test.tsx packages/plugin-detail/src/__tests__/RelatedList.readFailed-26415.test.tsx`,预期全绿。 - 需要你做的:无,席位落地。 Fixes #27841 Clause-②: no Metadata-protocol: none ## What changes - **List view (`plugin-list` `ListView`).** The load-error panel no longer draws the Retry button for the `forbidden` kind (a 403 / `PERMISSION_DENIED` read), joining `api-disabled`. A refusal is a grant only an administrator can give, so a retry re-reaches the same refusal. `network` (network failures and 5xx), `unauthorized` and `rejected` keep Retry. - **Related list (`plugin-detail` `RelatedListLoadFailed`).** A `forbidden` read is drawn as a neutral line: `role="status"` instead of `alert`, `border-border bg-muted/40` and a muted icon instead of the destructive tint, no Retry, and always the list's existing no-access sentence (`list.loadErrorForbiddenTitle` / `list.loadErrorForbiddenMessage`). A producer's `userMessage` on a 403 is no longer shown there: the server marks the generic action refusal ("You do not have permission to perform this action. Contact your administrator if you need access.") on a refused read, which was the red sentence the card saw. Every other kind is drawn exactly as before. - No new i18n key; no copy changed. ## Tests - New `plugin-list/src/__tests__/ListView.forbiddenNoRetry.test.tsx`: a 403 draws the no-access panel with no Retry; controls: a 500 and a network failure still draw Retry. - `plugin-detail/src/__tests__/RelatedList.readFailed-26415.test.tsx` gains a describe: a 403 carrying the action-refusal `userMessage` draws `role="status"`, no destructive class on the line or its icon, the no-access title and sentence, no "perform this action", no Retry; controls: a 5xx keeps `role="alert"`, the destructive styling and Retry; a readable related list draws its rows. - `ListView.errorStateComponent-7143.test.tsx`: the "retry button is still rendered" arm now fails with a 500 instead of a 403, since a 403 no longer offers one. ## Acceptance notes - **Repro on main = the ablation.** Both source files restored to the base `38651598` (tests unchanged): the two new refusal tests fail (ListView: Retry button present; RelatedList: `role="alert"` received where `status` expected), and their 14 sibling tests, the controls included, pass. Restored by `git checkout HEAD`, `git diff HEAD` empty, blob hashes equal HEAD. The red action sentence on main is read from the code (`declaredUserMessage` wins over the fixed copy) and is asserted absent by the new test on the fixed tree; it was not measured on a live stack. - `unauthorized` (401, "Sign in required") still offers Retry in both places. The card names 403 only; left as is. - The ListView forbidden panel still uses its `text-destructive/60` icon; the card asks only that it drop Retry. - Seat 4's draft PR #27793 regions (`RelatedList.tsx` docblock near the API-door note, `RecordDetailView.tsx` `apiHandler`) are not touched; this diff edits only `RelatedListLoadFailed` and its docblock. ## Verification All readings on head `fa457cd3` (branch merged with `origin/main`). <details><summary>Tests and type-check</summary> - `pnpm --dir ui exec vitest run --maxWorkers=2` on `ListView.forbiddenNoRetry.test.tsx`, `ListView.errorStateComponent-7143.test.tsx`, `ListView.loadErrorKind.test.tsx`, `RelatedList.readFailed-26415.test.tsx`: Test Files 4 passed (4), Tests 39 passed (39); os-verify-lock VERDICT command-exit 0. - Ablation (base source, new tests): Tests 2 failed, 14 passed (16); VERDICT command-exit 1; the two failures are the two refusal tests. - `pnpm --filter @object-ui/plugin-list --filter @object-ui/plugin-detail run type-check` (tsc --noEmit and tsconfig.test.json): VERDICT command-exit 0, on the pre-merge head; the merge brought nothing under these packages. </details> <details><summary>Gates</summary> - `dispatch-gates --commands` derived 34 families; all 34 ran, exit 0. `--ran` reconciliation: 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN. `node scripts/check-sdui-manifest.mjs` among them, exit 0. - `pnpm check:new-line-citations` (ui): 0 new citation(s), exit 0. - Repo-wide families and Test UI belong to CI; see this PR's check-runs. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CXjatjJmMJkdAQoyqn)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d07e8d1 commit 64e5efd

6 files changed

Lines changed: 163 additions & 12 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/plugin-list': patch
3+
'@object-ui/plugin-detail': patch
4+
---
5+
6+
fix(ui): a refused list or related-list read says "no access" and offers no Retry
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
- **List view.** A permission refusal (403) on a list read draws the no-access panel without the Retry button, the same way an object whose API is switched off already did: a retry re-reaches the same refusal. A network failure or a 5xx keeps Retry.
12+
- **Related list.** A related list the reader may not read draws a quiet, neutral line (`role="status"`, no destructive styling) with the list's no-access sentence, and no Retry. It no longer shows the generic action refusal ("You do not have permission to perform this action.") in red to someone who only opened the record. Other read failures are drawn as before.

‎ui/packages/plugin-detail/src/RelatedList.tsx‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -577,31 +577,41 @@ function serverSentence(err: unknown): string | null {
577577
* the shared fixed sentence and never the server's text (a refused read does
578578
* not name what it cannot see), unless the producer marked a `userMessage` for
579579
* the reader; every other failure says the server's sentence when it gave one.
580-
* Retry for every kind but `api-disabled`, whose verdict a retry cannot change
581-
* — the ListView precedent.
580+
* Retry for every kind but `api-disabled` and `forbidden`, whose verdicts a
581+
* retry cannot change — the ListView precedent.
582+
*
583+
* A `forbidden` read is not a failure of anything the reader did: it is drawn
584+
* as a quiet neutral line (`role="status"`, no destructive styling) and always
585+
* says the list's no-access sentence. A producer's `userMessage` on a 403 is
586+
* the generic action refusal ("...perform this action"), which reads as a
587+
* rebuke to someone who only opened the record to read it.
582588
*/
583589
const RelatedListLoadFailed: React.FC<{ error: unknown; onRetry: () => void }> = ({ error, onRetry }) => {
584590
const { t } = useDetailTranslation();
585591
const kind = classifyLoadError(error);
586592
const copy = LOAD_FAILED_COPY[kind];
593+
const refused = kind === 'forbidden';
587594
const message =
588-
declaredUserMessage(error)
595+
(refused ? null : declaredUserMessage(error))
589596
?? (copy.message
590597
? t(copy.message[0], { defaultValue: copy.message[1] })
591598
: serverSentence(error)
592599
?? t('list.loadErrorMessage', { defaultValue: 'Something went wrong while loading this data. Check your connection and try again.' }));
593600
const Icon = kind === 'network' || kind === 'rejected' ? AlertTriangle : ShieldAlert;
594601
return (
595602
<div
596-
role="alert"
603+
role={refused ? 'status' : 'alert'}
597604
data-testid="related-list-load-failed"
598605
data-error-kind={kind}
599-
className="flex flex-wrap items-center gap-x-2 gap-y-1 rounded-md border border-destructive/30 bg-destructive/5 px-3 py-2 text-sm"
606+
className={cn(
607+
'flex flex-wrap items-center gap-x-2 gap-y-1 rounded-md border px-3 py-2 text-sm',
608+
refused ? 'border-border bg-muted/40' : 'border-destructive/30 bg-destructive/5',
609+
)}
600610
>
601-
<Icon className="h-4 w-4 shrink-0 text-destructive/70" aria-hidden />
611+
<Icon className={cn('h-4 w-4 shrink-0', refused ? 'text-muted-foreground' : 'text-destructive/70')} aria-hidden />
602612
<span className="font-medium">{t(copy.title[0], { defaultValue: copy.title[1] })}</span>
603613
<span className="min-w-0 text-muted-foreground">{message}</span>
604-
{kind !== 'api-disabled' && (
614+
{kind !== 'api-disabled' && !refused && (
605615
<Button
606616
variant="outline"
607617
size="sm"

‎ui/packages/plugin-detail/src/__tests__/RelatedList.readFailed-26415.test.tsx‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,3 +281,58 @@ describe('record:related_list — a failed list keeps its own row in the stack f
281281
expect(quotesTitle).toBeVisible();
282282
});
283283
});
284+
285+
/*
286+
* A refused read is not a failure of anything the reader did. The line for a
287+
* 403 is neutral (`role="status"`, no destructive styling), offers no Retry (a
288+
* grant only an administrator can give does not change on a retry), and says
289+
* the list's no-access sentence even when the producer marked the generic
290+
* action refusal as the `userMessage`. The 5xx case above is the control for
291+
* the destructive styling and the Retry.
292+
*/
293+
describe('RelatedList — a refused read is a neutral no-access line, not a failure', () => {
294+
/** The 403 the server answers a refused read: the generic action refusal marked for the reader. */
295+
const ACTION_REFUSAL =
296+
'You do not have permission to perform this action. Contact your administrator if you need access.';
297+
const refused = () =>
298+
Object.assign(serverError(403, '[Security] Access denied on invoice_line.invoice', 'PERMISSION_DENIED'), {
299+
userMessage: ACTION_REFUSAL,
300+
});
301+
302+
it('draws a neutral status line with the no-access sentence and no Retry', async () => {
303+
const find = vi.fn(async () => {
304+
throw refused();
305+
});
306+
renderAdapterList(find);
307+
308+
const line = await screen.findByTestId('related-list-load-failed');
309+
expect(line).toHaveAttribute('data-error-kind', 'forbidden');
310+
expect(line).toHaveAttribute('role', 'status');
311+
expect(line.className).not.toMatch(/destructive/);
312+
expect(line.querySelector('svg')!.getAttribute('class')).not.toMatch(/destructive/);
313+
expect(line).toHaveTextContent(FORBIDDEN_TITLE);
314+
expect(line).toHaveTextContent(FORBIDDEN_MESSAGE);
315+
expect(line).not.toHaveTextContent('perform this action');
316+
expect(within(line).queryByRole('button', { name: /Retry/ })).toBeNull();
317+
});
318+
319+
it('CONTROL: a 5xx keeps the alert styling and the Retry', async () => {
320+
const find = vi.fn(async () => {
321+
throw serverError(500, 'The database did not answer in time.');
322+
});
323+
renderAdapterList(find);
324+
325+
const line = await screen.findByTestId('related-list-load-failed');
326+
expect(line).toHaveAttribute('role', 'alert');
327+
expect(line.className).toMatch(/destructive/);
328+
expect(within(line).getByRole('button', { name: /Retry/ })).toBeInTheDocument();
329+
});
330+
331+
it('CONTROL: a readable related list draws its rows', async () => {
332+
const find = vi.fn(async () => ({ data: [{ id: 'l1', name: 'Consulting hours' }] }));
333+
renderAdapterList(find);
334+
335+
await screen.findByText('Consulting hours');
336+
expect(failedLine()).toBeNull();
337+
});
338+
});

‎ui/packages/plugin-list/src/ListView.tsx‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6431,11 +6431,14 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
64316431
the empty state. `DataErrorState` grew three icon props for this
64326432
migration and no fourth — an `action` prop would be a second
64336433
spelling of an affordance it already has. */}
6434-
{loadErrorKind === 'api-disabled' ? null : (
6435-
// No Retry for an `enable`-block denial. The verdict is a pure
6436-
// function of the object's metadata, so every retry re-fetches
6437-
// the identical refusal — offering the button is the same wrong
6434+
{loadErrorKind === 'api-disabled' || loadErrorKind === 'forbidden' ? null : (
6435+
// No Retry for an `enable`-block denial or a permission refusal
6436+
// (403). Neither verdict is one a retry can change (the first is
6437+
// a pure function of the object's metadata, the second a grant
6438+
// only an administrator can give), so every retry re-fetches the
6439+
// identical refusal — offering the button is the same wrong
64386440
// advice as "check your connection", just spelled as a control.
6441+
// A transient failure (network, 5xx) keeps it.
64396442
<Button
64406443
variant="outline"
64416444
size="sm"

‎ui/packages/plugin-list/src/__tests__/ListView.errorStateComponent-7143.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ describe('ListView — the load failure is a DataErrorState (#7143)', () => {
107107
});
108108

109109
it('UNCHANGED: the retry button is still rendered inside the panel', async () => {
110-
const { container } = failing(forbidden());
110+
const { container } = failing(Object.assign(new Error('Internal error'), { httpStatus: 500 }));
111111
const box = await panel(container, 'list-error-state');
112112
const retry = box.querySelector('[data-testid="list-error-retry"]');
113113
expect(retry).not.toBeNull();
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*
8+
* A permission refusal (403) on a list read offers no Retry: the grant is one
9+
* only an administrator can give, so a retry re-reaches the same refusal. A
10+
* transient failure (5xx, network) keeps the Retry.
11+
*/
12+
13+
import { describe, it, expect, vi } from 'vitest';
14+
import { render, waitFor } from '@testing-library/react';
15+
import { ListView } from '../ListView';
16+
import { SchemaRendererProvider } from '@object-ui/react';
17+
import type { ListViewSchema } from '@object-ui/types';
18+
19+
const schema: ListViewSchema = {
20+
type: 'list-view',
21+
objectName: 'invoice',
22+
fields: ['name'],
23+
};
24+
25+
function renderFailing(error: unknown) {
26+
const ds = {
27+
find: vi.fn().mockRejectedValue(error),
28+
findOne: vi.fn(),
29+
create: vi.fn(),
30+
update: vi.fn(),
31+
delete: vi.fn(),
32+
};
33+
return render(
34+
<SchemaRendererProvider dataSource={ds as any}>
35+
<ListView schema={schema} dataSource={ds as any} />
36+
</SchemaRendererProvider>,
37+
);
38+
}
39+
40+
async function errorState(container: HTMLElement): Promise<HTMLElement> {
41+
await waitFor(() => {
42+
expect(container.querySelector('[data-testid="list-error-state"]')).not.toBeNull();
43+
});
44+
return container.querySelector('[data-testid="list-error-state"]') as HTMLElement;
45+
}
46+
47+
describe('ListView — a permission refusal offers no Retry', () => {
48+
it('a 403 draws the no-access panel without Retry', async () => {
49+
const err = Object.assign(new Error('Forbidden'), { httpStatus: 403, code: 'PERMISSION_DENIED' });
50+
const { container } = renderFailing(err);
51+
const panel = await errorState(container);
52+
expect(panel.getAttribute('data-error-kind')).toBe('forbidden');
53+
expect(panel.querySelector('[data-testid="list-error-retry"]')).toBeNull();
54+
// "No Retry" is not reached by drawing nothing: the panel still says why.
55+
expect(panel.textContent).toMatch(/permission/i);
56+
});
57+
58+
it('CONTROL: a 500 still draws Retry', async () => {
59+
const err = Object.assign(new Error('Internal error'), { httpStatus: 500 });
60+
const { container } = renderFailing(err);
61+
const panel = await errorState(container);
62+
expect(panel.getAttribute('data-error-kind')).toBe('network');
63+
expect(panel.querySelector('[data-testid="list-error-retry"]')).not.toBeNull();
64+
});
65+
66+
it('CONTROL: a network failure still draws Retry', async () => {
67+
const { container } = renderFailing(new TypeError('Failed to fetch'));
68+
const panel = await errorState(container);
69+
expect(panel.querySelector('[data-testid="list-error-retry"]')).not.toBeNull();
70+
});
71+
});

0 commit comments

Comments
 (0)