Fix: hacky pagination. #2786
Fix: hacky pagination. #2786
Conversation
WalkthroughA single-file refactoring in the payment history billing component where an internal function named Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/organization-[organization]/billing/paymentHistory.svelte:
- Around line 45-51: In loadInvoices(), remove the extra "+ 1" so
Query.offset(offset) is used (the +1 causes the first invoice to be skipped and
misaligns pagination), and wrap the sdk.forConsole.billing.listInvoices(...)
call in a try/finally that sets isLoadingInvoices = false in the finally block
so isLoadingInvoices is always reset on error; update references to invoiceList
and Query.orderDesc/Query.limit as needed inside the same try block.
🧹 Nitpick comments (1)
src/routes/(console)/organization-[organization]/billing/paymentHistory.svelte (1)
45-54: Ensure loading state clears on request failures.If
listInvoicesthrows, Line 53 won’t run andisLoadingInvoicesstaystrue. Wrap the await in atry/finallyto guarantee cleanup.♻️ Proposed fix
async function loadInvoices() { isLoadingInvoices = true; - invoiceList = await sdk.forConsole.billing.listInvoices(page.params.organization, [ - Query.orderDesc('$createdAt'), - Query.limit(limit), - Query.offset(offset + 1) - ]); - - isLoadingInvoices = false; + try { + invoiceList = await sdk.forConsole.billing.listInvoices(page.params.organization, [ + Query.orderDesc('$createdAt'), + Query.limit(limit), + Query.offset(offset + 1) + ]); + } finally { + isLoadingInvoices = false; + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/organization-[organization]/billing/paymentHistory.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/organization-[organization]/billing/paymentHistory.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/organization-[organization]/billing/paymentHistory.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/organization-[organization]/billing/paymentHistory.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/organization-[organization]/billing/paymentHistory.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/organization-[organization]/billing/paymentHistory.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (1)
src/routes/(console)/organization-[organization]/billing/paymentHistory.svelte (1)
43-43: Consistent wiring toloadInvoiceslooks clean.Nice simplification of the load triggers across mount, URL validation, and pagination.
Also applies to: 64-65, 188-188
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| async function loadInvoices() { | ||
| isLoadingInvoices = true; | ||
| invoiceList = await sdk.forConsole.billing.listInvoices(page.params.organization, [ | ||
| Query.orderDesc('$createdAt'), | ||
| // first page extra must have an extra limit! | ||
| Query.limit(patchQuery ? limit + 1 : limit), | ||
| // so an invoice isn't repeated on 2nd page! | ||
| Query.offset(patchQuery ? offset : offset + 1) | ||
| Query.limit(limit), | ||
| Query.offset(offset + 1) | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Appwrite Console SDK billing listInvoices offset parameter semantics
💡 Result:
Short answer: offset is the number of records to skip (not a 1-based page number). Pass offset = how many items to skip (e.g. 0 for start, limit for second page, limit*2 for third page). See Appwrite pagination docs. [1][2][3]
Sources:
- Appwrite pagination docs (offset = number of rows/documents to skip). [1]
- Community example + explanation showing offset used as skip (and an alternate description where some SDK examples treat it as page index by incrementing offset by 1 when limit is fixed). [2]
- Appwrite Console SDK repo (SDK surface for Console). [3]
🌐 Web query:
Appwrite Query.offset definition documentation
💡 Result:
Query.offset — skips a number of records before returning results (used with Query.limit for offset-style pagination). Example: Query.offset(25) skips the first 25 records. Use Query.offset when you want page-based pagination (but prefer cursor pagination for frequently changing or large datasets). [1][2]
Sources:
- Appwrite Pagination docs. [1]
- Appwrite Queries reference. [2]
🌐 Web query:
@appwrite.io/pink-svelte PaginationInline offset behavior
💡 Result:
Short answer: PaginationInline's offset is a page index (integer pages, 0-based) used to compute the record skip as offset * limit — so to get items starting at record R you set offset = floor(R / limit). See Appwrite docs on offset pagination (offset = number of pages to skip / integer page index) and Pink design docs for the Pink pagination components. [1][2]
Sources:
- Appwrite pagination (offset explanation). [1]
- Pink Design (Appwrite Pink design system / components). [2]
🏁 Script executed:
# Check the codebase for related files and usage patterns
fd -e svelte -e ts -e js | grep -i "pagination\|invoice" | head -20Repository: appwrite/console
Length of output: 971
🏁 Script executed:
# Look for package.json to check installed versions
cat package.json | grep -A 5 -B 5 "@appwrite"Repository: appwrite/console
Length of output: 923
🏁 Script executed:
cat -n src/routes/\(console\)/organization-\[organization\]/billing/paymentHistory.svelte | sed -n '40,60p'Repository: appwrite/console
Length of output: 823
🏁 Script executed:
# Also check what the limit variable is and how offset is being passed
cat -n src/routes/\(console\)/organization-\[organization\]/billing/paymentHistory.svelte | sed -n '1,100p' | grep -E "(limit|offset|let )"Repository: appwrite/console
Length of output: 312
🏁 Script executed:
# Let me check the PaginationInline component to understand what offset value it provides
cat -n src/lib/components/paginationInline.svelte | head -150Repository: appwrite/console
Length of output: 3287
🏁 Script executed:
# Also check how the offset is being used in the on:change handler
cat -n src/routes/\(console\)/organization-\[organization\]/billing/paymentHistory.svelte | sed -n '180,195p'Repository: appwrite/console
Length of output: 838
Remove the + 1 from Query.offset() — it causes the first invoice to be skipped and misaligns all subsequent pages.
PaginationInline already computes offset as the actual record skip count (0, 5, 10, etc.). Calling Query.offset(offset + 1) skips one extra record on every page. Use Query.offset(offset) instead.
Additionally, wrap the listInvoices call in a try/finally to reset isLoadingInvoices on error:
Suggested fix
async function loadInvoices() {
isLoadingInvoices = true;
try {
invoiceList = await sdk.forConsole.billing.listInvoices(page.params.organization, [
Query.orderDesc('$createdAt'),
Query.limit(limit),
Query.offset(offset)
]);
} finally {
isLoadingInvoices = false;
}
}🤖 Prompt for AI Agents
In
`@src/routes/`(console)/organization-[organization]/billing/paymentHistory.svelte
around lines 45 - 51, In loadInvoices(), remove the extra "+ 1" so
Query.offset(offset) is used (the +1 causes the first invoice to be skipped and
misaligns pagination), and wrap the sdk.forConsole.billing.listInvoices(...)
call in a try/finally that sets isLoadingInvoices = false in the finally block
so isLoadingInvoices is always reset on error; update references to invoiceList
and Query.orderDesc/Query.limit as needed inside the same try block.
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.