Skip to content

Commit e5a24dd

Browse files
fix(app-shell): a failed comments or activity read on a record page shows a load-failure notice with a Retry, not the empty state (#29358)
domain:ui#1 · zhuangjianguo · session_01QsR2jUJ19MstFvFhjgiYTu <!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9Ubb3KMJXK7TVJeUmNs3XdWYn3)_ Fixes #29188 Clause-②: yes (widening) Metadata-protocol: none ## 维护者速读 - **改了什么**:记录页的评论读取或活动读取失败(5xx 或网络断开)时,讨论面板、「动态」面板和「动态」标签页不再显示「暂无评论」/「暂无活动记录」,而是显示「无法加载此记录的评论/活动」并带「重试」按钮;读取失败期间「动态」标签不再显示数字 0。重试只重新读取失败的那一半。 - **为什么改**:staging 上托管环境的评论读取持续返回 500(cloud#3485),页面却显示「暂无评论」,用户以为自己写的评论丢了,故障只能在开发者工具里看到。 - **风险与回滚**:低。被拒绝(401/403)、空结果、404、接口被对象禁用和 400 的处理都与之前相同;只有 5xx 和无状态的网络失败走新状态。`@object-ui/react` 的 `DiscussionContextValue` 和时间线组件新增三个可选成员,均为加宽。附件面板 main 上已经能显示失败与重试,本 PR 只加了测试。回滚即 revert 本 PR。 - **先看**:① `ui/packages/app-shell/src/views/RecordDetailView.tsx` 的 `feedReadVerdictOf` 与 `handleRetryFeed`,以及 feed effect 里的 `retrying` 分支;② `ui/packages/plugin-detail/src/RecordActivityTimeline.tsx` 的 `failedNotice`;③ `ui/packages/plugin-detail/src/renderers/record-activity.tsx` 的计数置空。 - **两分钟验证**:`pnpm --dir ui exec vitest run packages/app-shell/src/views/RecordDetailView.feedLoadFailure-29188.test.tsx`,预期 10 个测试全绿,覆盖评论 500、活动 500、重试、空结果对照、拒绝对照和附件 500。 - **需要你做的**:无,席位落地。 ## What changes A record page's comments read and activity read each end in one of three verdicts per source: `answered`, `refused` or `failed`. Before this change a read could only be refused or not, so a 5xx fell through to the empty state. - **`failed`** is a 5xx, or a rejection with no status at all (the request never got an answer): `classifyLoadError`'s `network`, minus the 4xx statuses it also folds there (`feedReadVerdictOf` in `RecordDetailView.tsx`). A 404 (a table the deployment does not serve), an object that withholds its API (`api-disabled`) and a 400 (`rejected`) stay `answered`, as before. `refused` is unchanged (`isRefusedFeedRead`, objectui#11195). - **The notice.** `RecordActivityTimeline` draws "We couldn't load the comments for this record." / "…the activity…" (zh: 无法加载此记录的评论。/ 活动。) with a Retry button, in place of the empty state when no rows came back and above the rows when some did. This applies to the panel, the 动态 tab (`stream`) and the 动态 panel (`peek`). Only the i18n sentence is rendered: no status code, no server text. The header count, the collapsed chatter label and the 动态 count (`record:activity`'s reported count and `RecordTimelineCountProbe`) are unknown (no number) while a read has failed, as for a refusal. - **Retry** (`handleRetryFeed`) re-reads only the failed sources, directly through `dataSource.find`, never the page payload (which already answered). It does not touch the stream count the payload gave. The button is disabled while the re-read is in flight over rows on screen, and a retry that fails again keeps the notice. - **Wiring.** `DiscussionContextValue` (`@object-ui/react`) gains the optional `activityFailed`, `commentsFailed` and `onRetryFeed`. `record:activity` and `record:chatter` forward them, and `RecordChatterPanel` and `RecordActivityTimeline` take them as optional props. A host that sets none of them renders exactly as before. - **Locale keys**: `detail.activityLoadFailed`, `detail.commentsLoadFailed` and `detail.retryLoadFeed` in all ten packs (en, zh, ar, de, es, fr, ja, ko, pt, ru) at the en place, worded as each pack's `attachmentsLoadFailed` / `retryLoadAttachments`, plus the provider-less defaults in `useDetailTranslation.ts`. The eight non-en/zh packs came in claim amendment 2, after `all-locales-key-parity.test.ts` went red on Test UI 14/16 at 89ec072. - **Attachments**: no code change. Measured on main, a 500 on the attachments read (payload part or direct GET) already shows `RecordAttachmentsPanel`'s "We couldn't load the attachments for this record." with a Retry, and the count store holds no value, so no badge is drawn. The payload path is now pinned. ## Contract review Seat self-review, per the 10:43Z triage ruling. Every addition is optional and additive; no member is removed, renamed or narrowed. | Surface | Addition | Default when absent | |---|---|---| | `DiscussionContextValue` (`@object-ui/react`) | `activityFailed?: boolean`, `commentsFailed?: boolean`, `onRetryFeed?: () => void` | no failure state, no Retry | | `RecordActivityTimelineProps` (`@object-ui/plugin-detail`) | the same three props | `false`, `false`, no Retry button | | `RecordChatterPanelProps` (`@object-ui/plugin-detail`) | the same three props, forwarded | as above | | `detail` locale namespace (`@object-ui/i18n`) | `activityLoadFailed`, `commentsLoadFailed`, `retryLoadFeed`, in all ten packs | n/a (every pack carries them) | No spec, metadata or wire shape changes; the server's page payload is read as before. ## Pins changed - `RecordDetailView.feedRefusal-11195.test.tsx`, "a 500 is not \"no permission\"": an activity 500 now shows the load-failure notice instead of the empty state. It still asserts the no-permission line is absent. - `RecordDetailView.feedLoading.test.tsx` (claim amendment 1, test only): its rejections were bare `new Error('403 FEEDS_DISABLED')` / `new Error('404 sys_activity')` with no status. A statusless rejection is what a network failure looks like, so three rows went red. Each rejection now carries the status its message names, in the ObjectStack client's shape (`httpStatus`, `code`), as the 11195 test already does. - "leaves the loading state when a feed read REJECTS (both rejecting)": the comment read is a real 403 now, so the panel settles on the comments no-permission line (objectui#11195's behaviour for a 403 since that card) instead of the empty copy. The no-spinner assertion is unchanged. - "leaves the loading state when only one read rejects and the other resolves empty": the activity 404 carries its status and keeps the empty copy. Assertions unchanged. - "leaves the loading state when both reads reject" (authored discussion node): the same as the first row. ## Tests - New `RecordDetailView.feedLoadFailure-29188.test.tsx` (10 tests). It uses the real `ObjectStackAdapter` over a held transport, the `RecordDetailView.recordOpenOneRequest-28470` rig, under StrictMode. It covers a comments 500 as a payload part and as a direct GET (notice, Retry, no empty copy, no leak, 动态 tab with no count), Retry re-reading only `sys_comment` directly (no activity re-read, the notice gone once it answers), a retry that fails again, an activity 500 both ways, the empty-200 controls both ways, the 403 refusal control (no-permission line, no Retry) and the attachments payload 500 pin. - New `RecordActivityTimeline.loadFailed-29188.test.tsx` (9 tests), through the real en and zh packs and the `record:chatter` block under a host `DiscussionContext`, plus the `peek` face. It covers the notice in place of "No comments yet" (zh: 无法加载此记录的评论。, never 暂无评论), Retry calling the host, rows kept with the notice above them, the activity line, no Retry when the host offers none, a refusal and a failure side by side, the empty control, and the disabled Retry while loading. - **Measured before the change**, on main db6b408, with a temporary copy of the 28470 rig (deleted before code): comments 500 → "No activity recorded" and an `Activity0` tab badge; attachments 500 → the panel's unavailable state with Retry and no count. - **Ablation** (`scripts/ablation-replace.mjs`, `feedReadVerdictOf` forced back to `answered` for a 5xx): 7 of 17 red across the new page test and the 11195 file. Those are the six failure rows and the flipped 11195 row; the controls, the refusal row and the attachments pin stayed green. Restored: blob equals HEAD, `git diff HEAD` empty. - **Related sweep** from `ui/` under `scripts/pm/os-verify-lock.sh`, `--maxWorkers=2`, at merged head 89ec072: the 109 test files that name the timeline, the chatter panel, `DiscussionContext`, the feed renderers or the refusal flags, plus the `RecordDetailView` tests that read `sys_comment` / `sys_activity` / the payload / the timeline, plus the two new files. 111 files, 2609 tests, exit 0. `apps/console/src/__tests__/record-block-record-reach.test.tsx`: 13 passed. - `pnpm exec eslint` on the 13 changed files: 0 errors; the warnings are the files' existing `any` and hook warnings. - At 53d0b77 (the eight locale packs): `packages/i18n/` suite 95 files, 1443 passed, 13 skipped, exit 0; `all-locales-key-parity.test.ts` with the timeline test: 54 passed. ## Acceptance notes - Claim 6576137247, amendment 1 (`RecordDetailView.feedLoading.test.tsx`, test only) and amendment 2 (the eight other locale packs). The diff stays inside them. - The verdict reads `classifyLoadError`'s `network` kind and then excludes the 4xx statuses that kind also holds, so a status-carrying 404 keeps the empty state the feedLoading pins describe. This matches the claim's "5xx or network" reading. - An object that withholds its API (`api-disabled`) or answers 400 still shows the empty state, as the claim keeps. Neither is retryable, so a Retry would be wrong advice there. - Premise check: the issue's attachments half does not reproduce on main (see Tests); it is pinned rather than changed. ## Verification - `node scripts/pm/dispatch-gates.mjs --ran` at 89ec072: 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN; every recorded exit is 0. - `pnpm --dir ui exec vitest run scripts/` at 89ec072: 169 files passed, 2 skipped (4903 tests). `node ui/scripts/check-new-cross-file-line-citations.mjs`: 0 new citations. - `pnpm exec turbo run type-check --filter=@object-ui/app-shell --filter=@object-ui/react --filter=@object-ui/i18n --filter=@object-ui/plugin-detail --concurrency=2` at 89ec072: exit 0, 68 of 68 tasks. - `node scripts/check-sdui-manifest.mjs`: exit 0. - `node scripts/check-changeset-no-major.mjs --base origin/main`, `node scripts/check-adr-0087-registration.mjs --base origin/main`, `node scripts/check-closing-keyword-parity.mjs` and `node scripts/check-partof-closing-keyword.mjs`, with this body as the event: exit 0 each. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. (CI runs the repo lint) - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (not touched) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QsR2jT854xvoBjntpsRE1F --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ac4715a commit e5a24dd

22 files changed

Lines changed: 824 additions & 64 deletions
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
'@object-ui/plugin-detail': patch
4+
'@object-ui/react': patch
5+
'@object-ui/i18n': patch
6+
---
7+
8+
fix(app-shell): a record page whose comments or activity read fails (5xx, network) says it could not load them, with a Retry, instead of showing the empty state
9+
10+
Clause-②: yes (widening)
11+
Metadata-protocol: none
12+
13+
On a record page, a comments read or an activity read that answered a server error used to settle the discussion panel on its empty state (「暂无评论」, "No activity recorded"), and the 动态 tab's badge on 0. A comment users wrote looked lost, and an outage was visible only in developer tools.
14+
15+
- **A failed read has its own state.** A 5xx, or a request that never got an answer, now shows "We couldn't load the comments for this record." (or the activity) with a Retry, in place of the empty state, on the 动态 panel, the 动态 tab and the discussion panel. Rows the other read answered stay on screen with the notice above them, and the count is unknown (no number) until the read answers.
16+
- **Retry reads only what failed**, directly, never the page payload again. A retry that fails again keeps the notice.
17+
- **Unchanged:** a refusal (401 / 403) keeps its no-permission state and is never retried; an empty 200, a 404 (a table the deployment does not serve), an object that withholds its API and a 400 keep the empty state.
18+
- **New public members.** `DiscussionContextValue` gains the optional `activityFailed`, `commentsFailed` and `onRetryFeed`; `RecordActivityTimeline` and `RecordChatterPanel` gain the same three optional props; the `detail` locale namespace gains `activityLoadFailed`, `commentsLoadFailed` and `retryLoadFeed` in all ten packs.
19+
- The attachments panel already showed its own load-failure state with a Retry for a failed read; that is now pinned for the record page payload's attachments part too.

‎ui/packages/app-shell/src/views/RecordDetailView.feedLoadFailure-29188.test.tsx‎

Lines changed: 362 additions & 0 deletions
Large diffs are not rendered by default.

‎ui/packages/app-shell/src/views/RecordDetailView.feedLoading.test.tsx‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,13 @@
3737
* 1. during the fetch → loading row, no empty copy;
3838
* 2. after BOTH parallel reads settle with nothing → empty copy, no
3939
* spinner (one settled read is not an answer);
40-
* 3. after a read REJECTS → still the empty copy. A rejected fetch is an
41-
* answer too; the alternative is a panel that spins forever, which is
42-
* strictly worse than the bug being fixed.
40+
* 3. after a read REJECTS → no spinner. A rejected fetch is an answer too;
41+
* the alternative is a panel that spins forever, which is strictly worse
42+
* than the bug being fixed. A 404 settles on the empty copy; a refused
43+
* read (403) on its no-permission line (objectui#11195). Each rejection
44+
* carries the status the ObjectStack client puts on it: a rejection with
45+
* no status is a network failure, which shows the load-failure notice
46+
* (objectstack#29188).
4347
*/
4448

4549
import * as React from 'react';
@@ -106,6 +110,12 @@ const OBJECTS = [
106110

107111
/** A promise that never settles — "the fetch is still in flight". */
108112
const pending = () => new Promise(() => {});
113+
/**
114+
* A rejected read in the shape the ObjectStack client throws it: the HTTP
115+
* status on `httpStatus` and the ADR-0112 code on `code`.
116+
*/
117+
const rejection = (httpStatus: number, code: string) =>
118+
Object.assign(new Error(`${httpStatus} ${code}`), { httpStatus, code });
109119

110120
interface FeedResponses {
111121
sys_comment: () => Promise<any>;
@@ -294,20 +304,23 @@ describe('RecordDetailView — the feed fetch produces a loading signal (#3209)'
294304
// this fix would trade "wrongly says empty" for "spins forever", which is
295305
// the worse bug.
296306
const dataSource = makeDataSource({
297-
sys_comment: () => Promise.reject(new Error('403 FEEDS_DISABLED')),
298-
sys_activity: () => Promise.reject(new Error('404 sys_activity')),
307+
sys_comment: () => Promise.reject(rejection(403, 'FEEDS_DISABLED')),
308+
sys_activity: () => Promise.reject(rejection(404, 'OBJECT_NOT_FOUND')),
299309
});
300310

301311
renderDetail(dataSource);
302312

303-
expect(await screen.findByText(EMPTY_COMMENTS)).toBeTruthy();
313+
// The refused comment read settles on its no-permission line, the 404 on
314+
// nothing: an answer either way, never a spinner.
315+
const notice = await screen.findByTestId('activity-access-denied');
316+
expect(notice.querySelector('[data-denied-source="comments"]')).not.toBeNull();
304317
expect(screen.queryByTestId('activity-loading')).toBeNull();
305318
});
306319

307320
it('leaves the loading state when only one read rejects and the other resolves empty', async () => {
308321
const dataSource = makeDataSource({
309322
sys_comment: () => Promise.resolve({ data: [] }),
310-
sys_activity: () => Promise.reject(new Error('404 sys_activity')),
323+
sys_activity: () => Promise.reject(rejection(404, 'OBJECT_NOT_FOUND')),
311324
});
312325

313326
renderDetail(dataSource);
@@ -329,7 +342,7 @@ describe('RecordDetailView — the feed fetch produces a loading signal (#3209)'
329342
},
330343
],
331344
}),
332-
sys_activity: () => Promise.reject(new Error('404 sys_activity')),
345+
sys_activity: () => Promise.reject(rejection(404, 'OBJECT_NOT_FOUND')),
333346
});
334347

335348
renderDetail(dataSource);
@@ -368,13 +381,14 @@ describe('RecordDetailView — an AUTHORED discussion node is on the same chain
368381

369382
it('leaves the loading state when both reads reject', async () => {
370383
const dataSource = makeDataSource({
371-
sys_comment: () => Promise.reject(new Error('403 FEEDS_DISABLED')),
372-
sys_activity: () => Promise.reject(new Error('404 sys_activity')),
384+
sys_comment: () => Promise.reject(rejection(403, 'FEEDS_DISABLED')),
385+
sys_activity: () => Promise.reject(rejection(404, 'OBJECT_NOT_FOUND')),
373386
});
374387

375388
renderDetail(dataSource, [AUTHORED_PAGE_WITH_DISCUSSION]);
376389

377-
expect(await screen.findByText(AUTHORED_EMPTY_COMMENTS)).toBeTruthy();
390+
const notice = await screen.findByTestId('activity-access-denied');
391+
expect(notice.querySelector('[data-denied-source="comments"]')).not.toBeNull();
378392
expect(screen.queryByTestId('activity-loading')).toBeNull();
379393
});
380394
});

‎ui/packages/app-shell/src/views/RecordDetailView.feedRefusal-11195.test.tsx‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
* 2. the control: a 200 with zero rows shows the empty state, unchanged;
1818
* 3. a 404 is not a refusal, so the empty state stays;
1919
* 4. a 500 is not a refusal either, so the panel never says "no permission"
20-
* for it;
20+
* for it; since objectstack#29188 it shows the load-failure notice, not
21+
* the empty state;
2122
* 5. a refused `sys_comment` read beside an answered `sys_activity` read
2223
* keeps the activity rows and says the comments are withheld.
2324
*
@@ -260,9 +261,11 @@ describe('RecordDetailView: a refused sys_activity read shows the no-permission
260261

261262
renderDetail(dataSource);
262263

263-
// The panel has no error state of its own, so a failure that is not a
264-
// refusal still settles on the empty state.
265-
expect(await screen.findByText(EMPTY_COMMENTS)).toBeTruthy();
264+
// objectstack#29188: a failure that is not a refusal shows the panel's
265+
// load-failure notice, never the empty state and never "no permission".
266+
const notice = await screen.findByTestId('activity-load-failed');
267+
expect(notice.querySelector('[data-failed-source="activity"]')).not.toBeNull();
268+
expect(screen.queryByText(EMPTY_COMMENTS)).toBeNull();
266269
expect(screen.queryByTestId('activity-access-denied')).toBeNull();
267270
expect(screen.queryByText(ACTIVITY_DENIED)).toBeNull();
268271
});

0 commit comments

Comments
 (0)