fix(billing): derive history pagination from totalCount - #3595
Conversation
The History table built its page numbers from hasMore plus a hardcoded "page 3 + ellipsis on the first page" block and never used the stable totalCount already provided by the container. On accounts with ~2 pages this rendered phantom pages that collapsed after navigating and loaded empty pages past the end. Replace the hand-rolled pagination with the shared CustomPagination component (used by the alerts/providers/deployments tables), driven by ceil(totalCount / pageSize). Drop the now-dead hasMore/hasPrevious props from the view and container.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughBilling usage pagination now uses ChangesBilling Pagination
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The change replaces fabricated billing-history page counts with pagination derived from total transaction count, preventing phantom pages and empty results; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/deploy-web/src/components/billing-usage/BillingView/BillingView.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3595 +/- ##
==========================================
- Coverage 77.02% 76.20% -0.82%
==========================================
Files 1150 1057 -93
Lines 30026 27572 -2454
Branches 7492 7007 -485
==========================================
- Hits 23127 21011 -2116
+ Misses 6078 5770 -308
+ Partials 821 791 -30
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward pagination fix that adopts an already-proven shared component.
Extended reasoning...
Overview
Four files in apps/deploy-web: BillingView.tsx/.spec.tsx and BillingContainer.tsx/.spec.tsx. The change removes a hand-rolled, buggy pagination block (relying on hasMore/hardcoded page-3 rendering) and replaces it with the shared CustomPagination component, deriving totalPageCount from the backend-provided totalCount via Math.ceil(totalCount / pageSize) (floored at 1). Dead hasMore/hasPrevious props are removed from both the view and its container.
Security risks
None. This is a pure UI pagination change with no auth, data access, or injection surface — it only affects how existing, already-fetched billing transaction data is paginated for display.
Level of scrutiny
Low-to-moderate is appropriate: this is a UI bug fix, not a critical path, and it replaces bespoke logic with a component (CustomPagination) already in production use in ProviderList, LeaseList, DeploymentList, AlertsListView, and NotificationChannelsListView. I verified CustomPagination's prop contract (pageIndex, pageSize, totalPageCount, setPageIndex, setPageSize) in packages/ui/components/custom-pagination.tsx and confirmed BillingView.tsx wires it correctly, including the pageIndex: 0 reset on page-size change.
Other factors
Test coverage is solid: new regression tests directly encode the reported bug (15 rows → exactly 2 pages, no phantom 3rd page; 35 rows → 4 pages; clicking page 3 navigates to pageIndex: 2), and the removed dead props are reflected in updated specs. The noted minor UX change (pagination buttons no longer disable during background refetch) is explicitly called out and consistent with how the other list views already behave with CustomPagination, so it's not a regression worth blocking on.
1 participant
Why
The billing History table pagination was broken in production. On load an account with ~2 pages of transactions showed ~4 pages; clicking page 2 collapsed the count to 2; clicking the phantom page 3/4 loaded an empty table.
The view fabricated its page numbers from the
hasMoreflag plus a hardcoded "render a literal page 3 + ellipsis while on the first page" block, and never used the stabletotalCountthe container (and backendCOUNT(*)) already provided. So:pageIndex 0): rendered1,2(becausehasMore), then the hardcoded3+…+ Next → looked like ~4 pages.hasMoreis nowfalseand the first-page block is gone → collapsed to1,2.3/ Next: jumped to an offset pasttotalCount→ empty "No billing history found".What
BillingViewwith the sharedCustomPaginationcomponent already used by the alerts/providers/deployments tables, driven byMath.ceil(totalCount / pageSize).hasMore/hasPreviousprops fromBillingViewandBillingContainer.BillingView/BillingContainerspecs and added regression cases: 15 rows → exactly 2 pages (no phantom page 3), 35 rows → 4 pages, clicking page 3 navigates topageIndex: 2.Minor UX change: page buttons no longer disable during a background refetch (
CustomPaginationhas no fetching state, consistent with the other tables;keepPreviousDatakeeps rows visible meanwhile).Verification:
npm run test:unit -- BillingView BillingContainer(32 passed),npm run lint -- --quietclean,npx tsc --noEmitno new errors in the changed files.Summary by CodeRabbit
New Features
Tests