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
19 changes: 19 additions & 0 deletions .changeset/29188-feed-load-failure.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'@object-ui/app-shell': patch
'@object-ui/plugin-detail': patch
'@object-ui/react': patch
'@object-ui/i18n': patch
---

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

Clause-②: yes (widening)
Metadata-protocol: none

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.

- **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.
- **Retry reads only what failed**, directly, never the page payload again. A retry that fails again keeps the notice.
- **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.
- **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.
- 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.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,13 @@
* 1. during the fetch → loading row, no empty copy;
* 2. after BOTH parallel reads settle with nothing → empty copy, no
* spinner (one settled read is not an answer);
* 3. after a read REJECTS → still the empty copy. A rejected fetch is an
* answer too; the alternative is a panel that spins forever, which is
* strictly worse than the bug being fixed.
* 3. after a read REJECTS → no spinner. A rejected fetch is an answer too;
* the alternative is a panel that spins forever, which is strictly worse
* than the bug being fixed. A 404 settles on the empty copy; a refused
* read (403) on its no-permission line (objectui#11195). Each rejection
* carries the status the ObjectStack client puts on it: a rejection with
* no status is a network failure, which shows the load-failure notice
* (objectstack#29188).
*/

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

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

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

renderDetail(dataSource);

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

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

renderDetail(dataSource);
Expand All @@ -329,7 +342,7 @@ describe('RecordDetailView — the feed fetch produces a loading signal (#3209)'
},
],
}),
sys_activity: () => Promise.reject(new Error('404 sys_activity')),
sys_activity: () => Promise.reject(rejection(404, 'OBJECT_NOT_FOUND')),
});

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

it('leaves the loading state when both reads reject', async () => {
const dataSource = makeDataSource({
sys_comment: () => Promise.reject(new Error('403 FEEDS_DISABLED')),
sys_activity: () => Promise.reject(new Error('404 sys_activity')),
sys_comment: () => Promise.reject(rejection(403, 'FEEDS_DISABLED')),
sys_activity: () => Promise.reject(rejection(404, 'OBJECT_NOT_FOUND')),
});

renderDetail(dataSource, [AUTHORED_PAGE_WITH_DISCUSSION]);

expect(await screen.findByText(AUTHORED_EMPTY_COMMENTS)).toBeTruthy();
const notice = await screen.findByTestId('activity-access-denied');
expect(notice.querySelector('[data-denied-source="comments"]')).not.toBeNull();
expect(screen.queryByTestId('activity-loading')).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
* 2. the control: a 200 with zero rows shows the empty state, unchanged;
* 3. a 404 is not a refusal, so the empty state stays;
* 4. a 500 is not a refusal either, so the panel never says "no permission"
* for it;
* for it; since objectstack#29188 it shows the load-failure notice, not
* the empty state;
* 5. a refused `sys_comment` read beside an answered `sys_activity` read
* keeps the activity rows and says the comments are withheld.
*
Expand Down Expand Up @@ -260,9 +261,11 @@ describe('RecordDetailView: a refused sys_activity read shows the no-permission

renderDetail(dataSource);

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