unbridled-41 commented on PR #5898:
URL:
https://github.com/apache/rocketmq-dashboard/pull/5898#issuecomment-6090899701
### Follow-up review of this fix: the live read could land in the wrong
dialog
`loadLiveConfig` committed its response unconditionally, so a read that was
still in flight when the operator cancelled the dialog over cluster A and
opened cluster B instead would:
- write A's config into the shared form that B's dialog is now showing, and
- call `setConfigUnavailable(false)` / `setConfigLoading(false)`, clearing
B's refusal warning and re-enabling **OK** over values that were never read for
B.
That is the same class of bug this PR fixes (submitting values nobody read),
just triggered by a race instead of by a missing endpoint. Commit `fc7003b5`
closes it in this branch (first commit `5737a1bd` untouched, no force-push):
- added a `configLoadRequest` generation next to the page's existing
`configPreviewRequest` and applied the documented pattern (`begin()` on issue,
`isCurrent()` before committing, `invalidate()` when the dialog closes / on
unmount);
- `handleConfigOpen` passes the token to `loadLiveConfig`, which returns
before touching `applyConfigFormValues`, `setSelectedCluster`,
`setConfigUnavailable` or `setConfigLoading` when the token is stale;
- the modal's `onCancel` now invalidates it alongside `configPreviewRequest`.
New regression test `drops a live config response for a dialog the operator
already left`: cluster A's `getCluster` never settles, the dialog is cancelled,
cluster B's read rejects, then A resolves late — B must keep its warning and
keep **OK** disabled.
Red/green, both observed locally:
```
# without the isCurrent guard
cd web && npx vitest run src/pages/cluster/__tests__/ClusterPage.test.tsx -t
'drops a live config response'
Tests 1 failed | 32 skipped (33) <- B's OK was re-enabled by A's
response
# with fc7003b5
cd web && npx vitest run src/pages/cluster/__tests__/ClusterPage.test.tsx
Test Files 1 passed (1)
Tests 33 passed (33)
```
`npx tsc -b` and `npx eslint src/pages/cluster/index.tsx
src/pages/cluster/__tests__/ClusterPage.test.tsx` are clean.
Branch `fix/broker-config-dialog-live-values`, HEAD `fc7003b5` (on top of
`5737a1bd`).
--
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]
