unbridled-41 opened a new pull request, #4734:
URL: https://github.com/apache/rocketmq-dashboard/pull/4734
Fixes #4731.
### Problem
The asset/dashboard preview modal renders a failed request as an empty box.
`handleView` catches the failed preview request and only raises a toast, while
the modal body branches on `viewLoading` alone — so after loading ends the
`<pre>` renders an empty string, and for as long as the modal stays open there
is no error text and no retry.
Two sibling components share the defect:
- `web/src/components/AlertRuleAssetList.tsx` — 业务告警 asset list (View
button).
- `web/src/components/GrafanaDashboardList.tsx` — Grafana 看板 list (View
button).
Both already implement the correct pattern for their *list* request (an
error state plus a retry button, `AlertRuleAssetList.tsx:232-248` and the
Grafana equivalent); the preview request is the one load in these files whose
failure has no surface.
Trigger (deterministic): click 查看/View on any row while the preview request
fails (4xx/5xx/timeout, e.g. the backend restarts or the dashboard file is
unreadable). The modal opens, the toast flashes once, and the pane stays blank;
the operator cannot tell a failed request from an empty document and must close
and reopen the modal to try again.
### Root cause
`viewContent` is the only view state: it stays `''` on failure and the
render has no failure branch, so a rejected request and a successful-but-empty
response are indistinguishable.
### Fix
Add a `viewError` flag to both components: set on the catch (next to the
existing toast), cleared at the start of every attempt and on close. The modal
body now renders an error `Alert` with a retry button that re-issues the
request for the asset/dashboard currently being previewed, instead of the empty
`<pre>`. No i18n keys added — the existing `alertAssets.loadFailed` /
`grafana.loadFailed` messages and `common.retry` are reused, matching the
list-level failure UI in the same files.
### Priority
**70** = impact 26 + reach 10 + reproducibility 20 + maintenance value 14.
- Impact 26/40: a recoverable read failure is presented as an empty
document; the only signal lives for seconds.
- Reach 10/20: two components on two pages, any transient preview failure.
- Reproducibility 20/20: deterministic — stub the service to reject once.
- Maintenance value 14/20: restores parity with the list-level failure
handling already in these files.
`FIX_CONFIDENCE`: **92** — the failing test and the sibling pattern in the
same file fix the expected behavior; the change is additive state plus one
render branch.
### Tests
Red (source at base commit `7ce9a682`, new tests added):
```
npx vitest run src/components/__tests__/AlertRuleAssetList.test.tsx
src/components/__tests__/GrafanaDashboardList.test.tsx
× renders a failed preview as an error with a retry instead of an empty
pane (AlertRuleAssetList)
× renders a failed preview as an error with a retry instead of an empty
pane (GrafanaDashboardList)
TestingLibraryElementError: Unable to find role="button" and name
`/Retry|重试/`
```
Green (this branch):
```
npx vitest run src/components/__tests__/AlertRuleAssetList.test.tsx
src/components/__tests__/GrafanaDashboardList.test.tsx
Test Files 2 passed (2)
Tests 21 passed (21)
```
Each new test asserts the full contract: the failed preview shows a retry
inside the dialog, the retry re-issues the request (call count 1 → 2), the
content replaces the error, and the retry button is gone afterwards.
Related suites and gates on this branch (`fix/asset-preview-failure-state` @
16160cb5):
- `npx vitest run --maxWorkers=4` → **134 files / 1237 tests, all passed**
(5 min).
- `npx tsc -b` → clean.
- `npx eslint <the 4 changed files>` → clean.
- `npm run build` → `✓ built in 11.21s`.
### Risk
Low. The change is additive: a new boolean plus one render branch per
component, reusing the same loading/failure vocabulary as the list request
above it. The retry path calls the existing `handleView(viewing)`, which
already bumps the request-generation ref, so an in-flight/stale response still
cannot land (covered by the existing "keeps the latest preview" tests, which
stay green). The only visible change on the success path is that a stale error
can no longer survive a successful retry.
### Base branch
Targets `rocketmq-studio`, the development trunk. Note the repository's
default branch is still `master`; GitHub interprets the closing keyword only
for PRs that target the default branch, so the `Fixes` link above does not
auto-close #4731 when this merges — the issue needs to be closed by hand.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
