You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "unbridled-41 (via GitHub)" <gi...@apache.org> on 2026/09/08 14:51:28 UTC
[PR] fix(producer): clear connection results when the topic changes [rocketmq-dashboard]
unbridled-41 opened a new pull request, #4169:
URL: https://github.com/apache/rocketmq-dashboard/pull/4169
Fixes #4168.
## Problem / Evidence
On the Producer page (Studio), switching the selected topic only resets the producer-group suggestions (`handleTopicChange`, `web/src/pages/studio/Producer.tsx`, lines 162-166 on base `0a596661`), while the connection table and readiness banner keep showing the previous topic's results. `handleInstanceChange` (lines 121-131) already clears `connectionList`/`connectionSummary` for instance switches; the topic dimension was missed. `handleExport` (lines 277-294) stamps rows with `topic: connection.topic ?? selectedTopic` (line 286), so exporting from that state relabels topic A's client connections as topic B.
Two regression tests fail on the unmodified base:
1. `clears stale connection results when the topic changes` → `expected document not to contain element, found <td ...>producer-1</td>` — the stale client row survives a topic switch.
2. `exports connections with the form topic after switching topics` → the export button stays enabled with cached stale rows, so an export would stamp them with the newly selected topic.
## Root cause / Fix
Root cause: `handleTopicChange` resets only the group suggestions, not the previous query's visible results.
Fix: clear `connectionList` and `connectionSummary` in `handleTopicChange`, matching `handleInstanceChange`. The export button's existing `disabled={connectionList.length === 0}` then prevents exporting until a fresh query is run.
## Priority & scoring
- Impact 28/40 (wrong client inventory presented and exported under the wrong topic — misleading diagnostics on the producer page) + breadth 13/20 (single page, but the table/export/banner are its core outputs) + reproducibility 20/20 (deterministic UI regression) + maintainability value 12/20 (completes the state-reset symmetry the file already maintains for instance changes) = **PRIORITY 73** ≥ 70.
- Two-line state reset mirroring an in-file pattern, with red→green regressions → **FIX_CONFIDENCE 92** ≥ 80.
## Tests
- Red: `npx vitest run src/pages/studio/__tests__/Producer.test.tsx` on base with only the tests added → `Tests 2 failed | 13 passed (15)` (both new regressions above).
- Green: same command after the fix → `Test Files 1 passed (1)`, `Tests 15 passed (15)`.
- Full web suite after the fix: `npx vitest run` → `Test Files 3 failed | 112 passed (115)`, `Tests 3 failed | 943 passed (946)` — the failing files are `ClusterPage` ×1, `ConsumerPage` ×1, `NotificationDeliveriesPage` ×1, none of which this change touches; running those three files in isolation with the fix applied → `Tests 59 passed (59)`, consistent with the documented load-flakiness under full-suite parallelism (see verification comment for the base stash comparison).
- `npm run build` → `✓ built in 11.08s`; `eslint` clean on both changed files (after fixing one unused-var warning introduced by the new test); `tsc --noEmit` clean.
## Risk
Low. Two state resets added to an existing change handler; no API, data, or visual changes beyond the intended clearing. Users who relied on switching the topic while keeping the old table on screen now see an empty table (with the export button disabled) until they run a new query — the same behavior an instance switch already produces.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(producer): clear connection results when the topic changes [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #4169:
URL: https://github.com/apache/rocketmq-dashboard/pull/4169#issuecomment-5587085786
## Verification evidence
- **CI**: for this PR's head SHA `29c0f4f0` (verified before commenting): `gh api /repos/apache/rocketmq-dashboard/actions/runs?head_sha=<sha>` returns exactly one `CI (pull_request)` run with `conclusion=startup_failure`, and `/commits/<sha>/check-runs` reports `total_count=0`. The upstream workflow never starts for any branch here (recent base-branch runs are also startup_failure), so nothing can pass or fail CI.
- **Red** (base `0a596661`, only the two new tests added; fix stashed): `npx vitest run src/pages/studio/__tests__/Producer.test.tsx` → `Tests 2 failed | 13 passed (15)`:
- `clears stale connection results when the topic changes`: `expect(element).not.toBeInTheDocument() ... found <td class="ant-table-cell">producer-1</td>` — the previous topic's client row survived the topic switch.
- `exports connections with the form topic after switching topics`: the export button remained enabled with the stale rows cached (a click would export topic A's clients labelled as topic B).
- **Module tests** after the fix: `npx vitest run src/pages/studio/__tests__/Producer.test.tsx` → `Test Files 1 passed (1)`, `Tests 15 passed (15)` (13 pre-existing + 2 new).
- **Full suite + baseline comparison** after the fix: `npx vitest run` → `Test Files 3 failed | 112 passed (115)`, `Tests 3 failed | 943 passed (946)`. Failing files: `ClusterPage` ×1, `ConsumerPage` ×1, `NotificationDeliveriesPage` ×1 — none is touched by this change. Running those three files in isolation with the fix applied → `Test Files 3 passed (3)`, `Tests 59 passed (59)`, matching the documented load-flakiness of these files under full-suite parallelism.
- **Build**: `npm run build` → `✓ built in 11.08s`; `node_modules/.bin/eslint` on both changed files clean (one unused-var warning in the new test itself was fixed before commit); `tsc --noEmit` clean. These cover the CI web build/lint steps, which cannot run due to the startup_failure above.
- **Diff self-check**: `git show 29c0f4f0 --numstat` → `2 files changed, 80 insertions(+)`: `web/src/pages/studio/Producer.tsx` +2/−0 (the two `setConnectionList([])`/`setConnectionSummary(null)` lines in `handleTopicChange`), `web/src/pages/studio/__tests__/Producer.test.tsx` +78/−0 (two new regressions). No unrelated changes.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(producer): clear connection results when the topic changes [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins commented on PR #4169:
URL: https://github.com/apache/rocketmq-dashboard/pull/4169#issuecomment-5679165238
Thanks for looking at the producer page's scope-change cleanup — the inconsistency you noticed is real: switching instance clears the connection results while switching topic does not. We can't take the change in this form, though. Clearing the state without also bumping `queryRequestIdRef` / clearing `queryInFlightRef` (as `handleInstanceChange` does) means a slow in-flight response still repopulates exactly what you just cleared, so the fix doesn't hold under the race it targets. The second test also rests on a mock that omits `topic` and `producerGroup`; the server populates both on every row, so the `connection.topic ?? selectedTopic` export fallback it asserts is never reached in production. A resubmission that invalidates the in-flight query, extracts one shared reset helper reused by both scope changes, and drops the export test would be a clear win — please do send it.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(producer): clear connection results when the topic changes [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins closed pull request #4169: fix(producer): clear connection results when the topic changes
URL: https://github.com/apache/rocketmq-dashboard/pull/4169
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(producer): clear connection results when the topic changes [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #4169:
URL: https://github.com/apache/rocketmq-dashboard/pull/4169#issuecomment-5680371232
@lizhimins Thanks for the review — resubmitted as #4322. It invalidates the in-flight query (bumps `queryRequestIdRef` and releases `queryInFlightRef`) as part of a shared reset helper now reused by both `handleInstanceChange` and `handleTopicChange`, and the old export test is replaced by a race regression where a slow topic A response resolves after switching to topic B and must not write back.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org