feat(dashboard): paginate Top Queries and filter it by database - #379
feat(dashboard): paginate Top Queries and filter it by database#379dpage wants to merge 2 commits into
Conversation
The Top Queries panel showed a fixed handful of rows with no way to reach the rest, and it offered no way to narrow the list when a connection monitors several databases; this addresses both. On the server, GET /api/v1/metrics/top-queries gains an `offset` parameter and a `database_name` filter that matches the resolved database name, and it now returns an `X-Total-Count` header giving the total number of matching rows whilst ignoring limit and offset. The JSON body remains a bare array, so existing consumers such as the query drill-down are unaffected. The count comes from a separate COUNT(*) over the same CTE rather than a window function, because a window count disappears exactly when the offset runs past the end, which is the case a pager most needs it for. The CORS middleware now exposes the new header so cross-origin browser clients can read it. On the client, the panel gains a footer pager offering 10, 20, 50 or 100 rows per page (defaulting to 20) with previous/next controls and a range indicator, plus a database filter in the header that appears only when the connection monitors more than one database. The pager degrades gracefully when the header is missing or unparseable, and it clamps back into range if a refresh shrinks the result set. Closes #335
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughTop Queries now supports server-side pagination, database filtering, page-size controls, and total-count metadata. The dashboard reuses a new database-summary hook, while the API, OpenAPI documentation, CORS middleware, tests, and user-facing documentation are updated accordingly. ChangesTop Queries pagination and database filtering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant API
participant Database
Dashboard->>API: Request page with limit, offset, and database_name
API->>Database: Count filtered matching queries
Database-->>API: Return total count
API->>Database: Fetch filtered page
Database-->>API: Return query rows
API-->>Dashboard: JSON rows and X-Total-Count
Dashboard-->>Dashboard: Render rows and pagination controls
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 208 |
| Duplication | -1 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
client/src/components/Dashboard/ServerDashboard/__tests__/TopQueriesSection.test.tsx (1)
591-666: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThese two tests don't reach the stranded-recovery branch.
Both drive the scenario through a page-size click, but
handlePageSizeChangecallssetPage(0)(Line 398-401 ofTopQueriesSection.tsx), so the refetch always runs withoffset=0and returns a non-empty page —rows.length === 0 && page > 0is never true, and the final assertions pass trivially. To exercise the branch, stay on a later page: seedpages: { '0': makeRows(20), '20': makeRows(20) }(no total header), click Next, then swap the mock so offset 20 returns[]and re-trigger a fetch without resettingpage(e.g. via the collector toggle... that also resets page — so use arefreshTriggerbump from the mockeduseDashboard).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/Dashboard/ServerDashboard/__tests__/TopQueriesSection.test.tsx` around lines 591 - 666, Rewrite both stranded-recovery tests so they remain on page 2 when the empty response is fetched: seed non-empty page 0 and page 20 data, click the “Next page of queries” control, then update the mocked fetch to return [] for offset 20 and trigger a refresh without invoking handlePageSizeChange or another page-resetting action. Assert the component falls back to page 1, using the mocked useDashboard refreshTrigger mechanism to re-fetch while preserving the current page.client/src/hooks/useDatabaseSummaries.ts (1)
60-119: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider guarding against out-of-order responses.
fetchDatahas noAbortController/request-sequence guard, so whenconnectionId(ortimeRange) changes while a request is in flight, a late response from the previous connection can still land indatabases(the mounted check doesn't discriminate). Also note this hook is now mounted twice per server dashboard (Database Summaries panel + Top Queries filter), producing two identical requests per refresh cycle; a shared cache/context would avoid that.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/hooks/useDatabaseSummaries.ts` around lines 60 - 119, Guard fetchData against stale responses when connectionId or timeRange changes by using an AbortController or request-sequence check, and only apply results from the latest active request. Ensure cleanup aborts or invalidates in-flight requests alongside the existing isMountedRef handling. Also consolidate the duplicate Database Summaries requests from the two dashboard mounts through a shared cache or context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/components/Dashboard/ServerDashboard/TopQueriesSection.tsx`:
- Around line 460-479: Update the FormControlLabel in TopQueriesSection to use
the MUI v5-compatible componentsProps property instead of slotProps, preserving
the existing typography sx styling and all other switch and label behavior.
In `@server/src/internal/api/perf_summary_handlers.go`:
- Around line 1206-1215: Update the pre-existing deferred rollback in the
enclosing handler to call tx.Rollback with context.Background() instead of the
request-scoped ctx. Preserve the existing deferred cleanup behavior and its
no-error-check annotation, ensuring both count-query and page-query early
returns use the background context.
- Around line 1217-1227: Update the paginated query’s ORDER BY construction in
the handler to append a deterministic secondary key, such as the query’s unique
identifier, while preserving the requested primary column and direction. Ensure
the same ordering is used for all offset-based pages, and add a tied-value
fixture case in TestTopQueries_OffsetPaging to verify rows are neither
duplicated nor skipped.
---
Nitpick comments:
In
`@client/src/components/Dashboard/ServerDashboard/__tests__/TopQueriesSection.test.tsx`:
- Around line 591-666: Rewrite both stranded-recovery tests so they remain on
page 2 when the empty response is fetched: seed non-empty page 0 and page 20
data, click the “Next page of queries” control, then update the mocked fetch to
return [] for offset 20 and trigger a refresh without invoking
handlePageSizeChange or another page-resetting action. Assert the component
falls back to page 1, using the mocked useDashboard refreshTrigger mechanism to
re-fetch while preserving the current page.
In `@client/src/hooks/useDatabaseSummaries.ts`:
- Around line 60-119: Guard fetchData against stale responses when connectionId
or timeRange changes by using an AbortController or request-sequence check, and
only apply results from the latest active request. Ensure cleanup aborts or
invalidates in-flight requests alongside the existing isMountedRef handling.
Also consolidate the duplicate Database Summaries requests from the two
dashboard mounts through a shared cache or context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a8d4ec27-35d1-40a3-bfc3-0ec065ef4eef
📒 Files selected for processing (14)
client/src/components/Dashboard/ServerDashboard/DatabaseSummariesSection.tsxclient/src/components/Dashboard/ServerDashboard/TopQueriesSection.tsxclient/src/components/Dashboard/ServerDashboard/__tests__/TopQueriesSection.test.tsxclient/src/hooks/__tests__/useDatabaseSummaries.test.tsclient/src/hooks/useDatabaseSummaries.tsclient/src/theme/tokens.tsdocs/admin-guide/api/openapi.jsondocs/changelog.mddocs/user-guide/dashboards/server.mdserver/src/internal/api/openapi.goserver/src/internal/api/perf_summary_handlers.goserver/src/internal/api/perf_summary_top_queries_test.goserver/src/internal/mcp/cors_middleware_test.goserver/src/internal/mcp/http_server.go
Adds a deterministic `queryid` tiebreaker to the page query's ORDER BY. Sorting by the user-selected column alone left rows that tie on that column with no guaranteed relative order between the count query and successive page requests, so a client paging through ties could see a row twice or miss one entirely; a new tied-value fixture pages through seven rows sharing a value and asserts each is visited exactly once. The SQL is now assembled by a pure `buildTopQueriesSQL` helper, and the order column and direction are interpolated from whitelist map *values* rather than the validated request strings, so no byte of the statement text originates from the request. Table-driven unit tests assert the exact SQL and argument slices for all eight clause combinations, which pins the `$N` numbering against future refactoring. The deferred rollback in this handler now uses a non-cancelable context, following the precedent in connection_queries.go, so a cancelled request context cannot trigger the pgx v5 panic in jackc/pgx#2470 and leak the pooled connection in an aborted-transaction state. Also reduces the cyclomatic complexity of the `useDatabaseSummaries` fetch below the project's limit by extracting four small helpers, and records the Opengrep SQL-injection findings on this file as audited false positives in .codacy.yaml, alongside the new test files in the duplication exclusion list.
1 participant
Summary
The Top Queries panel showed a fixed handful of rows with no way to reach the rest, and it offered no way to narrow the list when a connection monitors several databases; this addresses both halves of the issue.
GET /api/v1/metrics/top-queriesgains anoffsetparameter and adatabase_namefilter matching the resolved database name, and returns anX-Total-Countheader giving the total number of matching rows whilst ignoring limit and offset. The JSON body stays a bare array, so the query drill-down is unaffected. The count comes from a separateCOUNT(*)over the same CTE rather thanCOUNT(*) OVER (), because a window count vanishes exactly when the offset runs past the end, which is the case a pager most needs it for. The CORS middleware now exposes the new header.useDatabaseSummarieshook.Test plan
database_namefilter against the resolved name,X-Total-Countcorrectness, and invalid-parameter rejection.CORSMiddlewaregained tests (it had none); coverage 0% to 100%.TopQueriesSection.tsx99.2%,useDatabaseSummaries.ts100%,handleTopQueries92.9%.make test-allpasses apart from two pre-existing failures ininternal/tools(a pgvector dimension mismatch in the memory-embedding tests) that reproduce unchanged onmain.gofmt,golangci-lintandnpm run lintclean on the touched files.Closes #335
Summary by CodeRabbit
New Features
Bug Fixes
Documentation