unbridled-41 opened a new pull request, #5902:
URL: https://github.com/apache/rocketmq-dashboard/pull/5902
### Which Issue(s) This PR Fixes
Fixes #5901
### Problem / Evidence
Both sortable lag/connection columns use a one-sided sort key:
```ts
// web/src/utils/consumerLag.ts (before)
export const lagSortValue = (lag: number | null | undefined): number =>
isLagAvailable(lag) ? lag : Number.MAX_SAFE_INTEGER;
```
Ant Design negates the comparator for a descending column
(`antd/es/table/hooks/useSorter.js`: `return sortOrder === ASCEND ?
compareResult : -compareResult;`), so `MAX_SAFE_INTEGER` becomes the smallest
value and the rows without a measurement lead the descending list:
```
ConsumerPage: keeps an unknown lag after known backlogs in the descending
order too
AssertionError: expected [ 'unknown', 'known' ] to deeply equal [ 'known',
'unknown' ]
```
The instance page already handles this (`compareResourceCounts`: "Ant Design
reverses the comparator for descending order, so invert this branch to keep
unavailable counts after numeric values in either order", pinned by
`InstancePage.test.tsx`), and the consumer page's own tests only ever click
each header once, so the direction was never covered.
### Root cause / Fix
A sort key written for one direction used as a two-direction comparator.
Make both keys direction-aware (flip the sentinel to `MIN_SAFE_INTEGER` for
`descend`) and pass the column's `sortOrder` through from `consumer.tsx`.
### Priority and scoring
**PRIORITY 52** — impact 16/40 (a wrong order that misdirects triage, no
data loss), blast radius 12/20 (two columns of the consumer-group list, for
every group whose lag/connection read is unavailable), reproducibility 20/20
(deterministic, pinned by tests in both directions), maintenance value 8/20
(the sibling page already established the pattern).
**FIX_CONFIDENCE 90** — the mechanism is pinned by an in-repo precedent and
now by tests; the change is two helpers plus their two call sites.
### Tests
`cd web && npx vitest run src/utils/consumerLag.test.ts
src/utils/consumerConnections.test.ts
src/pages/instance/__tests__/ConsumerPage.test.tsx` -> `Test Files 3 passed
(3), Tests 56 passed (56)`.
| Test | Before | After |
|---|---|---|
| `ConsumerPage: keeps an unknown lag after known backlogs in the descending
order too` | FAIL (`['unknown','known']`) | PASS |
| `consumerLag.test.ts: keeps unknown lags after known ones in the
descending order too` | FAIL (compile/behaviour red on the old signature) |
PASS |
| `consumerConnections.test.ts: keeps unavailable connection counts last in
the descending order too` | FAIL (same) | PASS |
The two existing ascending cases are unchanged and still pass; `npx eslint`
reports no errors; `npx tsc -b` passes.
### Risk
Only the relative order of rows that carry no measurement changes; measured
values sort 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
