Skip to content

feat: pagination on billing ui table #2350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: pagination on billing ui table #2350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
},
"dependencies": {
"@ai-sdk/svelte": "^1.1.24",
"@appwrite.io/console": "https://pkg.pr.new/appwrite-labs/cloud/@appwrite.io/console@6031134",
"@appwrite.io/console": "https://pkg.pr.new/appwrite-labs/cloud/@appwrite.io/console@2515",
"@appwrite.io/pink-icons": "0.25.0",
"@appwrite.io/pink-icons-svelte": "https://pkg.vc/-/@appwrite/@appwrite.io/pink-icons-svelte@2cf27e0",
"@appwrite.io/pink-legacy": "^1.0.3",
Expand Down
10 changes: 5 additions & 5 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 9 additions & 2 deletions src/lib/sdk/billing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -934,12 +934,19 @@ export class Billing {
);
}

async getAggregation(organizationId: string, aggregationId: string): Promise<AggregationTeam> {
async getAggregation(
organizationId: string,
aggregationId: string,
limit?: number,
offset?: number
): Promise<AggregationTeam> {
const path = `/organizations/${organizationId}/aggregations/${aggregationId}`;
const params = {
const params: Record<string, unknown> = {
organizationId,
aggregationId
};
if (typeof limit === 'number') params['limit'] = limit;
if (typeof offset === 'number') params['offset'] = offset;
const uri = new URL(this.client.config.endpoint + path);
return await this.client.call(
'get',
Expand Down
5 changes: 4 additions & 1 deletion src/routes/(console)/organization-[organization]/billing/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@
availableCredit={data?.availableCredit}
currentPlan={data?.currentPlan}
nextPlan={data?.nextPlan}
currentAggregation={data?.billingAggregation} />
currentAggregation={data?.billingAggregation}
limit={data?.limit}
offset={data?.offset}
aggregationKey={data?.aggregationKey} />
{:else}
<PlanSummaryOld
availableCredit={data?.availableCredit}
Expand Down
20 changes: 17 additions & 3 deletions src/routes/(console)/organization-[organization]/billing/+page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import { redirect } from '@sveltejs/kit';
import type { PageLoad } from './$types';
import { isCloud } from '$lib/system';

export const load: PageLoad = async ({ parent, depends }) => {
import { getLimit, getPage, pageToOffset } from '$lib/helpers/load';

export const load: PageLoad = async ({ parent, depends, url, route }) => {
const { organization, scopes, currentPlan, countryList, locale } = await parent();

if (!scopes.includes('billing.read')) {
Expand All @@ -18,6 +20,8 @@ export const load: PageLoad = async ({ parent, depends }) => {
depends(Dependencies.CREDIT);
depends(Dependencies.INVOICES);
depends(Dependencies.ADDRESS);
//aggregation reloads on page param changes
depends('billing:aggregation');

const billingAddressId = (organization as Organization)?.billingAddressId;
const billingAddressPromise: Promise<Address> = billingAddressId
Expand All @@ -33,9 +37,14 @@ export const load: PageLoad = async ({ parent, depends }) => {
*/
let billingAggregation = null;
try {
const currentPage = getPage(url) || 1;
const limit = getLimit(url, route, 5);
const offset = pageToOffset(currentPage, limit);
billingAggregation = await sdk.forConsole.billing.getAggregation(
organization.$id,
(organization as Organization)?.billingAggregationId
(organization as Organization)?.billingAggregationId,
limit,
offset
);
Comment on lines +40 to 48
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Sanitize page/limit to avoid NaN/invalid values reaching the SDK.

getPage/getLimit can return NaN or <=0 from URL. Guard and normalize before computing offset and calling the SDK.

Apply:

-        const currentPage = getPage(url) || 1;
-        const limit = getLimit(url, route, 5);
-        const offset = pageToOffset(currentPage, limit);
+        const rawPage = getPage(url);
+        const rawLimit = getLimit(url, route, 5);
+        const currentPage =
+            Number.isFinite(rawPage) && rawPage > 0 ? Math.floor(rawPage) : 1;
+        const limit =
+            Number.isFinite(rawLimit) && rawLimit > 0 ? Math.floor(rawLimit) : 5;
+        const offset = pageToOffset(currentPage, limit);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const currentPage = getPage(url) || 1;
const limit = getLimit(url, route, 5);
const offset = pageToOffset(currentPage, limit);
billingAggregation = await sdk.forConsole.billing.getAggregation(
organization.$id,
(organization as Organization)?.billingAggregationId
(organization as Organization)?.billingAggregationId,
limit,
offset
);
const rawPage = getPage(url);
const rawLimit = getLimit(url, route, 5);
const currentPage =
Number.isFinite(rawPage) && rawPage > 0 ? Math.floor(rawPage) : 1;
const limit =
Number.isFinite(rawLimit) && rawLimit > 0 ? Math.floor(rawLimit) : 5;
const offset = pageToOffset(currentPage, limit);
billingAggregation = await sdk.forConsole.billing.getAggregation(
organization.$id,
(organization as Organization)?.billingAggregationId,
limit,
offset
);
🤖 Prompt for AI Agents 
In src/routes/(console)/organization-[organization]/billing/+page.ts around
lines 40 to 48, getPage/getLimit can return NaN or non-positive values from the
URL which then produce an invalid offset and bad args for the SDK; before
computing offset and calling sdk.forConsole.billing.getAggregation, coerce and
validate the values: parse/convert the returned page and limit to numbers, use
Number.isFinite and >0 checks, fallback page to 1 and limit to the existing
default (5) when invalid, then compute offset with the sanitized integers and
pass those to the SDK; ensure types remain number and avoid passing NaN or <=0
to pageToOffset or the SDK.

} catch (e) {
// ignore error
Expand Down Expand Up @@ -83,6 +92,11 @@ export const load: PageLoad = async ({ parent, depends }) => {
areCreditsSupported,
countryList,
locale,
nextPlan: billingPlanDowngrade
nextPlan: billingPlanDowngrade,
// expose pagination for components
limit: getLimit(url, route, 5),
offset: pageToOffset(getPage(url) || 1, getLimit(url, route, 5)),
// unique key to force component refresh on page change
aggregationKey: `agg:${getPage(url) || 1}:${getLimit(url, route, 5)}`
};
};
Loading
Loading