unbridled-41 opened a new pull request, #5092:
URL: https://github.com/apache/rocketmq-dashboard/pull/5092
## Problem
The audit console's cleanup dialog (`web/src/pages/ops/audit.tsx`) confirms
a **destructive** operation — `POST /api/audit-logs/cleanup` deletes every
audit row older than N days — and nothing prevents a second submission while
the first request is on the wire:
- `handleCleanup` (`:293`) has no in-flight guard; it runs straight into
`cleanupAuditLogs(cleanupDays)`.
- The dialog stays open until the request resolves
(`setCleanupModalOpen(false)` happens after the `await`), and the OK button is
only given `okButtonProps={{ danger: true }}` — no `confirmLoading`.
- antd v5's `Modal` does **not** disable the OK button for a
promise-returning `onOk`; its `handleOk` is just `onOk === null || onOk ===
void 0 ? void 0 : onOk(e)` (`node_modules/antd/es/modal/Modal.js:88`), so the
loading state must be passed in explicitly.
Result: a second click on 确认清理 re-enters the handler and issues a second
cleanup. The first response closes the dialog and toasts success; the second
still runs and toasts success again (with its own count) after the dialog is
gone, plus a second `setPage(1)`/refresh cycle and a second scan-and-delete
over the same retention window.
## Evidence
Reproduction, added as a regression test (`AuditPage.test.tsx`, `does not
submit the cleanup again while the first request is in flight`): mock
`cleanupAuditLogs` with a deferred promise, open 清理日志, click 确认清理 twice, assert
`cleanupAuditLogs` was called once.
Before the fix the test fails with `expected "vi.fn()" to be called 1 times,
but got 2 times`. After the fix it passes, and the OK button carries
`ant-btn-loading` while the request is pending.
Same class and same fix as the NameServer registry dialog
(`web/src/pages/cluster/index.tsx:288-329`), which was merged for exactly this
reason: `nsSubmittingRef` + `nsSubmitting` + `confirmLoading`. 20 of the 42
modals in the console already pass `confirmLoading`; this dialog was the one
destructive confirm that did not.
## Root cause
The confirm handler was written as a bare `async` function and the dialog
was never given an in-flight state, so nothing — neither the component nor antd
— blocks a second confirm click during the request.
## Fix
Reuse the page's existing single-flight idiom: a `cleanupSubmittingRef`
guard at the top of `handleCleanup` with a `cleanupSubmitting` state
set/cleared in `finally`, and `confirmLoading={cleanupSubmitting}` on the
dialog.
## Priority
**54** = impact 16 (a duplicated destructive request and a duplicated
success confirmation the operator reads as two separate deletions) + blast
radius 6 (one dialog, but the operation it guards is irreversible) +
reproducibility 18 (deterministic in RTL with a deferred promise) + maintenance
14 (aligns the dialog with the pattern the rest of the console and the same
page's request refs already use).
`FIX_CONFIDENCE` **92**.
## Tests
```
$ npx vitest run src/pages/ops/__tests__/AuditPage.test.tsx
Test Files 1 passed (1)
Tests 12 passed (12) # the new test fails (2 calls) before the
fix
$ npx vitest run src/pages/ops src/services
Test Files 23 passed (23)
Tests 192 passed (192)
$ npx eslint src/pages/ops/audit.tsx
src/pages/ops/__tests__/AuditPage.test.tsx # 0 problems
$ npx prettier --check ...
# clean
$ npx tsc -b
# clean
```
## Risk
Low. The guard only refuses a re-entry while a cleanup is pending; the
confirm button now shows its loading state instead of appearing idle, and
`finally` always releases the guard, so a failed cleanup can be retried
immediately (covered by the existing failure-path behaviour of the dialog).
--
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]
