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/22 01:38:59 UTC
[PR] fix(alerts): reset the deliveries page when the page size changes [rocketmq-dashboard]
unbridled-41 opened a new pull request, #4833:
URL: https://github.com/apache/rocketmq-dashboard/pull/4833
## Problem / Evidence
The notification-deliveries inventory is server-paginated, but its pagination `onChange` always stored the page antd handed it, including when the *size changer* fired:
```ts
onChange: (nextPage, nextPageSize) => {
setLoading(true);
setPage(nextPage); // ← stored even when the size changed
setPageSize(nextPageSize);
},
```
Reproducible sequence: 45 deliveries, pageSize 20, operator on page 3 → switch the size changer to "50 / page":
1. antd fires `onChange(3, 50)`: the code requests page 3 at size 50.
2. Page 3 no longer exists (45 rows / 50 = 1 page). `total` updates to 45, but `current: 3` stays past the last page.
3. The table renders **permanently empty** with a pager claiming page 3 of 1 — this list has no empty-page clamp (unlike the rule and audit inventories), so the only escape is changing a filter or manually clicking back.
The codebase already recognized this exact hazard elsewhere: `web/src/pages/ops/audit.tsx` resets the page on size change with a comment spelling out the same scenario, and the alerts-rule page clamps an out-of-range page after a shrunk total. The deliveries page — added later — missed both protections.
## Root cause / Fix
Minimal fix, mirroring the established pattern:
- `web/src/pages/ops/notificationDeliveries.tsx`: when `nextPageSize !== pageSize`, land on page 1 with the new size; an ordinary page change keeps the picked page.
## Priority / scoring
- Impact 22/40: the operator lands on a blank inventory with no in-page recovery, reading "no deliveries" from a list that has 45 rows — on a failure-verification screen where a stale "nothing to see" is actively misleading.
- Reach 12/20: the deliveries page is the alert-notification verification workflow.
- Reproducibility 16/20: fully deterministic clicks; no timing or data subtlety.
- Maintenance value 8/20: converges the pagination handler on the pattern the audit page already documents.
- **PRIORITY 52/100**, **FIX_CONFIDENCE 90/100**.
## Tests
- New regression test "returns to the first page when the page size changes" (`web/src/pages/ops/__tests__/NotificationDeliveriesPage.test.tsx`): opens the size changer, selects 50/page, and asserts `listAlertDeliveriesPage` is last called with `{ page: 1, pageSize: 50 }`. On the pre-fix code the call carried `page: 3`-style stale page values, so the assertion fails without the fix.
- `npx vitest run src/pages/ops/__tests__/NotificationDeliveriesPage.test.tsx` → 5 passed (5).
- `npx tsc --noEmit -p tsconfig.app.json` → clean; `npx eslint` on the touched files → clean.
## Risk
Minimal. Only the size-changer branch of one pagination handler changes; ordinary page navigation behaves exactly as before.
--
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): reset the deliveries page when the page size changes [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #4833:
URL: https://github.com/apache/rocketmq-dashboard/pull/4833#issuecomment-5770426024
Evidence-chain verification pass (self-review), with a follow-up test commit `72d17841`:
- **Gap found and closed:** the original regression test switched the page size from page 1, where the pre-fix handler and the fixed handler record the same request — so it could not actually fail on the bug. The reworked test walks to **page 3** of a 45-row / 3-page mock first, then switches the size changer to 50/page. The mock serves rows per requested page, so the assertions are meaningful:
- the follow-up request must carry `page: 1, pageSize: 50` (the pre-fix handler records `page: 3`),
- page 1's rows (`Delivery 1`) are visible again after the reset.
- **Pre-fix failure re-verified:** with the handler reverted to `setPage(nextPage); setPageSize(nextPageSize);` (the `origin/rocketmq-studio` state), the reworked test fails its `toHaveBeenLastCalledWith({ page: 1, pageSize: 50 })` assertion; it passes against this branch's fix. Exact request sequences recorded via a probe: pre-fix `[{1,20},{3,20},{1,50}...]` — wait, the corrected sequence on the pre-fix code is `[{1,20},{3,20},{3,50}]` (the stale page is re-requested with the new size), versus `[{1,20},{3,20},{1,50}]` on the fixed code.
- Suite: `npx vitest run src/pages/ops/__tests__/NotificationDeliveriesPage.test.tsx` → **5 passed (5)** on this branch; `tsc --noEmit` clean.
No further code changes needed — the component fix itself was correct; only the test's sensitivity to the bug needed strengthening.
--
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): reset the deliveries page when the page size changes [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4833: fix(alerts): reset the deliveries page when the page size changes
URL: https://github.com/apache/rocketmq-dashboard/pull/4833
--
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): reset the deliveries page when the page size changes [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4833:
URL: https://github.com/apache/rocketmq-dashboard/pull/4833#issuecomment-5963523177
This pull request has been automatically closed because there has been no activity for 7 days. If you would like to continue working on this, please reopen the PR with updated 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