unbridled-41 commented on PR #4616:
URL:
https://github.com/apache/rocketmq-dashboard/pull/4616#issuecomment-5763340817
@lizhimins Thanks for the review and the clear direction question. **This
re-submission implements real server-side sorting** — the version you preferred
— on branch head `b017fef1` (rebased onto trunk `4c697f07`; the original
premises were re-verified there before re-submitting).
**Backend**: `GET /api/audit-logs` and the alert-rule `/page` endpoints now
accept `sortField` + `sortOrder`. The ORDER BY is built exclusively from an
enum allow-list (`AuditSortField` on the audit side, `AlertSortField` on the
alert side) mapping caller-facing names to physical columns — no caller string
is ever interpolated into the query, and unknown fields answer 400. Defaults
are unchanged (`gmt_create DESC, id DESC` / `name ASC, id ASC`), with `id` as a
same-direction tiebreaker so pagination stays deterministic. `docs/api-spec.md`
documents both.
**Frontend**: both tables use `sorter: true` with controlled `sortOrder`;
header clicks re-query the server with the allow-listed params and reset to
page 1, and rows always render exactly the returned order. The meaningless
`defaultSortOrder`+`sorter: false` pair on the time column is gone (replaced by
a real sorter defaulting to the server's direction), and `alerts.tsx` kept its
six sorters — now backed by server behavior and coverage.
**Tests**:
- Audit page: rewritten regression asserts the query carries
`sortField/sortOrder` and that rendered order always equals the mocked server
order (the tautological `closest('tr')` assertions are replaced by a full
body-row order comparison — mutation-sensitive).
- Alerts page: new test pins that a non-name-ordered mock renders verbatim,
that the metric header re-queries with `sortField: 'METRIC'`, and that the rows
stay server-ordered.
- Backend: allow-list ORDER BY assertions + 400-on-unknown-field coverage
(`MybatisPlusAuditRepositoryTest` 14/14, `MybatisPlusAlertRepositoryTest`
17/17, `AuditServiceTest` 19/19, `AuditControllerTest` 12/12,
`AlertServiceTest` 82/82, `AlertRuleControllerTest` 22/22,
`ClusterAlertRuleControllerTest` 6/6) — **170 passed, 0 failed**.
- `tsc -b` clean; eslint 0 errors (5 warnings pre-existing on trunk); `npm
run build` ✓.
Ready for re-review.
--
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]
