Frun1na opened a new pull request, #6546:
URL: https://github.com/apache/rocketmq-dashboard/pull/6546
### Which Issue(s) This PR Fixes
- Fixes #6544
### Brief Description
Four list views stopped clamping their page back when the list shrank **to
zero**, because the guard
that does the clamping is gated on a non-zero total: `instance/acl.tsx:212`
(ACL rules) and `:233`
(ACL users), `instance/consumer.tsx:347` (consumer groups) and
`instance/topic.tsx:468` (topics) all
read
```ts
if (items.length === 0 && total > 0 && page > 1) {
setPage(Math.max(1, Math.ceil(total / pageSize)));
}
```
`items.length === 0 && total === 0 && page > 1` is the case the guard exists
for - the page the
operator is on no longer exists and the last valid page is 1 - but the
`total > 0` term makes it fall
through, so no follow-up query is issued and the view stays on an
out-of-range page; with `total` at
0 there is also no other page button to click. The two views that already
had this fixed,
`settings/CloudCredentialTab.tsx:102-108` and
`settings/DataSourceTab.tsx:158-165` (the change in
#6198 that named these four as a follow-up), use the un-gated form and move
the page only when it
really is beyond the last one; the four sites now use that shape and take
the last page from the
response's own page size. User-visible change: a list that empties out while
the operator is past
page 1 returns to page 1 instead of staying on a page it no longer has.
### How Did You Test This Change?
```
$ wb test # worktree
worktrees/rocketmq-dashboard/pagination-clamp-remaining
── web (web)
✓ 4.4s npx eslint src/pages/instance/__tests__/AclPage.test.tsx
src/pages/instance/__tests__/ConsumerPage.test.tsx
src/pages/instance/__tests__/TopicPage.test.tsx src/pages/instance/acl.tsx
src/pages/instance/consumer.tsx src/pages/instance/topic.tsx
✓ 1.3s npx prettier --check <the same six files>
✓ 176.8s npx vitest related --run <the three test files>
✓ 0.4s npx tsc -b
Test Files 5 passed (5)
Tests 139 passed (139)
$ npx vitest run src/pages/instance/__tests__/AclPage.test.tsx # 28
passed
$ npx vitest run src/pages/instance/__tests__/ConsumerPage.test.tsx # 55
passed
$ npx vitest run src/pages/instance/__tests__/TopicPage.test.tsx # 45
passed
```
The four new cases fail without the fix (the three product files were
reverted, the tests were kept):
```
$ npx vitest run src/pages/instance/__tests__/AclPage.test.tsx -t
'out-of-range page is also the last one'
FAIL src/pages/instance/__tests__/AclPage.test.tsx > ACL page > clamps
rules back to the first page when the out-of-range page is also the last one
FAIL src/pages/instance/__tests__/AclPage.test.tsx > ACL page > clamps the
user list back to the first page when the out-of-range page is also the last one
AssertionError: expected last "vi.fn()" call to have been called with [
ObjectContaining{…} ]
Tests 2 failed | 26 passed (28)
$ npx vitest run src/pages/instance/__tests__/ConsumerPage.test.tsx -t
'out-of-range page is also the last one'
Tests 1 failed | 54 passed (55)
$ npx vitest run src/pages/instance/__tests__/TopicPage.test.tsx -t
'out-of-range page is also the last one'
Tests 1 failed | 44 passed (45)
```
The assertion that turns red is on the *page argument of the follow-up list
call*: each case drives
the page to 2 with a 40-row first page and then has the service answer
`{items: [], total: 0,
page: 2}`, so with the old guard the last call is still `page: 2` and
`toHaveBeenLastCalledWith({
page: 1, pageSize: 20 })` fails; with the guard fixed the page falls back
and the re-query carries
page 1. The three suites' pre-existing clamps (`total: 15`) stay green in
both directions, so they
cannot cover this case - which is why each page gets its own test here.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`feat:` / `fix:` /
`refactor:` / `chore:` / `docs:` / `perf:`)
- [x] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [ ] New UI text has both Chinese and English entries under `web/src/i18n/`
- [ ] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [ ] New source files carry the ASF license header
- [ ] Documentation touched where behaviour changed (README / `docs/` /
in-app help)
--
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]
