You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "messere1 (via GitHub)" <gi...@apache.org> on 2026/10/10 05:30:07 UTC
[PR] fix(alerts): deduplicate system alert acknowledgements [rocketmq-dashboard]
messere1 opened a new pull request, #6045:
URL: https://github.com/apache/rocketmq-dashboard/pull/6045
### Which Issue(s) This PR Fixes
- Resubmission of #1574 (closed during the August branch-rebase sweep, with an invitation to resubmit against the current branch). Original report: #1573, closed as not planned — the defect is still present on the current `rocketmq-studio` tip.
### Brief Description
`handleAck` on the system-alerts page guards duplicate acknowledgements only through the `acknowledgingIds` **state** set. State updates are asynchronous: a rapid double-click on the confirm button (or button + keyboard activation) before React re-renders passes the guard twice and sends two `acknowledgeAlert()` requests for the same alert.
Add a synchronous ref-based in-flight guard alongside the state set (the ref is checked/set immediately, before any re-render can happen), mirroring the request-id guards the page already uses for its list loads. Rebased onto the current tip; the newer delivery/silence state on the page is untouched.
### How Did You Test This Change?
```text
$ cd web && npx vitest run src/pages/ops/__tests__/SystemAlertsPage.test.tsx
Test Files 1 passed (1)
Tests 22 passed (22)
$ npx eslint src/pages/ops/systemAlerts.tsx src/pages/ops/__tests__/SystemAlertsPage.test.tsx # clean
```
The regression test double-clicks the acknowledge button against a pending promise and asserts the API is called exactly once; the 20 pre-existing silence/delivery tests stay green.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits
- [x] Tests added or updated for non-trivial changes
- [x] New UI text has both Chinese and English entries under `web/src/i18n/` (no new UI text)
- [x] Architecture constraints stay green (frontend-only change)
- [x] New source files carry the ASF license header (no new files)
- [x] Documentation touched where behaviour changed (bug fix only, no behaviour spec change)
--
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(alerts): deduplicate system alert acknowledgements [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins commented on PR #6045:
URL: https://github.com/apache/rocketmq-dashboard/pull/6045#issuecomment-6120330858
Thanks for the resubmission. I don't think this one is needed on the current tip: `handleAck` already sets `acknowledgingIds` **synchronously before the first `await`** (`web/src/pages/ops/systemAlerts.tsx:246`, `await acknowledgeAlert(id)` is at `:248`), and the confirm button consumes it as `loading={acknowledgingIds.has(alert.id)}` (`:835`). In a browser, two physical clicks are two separate discrete-event tasks and React 18 flushes sync work at the end of each one, so the second click lands on a button that is already in `ant-btn-loading`, which antd v5 swallows (`Button.handleClick` returns early when `innerLoading`). The ref only helps for same-tick re-entry, and `handleAck` has exactly one call site (`:834`) with no loop / bulk / keyboard path that could produce it. Related: the new test uses `userEvent.dblClick`, which gives React a chance to flush between the two clicks, so it also passes on the unpatched tip and therefore does not pin the ref. Contrast with the audit cle
anup modal (#6044), where the trunk genuinely has no guard at all — that one is a real fix. If you want to keep this as belt-and-braces hardening, please make the test click twice inside a single tick (`fireEvent.click(btn); fireEvent.click(btn);`) so removing the ref would actually turn it red; otherwise I'd suggest closing this in favour of a change that covers a path the state guard does not (e.g. the silence/delivery retry buttons on the same page).
--
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(alerts): deduplicate system alert acknowledgements [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins closed pull request #6045: fix(alerts): deduplicate system alert acknowledgements
URL: https://github.com/apache/rocketmq-dashboard/pull/6045
--
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