Skip to content

Conversation

HarshMN2345
Copy link
Member

@HarshMN2345 HarshMN2345 commented Sep 10, 2025

What does this PR do?

Show badge with count next to “Additional projects”
Implement backend-driven pagination (limit/offset) for project breakdown
Update SDK getAggregation(orgId, aggregationId, limit?, offset?)
Wire loader to read page/limit from URL and pass to component
Add dependency key and {#key} to force reload on page changes

Test Plan

image image

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?

yes

Summary by CodeRabbit

  • New Features
    • Pagination for Billing usage with page/limit controls and pagination-aware loading.
    • Revamped Plan Summary: accordion-style breakdown, client-side project pagination, badges for add-ons/projects, and updated totals/credits handling.
  • Bug Fixes
    • Ensures summary refreshes when pagination/aggregation changes.
  • Style
    • Adjusted paginator sizing and responsive layout for the new accordion view.
  • Chores
    • Updated several UI-related dependencies to newer sources.

Copy link

appwrite bot commented Sep 10, 2025

Console

Project ID: 688b7bf400350cbd60e9

Sites (2)
Site Status Logs Preview QR
 console-stage
688b7cf6003b1842c9dc
Ready Ready View Logs Preview URL QR Code
 console-cloud
688b7c18002b9b871a8f
Ready Ready View Logs Preview URL QR Code

Note

Appwrite has a Discord community with over 16 000 members.

Copy link
Contributor

coderabbitai bot commented Sep 10, 2025

Warning

Rate limit exceeded

@HarshMN2345 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 38 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 31b66ad and 6b3e79d.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • package.json (1 hunks)
  • src/lib/constants.ts (1 hunks)
  • src/routes/(console)/organization-[organization]/billing/+page.ts (4 hunks)
  • src/routes/(console)/organization-[organization]/billing/planSummary.svelte (6 hunks)

Walkthrough

  • Updated dependency sources in package.json for @appwrite.io/console, @appwrite.io/pink-icons-svelte, and @appwrite.io/pink-svelte.
  • SDK: Billing.getAggregation signature changed to accept optional pagination params: (organizationId, aggregationId, limit?, offset?) and will include numeric limit/offset as query params.
  • Billing page load (+page.ts) now accepts url and route, computes limit/offset (getLimit/getPage/pageToOffset), returns limit, offset, aggregationKey, and calls depends('billing:aggregation').
  • +page.svelte now passes limit, offset, and aggregationKey to PlanSummary.
  • planSummary.svelte adds exported props (limit, offset, aggregationKey), replaces ExpandableTable with AccordionTable, introduces client-side pagination for project items, adjusts row/summary rendering and total calculation, and wraps main content with a {#key aggregationKey} to force remount on key changes.
  • constants.ts adds Dependencies.BILLING_AGGREGATION = 'dependency:billing_aggregation'.

Possibly related PRs

  • Feat: New Billing UI changes #2295: Changes to billing SDK/types and getAggregation signature plus UI/load updates — directly overlaps the SDK getAggregation signature change.
  • fix: calculation. #2305: Modifies planSummary component props and billing calculation logic — overlaps AccordionTable migration and total/credits computation changes.
  • feat: added image transformation #2303: Updates per-project billing rows (imageTransformations) in planSummary — touches the same planSummary data structure and rendering.

Suggested reviewers

  • lohanidamodar
  • ItzNotABug

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly captures the primary change by indicating the addition of pagination to the billing table UI, aligning directly with the pull request objectives around backend-driven pagination, updated SDK signatures, and UI adjustments for page navigation. It is concise, clear, and focused on the main feature without unnecessary details.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (3)

341-346: NaN bug when availableCredit is undefined

Math.min with undefined yields NaN, propagating to totalAmount.

-$: totalAmount = Math.max(currentAggregation?.amount - creditsApplied, 0);
-
-$: creditsApplied = Math.min(
-    currentAggregation?.amount ?? currentPlan?.price ?? 0,
-    availableCredit
-);
+$: creditsApplied = Math.min(
+    currentAggregation?.amount ?? currentPlan?.price ?? 0,
+    availableCredit ?? 0
+);
+$: totalAmount = Math.max((currentAggregation?.amount ?? 0) - creditsApplied, 0);

218-316: Possible runtime errors: missing optional chaining on project resource fields

project.* lookups (find) can be undefined; direct .value/.amount access will throw.

-usage: `${formatBandwidthUsage(project.bandwidth.value, currentPlan?.bandwidth)}`,
-price: formatCurrency(project.bandwidth.amount || 0)
+usage: `${formatBandwidthUsage(project.bandwidth?.value ?? 0, currentPlan?.bandwidth)}`,
+price: formatCurrency(project.bandwidth?.amount ?? 0)
...
-usage: `${formatNum(project.users.value || 0)} / ${currentPlan?.users ? formatNum(currentPlan.users) : 'Unlimited'}`,
-price: formatCurrency(project.users.amount || 0)
+usage: `${formatNum(project.users?.value ?? 0)} / ${currentPlan?.users ? formatNum(currentPlan.users) : 'Unlimited'}`,
+price: formatCurrency(project.users?.amount ?? 0)
...
-usage: `${formatNum(project.databasesReads.value || 0)} / ${currentPlan?.databasesReads ? formatNum(currentPlan.databasesReads) : 'Unlimited'}`,
-price: formatCurrency(project.databasesReads.amount || 0)
+usage: `${formatNum(project.databasesReads?.value ?? 0)} / ${currentPlan?.databasesReads ? formatNum(currentPlan.databasesReads) : 'Unlimited'}`,
+price: formatCurrency(project.databasesReads?.amount ?? 0)
...
-usage: `${formatNum(project.databasesWrites.value || 0)} / ${currentPlan?.databasesWrites ? formatNum(currentPlan.databasesWrites) : 'Unlimited'}`,
-price: formatCurrency(project.databasesWrites.amount || 0)
+usage: `${formatNum(project.databasesWrites?.value ?? 0)} / ${currentPlan?.databasesWrites ? formatNum(currentPlan.databasesWrites) : 'Unlimited'}`,
+price: formatCurrency(project.databasesWrites?.amount ?? 0)
...
-usage: `${formatNum(project.executions.value || 0)} / ${currentPlan?.executions ? formatNum(currentPlan.executions) : 'Unlimited'}`,
-price: formatCurrency(project.executions.amount || 0)
+usage: `${formatNum(project.executions?.value ?? 0)} / ${currentPlan?.executions ? formatNum(currentPlan.executions) : 'Unlimited'}`,
+price: formatCurrency(project.executions?.amount ?? 0)
...
-usage: `${formatHumanSize(project.storage.value || 0)} / ${currentPlan?.storage?.toString() || '0'} GB`,
-price: formatCurrency(project.storage.amount || 0)
+usage: `${formatHumanSize(project.storage?.value ?? 0)} / ${currentPlan?.storage?.toString() || '0'} GB`,
+price: formatCurrency(project.storage?.amount ?? 0)
...
-usage: `${formatNum(project.imageTransformations.value || 0)} / ${currentPlan?.imageTransformations ? formatNum(currentPlan.imageTransformations) : 'Unlimited'}`,
-price: formatCurrency(project.imageTransformations.amount || 0)
+usage: `${formatNum(project.imageTransformations?.value ?? 0)} / ${currentPlan?.imageTransformations ? formatNum(currentPlan.imageTransformations) : 'Unlimited'}`,
+price: formatCurrency(project.imageTransformations?.amount ?? 0)
...
-usage: `${formatNum(project.gbHours.value || 0)} / ${currentPlan?.GBHours ? formatNum(currentPlan.GBHours) : 'Unlimited'}`,
-price: formatCurrency(project.gbHours.amount || 0)
+usage: `${formatNum(project.gbHours?.value ?? 0)} / ${currentPlan?.GBHours ? formatNum(currentPlan.GBHours) : 'Unlimited'}`,
+price: formatCurrency(project.gbHours?.amount ?? 0)
...
-usage: `${formatNum(project.authPhone.value || 0)} SMS messages`,
-price: formatCurrency(project.authPhone.amount || 0)
+usage: `${formatNum(project.authPhone?.value ?? 0)} SMS messages`,
+price: formatCurrency(project.authPhone?.amount ?? 0)

Also update related progressData inputs to use ?.value ?? 0.


326-333: Avoid {@html} for links (XSS risk) and honor base path

Build the link via Svelte markup to prevent injection; also prefix with base for subpath deployments.

-{
-    id: `usage-details`,
-    cells: {
-        item: `<a href="/console/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`,
-        usage: '',
-        price: ''
-    }
-}
+{
+    id: `usage-details`,
+    cells: { item: 'Usage details', usage: '', price: '' },
+    linkHref: `${base}/console/project-${String(project.region || 'default')}-${project.projectId}/settings/usage`
+}
-{#if child.cells?.[col.id]?.includes('<a href=')}
-    {@html child.cells?.[col.id] ?? ''}
+{#if col.id === 'item' && child.linkHref}
+    <a href={child.linkHref} rel="noopener noreferrer" class="u-underline" style="color: var(--fgcolor-accent-neutral);">
+        {child.cells?.[col.id]}
+    </a>

Also applies to: 434-436

src/routes/(console)/organization-[organization]/billing/+page.svelte (1)

110-113: Fix user-facing grammar.

“In organization's your projects” is incorrect.

Apply:

-                To avoid service disruptions in organization's your projects, please verify your
-                payment details and update them if necessary.
+                To avoid service disruptions in your organization's projects, please verify your
+                payment details and update them if necessary.
🧹 Nitpick comments (10)
src/lib/sdk/billing.ts (2)

937-950: Harden limit/offset handling (avoid NaN/negatives) and send ints

Guard against NaN and negative values; truncate to integers before sending.

-        if (typeof limit === 'number') params['limit'] = limit;
-        if (typeof offset === 'number') params['offset'] = offset;
+        if (Number.isFinite(limit) && (limit as number) > 0) {
+            params['limit'] = Math.trunc(limit as number);
+        }
+        if (Number.isFinite(offset) && (offset as number) >= 0) {
+            params['offset'] = Math.trunc(offset as number);
+        }

937-942: Return type may need optional totals for pagination

The UI reads breakdownTotal/projectsTotal for pagination. Consider extending AggregationTeam with optional totals to reflect backend shape.

Would you like me to patch the type to:

export type AggregationTeam = { /* existing */ } & {
  breakdownTotal?: number;
  projectsTotal?: number;
};

and wire tests accordingly?

src/routes/(console)/organization-[organization]/billing/planSummary.svelte (6)

154-159: Clamp URL-driven pagination to safe bounds

Prevent negative/invalid values from URL.

-$: projectsLimit = limit ?? (Number(page.url.searchParams.get('limit')) || 5);
-$: projectsOffset =
-    offset ?? ((Number(page.url.searchParams.get('page')) || 1) - 1) * projectsLimit;
+$: projectsLimit = limit ?? Math.max(1, Number(page.url.searchParams.get('limit')) || 5);
+$: projectsOffset =
+    offset ??
+    ((Math.max(1, Number(page.url.searchParams.get('page')) || 1) - 1) * projectsLimit);

781-785: Selector likely mismatched with row id

CSS targets [data-expandable-row-id='additional-projects'] but row id is "addon-projects".

-:global([data-expandable-row-id='additional-projects']),
+:global([data-expandable-row-id='addon-projects']),
 :global([data-expandable-row-id^='addon-']) {

798-804: Fix typo in comment

Minor nit.

-/* reducingh size of paginator */
+/* reduce size of paginator */

1-47: Optional: tighten utility function guards

formatHumanSize/formatBandwidthUsage handle falsy values; consider coalescing inputs to 0 explicitly for consistency.


167-187: Null-safety for addons map

currentPlan.addons is assumed defined when filtering. Consider local var to avoid accidental undefined access.

-const addons = (currentAggregation?.resources || [])
+const planAddons = currentPlan?.addons ?? {};
+const addons = (currentAggregation?.resources ?? [])
     .filter(
         (r) =>
             r.amount &&
             r.amount > 0 &&
-            Object.keys(currentPlan?.addons || {}).includes(r.resourceId) &&
-            currentPlan.addons[r.resourceId]?.price > 0
+            Object.prototype.hasOwnProperty.call(planAddons, r.resourceId) &&
+            planAddons[r.resourceId]?.price > 0
     )

31-35: Prop defaults: consider number defaults for pagination inputs

Defaulting limit/offset to numbers (0/undefined mix) causes conditional checks later. Prefer numeric defaults upstream to simplify.

src/routes/(console)/organization-[organization]/billing/+page.ts (2)

23-25: Optional: depends key may be unnecessary for query param nav.

SvelteKit re-runs load on search param changes. Keep depends('billing:aggregation') only if you plan to invalidate it elsewhere.


95-101: Avoid recomputing pagination; reuse computed values.

Prevents drift and keeps a single source of truth.

Apply:

-        // 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)}`
+        // expose pagination for components
+        limit,
+        offset,
+        // unique key to force component refresh on page change
+        aggregationKey: `agg:${currentPage}:${limit}`
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ba5bfd and 29c12d6.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • package.json (1 hunks)
  • src/lib/sdk/billing.ts (1 hunks)
  • src/routes/(console)/organization-[organization]/billing/+page.svelte (1 hunks)
  • src/routes/(console)/organization-[organization]/billing/+page.ts (4 hunks)
  • src/routes/(console)/organization-[organization]/billing/planSummary.svelte (7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/routes/(console)/organization-[organization]/billing/+page.ts (3)
src/routes/(console)/organization-[organization]/+page.ts (1)
  • load (8-42)
src/lib/helpers/load.ts (3)
  • getPage (8-10)
  • getLimit (12-16)
  • pageToOffset (4-6)
src/lib/stores/organization.ts (2)
  • organization (62-62)
  • Organization (16-38)
🪛 ESLint
src/routes/(console)/organization-[organization]/billing/planSummary.svelte

[error] 160-160: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)


[error] 161-161: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

⏰ 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 (9)
package.json (1)

25-25: pnpm-lock update verified; confirm CI artifact resolution

  • pnpm-lock.yaml includes the new @appwrite.io/console@2515 entry.
  • Verify your CI pipeline can authenticate to pkg.pr.new, resolve and cache this PR artifact URL reliably.
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (5)

196-203: Nice: badge for “Additional projects”

Clean UX improvement; keeps item text concise and surfaces the count visually.


350-365: Good use of keyed block to force re-render on page changes

{#key aggregationKey} ensures a clean remount when pagination changes.


556-575: Analytics: good switch to typed constant

Using Click.OrganizationClickUpgrade improves event hygiene.


152-153: Incorrect page store import

The page store should come from $app/stores. Using $app/state will fail.

-import { page } from '$app/state';
+import { page } from '$app/stores';

Likely an incorrect or invalid review comment.


494-503: No changes needed: PaginationComponent correctly updates the page query-param
PaginationComponent’s getLink sets/removes the page parameter on its generated links, ensuring SvelteKit navigation triggers the +page.ts loader and re-runs depends('billing:aggregation').

src/routes/(console)/organization-[organization]/billing/+page.svelte (2)

136-139: Prop pass-through for backend pagination looks correct.

Passing limit, offset, and aggregationKey to PlanSummary aligns with the new load API.


136-139: Verify PlanSummary exports and wrap in key block
Ensure the PlanSummary component exports limit, offset, and aggregationKey, and in src/routes/(console)/organization-[organization]/billing/+page.svelte wrap the <PlanSummary> invocation in

{#key data.aggregationKey}
  <PlanSummary … />
{/key}

to force a remount when the key changes.

src/routes/(console)/organization-[organization]/billing/+page.ts (1)

9-10: Imports for pagination helpers look good.

Comment on lines 160 to 165
(currentAggregation && (currentAggregation as any).breakdownTotal) ||
(currentAggregation && (currentAggregation as any).projectsTotal) ||
(currentAggregation?.resources?.find?.((r) => r.resourceId === 'projects')?.value ??
null) ||
currentAggregation?.breakdown?.length ||
0;
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

Fix ESLint “no-explicit-any” and keep strong typing for totals

Avoid casts to any by introducing a local augmentation type.

+$: type AggregationTotals = Partial<{ breakdownTotal: number; projectsTotal: number }>;
 $: totalProjects =
-    (currentAggregation && (currentAggregation as any).breakdownTotal) ||
-    (currentAggregation && (currentAggregation as any).projectsTotal) ||
+    (currentAggregation && (currentAggregation as AggregationTotals).breakdownTotal) ||
+    (currentAggregation && (currentAggregation as AggregationTotals).projectsTotal) ||
     (currentAggregation?.resources?.find?.((r) => r.resourceId === 'projects')?.value ??
         null) ||
     currentAggregation?.breakdown?.length ||
     0;

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 ESLint

[error] 160-160: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)


[error] 161-161: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/billing/planSummary.svelte
around lines 160 to 165, avoid the explicit any casts by introducing a local
typed interface/alias that augments the shape of currentAggregation (e.g.,
fields breakdownTotal?: number, projectsTotal?: number, resources?: {
resourceId: string; value?: number }[], breakdown?: unknown[]), annotate
currentAggregation with that type, and then rewrite the expression to use the
typed properties with optional chaining and nullish coalescing (no as any casts)
so the totals resolution remains the same but with strong typing.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (2)

341-347: Avoid NaN flicker and undefined credit: compute credits first and default to 0

totalAmount uses creditsApplied before it’s defined, and Math.min(..., availableCredit) yields NaN when availableCredit is undefined.

Apply:

-$: totalAmount = Math.max(currentAggregation?.amount - creditsApplied, 0);
-
-$: creditsApplied = Math.min(
-    currentAggregation?.amount ?? currentPlan?.price ?? 0,
-    availableCredit
-);
+$: creditsApplied = Math.min(
+    (currentAggregation?.amount ?? currentPlan?.price ?? 0),
+    (availableCredit ?? 0)
+);
+$: totalAmount = Math.max((currentAggregation?.amount ?? 0) - creditsApplied, 0);

205-334: Guard resource access with optional chaining to prevent crashes

Several project.<resource>.value/amount reads assume the resource exists. When absent, this dereferences undefined and crashes the UI.

Apply the pattern project.<res>?.value ?? 0 and project.<res>?.amount ?? 0. For example:

-usage: `${formatBandwidthUsage(project.bandwidth.value, currentPlan?.bandwidth)}`,
-price: formatCurrency(project.bandwidth.amount || 0)
+usage: `${formatBandwidthUsage(project.bandwidth?.value ?? 0, currentPlan?.bandwidth)}`,
+price: formatCurrency(project.bandwidth?.amount ?? 0)
...
-usage: `${formatNum(project.users.value || 0)} / ${currentPlan?.users ? ...}`,
-price: formatCurrency(project.users.amount || 0)
+usage: `${formatNum(project.users?.value ?? 0)} / ${currentPlan?.users ? ...}`,
+price: formatCurrency(project.users?.amount ?? 0)
...
-usage: `${formatNum(project.databasesReads.value || 0)} / ...`,
-price: formatCurrency(project.databasesReads.amount || 0)
+usage: `${formatNum(project.databasesReads?.value ?? 0)} / ...`,
+price: formatCurrency(project.databasesReads?.amount ?? 0)
...
-usage: `${formatNum(project.databasesWrites.value || 0)} / ...`,
-price: formatCurrency(project.databasesWrites.amount || 0)
+usage: `${formatNum(project.databasesWrites?.value ?? 0)} / ...`,
+price: formatCurrency(project.databasesWrites?.amount ?? 0)
...
-usage: `${formatNum(project.executions.value || 0)} / ...`,
-price: formatCurrency(project.executions.amount || 0)
+usage: `${formatNum(project.executions?.value ?? 0)} / ...`,
+price: formatCurrency(project.executions?.amount ?? 0)
...
-usage: `${formatHumanSize(project.storage.value || 0)} / ...`,
-price: formatCurrency(project.storage.amount || 0)
+usage: `${formatHumanSize(project.storage?.value ?? 0)} / ...`,
+price: formatCurrency(project.storage?.amount ?? 0)
...
-usage: `${formatNum(project.imageTransformations.value || 0)} / ...`,
-price: formatCurrency(project.imageTransformations.amount || 0)
+usage: `${formatNum(project.imageTransformations?.value ?? 0)} / ...`,
+price: formatCurrency(project.imageTransformations?.amount ?? 0)
...
-usage: `${formatNum(project.gbHours.value || 0)} / ...`,
+usage: `${formatNum(project.gbHours?.value ?? 0)} / ...`,
...
-                        ? createProgressData(project.gbHours.value || 0, currentPlan.GBHours)
+                        ? createProgressData(project.gbHours?.value ?? 0, currentPlan.GBHours)
...
-usage: `${formatNum(project.authPhone.value || 0)} SMS messages`,
-price: formatCurrency(project.authPhone.amount || 0)
+usage: `${formatNum(project.authPhone?.value ?? 0)} SMS messages`,
+price: formatCurrency(project.authPhone?.amount ?? 0)

Also update progress inputs similarly:

-progressData: createStorageProgressData(project.bandwidth.value || 0, ...),
+progressData: createStorageProgressData(project.bandwidth?.value ?? 0, ...),
♻️ Duplicate comments (1)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (1)

159-165: Use nullish coalescing and proper typing; drop any casts

|| will skip legitimate zeros; as any violates lint rules. Type narrow and use ??.

Apply:

+$: type AggregationTotals = Partial<{ breakdownTotal: number; projectsTotal: number }>;
 $: totalProjects =
-    (currentAggregation && (currentAggregation as any).breakdownTotal) ||
-    (currentAggregation && (currentAggregation as any).projectsTotal) ||
-    (currentAggregation?.resources?.find?.((r) => r.resourceId === 'projects')?.value ??
-        null) ||
-    currentAggregation?.breakdown?.length ||
-    0;
+    (currentAggregation as AggregationTotals)?.breakdownTotal ??
+    (currentAggregation as AggregationTotals)?.projectsTotal ??
+    currentAggregation?.resources?.find?.((r) => r.resourceId === 'projects')?.value ??
+    currentAggregation?.breakdown?.length ??
+    0;
🧹 Nitpick comments (10)
package.json (1)

25-29: Pin PR/VC tarball deps or swap to stable tags before merge

Using URL tarballs from pkg.pr.new/pkg.vc is fine for iterating, but it can hurt reproducibility and break in the future. Either:

  • keep them commit/immutable and ensure CI caches/allowlists them, or
  • switch to published, semver-pinned releases before merging to main.

Also verify the lockfile captures integrity for these URLs.

src/routes/(console)/organization-[organization]/billing/planSummary.svelte (9)

326-333: Avoid injecting HTML; render an element and include base

String HTML + {@html} is unnecessary and bypasses type checks. Also, prefix with base for subpath deployments.

Apply:

- item: `<a href="/console/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`,
+ item: {
+   type: 'link',
+   href: `${base}/console/project-${String(project.region || 'default')}-${project.projectId}/settings/usage`,
+   label: 'Usage details'
+ },

And in the summary cell:

-{#if child.cells?.item?.includes('<a href=')}
-    {@html child.cells?.item ?? ''}
-{:else}
+{#if typeof child.cells?.item === 'object' && child.cells?.item?.type === 'link'}
+    <a href={child.cells.item.href} class="usage-link"> {child.cells.item.label} </a>
+{:else}

187-203: Badge logic: handle zero count explicitly

row.badge is falsy for zero and won’t render. If zero should be shown, coerce to string and check !== null && !== undefined.

-badge: excess.resourceId === 'projects' ? formatNum(excess.value) : null,
+badge: excess.resourceId === 'projects' ? String(formatNum(excess.value ?? 0)) : null,

And:

-{#if row.badge}
+{#if row.badge !== null && row.badge !== undefined}

742-750: Dead selector? Align CSS with runtime IDs

Selector targets [data-expandable-row-id='additional-projects'] but no row uses that ID; addons use id: addon-.... Either remove or update to the actual attribute used by AccordionTable.Row.


763-769: Typo and styling nit

Fix comment typo and consider using a tokenized size instead of scale transform (better for a11y and crispness).

-/* reducingh size of paginator */
+/* reducing size of paginator */

38-43: Columns widths: guard against narrow viewports

Static min widths of 500 + 200 can overflow on small screens. Consider responsive widths or CSS clamp.


118-150: Type currentAggregation and return shape of getProjectsList

Add types to eliminate implicit any and catch resource ID typos at compile time.

-function getProjectsList(currentAggregation) {
+type ProjectResource = { resourceId: string; value?: number; amount?: number };
+type ProjectBreakdown = {
+  $id: string; name?: string; region?: string; amount?: number; resources?: ProjectResource[];
+};
+function getProjectsList(currentAggregation: { breakdown?: ProjectBreakdown[] } | undefined) {

493-506: Pagination row condition: prefer ≥ and null-safe sums

If totalProjects === projectsLimit, consider whether to show pagination for page=1. Also ensure sum is a number.

-{#if totalProjects > projectsLimit}
+{#if (Number(totalProjects) || 0) >= (Number(projectsLimit) || 0)}

507-531: Credits row visibility matches creditsApplied not availableCredit

Currently hidden when availableCredit is unset but computed credits > 0 (after defaulting to 0). Show row based on creditsApplied > 0.

-{#if availableCredit > 0}
+{#if creditsApplied > 0}

518-529: Currency on credits: ensure negative formatting once

You already prefix - in template; ensure formatCurrency doesn’t double-negative on negative values. Safer to pass positive and keep the - in template, or pass -creditsApplied and drop the manual -.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 29c12d6 and c0e3043.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (2)
  • package.json (1 hunks)
  • src/routes/(console)/organization-[organization]/billing/planSummary.svelte (7 hunks)
🧰 Additional context used
🪛 ESLint
src/routes/(console)/organization-[organization]/billing/planSummary.svelte

[error] 160-160: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)


[error] 161-161: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

🔇 Additional comments (4)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (4)

349-365: Keyed re-render: good use of {#key} for pagination reloads

This prevents stale rows on page changes and matches the loader’s aggregationKey.

Ensure loaders update aggregationKey whenever page or limit changes so this reliably remounts.


554-573: Analytics event: good switch to typed enum

Consistent with the rest of the codebase and safer than string literals.


320-336: Units and max value for bandwidth/storage

formatBandwidthUsage treats max as GB; progress bars set maxValue in bytes. Confirm resource units are consistent across usage and progress.


1-23: Imports look coherent; keep components sourced from one bundle

Nice consolidation of UI imports through pink-svelte and $lib/components.

Comment on lines 152 to 158
import { page } from '$app/state';
let projectsLimit: number = 5;
let projectsOffset: number = 0;
$: projectsLimit = limit ?? (Number(page.url.searchParams.get('limit')) || 5);
$: projectsOffset =
offset ?? ((Number(page.url.searchParams.get('page')) || 1) - 1) * projectsLimit;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix store usage: use $page (value) not page (store object)

Accessing page.url reads the store object, not its value, causing runtime errors. Use $page in reactive statements.

Apply:

-import { page } from '$app/state';
+import { page } from '$app/stores';
...
-$: projectsLimit = limit ?? (Number(page.url.searchParams.get('limit')) || 5);
-$: projectsOffset =
-    offset ?? ((Number(page.url.searchParams.get('page')) || 1) - 1) * projectsLimit;
+$: projectsLimit = limit ?? (Number($page.url.searchParams.get('limit')) || 5);
+$: projectsOffset =
+    offset ?? ((Number($page.url.searchParams.get('page')) || 1) - 1) * projectsLimit;
📝 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
import { page } from '$app/state';
let projectsLimit: number = 5;
let projectsOffset: number = 0;
$: projectsLimit = limit ?? (Number(page.url.searchParams.get('limit')) || 5);
$: projectsOffset =
offset ?? ((Number(page.url.searchParams.get('page')) || 1) - 1) * projectsLimit;
import { page } from '$app/stores';
let projectsLimit: number = 5;
let projectsOffset: number = 0;
$: projectsLimit = limit ?? (Number($page.url.searchParams.get('limit')) || 5);
$: projectsOffset =
offset ?? ((Number($page.url.searchParams.get('page')) || 1) - 1) * projectsLimit;
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/billing/planSummary.svelte
around lines 152 to 158, the reactive statements use the store object `page`
instead of its value which causes runtime errors; update the reactive
assignments to reference the store value `$page` (e.g., use
`$page.url.searchParams.get(...)`) so the reactive expressions read the current
URL values, keeping the existing import of `page` intact and leaving the rest of
the logic unchanged.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (3)

341-347: Fix NaN and dependency ordering in totals calculation.

If availableCredit is undefined or currentAggregation is undefined, creditsApplied/totalAmount can become NaN. Also compute base first for clarity.

-$: totalAmount = Math.max(currentAggregation?.amount - creditsApplied, 0);
-
-$: creditsApplied = Math.min(
-    currentAggregation?.amount ?? currentPlan?.price ?? 0,
-    availableCredit
-);
+$: const baseAmount = (currentAggregation?.amount ?? currentPlan?.price ?? 0);
+$: creditsApplied = Math.min(baseAmount, availableCredit ?? 0);
+$: totalAmount = Math.max(baseAmount - creditsApplied, 0);

214-334: Guard all project resource reads with optional chaining to avoid runtime crashes.

project.<resource>.value/amount can be undefined (resources are looked up with find). Direct property access will throw.

Apply optional chaining across the child rows (pattern shown; repeat for all similar fields):

@@
-                        usage: `${formatBandwidthUsage(project.bandwidth.value, currentPlan?.bandwidth)}`,
-                        price: formatCurrency(project.bandwidth.amount || 0)
+                        usage: `${formatBandwidthUsage(project.bandwidth?.value || 0, currentPlan?.bandwidth)}`,
+                        price: formatCurrency(project.bandwidth?.amount || 0)
@@
-                    progressData: createStorageProgressData(
-                        project.bandwidth.value || 0,
+                    progressData: createStorageProgressData(
+                        project.bandwidth?.value || 0,
                         currentPlan?.bandwidth || 0
                     ),
@@
-                        usage: `${formatNum(project.users.value || 0)} / ${currentPlan?.users ? formatNum(currentPlan.users) : 'Unlimited'}`,
-                        price: formatCurrency(project.users.amount || 0)
+                        usage: `${formatNum(project.users?.value || 0)} / ${currentPlan?.users ? formatNum(currentPlan.users) : 'Unlimited'}`,
+                        price: formatCurrency(project.users?.amount || 0)
@@
-                    progressData: createProgressData(project.users.value || 0, currentPlan?.users),
+                    progressData: createProgressData(project.users?.value || 0, currentPlan?.users),
@@
-                        usage: `${formatNum(project.databasesReads.value || 0)} / ${currentPlan?.databasesReads ? formatNum(currentPlan.databasesReads) : 'Unlimited'}`,
-                        price: formatCurrency(project.databasesReads.amount || 0)
+                        usage: `${formatNum(project.databasesReads?.value || 0)} / ${currentPlan?.databasesReads ? formatNum(currentPlan.databasesReads) : 'Unlimited'}`,
+                        price: formatCurrency(project.databasesReads?.amount || 0)
@@
-                    progressData: createProgressData(
-                        project.databasesReads.value || 0,
+                    progressData: createProgressData(
+                        project.databasesReads?.value || 0,
                         currentPlan?.databasesReads
                     ),
@@
-                        usage: `${formatNum(project.databasesWrites.value || 0)} / ${currentPlan?.databasesWrites ? formatNum(currentPlan.databasesWrites) : 'Unlimited'}`,
-                        price: formatCurrency(project.databasesWrites.amount || 0)
+                        usage: `${formatNum(project.databasesWrites?.value || 0)} / ${currentPlan?.databasesWrites ? formatNum(currentPlan.databasesWrites) : 'Unlimited'}`,
+                        price: formatCurrency(project.databasesWrites?.amount || 0)
@@
-                    progressData: createProgressData(
-                        project.databasesWrites.value || 0,
+                    progressData: createProgressData(
+                        project.databasesWrites?.value || 0,
                         currentPlan?.databasesWrites
                     ),
@@
-                        usage: `${formatNum(project.executions.value || 0)} / ${currentPlan?.executions ? formatNum(currentPlan.executions) : 'Unlimited'}`,
-                        price: formatCurrency(project.executions.amount || 0)
+                        usage: `${formatNum(project.executions?.value || 0)} / ${currentPlan?.executions ? formatNum(currentPlan.executions) : 'Unlimited'}`,
+                        price: formatCurrency(project.executions?.amount || 0)
@@
-                    progressData: createProgressData(
-                        project.executions.value || 0,
+                    progressData: createProgressData(
+                        project.executions?.value || 0,
                         currentPlan?.executions
                     ),
@@
-                        usage: `${formatHumanSize(project.storage.value || 0)} / ${currentPlan?.storage?.toString() || '0'} GB`,
-                        price: formatCurrency(project.storage.amount || 0)
+                        usage: `${formatHumanSize(project.storage?.value || 0)} / ${currentPlan?.storage?.toString() || '0'} GB`,
+                        price: formatCurrency(project.storage?.amount || 0)
@@
-                    progressData: createStorageProgressData(
-                        project.storage.value || 0,
+                    progressData: createStorageProgressData(
+                        project.storage?.value || 0,
                         currentPlan?.storage || 0
                     ),
@@
-                        usage: `${formatNum(project.imageTransformations.value || 0)} / ${currentPlan?.imageTransformations ? formatNum(currentPlan.imageTransformations) : 'Unlimited'}`,
-                        price: formatCurrency(project.imageTransformations.amount || 0)
+                        usage: `${formatNum(project.imageTransformations?.value || 0)} / ${currentPlan?.imageTransformations ? formatNum(currentPlan.imageTransformations) : 'Unlimited'}`,
+                        price: formatCurrency(project.imageTransformations?.amount || 0)
@@
-                    progressData: createProgressData(
-                        project.imageTransformations.value || 0,
+                    progressData: createProgressData(
+                        project.imageTransformations?.value || 0,
                         currentPlan?.imageTransformations
                     ),
@@
-                        usage: `${formatNum(project.gbHours.value || 0)} / ${currentPlan?.GBHours ? formatNum(currentPlan.GBHours) : 'Unlimited'}`,
-                        price: formatCurrency(project.gbHours.amount || 0)
+                        usage: `${formatNum(project.gbHours?.value || 0)} / ${currentPlan?.GBHours ? formatNum(currentPlan.GBHours) : 'Unlimited'}`,
+                        price: formatCurrency(project.gbHours?.amount || 0)
@@
-                        ? createProgressData(project.gbHours.value || 0, currentPlan.GBHours)
+                        ? createProgressData(project.gbHours?.value || 0, currentPlan.GBHours)
@@
-                        usage: `${formatNum(project.authPhone.value || 0)} SMS messages`,
-                        price: formatCurrency(project.authPhone.amount || 0)
+                        usage: `${formatNum(project.authPhone?.value || 0)} SMS messages`,
+                        price: formatCurrency(project.authPhone?.amount || 0)

326-333: Avoid {@html ...}; render links safely to prevent XSS.

The current approach injects raw HTML with user-derived data (project.region, projectId). Replace with a bound <a> element.

-                {
-                    id: `usage-details`,
-                    cells: {
-                        item: `<a href="/console/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`,
-                        usage: '',
-                        price: ''
-                    }
-                },
+                {
+                    id: `usage-details`,
+                    cells: {
+                        item: 'Usage details',
+                        linkHref: `${base}/project-${String(project.region || 'default')}-${project.projectId}/settings/usage`,
+                        usage: '',
+                        price: ''
+                    }
+                },
@@
-                                                {#if child.cells?.item?.includes('<a href=')}
-                                                    {@html child.cells?.item ?? ''}
-                                                {:else}
-                                                    <Typography.Text
-                                                        variant="m-400"
-                                                        color="--fgcolor-neutral-secondary">
-                                                        {child.cells?.item ?? ''}
-                                                    </Typography.Text>
-                                                {/if}
+                                                {#if child.cells?.linkHref}
+                                                    <a href={child.cells.linkHref}
+                                                        style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">
+                                                        Usage details
+                                                    </a>
+                                                {:else}
+                                                    <Typography.Text variant="m-400" color="--fgcolor-neutral-secondary">
+                                                        {child.cells?.item ?? ''}
+                                                    </Typography.Text>
+                                                {/if}

Also applies to: 421-429

♻️ Duplicate comments (2)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (2)

152-158: Fix SvelteKit store usage: import from $app/stores and read $page (value), not the store object.

Using '$app/state' and page.url reads the store object and will break at runtime. Use '$app/stores' and $page.url.

-import { page } from '$app/state';
+import { page } from '$app/stores';
@@
-$: projectsLimit = limit ?? (Number(page.url.searchParams.get('limit')) || 5);
-$: projectsOffset =
-    offset ?? ((Number(page.url.searchParams.get('page')) || 1) - 1) * projectsLimit;
+$: projectsLimit = limit ?? (Number($page.url.searchParams.get('limit')) || 5);
+$: projectsOffset =
+    offset ?? ((Number($page.url.searchParams.get('page')) || 1) - 1) * projectsLimit;

160-165: Remove any and preserve 0-values with nullish coalescing.

Two issues:

  • as any violates lint rules.
  • Using || drops valid 0 values; use ??.

Apply this diff for the reactive expression:

-$: totalProjects =
-    (currentAggregation && (currentAggregation as any).breakdownTotal) ||
-    (currentAggregation && (currentAggregation as any).projectsTotal) ||
-    (currentAggregation?.resources?.find?.((r) => r.resourceId === 'projects')?.value ??
-        null) ||
-    currentAggregation?.breakdown?.length ||
-    0;
+$: totalProjects =
+    (currentAggregation as AggregationTotals)?.breakdownTotal ??
+    (currentAggregation as AggregationTotals)?.projectsTotal ??
+    currentAggregation?.resources?.find?.((r) => r.resourceId === 'projects')?.value ??
+    currentAggregation?.breakdown?.length ??
+    0;

Add this local type (outside the range, e.g., below imports):

type AggregationTotals = Partial<{
  breakdownTotal: number;
  projectsTotal: number;
  resources: { resourceId: string; value?: number }[];
  breakdown: unknown[];
}>;
🧹 Nitpick comments (4)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (4)

367-372: Use the existing date helper consistently and guard for missing dates.

Minor polish: prefer toLocaleDate(...) everywhere and avoid Invalid Date when org fields are absent.

-                    Current billing cycle ({new Date(
-                        $organization?.billingCurrentInvoiceDate
-                    ).toLocaleDateString('en', { day: 'numeric', month: 'short' })}-{new Date(
-                        $organization?.billingNextInvoiceDate
-                    ).toLocaleDateString('en', { day: 'numeric', month: 'short' })})
+                    Current billing cycle ({toLocaleDate($organization?.billingCurrentInvoiceDate, 'en', { day: 'numeric', month: 'short' })}-
+                    {toLocaleDate($organization?.billingNextInvoiceDate, 'en', { day: 'numeric', month: 'short' })})

746-751: Tidy selector: stray target for additional-projects.

There’s no row with id additional-projects. Either drop it or target the actual addon-projects id (you already have the ^='addon-' selector).

-        :global([data-expandable-row-id='additional-projects']),
-        :global([data-expandable-row-id^='addon-']) {
+        :global([data-expandable-row-id^='addon-']) {
             background-color: var(--bgcolor-neutral-tertiary);
             border-left: 3px solid var(--bgcolor-accent-neutral);
         }

763-768: Fix typo in comment.

Micro nit.

-/* reducingh size of paginator */
+/* reduce size of paginator */

555-573: Unify analytics event naming.

One button uses Click.OrganizationClickUpgrade, the other a string literal. Prefer typed constants for both if available.

-    on:click={() =>
-        trackEvent('click_organization_plan_update', {
+    on:click={() =>
+        trackEvent(Click.OrganizationClickUpgrade, {
             from: 'button',
             source: 'billing_tab'
         })}

If a dedicated event exists for “plan update,” switch to that constant instead.

Also applies to: 575-601

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c0e3043 and 9de7ece.

📒 Files selected for processing (1)
  • src/routes/(console)/organization-[organization]/billing/planSummary.svelte (7 hunks)
🧰 Additional context used
🪛 ESLint
src/routes/(console)/organization-[organization]/billing/planSummary.svelte

[error] 160-160: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)


[error] 161-161: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

🔇 Additional comments (3)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (3)

195-202: Nice UX: "Additional projects" label + badge.

Good call to simplify the label and surface the count via Badge.


351-353: Keyed block to force re-mount is appropriate.

This will reset internal state on page changes, avoiding stale data.


497-501: Pagination updates 'page' (not 'offset') and preserves 'limit' — loader will re-run

Confirmed: src/lib/components/pagination.svelte builds links from pageStore.url and sets/deletes the 'page' param (not 'offset'), preserving other query params; planSummary.svelte reads page.url.searchParams to derive projectsLimit/projectsOffset (src/routes/.../planSummary.svelte:156–158) and +page.ts uses getPage/getLimit to compute offset and depends('billing:aggregation'), so clicking the PaginationComponent updates the query string (page, with limit preserved) and triggers the loader to refresh.

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)}`
Copy link
Member

Choose a reason for hiding this comment

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

why not just invalidate the dependency?

Copy link
Member Author

Choose a reason for hiding this comment

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

depends() would refresh the data but not reset the component state, so expanded rows from the previous page would still persist. Using aggregationKey forces a full component reset on pagination change, which cleanly fixes the expansion state issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

2025-09-27.17-31-11.mp4

When only invalidating dependency

Copy link
Member Author

Choose a reason for hiding this comment

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

Need to do a hard refresh for rows to populate with data

Copy link
Member

Choose a reason for hiding this comment

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

depends() would refresh the data but not reset the component state

then we should derive the component values via runes or use reactive vars if on svelte4.

Copy link
Member

Choose a reason for hiding this comment

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

this still applies @HarshMN2345?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (3)

216-226: Prevent crashes when resource entries are missing

Several lookups assume a resource exists (e.g., project.bandwidth.value). If undefined, this throws. Use optional chaining and default values.

-usage: `${formatBandwidthUsage(project.bandwidth.value, currentPlan?.bandwidth)}`,
-price: formatCurrency(project.bandwidth.amount || 0)
+usage: `${formatBandwidthUsage(project.bandwidth?.value ?? 0, currentPlan?.bandwidth)}`,
+price: formatCurrency(project.bandwidth?.amount ?? 0)
- progressData: createStorageProgressData(
-    project.bandwidth.value || 0,
+ progressData: createStorageProgressData(
+    project.bandwidth?.value ?? 0,
     currentPlan?.bandwidth || 0
   ),

-usage: `${formatNum(project.users.value || 0)} / ${currentPlan?.users ? formatNum(currentPlan.users) : 'Unlimited'}`,
-price: formatCurrency(project.users.amount || 0)
+usage: `${formatNum(project.users?.value ?? 0)} / ${currentPlan?.users ? formatNum(currentPlan.users) : 'Unlimited'}`,
+price: formatCurrency(project.users?.amount ?? 0)
- progressData: createProgressData(project.users.value || 0, currentPlan?.users),
+ progressData: createProgressData(project.users?.value ?? 0, currentPlan?.users),

-usage: `${formatNum(project.databasesReads.value || 0)} / ${currentPlan?.databasesReads ? formatNum(currentPlan.databasesReads) : 'Unlimited'}`,
-price: formatCurrency(project.databasesReads.amount || 0)
+usage: `${formatNum(project.databasesReads?.value ?? 0)} / ${currentPlan?.databasesReads ? formatNum(currentPlan.databasesReads) : 'Unlimited'}`,
+price: formatCurrency(project.databasesReads?.amount ?? 0)
- progressData: createProgressData(
-    project.databasesReads.value || 0,
+ progressData: createProgressData(
+    project.databasesReads?.value ?? 0,
     currentPlan?.databasesReads
   ),

-usage: `${formatNum(project.databasesWrites.value || 0)} / ${currentPlan?.databasesWrites ? formatNum(currentPlan.databasesWrites) : 'Unlimited'}`,
-price: formatCurrency(project.databasesWrites.amount || 0)
+usage: `${formatNum(project.databasesWrites?.value ?? 0)} / ${currentPlan?.databasesWrites ? formatNum(currentPlan.databasesWrites) : 'Unlimited'}`,
+price: formatCurrency(project.databasesWrites?.amount ?? 0)
- progressData: createProgressData(
-    project.databasesWrites.value || 0,
+ progressData: createProgressData(
+    project.databasesWrites?.value ?? 0,
     currentPlan?.databasesWrites
   ),

-usage: `${formatNum(project.executions.value || 0)} / ${currentPlan?.executions ? formatNum(currentPlan.executions) : 'Unlimited'}`,
-price: formatCurrency(project.executions.amount || 0)
+usage: `${formatNum(project.executions?.value ?? 0)} / ${currentPlan?.executions ? formatNum(currentPlan.executions) : 'Unlimited'}`,
+price: formatCurrency(project.executions?.amount ?? 0)
- progressData: createProgressData(
-    project.executions.value || 0,
+ progressData: createProgressData(
+    project.executions?.value ?? 0,
     currentPlan?.executions
   ),

-usage: `${formatHumanSize(project.storage.value || 0)} / ${currentPlan?.storage?.toString() || '0'} GB`,
-price: formatCurrency(project.storage.amount || 0)
+usage: `${formatHumanSize(project.storage?.value ?? 0)} / ${currentPlan?.storage?.toString() || '0'} GB`,
+price: formatCurrency(project.storage?.amount ?? 0)
- progressData: createStorageProgressData(
-    project.storage.value || 0,
+ progressData: createStorageProgressData(
+    project.storage?.value ?? 0,
     currentPlan?.storage || 0
   ),

-usage: `${formatNum(project.imageTransformations.value || 0)} / ${currentPlan?.imageTransformations ? formatNum(currentPlan.imageTransformations) : 'Unlimited'}`,
-price: formatCurrency(project.imageTransformations.amount || 0)
+usage: `${formatNum(project.imageTransformations?.value ?? 0)} / ${currentPlan?.imageTransformations ? formatNum(currentPlan.imageTransformations) : 'Unlimited'}`,
+price: formatCurrency(project.imageTransformations?.amount ?? 0)
- progressData: createProgressData(
-    project.imageTransformations.value || 0,
+ progressData: createProgressData(
+    project.imageTransformations?.value ?? 0,
     currentPlan?.imageTransformations
   ),

-usage: `${formatNum(project.gbHours.value || 0)} / ${currentPlan?.GBHours ? formatNum(currentPlan.GBHours) : 'Unlimited'}`,
-price: formatCurrency(project.gbHours.amount || 0)
+usage: `${formatNum(project.gbHours?.value ?? 0)} / ${currentPlan?.GBHours ? formatNum(currentPlan.GBHours) : 'Unlimited'}`,
+price: formatCurrency(project.gbHours?.amount ?? 0)
- progressData: currentPlan?.GBHours
-    ? createProgressData(project.gbHours.value || 0, currentPlan.GBHours)
+ progressData: currentPlan?.GBHours
+    ? createProgressData(project.gbHours?.value ?? 0, currentPlan.GBHours)
     : [],

-usage: `${formatNum(project.authPhone.value || 0)} SMS messages`,
-price: formatCurrency(project.authPhone.amount || 0)
+usage: `${formatNum(project.authPhone?.value ?? 0)} SMS messages`,
+price: formatCurrency(project.authPhone?.amount ?? 0)

Also applies to: 230-236, 240-249, 253-262, 266-275, 279-288, 292-301, 305-313, 317-321


167-175: Remove unsafe {@html}; render link safely and stop shadowing base

Using {@html} with dynamic data is an XSS risk. Also, local const base shadows $app/paths base. Rename row var and pass a dedicated href property.

-        const base = {
+        const basePlanRow = {
             id: 'base-plan',
             expandable: false,
             cells: {
                 item: 'Base plan',
                 usage: '',
                 price: formatCurrency(nextPlan?.price ?? currentPlan?.price ?? 0)
             },
             children: []
         };
...
-                {
-                    id: `usage-details`,
-                    cells: {
-                        item: `<a href="/console/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`,
-                        usage: '',
-                        price: ''
-                    }
-                }
+                {
+                    id: `usage-details`,
+                    linkHref: `${base}/project-${String(project.region || 'default')}-${project.projectId}/settings/usage`,
+                    cells: {
+                        item: 'Usage details',
+                        usage: '',
+                        price: ''
+                    }
+                }
...
-        return [base, ...addons, ...projects, ...noProjects];
+        return [basePlanRow, ...addons, ...projects, ...noProjects];
-                                                {#if child.cells?.item?.includes('<a href=')}
-                                                    {@html child.cells?.item ?? ''}
+                                                {#if child.linkHref}
+                                                    <a
+                                                        href={child.linkHref}
+                                                        style="text-decoration: underline; color: var(--fgcolor-accent-neutral);"
+                                                    >
+                                                        Usage details
+                                                    </a>
                                                 {:else}
                                                     <Typography.Text
                                                         variant="m-400"
                                                         color="--fgcolor-neutral-secondary">
                                                         {child.cells?.item ?? ''}
                                                     </Typography.Text>
                                                 {/if}

Also applies to: 323-329, 418-426, 333-334


338-344: Avoid NaN totals when credits are undefined

Coalesce availableCredit to 0 and base the total off a single base amount to keep numbers stable.

-$: totalAmount = Math.max(currentAggregation?.amount - creditsApplied, 0);
-
-$: creditsApplied = Math.min(
-    currentAggregation?.amount ?? currentPlan?.price ?? 0,
-    availableCredit
-);
+$: baseAmount = currentAggregation?.amount ?? currentPlan?.price ?? 0;
+$: creditsApplied = Math.min(baseAmount, availableCredit ?? 0);
+$: totalAmount = Math.max(baseAmount - creditsApplied, 0);
🧹 Nitpick comments (1)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (1)

768-773: Fix typo in comment

Minor nit.

-    /* reducingh size of paginator */
+    /* reducing size of paginator */
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb41f8f and 926d46e.

📒 Files selected for processing (3)
  • src/lib/constants.ts (1 hunks)
  • src/routes/(console)/organization-[organization]/billing/+page.ts (4 hunks)
  • src/routes/(console)/organization-[organization]/billing/planSummary.svelte (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/routes/(console)/organization-[organization]/billing/+page.ts
🔇 Additional comments (3)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (2)

26-26: Fix invalid SvelteKit store import and usage

Import page from $app/stores and dereference the store with $page in reactive statements. Current code will error at runtime. (Previously flagged)

-import { page } from '$app/state';
+import { page } from '$app/stores';

-$: projectsLimit = limit ?? (Number(page.url.searchParams.get('limit')) || 5);
-$: projectsOffset =
-    offset ?? ((Number(page.url.searchParams.get('page')) || 1) - 1) * projectsLimit;
+$: projectsLimit = limit ?? (Number($page.url.searchParams.get('limit')) || 5);
+$: projectsOffset =
+    offset ?? ((Number($page.url.searchParams.get('page')) || 1) - 1) * projectsLimit;

Also applies to: 155-157


490-503: Confirm pagination updates URL and triggers reload

The PaginationComponent generates <a> links via SvelteKit’s page store (getLink) and only mutates the page query param—clicking a page number updates the URL and remounts the {#key} block.

src/lib/constants.ts (1)

23-24: Dependency key usage is correct
depends calls use Dependencies.BILLING_AGGREGATION and no hardcoded ‘billing:aggregation’ remains.

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)}`
Copy link
Member

Choose a reason for hiding this comment

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

depends() would refresh the data but not reset the component state

then we should derive the component values via runes or use reactive vars if on svelte4.

Copy link
Member

Choose a reason for hiding this comment

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

lets move this to Svelte5 as well. using $derived might also help us get rid of the aggregationKey. If not, we might have to use the key!

Copy link
Member Author

Choose a reason for hiding this comment

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

we need to use the key!

Copy link
Member

Choose a reason for hiding this comment

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

we should be able to build it on UI then ig.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5ef9b7 and 31b66ad.

📒 Files selected for processing (1)
  • src/routes/(console)/organization-[organization]/billing/planSummary.svelte (6 hunks)
🔇 Additional comments (1)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (1)

26-27: Fix page store import and subscription

$app/state doesn’t export page, and reading page.url hits the store object instead of its value—so page.url is undefined, breaking pagination as soon as the component mounts. Import the store from $app/stores and access $page inside the derived runes to keep the table responsive to URL changes.

-import { page } from '$app/state';
+import { page } from '$app/stores';-const projectsLimit = $derived(limit ?? (Number(page.url.searchParams.get('limit')) || 5));
+const projectsLimit = $derived(limit ?? (Number($page.url.searchParams.get('limit')) || 5));
 const projectsOffset = $derived(
-    offset ?? ((Number(page.url.searchParams.get('page')) || 1) - 1) * projectsLimit
+    offset ?? ((Number($page.url.searchParams.get('page')) || 1) - 1) * projectsLimit
 );

Also applies to: 162-165

Comment on lines +947 to +948
limit?: number;
offset?: number;
Copy link
Member

Choose a reason for hiding this comment

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

does the endpoint now allow these? 👀
quite nice if it fully supports pagination, does it also support Query.select?

speaking of queries, lets make this as a queries param instead of passing every param individually. Refer - https://github.com/appwrite/console/blob/main/src/lib/sdk/billing.ts#L455

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)}`
Copy link
Member

Choose a reason for hiding this comment

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

this still applies @HarshMN2345?

Copy link
Member

Choose a reason for hiding this comment

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

we should be able to build it on UI then ig.

disabled={$organization?.markedForDeletion}
href={$upgradeURL}
on:click={() =>
trackEvent('click_organization_plan_update', {
Copy link
Member

Choose a reason for hiding this comment

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

Click.* constant if it exists.

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