Skip to content

feat: paginate alert rules - #2590

Open
tju-yxq wants to merge 1 commit into
apache:rocketmq-studiofrom
tju-yxq:codex/alert-rule-pagination
Open

feat: paginate alert rules#2590
tju-yxq wants to merge 1 commit into
apache:rocketmq-studiofrom
tju-yxq:codex/alert-rule-pagination

Conversation

@tju-yxq

@tju-yxq tju-yxq commented Aug 25, 2026

Copy link
Copy Markdown

Closes #2589

Summary

  • add GET /api/alert-rules/page with name search, enabled filter, bounded page, and bounded page size
  • apply filtering and deterministic name ASC, id ASC ordering in SQL through MyBatis-Plus Page
  • reject invalid page/pageSize before repository access
  • preserve the unpaginated endpoint for export and existing callers
  • replace full-inventory reads in toggleRule with direct findRuleById
  • resolve bulk-toggle IDs with an ID-bounded findRulesByIds query
  • wire the Alert Rules page to server-driven pagination, search, enabled filtering, server total, and stale-response protection
  • add mock-mode support for the same page/filter contract

Why

The page previously fetched every rule and rendered it with pagination={false}. Toggling one rule also scanned the complete inventory, and bulk toggle read all rules to resolve a small ID list. This makes the read path bounded by page size and makes targeted mutations use targeted lookups.

Tests

  • focused backend: AlertServiceTest, AlertRuleControllerTest, MybatisPlusAlertRepositoryTest — 80 tests passed, Checkstyle 0 violations
  • focused frontend: AlertsPage.test.tsx, opsService.test.ts, ops.test.ts — 3 files / 32 tests passed
  • backend full suite: 1,632 tests run; only the 2 pre-existing RocketMQMessageProviderTest failures remain, reproduced earlier on untouched upstream 14da4b95
  • npm run lint -- --quiet: 0 errors, 2 pre-existing warnings
  • npm run build: passed
  • git diff --check: passed

Production changes: 176 additions / 48 deletions across backend and frontend, naturally exceeding 100 production lines without test padding.

Note

I also checked open PR #2533. Its later commits already contain a broader native-alerting implementation with alert-rule pagination, but that PR is still open and has been rebased several times. This PR is intentionally scoped to the current upstream code path, avoids the unrelated native-alerting domain changes, and provides a smaller standalone fix. If maintainers prefer to land #2533 first, this PR can be rebased or closed as superseded.

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — code changes look good.


Automated review by github-manager-bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants