Billing SDK Refactor #2722
Billing SDK Refactor #2722
Conversation
replace: `billing.listOrganization` calls with `organizations.list`.
…tionDeleteOrganization`, `getOrganizationPlan` and `getRoles`.
Console (appwrite/console)Project ID: Tip Custom domains work with both CNAME for subdomains and NS records for apex domains |
WalkthroughThe PR migrates billing and organization functionality to Appwrite Console Models and organization-scoped APIs. It replaces many sdk.forConsole.billing.* calls with sdk.forConsole.organizations., sdk.forConsole.account., or sdk.forConsole.console.; consolidates team/organization listing via getTeamOrOrganizationList; removes the Billing class and BillingPlan enum; introduces plan utilities (billingIdToPlan, planHasGroup, getBasePlanFromGroup, getNextTierBillingPlan, makePlansMap) and IconsMap; updates numerous components, stores, routes, and props to use Models. types; standardizes route construction using resolve(); and adjusts Stripe/account payment flows and related signatures. Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
…od and its usages.
…hods and its usages.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/routes/(console)/project-[region]-[project]/settings/smtp/+page.svelte (1)
126-200: Avoid direct access to$currentPlan.customSmtpwhile loading.If
$currentPlanis undefined during initial render, the current checks will throw. Guard with optional chaining.🛠️ Proposed fix
-{`#if` !$currentPlan.customSmtp} +{`#if` !$currentPlan?.customSmtp} ... -<Button submit disabled={isButtonDisabled || !$currentPlan.customSmtp}> +<Button submit disabled={isButtonDisabled || !$currentPlan?.customSmtp}>e2e/steps/pro-project.ts (1)
56-68: Avoid hardcoded fallback if no enabled options exist.When no enabled options are found, the project may still be created in the currently selected region (not necessarily
fra), which can makewaitForURLflaky. Consider deriving the current selection before falling back.🛠️ Proposed fix
let region = 'fra'; // for fallback const regionPicker = dialog.locator('button[role="combobox"]'); if (await regionPicker.isVisible()) { + const currentRegion = await regionPicker.getAttribute('data-value'); + if (currentRegion) { + region = currentRegion.replace(/"/g, ''); + } await regionPicker.click(); const firstEnabledOption = page .locator('[role="option"]:not([data-disabled="true"])') .first(); if ((await firstEnabledOption.count()) > 0) { const selectedRegion = await firstEnabledOption.getAttribute('data-value'); await firstEnabledOption.click(); region = selectedRegion?.replace(/"/g, '') || 'fra'; } }src/routes/(console)/+layout.svelte (1)
294-309: GuardbillingPlanDetailsbefore accessingrequiresPaymentMethod.If
billingPlanDetailsis missing, this will throw during usage checks.🛠️ Proposed fix
- if (org?.billingPlanDetails.requiresPaymentMethod) { + if (org?.billingPlanDetails?.requiresPaymentMethod) {src/routes/(console)/project-[region]-[project]/+layout.ts (1)
60-90: Avoid caching null/undefined plans inplansInfo.When
isCloudis false,organizationPlanbecomesnull, yet it’s cached. Downstream helpers assume aModels.BillingPlanand may throw on.order/.group. Guard before caching.🩹 Suggested guard
- if (!includedInBasePlans) { - // save the custom plan to `plansInfo` cache. - plansInfo.set(organization.billingPlanId, organizationPlan); - } + if (!includedInBasePlans && organizationPlan) { + // save the custom plan to `plansInfo` cache. + plansInfo.set(organization.billingPlanId, organizationPlan); + }src/routes/(console)/organization-[organization]/+page.svelte (1)
222-242: Guard free‑plan alert against missing currentPlan.
data.currentPlan.projectscan throw if the loader ever returns a null/undefined plan. Add an explicit guard to avoid a hard crash during partial loads.🛠️ Suggested fix
- {`#if` isCloud && data.currentPlan.projects !== 0 && projectsToArchive.length === 0 && !freePlanAlertDismissed} + {`#if` isCloud && data.currentPlan && data.currentPlan.projects !== 0 && projectsToArchive.length === 0 && !freePlanAlertDismissed}src/routes/(console)/create-organization/+page.svelte (2)
85-87: Potential runtime error when parsing invites from URL.If
page.url.searchParams.get('invites')returnsnull, calling.split(',')will throw a TypeError. This can happen if someone navigates to the page withtype=payment_confirmedbut without theinvitesparameter.🛠️ Suggested fix
if (type === 'payment_confirmed') { const organizationId = page.url.searchParams.get('id'); - const invites = page.url.searchParams.get('invites').split(','); + const invites = page.url.searchParams.get('invites')?.split(',') ?? []; await validate(organizationId, invites); }
101-122: Missing null check onorganizationIdparameter.If
organizationIdisnull(e.g., when URL param is missing), thevalidatePaymentcall may fail or behave unexpectedly. Consider adding validation before making the API call.🛠️ Suggested fix
async function validate(organizationId: string, invites: string[]) { + if (!organizationId) { + addNotification({ + type: 'error', + message: 'Organization ID is missing' + }); + return; + } try { const org = await sdk.forConsole.organizations.validatePayment({src/routes/(console)/organization-[organization]/change-plan/+page.svelte (1)
92-107: Plan query parameter is immediately overwritten.Lines 93-97 set
selectedPlanfrom theplanquery parameter, but lines 105-107 unconditionally overwrite it. The query parameter logic is effectively dead code.🐛 Proposed fix
const plan = params.get('plan'); if (plan) { // if the org has access, // the plan should be available! selectedPlan = billingIdToPlan(plan); - } - - if (params.get('type') === 'payment_confirmed') { - const organizationId = params.get('id'); - const invites = params.get('invites')?.split(',') ?? []; - await validate(organizationId, invites); - } + } else { + const pro = getBasePlanFromGroup(BillingPlanGroup.Pro); + const scale = getBasePlanFromGroup(BillingPlanGroup.Scale); + selectedPlan = $currentPlan?.group === BillingPlanGroup.Scale ? scale : pro; + } - const pro = getBasePlanFromGroup(BillingPlanGroup.Pro); - const scale = getBasePlanFromGroup(BillingPlanGroup.Scale); - selectedPlan = $currentPlan?.group === BillingPlanGroup.Scale ? scale : pro; + if (params.get('type') === 'payment_confirmed') { + const organizationId = params.get('id'); + const invites = params.get('invites')?.split(',') ?? []; + await validate(organizationId, invites); + }
🤖 Fix all issues with AI agents
In `@src/lib/components/backupDatabaseAlert.svelte`:
- Around line 19-27: The code reads
$organization?.billingPlanDetails.backupsEnabled directly which can throw if
billingPlanDetails is missing; update the guard used by areBackupsAvailable to
safely access backupsEnabled (e.g., use optional chaining and a default false:
$organization?.billingPlanDetails?.backupsEnabled ?? false) and ensure all
derived consts (subtitle, ctaText, ctaURL that reference areBackupsAvailable and
$upgradeURL) continue to use that safe value so the component won’t throw when
billingPlanDetails is absent.
In `@src/lib/components/billing/alerts/limitReached.svelte`:
- Around line 11-18: The conditional and JSX-like usage dereference
$organization.billingPlanDetails (specifically .usage and .name) without
ensuring billingPlanDetails exists, which can cause runtime crashes; update the
top-level if check to include a presence guard like
$organization?.billingPlanDetails (e.g., require
$organization?.billingPlanDetails && $organization?.billingPlanDetails.usage)
and update the HeaderAlert title and the fragment text to use the guarded value
(or a local safe variable) instead of directly accessing
$organization.billingPlanDetails.name/usage so all references to
billingPlanDetails are only used when defined.
In `@src/lib/components/billing/alerts/missingPaymentMethod.svelte`:
- Line 10: The template condition directly accesses orgMissingPaymentMethod and
its nested properties (billingPlanDetails.requiresPaymentMethod,
paymentMethodId, backupPaymentMethodId) which can be null; update the
conditional in the {`#if` ...} to use optional chaining on orgMissingPaymentMethod
and its nested fields (e.g.,
orgMissingPaymentMethod?.billingPlanDetails?.requiresPaymentMethod,
orgMissingPaymentMethod?.paymentMethodId,
orgMissingPaymentMethod?.backupPaymentMethodId) while keeping the existing
hideBillingHeaderRoutes and page.url.pathname checks to defensively prevent
runtime errors during initial null state.
In `@src/lib/components/billing/planSelection.svelte`:
- Line 22: The initialization of selectedPlan reads selectedBillingPlan.$id
directly which can throw if selectedBillingPlan is undefined; change the
initialization of selectedPlan (and any places that read
selectedBillingPlan.$id) to safely handle undefined by using optional chaining
and a sensible default (e.g., use selectedBillingPlan?.$id or fall back to
null/empty id) so that $state(selectedBillingPlan.$id) cannot be invoked with an
undefined property; update the selectedPlan initialization and any related
reactive logic that assumes a non-null id to handle the fallback.
In `@src/lib/components/roles/upgrade.svelte`:
- Around line 12-13: Guard access to billingPlanDetails in the Upgrade component
template to avoid a render-time TypeError: update the conditional that currently
checks isCloud and $organization?.billingPlanDetails.supportsOrganizationRoles
to safely handle undefined billingPlanDetails by using optional chaining or a
default boolean; specifically, change the second conditional to check
$organization?.billingPlanDetails?.supportsOrganizationRoles (or use
($organization?.billingPlanDetails?.supportsOrganizationRoles ?? false)) so
references to billingPlanDetails and supportsOrganizationRoles inside
upgrade.svelte are safe during render.
In `@src/lib/layout/containerButton.svelte`:
- Around line 8-13: The tooltipContent export uses
$organization?.billingPlanDetails.group which can throw if billingPlanDetails is
undefined; change accesses to use consistent optional chaining (e.g.
$organization?.billingPlanDetails?.group) so the comparison with
BillingPlanGroup.Starter safely handles missing billingPlanDetails and adjust
the ternary to handle a possible undefined group gracefully.
In `@src/lib/layout/shell.svelte`:
- Around line 156-164: Replace the bracket-notation hack by properly typing the
map parameter as Models.Organization so billingPlanDetails is accessed directly;
change the organizations mapping in the $organizationList.teams.map callback to
accept (org: Models.Organization) (or cast via (org as Models.Organization)) and
then use org.billingPlanDetails to derive const billingPlan =
org.billingPlanDetails as Models.BillingPlan and keep the rest (name, $id,
isSelected, tierName, showUpgrade) unchanged.
In `@src/lib/stores/billing.ts`:
- Around line 81-87: getPlansInfoStore currently uses "get(plansInfo) ??
get(page).data?.plansInfo ?? null" but get(plansInfo) always returns a Map (even
empty), preventing fallback to SSR page data; change getPlansInfoStore to treat
an empty Map as absent (e.g., check the returned Map's size or use a conditional
that returns null when size === 0) so that when plansInfo is empty it falls back
to get(page).data?.plansInfo; update references in getPlansInfoStore and ensure
makeBillingPlan/billingIdToPlan consumers still handle a null return.
In `@src/routes/`(console)/account/organizations/+page.svelte:
- Around line 84-86: The type-guard isCloudOrg currently tests for 'billingPlan'
which doesn't exist on Models.Organization; update the discriminant to check for
'billingPlanId' instead so the predicate can correctly identify cloud
organizations (ensure the function isCloudOrg, the parameter data:
Partial<Models.TeamList<Models.Preferences>> | Models.Organization, and the
Models.Organization type are used to locate the change).
In `@src/routes/`(console)/apply-credit/+page.svelte:
- Around line 283-293: The reactive expression needsPaymentMethod is incorrectly
using OR and becomes true even when payment isn't required; change its logic to
be true only when a payment method is required AND missing by replacing the
current condition with: selectedOrgId &&
selectedOrg?.billingPlanDetails.requiresPaymentMethod &&
!selectedOrg?.paymentMethodId; then update any other duplicate reactive block
(the similar occurrence around the second needsPaymentMethod/needsCouponInput
logic) to use the same AND semantics and ensure showPaymentSection still
combines needsPaymentMethod || needsCouponInput.
- Around line 92-95: The query-param branch sets selectedOrgId from
page.url.searchParams but a later reactive assignment ($: selectedOrgId =
tempOrgId) will overwrite it; to fix, make the sources consistent by also
assigning tempOrgId when an org query param is present (or change the reactive
to only run when no query param exists). Concretely, update the block that
checks page.url.searchParams.has('org') to set tempOrgId = selectedOrgId (and
keep canSelectOrg = false), or guard the reactive assignment that assigns
selectedOrgId from tempOrgId so it only runs when the org query param is not
present.
- Around line 107-113: The code reads organizationId from
page.url.searchParams.get('id') but the confirm route now appends 'org', causing
organizationId to be null and validatePayment to fail; update the logic that
sets organizationId (and any other occurrences around the second block at lines
~170-191) to first try page.url.searchParams.get('org') and fall back to
page.url.searchParams.get('id') (or vice versa based on canonical param), ensure
collaborators is parsed only when invites exists, and then pass the resolved
organizationId into sdk.forConsole.organizations.validatePayment so
validatePayment and variables like collaborators are never called with null
values.
In `@src/routes/`(console)/create-organization/+page.ts:
- Around line 39-56: The cache helper getPlanFromCache currently calls
billingIdToPlan which may return a default plan for unknown ids; change
getPlanFromCache to only treat a cache hit as valid if the returned plan's $id
exactly equals the requested plan string (or else return null), and in
getPlanFromUrl ensure you pass the raw planId to getPlanFromCache and fall back
to calling sdk.forConsole.console.getPlan({ planId }) when the cache returns
null or a mismatched plan; this guarantees stale/tampered query params trigger
the API fetch instead of silently using a default plan.
In
`@src/routes/`(console)/organization-[organization]/billing/deleteOrgPayment.svelte:
- Line 19: Guard against $currentPlan being undefined before accessing
.requiresPaymentMethod: update the conditional that reads "if
($currentPlan.requiresPaymentMethod && !hasOtherMethod) return;" to first ensure
$currentPlan exists (for example "if (!$currentPlan) return;" or use optional
chaining "if ($currentPlan?.requiresPaymentMethod && !hasOtherMethod) return;")
and apply the same defensive check where $currentPlan.requiresPaymentMethod is
used later in this file (the other occurrence referencing
requiresPaymentMethod).
In `@src/routes/`(console)/organization-[organization]/change-plan/+page.svelte:
- Around line 472-483: The conditional in the Aside slot compares
data.organization.billingPlanId (a plan ID string) to selectedPlan.group (a
BillingPlanGroup enum) which is a type mismatch; update the comparison in the
EstimatedTotalBox/PlanComparisonBox conditional to compare IDs instead—replace
data.organization.billingPlanId !== selectedPlan.group with
data.organization.billingPlanId !== selectedPlan.$id (or, if the intent is to
compare groups, use data.organization.billingPlanDetails.group !==
selectedPlan.group); ensure the rest of the condition (BillingPlanGroup.Starter
check and billingPlanDetails.selfService) remains unchanged so the logic still
shows EstimatedTotalBox only when appropriate.
In `@src/routes/`(console)/organization-[organization]/createProjectCloud.svelte:
- Line 102: The disabled expression reads currentPlan.projects which can be
undefined; change it to use optional chaining and a safe default so it won't
throw: replace uses of currentPlan.projects with currentPlan?.projects and use a
nullish coalescing fallback when comparing (e.g. (currentPlan?.projects ?? 0))
so the full boolean becomes something like (currentPlan?.projects ?? 0) > 0 &&
projects && projects >= (currentPlan?.projects ?? 0); update the disabled
attribute accordingly.
- Line 34: The code uses const projectIdForLog = $derived(projectId) which
reactively mirrors projectId so the later comparison (projectId !==
projectIdForLog / customId check) will never detect a user-provided custom ID;
instead capture the initial non-reactive value once (e.g., read the store with
get(projectId) or subscribe once into a local variable like initialProjectId) at
component mount or before edits, replace projectIdForLog with that
initialProjectId, and update the tracking call and any checks that compare
projectId to projectIdForLog to use initialProjectId so the customId detection
works correctly.
In `@src/routes/`(console)/organization-[organization]/header.svelte:
- Around line 154-159: The tooltip currently uses
organization?.billingPlanDetails.addons.seats.supported and always shows an
upgrade/limit message; change it to only show when the plan actually imposes a
member limit (e.g., check a dedicated flag or numeric limit on
organization?.billingPlanDetails.addons.seats such as seats.limited or
seats.limit > 0) and otherwise hide/suppress the tooltip (or show a neutral
message). Update the conditional around
organization?.billingPlanDetails.addons.seats.supported and the displayed string
that references organization?.billingPlanDetails?.name so it only appears when
seats are limited.
In
`@src/routes/`(console)/organization-[organization]/usage/[[invoice]]/totalMembers.svelte:
- Around line 14-16: billingPlan is fetched with optional chaining
($organization?.billingPlanDetails) but organizationMembersSupported reads
billingPlan.addons.seats.supported without null checks and its name/comment are
misleading; fix by guarding the access (treat undefined billingPlan as no seats
supported or the safe default you want) and rename/invert the boolean to reflect
semantics (e.g., isMembersLimited or isFreePlanWithLimitedMembers) so that true
means "members are limited/unsupported"; update every usage (e.g., the template
condition that currently uses {`#if` !organizationMembersSupported}) to use the
new name/logic and adjust the comment to match the behavior.
In `@src/routes/`(console)/project-[region]-[project]/settings/smtp/+page.svelte:
- Around line 126-130: Update the alert title text in the SMTP settings UI to
fix the typo: change the string passed to Alert.Inline's title prop from "Custom
SMTP is a paid plan feature. Upgrade to enable custom SMTP sever." to use
"server" instead of "sever" (the conditional block checking
$currentPlan.customSmtp contains this title). Ensure you only modify the title
prop text within the Alert.Inline component so the rest of the conditional and
slot markup (svelte:fragment slot="actions") remains unchanged.
♻️ Duplicate comments (2)
src/routes/(console)/+layout.svelte (1)
311-313: VerifybillingTrialDaysexists onModels.Organization.This guard was previously questioned; if the property isn’t part of the SDK type, the condition will always be false and trial-day logic won’t run. Please verify against the current SDK types/docs.
In `@appwrite.io/console`, does Models.Organization include a billingTrialDays property? If not, what is the correct field to use for trial-day calculations?src/routes/(console)/onboarding/create-project/+page.ts (1)
27-34: Inconsistent null safety between lines 31 and 34.Line 31 accesses
starterPlan.$iddirectly while line 34 uses optional chainingstarterPlan?.name. This inconsistency suggests uncertainty about whethergetBasePlanFromGroupcan returnundefined.While the PR author asserts that plans are guaranteed via root layouts, the defensive approach on line 34 should be applied consistently to line 31 as well, or both should omit optional chaining if the guarantee is absolute.
🔧 Suggested fix for consistency
const starterPlan = getBasePlanFromGroup(BillingPlanGroup.Starter); const org = await sdk.forConsole.organizations.create({ organizationId: ID.unique(), name: 'Personal projects', - billingPlan: starterPlan.$id + billingPlan: starterPlan?.$id }); trackEvent(Submit.OrganizationCreate, { - plan: starterPlan?.name, + plan: starterPlan?.name, budget_cap_enabled: false, members_invited: 0 });Note: If the API requires
billingPlanto be non-null, consider adding explicit error handling whenstarterPlanis undefined.
🧹 Nitpick comments (16)
e2e/steps/free-project.ts (2)
27-35: Consider waiting for options to appear after opening the combobox.After clicking
regionPicker, the dropdown options may not render immediately. Using.count()doesn't auto-wait for elements, which could cause flakiness. Additionally, if no enabled options are found, the combobox remains open without any selection.♻️ Suggested improvement
await regionPicker.click(); - const firstEnabledOption = page - .locator('[role="option"]:not([data-disabled="true"])') - .first(); - - if ((await firstEnabledOption.count()) > 0) { - const selectedRegion = await firstEnabledOption.getAttribute('data-value'); - await firstEnabledOption.click(); - region = selectedRegion?.replace(/"/g, '') || 'fra'; - } + const enabledOptions = page.locator('[role="option"]:not([data-disabled="true"])'); + const firstEnabledOption = enabledOptions.first(); + + // Wait for at least one enabled option to appear + await firstEnabledOption.waitFor({ state: 'visible', timeout: 5000 }).catch(() => null); + + if ((await enabledOptions.count()) > 0) { + const selectedRegion = await firstEnabledOption.getAttribute('data-value'); + await firstEnabledOption.click(); + region = selectedRegion?.replace(/"/g, '') || 'fra'; + } else { + // Close the combobox if no enabled options found + await page.keyboard.press('Escape'); + }
34-34: Clarify the need for quote stripping fromdata-value.The
.replace(/"/g, '')suggests thedata-valueattribute may contain embedded quotes. If this is expected behavior from the component, consider adding a brief comment. Otherwise, this might be masking an issue with how the attribute is being set upstream.src/routes/(console)/organization-[organization]/billing/availableCredit.svelte (1)
108-115: Consider removing unnecessary optional chaining.Since
creditListis always initialized with default values (lines 25-29), it will never benullorundefined. The optional chaining (creditList?.total) on lines 108, 115, 157, and 159 is unnecessary and could be simplified tocreditList.totalfor consistency.This is a minor inconsistency that doesn't affect functionality.
src/routes/(console)/organization-[organization]/usage/[[invoice]]/totalMembers.svelte (1)
51-51: Type annotation removed from snippet parameter.The explicit type annotation
typeof members.membershipswas removed frompaginatedItems. While TypeScript can likely infer the type from thePaginatorcomponent'sitemsprop, the explicit annotation provided better IDE support and documentation. This is a minor readability regression.Consider restoring explicit typing
- {`#snippet` children(paginatedItems)} + {`#snippet` children(paginatedItems: typeof members.memberships)}src/routes/(console)/organization-[organization]/billing/paymentMethods.svelte (1)
214-216: Consider extracting a filter helper.The filter logic appears twice with similar structure but different exclusion criteria. A small helper could reduce duplication:
const filterMethods = (excludeId?: string) => paymentMethods.paymentMethods.filter(o => !!o.last4 && o.$id !== excludeId);Also applies to: 268-270
src/lib/components/billing/alerts/newDevUpgradePro.svelte (1)
2-10: Use $lib alias for GradientBanner import.The relative import violates the aliasing guideline for
src/libmodules.♻️ Proposed change
-import GradientBanner from '../gradientBanner.svelte'; +import GradientBanner from '$lib/components/billing/alerts/gradientBanner.svelte';src/routes/(console)/organization-[organization]/billing/+page.ts (2)
31-39: Align billingAddressPromise typing with nullable outcomes.
Line 32 can returnnull(or resolve tonull), so reflect that to avoid misleading non‑null assumptions.♻️ Suggested tweak
- const billingAddressPromise: Promise<Models.BillingAddress> = billingAddressId + const billingAddressPromise: Promise<Models.BillingAddress | null> = billingAddressId ? sdk.forConsole.organizations .getBillingAddress({ organizationId: organization.$id, billingAddressId }) .catch(() => null) - : null; + : Promise.resolve(null);
46-66: Guard aggregation/invoice calls when IDs are missing.
Line 53 and Line 65 can be undefined; short‑circuiting avoids unnecessary requests and noisy errors.♻️ Suggested tweak
- billingAggregation = await sdk.forConsole.organizations.getAggregation({ - organizationId: organization.$id, - aggregationId: (organization as Models.Organization)?.billingAggregationId, - limit, - offset - }); + const aggregationId = (organization as Models.Organization)?.billingAggregationId; + if (aggregationId) { + billingAggregation = await sdk.forConsole.organizations.getAggregation({ + organizationId: organization.$id, + aggregationId, + limit, + offset + }); + }- billingInvoice = await sdk.forConsole.organizations.getInvoice({ - organizationId: organization.$id, - invoiceId: (organization as Models.Organization)?.billingInvoiceId - }); + const invoiceId = (organization as Models.Organization)?.billingInvoiceId; + if (invoiceId) { + billingInvoice = await sdk.forConsole.organizations.getInvoice({ + organizationId: organization.$id, + invoiceId + }); + }src/routes/(console)/account/organizations/+page.svelte (1)
40-45: Remove stale “tierToPlan” wording and renametierData.The comment no longer matches the implementation, and the variable name is misleading after the
billingIdToPlanswitch. Consider removing the comment and renaming to reduce confusion.♻️ Suggested cleanup
- // For known plans, use tierToPlan - const tierData = billingIdToPlan(billingPlan); - - // If it's not a custom plan, or we got a non-custom result, return the name - if (tierData.name !== 'Custom') { - return tierData.name; - } + const planData = billingIdToPlan(billingPlan); + if (planData.name !== 'Custom') { + return planData.name; + }As per coding guidelines, minimize comments unless they clarify complex logic.
src/lib/helpers/program.ts (1)
10-12: LGTM!Clean helper that centralizes program icon mapping. The documentation comment clearly explains the data path relationship.
Consider adding an index signature type for better maintainability and to document expected keys:
Optional type annotation
-export const IconsMap = { +export const IconsMap: Record<string, typeof IconGithub> = { github: IconGithub };src/lib/components/breadcrumbs.svelte (1)
255-258: Consider adding optional chaining forbillingPlanDetails.Similar to other files in this PR, the access to
$organization.billingPlanDetails.groupassumesbillingPlanDetailsis always defined. If the organization object could exist withoutbillingPlanDetailspopulated, this would throw.Suggested defensive fix
$: badgeType = - $organization && $organization.billingPlanDetails.group !== BillingPlanGroup.Starter + $organization && $organization.billingPlanDetails?.group !== BillingPlanGroup.Starter ? 'success' : undefined;src/routes/(console)/organization-[organization]/usage/[[invoice]]/+page.svelte (1)
30-33: Use Svelte 5's$derivedpattern for reactive computed values.
tierandcurrentBillingPlanshould use$derivedinstead ofconstto ensure they reactively track changes to$organization. While component remounting on route param navigation mitigates staleness, using$derivedis the recommended Svelte 5 pattern and ensures correctness if the store updates through other mechanisms (e.g., invalidation).Suggested fix
- const tier = data?.plan - ? data.plan.$id - : (data?.currentInvoice?.plan ?? $organization?.billingPlanId); + const tier = $derived(data?.plan + ? data.plan.$id + : (data?.currentInvoice?.plan ?? $organization?.billingPlanId));- const currentBillingPlan = $organization.billingPlanDetails; + const currentBillingPlan = $derived($organization.billingPlanDetails);Also applies to: 59-59
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (1)
354-357: Track TODO for SDK type fix.The bracket notation access
currentPlan?.['screenshotsGenerated']with the TODO comment indicates a known SDK typing issue. Ensure this is tracked for follow-up once the SDK is updated.Would you like me to open an issue to track this SDK type fix?
src/lib/components/billing/planSelection.svelte (1)
36-38:billingIdToPlanmay return undefined.Per the PR comments,
billingIdToPlanis asserted to always return a valid plan. However, if it returnsundefined, this$effectwould setselectedBillingPlantoundefined, potentially causing issues in parent components expecting a validModels.BillingPlan.Consider adding a guard or assertion:
♻️ Optional defensive check
$effect(() => { - selectedBillingPlan = billingIdToPlan(selectedPlan); + const plan = billingIdToPlan(selectedPlan); + if (plan) { + selectedBillingPlan = plan; + } });src/routes/(console)/organization-[organization]/change-plan/+page.svelte (2)
67-67: Reactive statement with constant value.
$: paymentMethods = null;is misleading—this runs once on init but the$:label suggests reactivity. Since there are no dependencies, use a plain declaration instead.♻️ Suggested fix
- $: paymentMethods = null; + let paymentMethods = null;
261-290: Redundant condition checks.Lines 261 and 286 check the same condition inversely. Using
elsewould be cleaner and prevent potential inconsistency ifisPaymentAuthenticationRequiredhad side effects.♻️ Suggested refactor
if (isPaymentAuthenticationRequired(org)) { // ... payment confirmation logic ... await validate(org.organizationId, collaborators); - } - - if (!isPaymentAuthenticationRequired(org)) { + } else { /** * Reload on upgrade (e.g. Free → Paid) */ // ... success handling ... }
src/routes/(console)/organization-[organization]/createProjectCloud.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/organization-[organization]/createProjectCloud.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/organization-[organization]/usage/[[invoice]]/totalMembers.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/settings/smtp/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
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/deleteOrgPayment.svelte (1)
40-59: Dialog closes prematurely before API completes, hiding errors from user.Line 42 sets
showDelete = falsebefore the try block, causing the confirmation dialog to close immediately. If the API call fails, the error is set (line 56) but the dialog is already closed, so the user never sees the error message.This differs from
removeDefaultMethodwhich keeps the dialog open until the finally block.Suggested fix
async function removeBackupMethod() { if ($currentPlan?.requiresPaymentMethod && !hasOtherMethod) return; - showDelete = false; try { await sdk.forConsole.organizations.deleteBackupPaymentMethod({ organizationId: $organization.$id }); addNotification({ type: 'success', message: `The payment method has been removed from ${$organization.name}` }); trackEvent(Submit.OrganizationBackupPaymentDelete); await invalidate(Dependencies.PAYMENT_METHODS); showDelete = false; } catch (e) { error = e.message; trackError(e, Submit.OrganizationBackupPaymentDelete); + } finally { + showDelete = false; } }src/routes/(console)/organization-[organization]/header.svelte (1)
141-159: Guard billingPlanDetails access on self‑hosted.The tooltip content is always rendered, and
billingPlanDetailsis cloud-only. On self-hosted, this can throw even when the tooltip is disabled. Gate the content (or the tooltip) withisCloudto avoid runtime errors. Based on learnings, billingPlanDetails is cloud-only and should be guarded.🛠️ Suggested guard
- <div slot="tooltip"> - {!organization?.billingPlanDetails.addons.seats.supported - ? 'Upgrade to add more members' - : `You've reached the members limit for the ${ - organization?.billingPlanDetails.name - } plan`} - </div> + <div slot="tooltip"> + {`#if` isCloud} + {!organization.billingPlanDetails.addons.seats.supported + ? 'Upgrade to add more members' + : `You've reached the members limit for the ${ + organization.billingPlanDetails.name + } plan`} + {:else} + Member limit reached + {/if} + </div>src/routes/(console)/apply-credit/+page.svelte (1)
288-357: AddisCloudguards before billingPlanDetails access.
billingPlanDetailsis cloud-only, but it’s accessed in derived state and the invite-members block without any cloud guard. On self-hosted, this can throw. Guard these withisCloud(or equivalent) so the evaluation short-circuits. Based on learnings, billingPlanDetails is cloud-only and should be guarded.🛠️ Suggested guard
+ import { isCloud } from '$lib/system';- $: needsPaymentMethod = selectedOrgId && selectedOrg?.billingPlanDetails.requiresPaymentMethod; + $: needsPaymentMethod = + isCloud && selectedOrgId && selectedOrg?.billingPlanDetails.requiresPaymentMethod;- {`#if` selectedOrgId && !selectedOrg?.billingPlanDetails.addons.seats.supported} + {`#if` isCloud && selectedOrgId && !selectedOrg?.billingPlanDetails.addons.seats.supported}src/routes/(console)/organization-[organization]/change-plan/+page.svelte (1)
81-107: Don’t override the plan query param selection.
selectedPlanis set from?plan=..., then unconditionally overwritten by the base-plan fallback. This likely breaks deep links. Only apply the fallback when no plan param (or when the param is invalid).🛠️ Suggested fix
- const plan = params.get('plan'); - if (plan) { - // if the org has access, - // the plan should be available! - selectedPlan = billingIdToPlan(plan); - } - - const pro = getBasePlanFromGroup(BillingPlanGroup.Pro); - const scale = getBasePlanFromGroup(BillingPlanGroup.Scale); - selectedPlan = $currentPlan?.group === BillingPlanGroup.Scale ? scale : pro; + const plan = params.get('plan'); + const pro = getBasePlanFromGroup(BillingPlanGroup.Pro); + const scale = getBasePlanFromGroup(BillingPlanGroup.Scale); + + if (plan) { + // if the org has access, + // the plan should be available! + selectedPlan = billingIdToPlan(plan); + } else { + selectedPlan = $currentPlan?.group === BillingPlanGroup.Scale ? scale : pro; + }
🤖 Fix all issues with AI agents
In `@src/routes/`(console)/apply-credit/+page.svelte:
- Around line 247-251: getBillingPlan currently calls
billingIdToPlan(campaign.plan) without ensuring campaign is defined; this can
throw when campaign is undefined. Fix by guarding access to campaign.plan before
calling billingIdToPlan: compute planInCache only if campaign is truthy (e.g.,
using campaign && campaign.plan) or by early-return/setting planInCache to
undefined when campaign is falsy, then use that safe value when computing
campaignPlan. Update getBillingPlan and references to billingIdToPlan so they
never dereference campaign when campaign is undefined.
In `@src/routes/`(console)/organization-[organization]/change-plan/+page.svelte:
- Around line 69-72: Declare the paymentMethods variable near the other state
variables (e.g., alongside paymentMethodId and data) so it exists before the
reactive statement; add a declaration like a let paymentMethods = null (or a
typed initial value if using TypeScript, e.g., let paymentMethods: {
paymentMethods?: Models.PaymentMethod[] } | null = null) to ensure the reactive
assignment to paymentMethodId can reference paymentMethods without a compile
error.
In `@src/routes/`(console)/organization-[organization]/members/+page.svelte:
- Around line 47-48: The reactive statement accesses
$organization?.billingPlanDetails.addons.seats which throws when
billingPlanDetails is undefined on self‑hosted; change it to safely traverse the
optional properties (use optional chaining for billingPlanDetails and addons)
and base supportsMembers on the resulting seats object (and its supported flag
like header.svelte uses), e.g. compute seats =
$organization?.billingPlanDetails?.addons?.seats and set supportsMembers =
!!seats?.supported, then keep isFreeWithMembers = !supportsMembers &&
memberCount >= 1 so the expression never dereferences undefined.
🧹 Nitpick comments (2)
src/routes/(console)/organization-[organization]/members/+page.svelte (1)
91-93: Consider defensive chaining onbillingPlanDetails.namefor consistency.Same pattern as line 47 — on self-hosted deployments, accessing
.nameon undefinedbillingPlanDetailscould throw if this slot content is ever evaluated. While the tooltip is disabled on non-cloud and the!supportsMembersbranch would be taken, adding defensive chaining ensures robustness.Suggested change
{!supportsMembers ? 'Upgrade to add more members' - : `You've reached the members limit for the ${$organization?.billingPlanDetails.name} plan`} + : `You've reached the members limit for the ${$organization?.billingPlanDetails?.name} plan`}src/routes/(console)/account/organizations/+page.svelte (1)
37-57: RenametierDataand remove the staletierToPlancomment for clarity.The variable name/comment still reflect the old tier-based naming and are now misleading with
billingIdToPlan.♻️ Suggested cleanup
- // For known plans, use tierToPlan - const tierData = billingIdToPlan(billingPlan); + const planData = billingIdToPlan(billingPlan); - if (tierData.name !== 'Custom') { - return tierData.name; + if (planData.name !== 'Custom') { + return planData.name; }
src/routes/(console)/organization-[organization]/change-plan/+page.svelte
Show resolved
Hide resolved
src/routes/(console)/organization-[organization]/members/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/lib/stores/billing.ts`:
- Around line 348-350: The code reads org.billingPlanDetails directly causing
crashes on self-hosted installs; update calculateTrialDay, checkForUsageLimit,
checkPaymentAuthorizationRequired, and checkForNewDevUpgradePro to guard access
by either checking org.isCloud first or using optional chaining (e.g.,
org.billingPlanDetails?....) before any property access, and return the
appropriate default (false/no-op) when billingPlanDetails is absent; ensure
every access in those functions (trial, limits, paymentAuthorization, upgrade
flags) uses the guard so self-hosted org objects don't throw.
🧹 Nitpick comments (1)
src/lib/stores/billing.ts (1)
81-169: Guard against missing billing plans before dereferencing.Line 98 and Line 260 assume
billingIdToPlanalways returns a plan; if it returnsnull(e.g., empty cache or unexpected ID), callers will throw on.$id,.usage, or.projects. Consider early returns or an explicit error to make failures deterministic.♻️ Suggested guard pattern
-function makeBillingPlan(billingPlanOrId: string | Models.BillingPlan): Models.BillingPlan { - return typeof billingPlanOrId === 'string' ? billingIdToPlan(billingPlanOrId) : billingPlanOrId; -} +function makeBillingPlan( + billingPlanOrId: string | Models.BillingPlan +): Models.BillingPlan | null { + return typeof billingPlanOrId === 'string' ? billingIdToPlan(billingPlanOrId) : billingPlanOrId; +} export function canUpgrade(billingPlanOrId: string | Models.BillingPlan): boolean { const billingPlan = makeBillingPlan(billingPlanOrId); + if (!billingPlan) return false; const nextTier = getNextTierBillingPlan(billingPlan.$id);export function checkForUsageFees(plan: string, id: PlanServices) { const billingPlan = billingIdToPlan(plan); + if (!billingPlan?.usage) return false; const supportsUsage = Object.keys(billingPlan.usage).length > 0;Also applies to: 259-282, 303-318
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@e2e/steps/pro-project.ts`:
- Around line 60-68: The page-level locator for firstEnabledOption can match
hidden options from other listboxes, causing flaky region selection; update the
locator used where firstEnabledOption is defined (page.locator(...).first()) to
either include the :visible filter (e.g.,
'[role="option"]:not([data-disabled="true"]):visible') or scope the query to the
dialog element
(dialog.locator('[role="option"]:not([data-disabled="true"])').first()) so the
selectedRegion/region logic only acts on visible options within the intended
dialog.
There was a problem hiding this comment.
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 (2)
src/lib/stores/billing.ts (2)
258-275: Null dereference risk incheckForUsageFees.
billingIdToPlan(plan)can returnnull, causingbillingPlan.usage(line 260) to throw aTypeError.🐛 Suggested fix
export function checkForUsageFees(plan: string, id: PlanServices) { const billingPlan = billingIdToPlan(plan); + if (!billingPlan) return false; const supportsUsage = Object.keys(billingPlan.usage).length > 0;
277-300: Same null dereference risk incheckForProjectLimitation.
billingIdToPlan(plan)can returnnull, causingbillingPlan.projects(line 280) to throw.🐛 Suggested fix
export function checkForProjectLimitation(plan: string, id: PlanServices) { if (id === 'members') { const billingPlan = billingIdToPlan(plan); + if (!billingPlan) return true; // Default to limited if plan unknown const hasUnlimitedProjects = billingPlan.projects === 0;
🤖 Fix all issues with AI agents
In `@src/lib/stores/billing.ts`:
- Around line 142-154: getNextTierBillingPlan currently assumes
billingIdToPlan(billingPlanId) returns a non-null value and directly accesses
currentPlanData.order; add a null check after calling billingIdToPlan and handle
the null case by returning a sensible default (e.g., call
getBasePlanFromGroup(BillingPlanGroup.Pro) or throw a clear error), then proceed
to compute currentOrder and search plans as before; reference the
billingIdToPlan call, the currentPlanData variable, the plans = get(plansInfo)
usage, and fallback to getBasePlanFromGroup(BillingPlanGroup.Pro) when null.
- Around line 156-168: getPreviousTierBillingPlan currently assumes
billingIdToPlan(billingPlanId) and its .order exist and can dereference them;
mirror the defensive checks used in getNextTierBillingPlan by validating
currentPlanData is non-null and has a numeric order before using it, and if
invalid return getBasePlanFromGroup(BillingPlanGroup.Starter); iterate plans
from get(plansInfo) only after the guard and compare plan.order to currentOrder
- 1 to find and return the previous plan.
🧹 Nitpick comments (2)
src/routes/(console)/regions.ts (1)
31-31: Inconsistent terminology in error message.The error message references
teamId, but the parameter is namedorgIdand the API usesorganizationId. Consider updating for consistency.Suggested fix
- console.error(`Failed to load regions for teamId: ${orgId}`, error); + console.error(`Failed to load regions for organizationId: ${orgId}`, error);src/lib/stores/billing.ts (1)
85-87: Null propagation risk frommakeBillingPlanto consumer functions.
billingIdToPlanreturnsModels.BillingPlan | null, butmakeBillingPlanhas no null guard. Consumer functions likeisStarterPlan,canUpgrade,canDowngrade, andplanHasGroupaccess properties (e.g.,.group,.$id) on the result without null checks.If an invalid
billingIdis passed, this will throw aTypeError.♻️ Suggested defensive handling
function makeBillingPlan(billingPlanOrId: string | Models.BillingPlan): Models.BillingPlan { - return typeof billingPlanOrId === 'string' ? billingIdToPlan(billingPlanOrId) : billingPlanOrId; + const plan = typeof billingPlanOrId === 'string' ? billingIdToPlan(billingPlanOrId) : billingPlanOrId; + if (!plan) { + throw new Error(`Invalid billing plan: ${billingPlanOrId}`); + } + return plan; }Alternatively, update the return type to
Models.BillingPlan | nulland add null checks in all consumer functions.
| export function getNextTierBillingPlan(billingPlanId: string): Models.BillingPlan { | ||
| const currentPlanData = billingIdToPlan(billingPlanId); | ||
| const currentOrder = currentPlanData.order; | ||
| const plans = get(plansInfo); | ||
|
|
||
| for (const [, plan] of plans) { | ||
| if (plan.order === currentOrder + 1) { | ||
| return plan; | ||
| } | ||
| } | ||
|
|
||
| return getBasePlanFromGroup(BillingPlanGroup.Pro); | ||
| } |
There was a problem hiding this comment.
Potential null dereference in getNextTierBillingPlan.
Line 143 calls billingIdToPlan(billingPlanId) which can return null. Line 144 then accesses currentPlanData.order without a null check, which would throw a TypeError.
🐛 Suggested fix
export function getNextTierBillingPlan(billingPlanId: string): Models.BillingPlan {
const currentPlanData = billingIdToPlan(billingPlanId);
+ if (!currentPlanData) {
+ return getBasePlanFromGroup(BillingPlanGroup.Pro);
+ }
const currentOrder = currentPlanData.order;
const plans = get(plansInfo);🤖 Prompt for AI Agents
In `@src/lib/stores/billing.ts` around lines 142 - 154, getNextTierBillingPlan
currently assumes billingIdToPlan(billingPlanId) returns a non-null value and
directly accesses currentPlanData.order; add a null check after calling
billingIdToPlan and handle the null case by returning a sensible default (e.g.,
call getBasePlanFromGroup(BillingPlanGroup.Pro) or throw a clear error), then
proceed to compute currentOrder and search plans as before; reference the
billingIdToPlan call, the currentPlanData variable, the plans = get(plansInfo)
usage, and fallback to getBasePlanFromGroup(BillingPlanGroup.Pro) when null.
| export function getPreviousTierBillingPlan(billingPlanId: string): Models.BillingPlan { | ||
| const currentPlanData = billingIdToPlan(billingPlanId); | ||
| const currentOrder = currentPlanData.order; | ||
| const plans = get(plansInfo); | ||
|
|
||
| for (const [, plan] of plans) { | ||
| if (plan.order === currentOrder - 1) { | ||
| return plan; | ||
| } | ||
| } | ||
|
|
||
| return getBasePlanFromGroup(BillingPlanGroup.Starter); | ||
| } |
There was a problem hiding this comment.
Same null dereference risk in getPreviousTierBillingPlan.
Apply the same defensive pattern as suggested for getNextTierBillingPlan.
🐛 Suggested fix
export function getPreviousTierBillingPlan(billingPlanId: string): Models.BillingPlan {
const currentPlanData = billingIdToPlan(billingPlanId);
+ if (!currentPlanData) {
+ return getBasePlanFromGroup(BillingPlanGroup.Starter);
+ }
const currentOrder = currentPlanData.order;
const plans = get(plansInfo);🤖 Prompt for AI Agents
In `@src/lib/stores/billing.ts` around lines 156 - 168, getPreviousTierBillingPlan
currently assumes billingIdToPlan(billingPlanId) and its .order exist and can
dereference them; mirror the defensive checks used in getNextTierBillingPlan by
validating currentPlanData is non-null and has a numeric order before using it,
and if invalid return getBasePlanFromGroup(BillingPlanGroup.Starter); iterate
plans from get(plansInfo) only after the guard and compare plan.order to
currentOrder - 1 to find and return the previous plan.
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/lib/stores/billing.ts (1)
258-285: GuardbillingIdToPlanresults before accessing fields.Both
checkForUsageFeesandcheckForProjectLimitationdereferencebillingPlanwithout null checks.🛠️ Proposed fix
export function checkForUsageFees(plan: string, id: PlanServices) { const billingPlan = billingIdToPlan(plan); + if (!billingPlan) return false; - const supportsUsage = Object.keys(billingPlan.usage).length > 0; + const supportsUsage = Object.keys(billingPlan.usage ?? {}).length > 0; ... } export function checkForProjectLimitation(plan: string, id: PlanServices) { if (id === 'members') { const billingPlan = billingIdToPlan(plan); + if (!billingPlan) return false; const hasUnlimitedProjects = billingPlan.projects === 0;src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte (1)
210-235: Guard$projectbefore readingteamIdin upgrade navigation.Accessing
$project.teamIdcan throw if the project store is briefly undefined.🛠️ Proposed fix
- goto(getChangePlanUrl($project.teamId)); + goto(getChangePlanUrl($project?.teamId)); ... - goto(getChangePlanUrl($project.teamId)); + goto(getChangePlanUrl($project?.teamId));src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/containerHeader.svelte (1)
73-78: Avoid potential$projectnull dereference in upgrade action.Use optional chaining to keep the CTA safe during brief store gaps.
🛠️ Proposed fix
- goto(getChangePlanUrl($project.teamId)); + goto(getChangePlanUrl($project?.teamId));src/routes/(console)/organization-[organization]/usage/[[invoice]]/+page.svelte (1)
59-115: GuardbillingPlanDetailsfor self‑hosted instances.
billingPlanDetailsis cloud‑only, so direct access can throw when the organization exists but billing details are absent. Gate it withisCloudand avoid calling plan helpers withnull.Based on learnings: billingPlanDetails is cloud-only in the Appwrite Console and should be guarded with isCloud or defensive access.🛠️ Suggested fix
- import { onMount } from 'svelte'; + import { onMount } from 'svelte'; + import { isCloud } from '$lib/system'; @@ - const currentBillingPlan = $organization.billingPlanDetails; + let currentBillingPlan = null; + $: currentBillingPlan = isCloud ? $organization.billingPlanDetails : null;- {`#if` isStarterPlan(currentBillingPlan)} + {`#if` currentBillingPlan && isStarterPlan(currentBillingPlan)} @@ - {`#if` planHasGroup(currentBillingPlan, BillingPlanGroup.Scale)} + {`#if` currentBillingPlan && planHasGroup(currentBillingPlan, BillingPlanGroup.Scale)} @@ - {:else if planHasGroup(currentBillingPlan, BillingPlanGroup.Pro)} + {:else if currentBillingPlan && planHasGroup(currentBillingPlan, BillingPlanGroup.Pro)} @@ - {:else if isStarterPlan(currentBillingPlan)} + {:else if currentBillingPlan && isStarterPlan(currentBillingPlan)}src/lib/layout/containerHeader.svelte (1)
64-87: GuardbillingPlanDetailsaccess outside cloud.
planNameis computed unconditionally frombillingPlanDetails, which can be missing on self‑hosted deployments. Gate it withisCloud(or provide a safe fallback) to avoid runtime errors.Based on learnings: billingPlanDetails is cloud-only in the Appwrite Console and should be guarded with isCloud or defensive access.🛠️ Suggested fix
- $: planName = $organization?.billingPlanDetails.name; + $: planName = isCloud ? $organization.billingPlanDetails.name : 'Self-hosted';
🤖 Fix all issues with AI agents
In `@src/lib/components/billing/alerts/projectsLimit.svelte`:
- Line 41: The upgrade link currently calls getChangePlanUrl(organizationId)
even when organizationId may be undefined; change the element to only set href
when organizationId is available and otherwise disable the control — e.g., set
href to undefined or an empty string when organizationId is falsy and add a
disabled/aria-disabled state on the button/link so it cannot be clicked (use the
existing organizationId variable and getChangePlanUrl function to implement this
guard).
In `@src/lib/components/billing/emptyCardCloud.svelte`:
- Around line 10-39: The default rendering uses organizationId directly in the
upgrade link which can call getChangePlanUrl(null) when organizationId is
omitted; update the component so that when children is falsy you either require
organizationId or guard the Button/link: check organizationId before calling
getChangePlanUrl (e.g., render the Button only if organizationId is present or
compute the href conditionally), and ensure the on:click/ href use the guarded
value so getChangePlanUrl is never invoked with null; modify the logic around
children, organizationId, and the Button rendering to implement this guard.
In `@src/lib/components/bottomModalAlert.svelte`:
- Around line 142-151: In triggerWindowLink, guard calls to canUpgrade by
ensuring billingPlanDetails is defined before invoking it: replace direct calls
like canUpgrade($organization?.billingPlanDetails) with a check that
$organization?.billingPlanDetails exists (e.g., const hasBilling =
!!$organization?.billingPlanDetails; const shouldShowUpgrade = hasBilling &&
canUpgrade($organization.billingPlanDetails)) and apply the same pattern at the
other location that calls canUpgrade (both uses in
triggerWindowLink/currentModalAlert handling) so canUpgrade never receives
undefined.
In `@src/lib/components/cardContainer.svelte`:
- Around line 12-18: serviceId is currently initialized once from service and
won't update when the prop changes; make serviceId a reactive derived value from
service so planLimit recalculates. Replace the static initialization of
serviceId with a reactive assignment (using Svelte reactivity) that sets
serviceId from service (and keep planLimit as $: planLimit =
getServiceLimit(serviceId) || Infinity) so updates to service propagate through
serviceId to getServiceLimit; reference the service, serviceId, and planLimit
symbols when applying this change.
In `@src/lib/stores/billing.ts`:
- Around line 81-121: Guard against a null billing plan returned by
billingIdToPlan (via makeBillingPlan) in canUpgrade and canDowngrade: check that
billingPlan is truthy before accessing billingPlan.$id and return false (or
appropriate safe value) if lookup failed; similarly ensure planHasGroup handles
a null billingPlan by returning false. Update canUpgrade to call
makeBillingPlan, if billingPlan is null return false, otherwise
getNextTierBillingPlan(billingPlan.$id) and compare IDs; do the analogous guard
in canDowngrade using getPreviousTierBillingPlan, and adjust planHasGroup to
safely return false when makeBillingPlan returns null.
In `@src/routes/`(console)/organization-[organization]/+page.svelte:
- Line 222: The conditional rendering for the free-plan alert can throw if
data.currentPlan is undefined; update the condition that currently reads
"isCloud && data.currentPlan.projects !== 0 && projectsToArchive.length === 0 &&
!freePlanAlertDismissed" to guard access to currentPlan (e.g., use optional
chaining or an explicit existence check) such as "isCloud &&
data.currentPlan?.projects !== 0 && projectsToArchive.length === 0 &&
!freePlanAlertDismissed" or "isCloud && data.currentPlan &&
data.currentPlan.projects !== 0 && projectsToArchive.length === 0 &&
!freePlanAlertDismissed" so hydration won't throw when currentPlan is absent.
In `@src/routes/`(console)/organization-[organization]/billing/budgetAlert.svelte:
- Around line 81-84: Guard access to organization.billingPlanDetails in the
budgetAlert Svelte view to avoid crashes in self-hosted contexts by checking
isCloud and providing a fallback: when isCloud is true use
organization.billingPlanDetails.name, otherwise use currentPlan.name (or a safe
default); update the template expression around the bold text to reference this
guarded value and ensure organization.billingPlanDetails is not accessed
directly when isCloud is false.
In
`@src/routes/`(console)/project-[region]-[project]/auth/templates/emailSignature.svelte:
- Line 58: Guard access to $project before reading teamId: change the href call
that currently passes $project.teamId to use optional chaining so
getChangePlanUrl receives undefined when $project is missing (e.g. call
getChangePlanUrl($project?.teamId) or compute a local teamId = $project?.teamId
and pass that). Update the template reference where
href={getChangePlanUrl($project.teamId)} so it reads
href={getChangePlanUrl($project?.teamId)} and ensure getChangePlanUrl safely
handles an undefined teamId.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/backups/upgradeCard.svelte:
- Line 34: The expression that computes organizationId uses
page.data?.organization.$id which will throw if organization is undefined;
update the lookup to use optional chaining (page.data?.organization?.$id) so the
access is safe during transitions while keeping the existing fallback
(page.data?.project?.teamId) and ensure any other reads of
page.data.organization in the same component follow the same pattern.
In
`@src/routes/`(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte:
- Around line 54-55: currentPlanId and currentBillingPlan are declared as plain
consts so they won't react to changes in data or $organization; change them to
reactive declarations using $: so they update automatically (e.g., replace the
consts for currentPlanId and currentBillingPlan with $: currentPlanId = ... and
$: currentBillingPlan = billingIdToPlan(currentPlanId)), ensuring you still
reference data?.currentInvoice?.plan and $organization?.billingPlanId and call
billingIdToPlan(currentPlanId) to derive the plan reactively.
In
`@src/routes/`(console)/project-[region]-[project]/sites/site-[site]/settings/updateResourceLimits.svelte:
- Around line 88-94: Replace the direct access to billingPlanId when computing
isStarter so missing organization billing data is safe: update the isStarter
computation (symbol isStarter and function isStarterPlan) to pass the
organization billingPlanId using optional chaining (i.e., guard the access to
billingPlanId on $organization so undefined is passed to isStarterPlan when
absent) so the check matches the defensive pattern used elsewhere.
🧹 Nitpick comments (2)
src/lib/elements/table/tableScroll.svelte (1)
15-15: AlignActiongeneric and node parameter type (line 15).
The action is declared asAction<HTMLDivElement, unknown>but the parameter is typed asElement. In Svelte 5, the first generic parameter ofActionspecifies the expected node type, so the parameter should match: changenode: Elementtonode: HTMLDivElement.Suggested fix
- const hasOverflow: Action<HTMLDivElement, unknown> = (node: Element) => { + const hasOverflow: Action<HTMLDivElement, unknown> = (node: HTMLDivElement) => {src/lib/components/billing/emptyCardCloud.svelte (1)
2-2: Prefer$libalias for shared component imports.This component lives under
src/lib, so use the$libalias for consistency with repo conventions.🔧 Suggested fix
-import { Card } from '..'; +import { Card } from '$lib/components';As per coding guidelines: Import reusable modules from the src/lib directory using the $lib alias.
| }}>Manage projects</Button> | ||
| <Button | ||
| href={$upgradeURL} | ||
| href={getChangePlanUrl(organizationId)} |
There was a problem hiding this comment.
Guard the upgrade link when organizationId is not resolved.
If organizationId is undefined (e.g., during navigation), getChangePlanUrl(undefined) can produce a broken link. Consider disabling the button or only setting href once the id is available.
🔧 Suggested guard
- <Button
- href={getChangePlanUrl(organizationId)}
+ <Button
+ href={organizationId ? getChangePlanUrl(organizationId) : undefined}
+ disabled={!organizationId}
event={Click.OrganizationClickUpgrade}📝 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.
| href={getChangePlanUrl(organizationId)} | |
| href={organizationId ? getChangePlanUrl(organizationId) : undefined} | |
| disabled={!organizationId} |
🤖 Prompt for AI Agents
In `@src/lib/components/billing/alerts/projectsLimit.svelte` at line 41, The
upgrade link currently calls getChangePlanUrl(organizationId) even when
organizationId may be undefined; change the element to only set href when
organizationId is available and otherwise disable the control — e.g., set href
to undefined or an empty string when organizationId is falsy and add a
disabled/aria-disabled state on the button/link so it cannot be clicked (use the
existing organizationId variable and getChangePlanUrl function to implement this
guard).
| let { | ||
| service, | ||
| eventSource, | ||
| organizationId = null, | ||
| children = null | ||
| }: { | ||
| service: string; | ||
| eventSource: string; | ||
| organizationId?: string; | ||
| children?: Snippet; | ||
| } = $props(); | ||
| export let service: string; | ||
| export let eventSource: string; | ||
| const proPlanName = getBasePlanFromGroup(BillingPlanGroup.Pro).name; | ||
| </script> | ||
|
|
||
| <Card> | ||
| <slot> | ||
| {#if children} | ||
| {@render children()} | ||
| {:else} | ||
| <Layout.Stack alignItems="center"> | ||
| <Typography.Text variant="m-600">Upgrade to add {service}</Typography.Text> | ||
| <Typography.Text> | ||
| Upgrade to a {tierToPlan(BillingPlan.PRO).name} plan to add {service} to your organization | ||
| Upgrade to a {proPlanName} plan to add {service} to your organization | ||
| </Typography.Text> | ||
|
|
||
| <Button | ||
| secondary | ||
| fullWidthMobile | ||
| href={$upgradeURL} | ||
| href={getChangePlanUrl(organizationId)} | ||
| on:click={() => { |
There was a problem hiding this comment.
Avoid getChangePlanUrl(null) when organizationId is omitted.
organizationId is optional, but the default rendering path always uses it. If a caller provides children without an organizationId, this can produce a broken upgrade link. Consider guarding the default path or making organizationId required when children is absent.
🔧 Suggested fix
- {`#if` children}
+ {`#if` children}
{`@render` children()}
- {:else}
+ {:else if organizationId}
<Layout.Stack alignItems="center">
...
- <Button
+ <Button
secondary
fullWidthMobile
href={getChangePlanUrl(organizationId)}
on:click={() => {
trackEvent(Click.OrganizationClickUpgrade, {
from: 'button',
source: eventSource
});
}}>
Upgrade
</Button>
</Layout.Stack>
+ {/if}📝 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.
| let { | |
| service, | |
| eventSource, | |
| organizationId = null, | |
| children = null | |
| }: { | |
| service: string; | |
| eventSource: string; | |
| organizationId?: string; | |
| children?: Snippet; | |
| } = $props(); | |
| export let service: string; | |
| export let eventSource: string; | |
| const proPlanName = getBasePlanFromGroup(BillingPlanGroup.Pro).name; | |
| </script> | |
| <Card> | |
| <slot> | |
| {#if children} | |
| {@render children()} | |
| {:else} | |
| <Layout.Stack alignItems="center"> | |
| <Typography.Text variant="m-600">Upgrade to add {service}</Typography.Text> | |
| <Typography.Text> | |
| Upgrade to a {tierToPlan(BillingPlan.PRO).name} plan to add {service} to your organization | |
| Upgrade to a {proPlanName} plan to add {service} to your organization | |
| </Typography.Text> | |
| <Button | |
| secondary | |
| fullWidthMobile | |
| href={$upgradeURL} | |
| href={getChangePlanUrl(organizationId)} | |
| on:click={() => { | |
| let { | |
| service, | |
| eventSource, | |
| organizationId = null, | |
| children = null | |
| }: { | |
| service: string; | |
| eventSource: string; | |
| organizationId?: string; | |
| children?: Snippet; | |
| } = $props(); | |
| const proPlanName = getBasePlanFromGroup(BillingPlanGroup.Pro).name; | |
| </script> | |
| <Card> | |
| {`#if` children} | |
| {`@render` children()} | |
| {:else if organizationId} | |
| <Layout.Stack alignItems="center"> | |
| <Typography.Text variant="m-600">Upgrade to add {service}</Typography.Text> | |
| <Typography.Text> | |
| Upgrade to a {proPlanName} plan to add {service} to your organization | |
| </Typography.Text> | |
| <Button | |
| secondary | |
| fullWidthMobile | |
| href={getChangePlanUrl(organizationId)} | |
| on:click={() => { | |
| trackEvent(Click.OrganizationClickUpgrade, { | |
| from: 'button', | |
| source: eventSource | |
| }); | |
| }}> | |
| Upgrade | |
| </Button> | |
| </Layout.Stack> | |
| {/if} |
🤖 Prompt for AI Agents
In `@src/lib/components/billing/emptyCardCloud.svelte` around lines 10 - 39, The
default rendering uses organizationId directly in the upgrade link which can
call getChangePlanUrl(null) when organizationId is omitted; update the component
so that when children is falsy you either require organizationId or guard the
Button/link: check organizationId before calling getChangePlanUrl (e.g., render
the Button only if organizationId is present or compute the href conditionally),
and ensure the on:click/ href use the guarded value so getChangePlanUrl is never
invoked with null; modify the logic around children, organizationId, and the
Button rendering to implement this guard.
| function triggerWindowLink(alert: BottomModalAlertItem, event?: string) { | ||
| const alertAction = alert.cta; | ||
| const shouldShowUpgrade = showUpgrade(); | ||
| const shouldShowUpgrade = canUpgrade($organization?.billingPlanDetails); | ||
| // for correct event tracking after removal | ||
| const currentModalId = currentModalAlert.id; | ||
| const organizationId = $project.teamId ?? $organization.$id; | ||
| const url = shouldShowUpgrade | ||
| ? $upgradeURL | ||
| ? getChangePlanUrl(organizationId) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "canUpgrade" src/lib/stores/billing.ts -C 5Repository: appwrite/console
Length of output: 608
🏁 Script executed:
cat -n src/lib/components/bottomModalAlert.svelte | head -200Repository: appwrite/console
Length of output: 9361
🏁 Script executed:
rg -n "billingPlanDetails" src/lib/components/bottomModalAlert.svelte -C 3Repository: appwrite/console
Length of output: 777
🏁 Script executed:
rg -n "function makeBillingPlan|const makeBillingPlan" src/lib/stores/billing.ts -A 10Repository: appwrite/console
Length of output: 609
🏁 Script executed:
rg -n "export.*makeBillingPlan" src/lib/stores/billing.ts -A 10Repository: appwrite/console
Length of output: 42
🏁 Script executed:
rg -n "canUpgrade" src/lib/components/ -lRepository: appwrite/console
Length of output: 102
🏁 Script executed:
rg -n "canUpgrade" src/routes/ -B 2 -A 2Repository: appwrite/console
Length of output: 42
🏁 Script executed:
rg -n "billingPlanDetails" src/lib/components/ src/routes/ --max-count 20Repository: appwrite/console
Length of output: 6188
🏁 Script executed:
rg -n "organization\?\." src/lib/components/bottomModalAlert.svelteRepository: appwrite/console
Length of output: 227
🏁 Script executed:
rg -n "makeBillingPlan" src/lib/stores/billing.ts -B 2 -A 8Repository: appwrite/console
Length of output: 1790
🏁 Script executed:
rg -n "isCloud\|isSelfHosted" src/lib/stores/ -lRepository: appwrite/console
Length of output: 42
🏁 Script executed:
# Check billingIdToPlan to see what happens with undefined
rg -n "billingIdToPlan" src/lib/stores/billing.ts -A 5Repository: appwrite/console
Length of output: 1566
🏁 Script executed:
# Check if there's any isCloud or isSelfHosted store that should be used
rg -n "isCloud|isSelfHosted" src/lib/ --max-count 20Repository: appwrite/console
Length of output: 4931
Guard against undefined billingPlanDetails when calling canUpgrade.
canUpgrade() requires a valid BillingPlan object but receives potentially undefined when $organization?.billingPlanDetails is missing (self-hosted deployments or before organization loads). This causes a runtime error when trying to access the plan's $id property. Apply the guard at both locations:
Suggested fix
- const shouldShowUpgrade = canUpgrade($organization?.billingPlanDetails);
+ const shouldShowUpgrade = $organization?.billingPlanDetails
+ ? canUpgrade($organization.billingPlanDetails)
+ : false;- {`@const` shouldShowUpgrade = canUpgrade($organization?.billingPlanDetails)}
+ {`@const` shouldShowUpgrade = $organization?.billingPlanDetails
+ ? canUpgrade($organization.billingPlanDetails)
+ : false}Applies to lines 144 and 177.
📝 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.
| function triggerWindowLink(alert: BottomModalAlertItem, event?: string) { | |
| const alertAction = alert.cta; | |
| const shouldShowUpgrade = showUpgrade(); | |
| const shouldShowUpgrade = canUpgrade($organization?.billingPlanDetails); | |
| // for correct event tracking after removal | |
| const currentModalId = currentModalAlert.id; | |
| const organizationId = $project.teamId ?? $organization.$id; | |
| const url = shouldShowUpgrade | |
| ? $upgradeURL | |
| ? getChangePlanUrl(organizationId) | |
| function triggerWindowLink(alert: BottomModalAlertItem, event?: string) { | |
| const alertAction = alert.cta; | |
| const shouldShowUpgrade = $organization?.billingPlanDetails | |
| ? canUpgrade($organization.billingPlanDetails) | |
| : false; | |
| // for correct event tracking after removal | |
| const currentModalId = currentModalAlert.id; | |
| const organizationId = $project.teamId ?? $organization.$id; | |
| const url = shouldShowUpgrade | |
| ? getChangePlanUrl(organizationId) |
🤖 Prompt for AI Agents
In `@src/lib/components/bottomModalAlert.svelte` around lines 142 - 151, In
triggerWindowLink, guard calls to canUpgrade by ensuring billingPlanDetails is
defined before invoking it: replace direct calls like
canUpgrade($organization?.billingPlanDetails) with a check that
$organization?.billingPlanDetails exists (e.g., const hasBilling =
!!$organization?.billingPlanDetails; const shouldShowUpgrade = hasBilling &&
canUpgrade($organization.billingPlanDetails)) and apply the same pattern at the
other location that calls canUpgrade (both uses in
triggerWindowLink/currentModalAlert handling) so canUpgrade never receives
undefined.
| export let offset = 0; | ||
| export let service = ''; | ||
| export let disableEmpty = true; | ||
| export let event: string = null; | ||
| export let serviceId: PlanServices = service as PlanServices; | ||
| $: planLimit = getServiceLimit(serviceId) || Infinity; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/lib/components/cardContainer.svelteRepository: appwrite/console
Length of output: 1987
🏁 Script executed:
rg "cardContainer" --type svelte -A 2 -B 2 | head -100Repository: appwrite/console
Length of output: 90
🏁 Script executed:
rg "cardContainer" -i -A 2 -B 2 | head -150Repository: appwrite/console
Length of output: 10322
🏁 Script executed:
rg "CardContainer" -A 10 | grep -E "(service|serviceId)" | head -50Repository: appwrite/console
Length of output: 347
🏁 Script executed:
rg "<CardContainer" -A 5 | grep -B 2 -A 2 "serviceId"Repository: appwrite/console
Length of output: 42
🏁 Script executed:
rg "<CardContainer" -A 8 | head -100Repository: appwrite/console
Length of output: 7086
serviceId won't track service updates after mount.
The default expression service as PlanServices only evaluates at component initialization. If a parent updates the service prop without updating serviceId, the value remains stale and won't trigger reactive updates to planLimit.
♻️ Suggested adjustment
-export let service = '';
-export let serviceId: PlanServices = service as PlanServices;
+export let service = '';
+export let serviceId: PlanServices | null = null;
+
+$: effectiveServiceId = (serviceId ?? (service as PlanServices));
+
-$: planLimit = getServiceLimit(serviceId) || Infinity;
+$: planLimit = getServiceLimit(effectiveServiceId) || Infinity;📝 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.
| export let offset = 0; | |
| export let service = ''; | |
| export let disableEmpty = true; | |
| export let event: string = null; | |
| export let serviceId: PlanServices = service as PlanServices; | |
| $: planLimit = getServiceLimit(serviceId) || Infinity; | |
| export let offset = 0; | |
| export let service = ''; | |
| export let disableEmpty = true; | |
| export let event: string = null; | |
| export let serviceId: PlanServices | null = null; | |
| $: effectiveServiceId = (serviceId ?? (service as PlanServices)); | |
| $: planLimit = getServiceLimit(effectiveServiceId) || Infinity; |
🤖 Prompt for AI Agents
In `@src/lib/components/cardContainer.svelte` around lines 12 - 18, serviceId is
currently initialized once from service and won't update when the prop changes;
make serviceId a reactive derived value from service so planLimit recalculates.
Replace the static initialization of serviceId with a reactive assignment (using
Svelte reactivity) that sets serviceId from service (and keep planLimit as $:
planLimit = getServiceLimit(serviceId) || Infinity) so updates to service
propagate through serviceId to getServiceLimit; reference the service,
serviceId, and planLimit symbols when applying this change.
| function getPlansInfoStore(): BillingPlansMap | null { | ||
| return get(plansInfo) ?? get(page).data?.plansInfo ?? null; | ||
| } | ||
|
|
||
| function makeBillingPlan(billingPlanOrId: string | Models.BillingPlan): Models.BillingPlan { | ||
| return typeof billingPlanOrId === 'string' ? billingIdToPlan(billingPlanOrId) : billingPlanOrId; | ||
| } | ||
|
|
||
| export function getRoleLabel(role: string) { | ||
| return roles.find((r) => r.value === role)?.label ?? role; | ||
| } | ||
|
|
||
| export function tierToPlan(tier: Tier) { | ||
| switch (tier) { | ||
| case BillingPlan.FREE: | ||
| return tierFree; | ||
| case BillingPlan.PRO: | ||
| return tierPro; | ||
| case BillingPlan.SCALE: | ||
| return tierScale; | ||
| case BillingPlan.GITHUB_EDUCATION: | ||
| return tierGitHubEducation; | ||
| case BillingPlan.CUSTOM: | ||
| return tierCustom; | ||
| case BillingPlan.ENTERPRISE: | ||
| return tierEnterprise; | ||
| default: | ||
| return tierCustom; | ||
| export function isStarterPlan(billingPlanOrId: string | Models.BillingPlan): boolean { | ||
| const billingPlan = makeBillingPlan(billingPlanOrId); | ||
| return planHasGroup(billingPlan, BillingPlanGroup.Starter); | ||
| } | ||
|
|
||
| export function canUpgrade(billingPlanOrId: string | Models.BillingPlan): boolean { | ||
| const billingPlan = makeBillingPlan(billingPlanOrId); | ||
| const nextTier = getNextTierBillingPlan(billingPlan.$id); | ||
|
|
||
| // defaults back to PRO, so adjust the check! | ||
| return billingPlan.$id !== nextTier.$id; | ||
| } | ||
|
|
||
| export function canDowngrade(billingPlanOrId: string | Models.BillingPlan): boolean { | ||
| const billingPlan = makeBillingPlan(billingPlanOrId); | ||
| const nextTier = getPreviousTierBillingPlan(billingPlan.$id); | ||
|
|
||
| // defaults back to Starter, so adjust the check! | ||
| return billingPlan.$id !== nextTier.$id; | ||
| } | ||
|
|
||
| export function planHasGroup( | ||
| billingPlanOrId: string | Models.BillingPlan, | ||
| group: BillingPlanGroup | ||
| ): boolean { | ||
| const billingPlan = makeBillingPlan(billingPlanOrId); | ||
|
|
||
| return billingPlan?.group === group; | ||
| } |
There was a problem hiding this comment.
Guard canUpgrade / canDowngrade when plan lookup fails.
billingIdToPlan can return null, which makes billingPlan.$id unsafe. A simple guard prevents a hard crash.
🛠️ Proposed fix
export function canUpgrade(billingPlanOrId: string | Models.BillingPlan): boolean {
const billingPlan = makeBillingPlan(billingPlanOrId);
+ if (!billingPlan) return false;
const nextTier = getNextTierBillingPlan(billingPlan.$id);
// defaults back to PRO, so adjust the check!
return billingPlan.$id !== nextTier.$id;
}
export function canDowngrade(billingPlanOrId: string | Models.BillingPlan): boolean {
const billingPlan = makeBillingPlan(billingPlanOrId);
+ if (!billingPlan) return false;
const nextTier = getPreviousTierBillingPlan(billingPlan.$id);
// defaults back to Starter, so adjust the check!
return billingPlan.$id !== nextTier.$id;
}🤖 Prompt for AI Agents
In `@src/lib/stores/billing.ts` around lines 81 - 121, Guard against a null
billing plan returned by billingIdToPlan (via makeBillingPlan) in canUpgrade and
canDowngrade: check that billingPlan is truthy before accessing billingPlan.$id
and return false (or appropriate safe value) if lookup failed; similarly ensure
planHasGroup handles a null billingPlan by returning false. Update canUpgrade to
call makeBillingPlan, if billingPlan is null return false, otherwise
getNextTierBillingPlan(billingPlan.$id) and compare IDs; do the analogous guard
in canDowngrade using getPreviousTierBillingPlan, and adjust planHasGroup to
safely return false when makeBillingPlan returns null.
| {#if !currentPlan.budgeting} | ||
| Get notified by email when your organization meets a percentage of your budget cap. <b | ||
| >{tierToPlan(organization.billingPlan).name} organizations will receive one notification | ||
| at 75% resource usage.</b> | ||
| >{organization.billingPlanDetails.name} organizations will receive one notification at 75% | ||
| resource usage.</b> |
There was a problem hiding this comment.
Guard billingPlanDetails for self-hosted contexts.
If this view can render outside cloud, organization.billingPlanDetails may be absent. Consider guarding with isCloud and falling back to currentPlan.name to avoid runtime crashes.
🔧 Suggested fix
- import { getChangePlanUrl } from '$lib/stores/billing';
+ import { getChangePlanUrl } from '$lib/stores/billing';
+ import { isCloud } from '$lib/system';
...
- Get notified by email when your organization meets a percentage of your budget cap. <b
- >{organization.billingPlanDetails.name} organizations will receive one notification at 75%
- resource usage.</b>
+ Get notified by email when your organization meets a percentage of your budget cap. <b
+ >{isCloud ? organization.billingPlanDetails.name : currentPlan.name} organizations will
+ receive one notification at 75% resource usage.</b>Based on learnings: billingPlanDetails is cloud-only in the Appwrite Console and should be guarded with isCloud or defensive access.
🤖 Prompt for AI Agents
In `@src/routes/`(console)/organization-[organization]/billing/budgetAlert.svelte
around lines 81 - 84, Guard access to organization.billingPlanDetails in the
budgetAlert Svelte view to avoid crashes in self-hosted contexts by checking
isCloud and providing a fallback: when isCloud is true use
organization.billingPlanDetails.name, otherwise use currentPlan.name (or a safe
default); update the template expression around the bold text to reference this
guarded value and ensure organization.billingPlanDetails is not accessed
directly when isCloud is false.
| secondary | ||
| fullWidth | ||
| href={$upgradeURL} | ||
| href={getChangePlanUrl($project.teamId)} |
There was a problem hiding this comment.
Guard $project before reading teamId.
If the store is temporarily undefined during navigation, $project.teamId will throw. Optional‑chain and let getChangePlanUrl fall back safely.
🛠️ Proposed fix
- href={getChangePlanUrl($project.teamId)}
+ href={getChangePlanUrl($project?.teamId)}🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/auth/templates/emailSignature.svelte
at line 58, Guard access to $project before reading teamId: change the href call
that currently passes $project.teamId to use optional chaining so
getChangePlanUrl receives undefined when $project is missing (e.g. call
getChangePlanUrl($project?.teamId) or compute a local teamId = $project?.teamId
and pass that). Update the template reference where
href={getChangePlanUrl($project.teamId)} so it reads
href={getChangePlanUrl($project?.teamId)} and ensure getChangePlanUrl safely
handles an undefined teamId.
| const desktopImg = isDark ? EmptyDark : EmptyLight; | ||
| const tabletImg = isDark ? EmptyDarkTablet : EmptyLightTablet; | ||
| const organizationId = page.data?.organization.$id ?? page.data?.project?.teamId; |
There was a problem hiding this comment.
Use optional chaining when reading page.data.organization.
page.data?.organization.$id will throw if organization is absent in the data payload (or during transitions). Use ?. to keep it safe.
🔧 Suggested fix
- const organizationId = page.data?.organization.$id ?? page.data?.project?.teamId;
+ const organizationId = page.data?.organization?.$id ?? page.data?.project?.teamId;📝 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.
| const organizationId = page.data?.organization.$id ?? page.data?.project?.teamId; | |
| const organizationId = page.data?.organization?.$id ?? page.data?.project?.teamId; |
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/backups/upgradeCard.svelte
at line 34, The expression that computes organizationId uses
page.data?.organization.$id which will throw if organization is undefined;
update the lookup to use optional chaining (page.data?.organization?.$id) so the
access is safe during transitions while keeping the existing fallback
(page.data?.project?.teamId) and ensure any other reads of
page.data.organization in the same component follow the same pattern.
| const currentPlanId = data?.currentInvoice?.plan ?? $organization?.billingPlanId; | ||
| const currentBillingPlan = billingIdToPlan(currentPlanId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the file
find . -name "+page.svelte" -path "*project-*settings/usage*" | head -5Repository: appwrite/console
Length of output: 149
🏁 Script executed:
# List files in the settings/usage directory to find the right file
fd "+page.svelte" settings/usageRepository: appwrite/console
Length of output: 161
🏁 Script executed:
# Search for the specific file pattern
find . -type f -path "*settings/usage*" -name "*.svelte" | grep -v node_modulesRepository: appwrite/console
Length of output: 149
🏁 Script executed:
cat -n "./src/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte" | head -100Repository: appwrite/console
Length of output: 5260
🏁 Script executed:
# Check the corresponding +page.ts loader
cat "./src/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.ts"Repository: appwrite/console
Length of output: 2753
🏁 Script executed:
# Search for how currentBillingPlan is used to understand impact
grep -n "currentBillingPlan\|currentPlanId" "./src/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte"Repository: appwrite/console
Length of output: 632
🏁 Script executed:
# Check if there are other similar const declarations in the codebase that might indicate a pattern
rg "const.*=.*\$organization" --type svelte | head -20Repository: appwrite/console
Length of output: 90
🏁 Script executed:
# Look at how billingIdToPlan is used elsewhere to understand if reactivity matters
rg "billingIdToPlan" --type svelte -B 2 -A 2 | head -40Repository: appwrite/console
Length of output: 90
🏁 Script executed:
# Search for similar patterns without specifying file type
rg "const.*\$organization" --include="*.svelte" | head -20Repository: appwrite/console
Length of output: 501
🏁 Script executed:
# Search for how billingIdToPlan is used elsewhere
rg "billingIdToPlan" --include="*.svelte" -B 2 -A 2 | head -50Repository: appwrite/console
Length of output: 501
🏁 Script executed:
# Search for const declarations with $organization pattern in svelte files
find . -name "*.svelte" -type f | xargs grep "const.*\$organization" 2>/dev/null | head -20Repository: appwrite/console
Length of output: 2261
🏁 Script executed:
# Search for how billingIdToPlan is used in other components
find . -name "*.svelte" -type f | xargs grep "billingIdToPlan" 2>/dev/null | head -30Repository: appwrite/console
Length of output: 2493
🏁 Script executed:
# Verify the pattern in the similar organization usage page
cat -n "./src/routes/(console)/organization-[organization]/usage/[[invoice]]/+page.svelte" | head -80Repository: appwrite/console
Length of output: 3582
🏁 Script executed:
# Verify how props update in Svelte - check if data prop would trigger const update
# Look for any instances where components re-render based on prop changes
grep -n "export let data" ./src/routes/\(console\)/project-[region]-[project]/settings/usage/\[\[invoice\]\]/+page.svelteRepository: appwrite/console
Length of output: 83
🏁 Script executed:
# Check the root layout to see how organization is provided
cat -n "./src/routes/(console)/+layout.ts" | head -50Repository: appwrite/console
Length of output: 2250
Use reactive declarations for plan derivation.
const currentPlanId and const currentBillingPlan won't update when $organization or data changes. This is inconsistent with the reactive pattern established throughout this file (lines 26-52 all use $:) and will cause stale values in the template conditions. Use $: declarations instead.
Suggested fix
- const currentPlanId = data?.currentInvoice?.plan ?? $organization?.billingPlanId;
- const currentBillingPlan = billingIdToPlan(currentPlanId);
+ $: currentPlanId = data?.currentInvoice?.plan ?? $organization?.billingPlanId;
+ $: currentBillingPlan = billingIdToPlan(currentPlanId);📝 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.
| const currentPlanId = data?.currentInvoice?.plan ?? $organization?.billingPlanId; | |
| const currentBillingPlan = billingIdToPlan(currentPlanId); | |
| $: currentPlanId = data?.currentInvoice?.plan ?? $organization?.billingPlanId; | |
| $: currentBillingPlan = billingIdToPlan(currentPlanId); |
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte
around lines 54 - 55, currentPlanId and currentBillingPlan are declared as plain
consts so they won't react to changes in data or $organization; change them to
reactive declarations using $: so they update automatically (e.g., replace the
consts for currentPlanId and currentBillingPlan with $: currentPlanId = ... and
$: currentBillingPlan = billingIdToPlan(currentPlanId)), ensuring you still
reference data?.currentInvoice?.plan and $organization?.billingPlanId and call
billingIdToPlan(currentPlanId) to derive the plan reactively.
| {@const isStarter = isStarterPlan($organization.billingPlanId)} | ||
| {#if isCloud && isStarter} | ||
| <Alert.Inline title="Customizing specs available with Pro or Scale plans"> | ||
| Upgrade to Pro or Scale to adjust your CPU and RAM beyond the default. | ||
| Upgrade your plan to adjust your CPU and RAM beyond the default. | ||
| <svelte:fragment slot="actions"> | ||
| <Button href={$upgradeURL} compact>Upgrade</Button> | ||
| <Button href={getChangePlanUrl($organization.$id)} compact>Upgrade</Button> | ||
| </svelte:fragment> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check organization store initialization and defaults
cat -n src/lib/stores/organization.ts | head -40Repository: appwrite/console
Length of output: 1679
🏁 Script executed:
#!/bin/bash
# Check the route layout hierarchy for this specific route
fd -t f "layout.ts" "src/routes/(console)/project-" | head -5Repository: appwrite/console
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Check how organization is accessed in similar billing/plan-related components
rg -l "billingPlanId" src/routes --type svelte | head -5Repository: appwrite/console
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Look for any organization initialization or guarding patterns in similar components
rg -B 2 -A 2 "isStarterPlan" src --type svelte | head -30Repository: appwrite/console
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Find layout files in the project routes
find src/routes -name "+layout.ts" | grep -E "console|project" | head -10Repository: appwrite/console
Length of output: 714
🏁 Script executed:
#!/bin/bash
# Search for organization loading in layout files
rg -A 5 "organization" src/routes/\(console\) --max-count=20 | head -40Repository: appwrite/console
Length of output: 3286
🏁 Script executed:
#!/bin/bash
# Check how billingPlanId is used across the codebase
rg "billingPlanId" src/routes -B 2 -A 2 | head -50Repository: appwrite/console
Length of output: 4589
🏁 Script executed:
#!/antml/bin/bash
# Look at the specific file to understand context better
cat -n src/routes/\(console\)/project-\[region\]-\[project\]/sites/site-\[site\]/settings/updateResourceLimits.svelte | grep -A 10 -B 10 "isStarter"Repository: appwrite/console
Length of output: 2292
🏁 Script executed:
#!/bin/bash
# Check the root console layout to see how organization is loaded
cat -n src/routes/\(console\)/+layout.ts | head -60Repository: appwrite/console
Length of output: 2538
🏁 Script executed:
#!/bin/bash
# Check the project layout file
cat -n src/routes/\(console\)/project-\[region\]-\[project\]/+layout.ts | head -60Repository: appwrite/console
Length of output: 2940
🏁 Script executed:
#!/bin/bash
# Check if there's a sites layout
find src/routes/\(console\)/project-\[region\]-\[project\] -name "+layout.ts"Repository: appwrite/console
Length of output: 2187
🏁 Script executed:
#!/bin/bash
# Let's verify if billingPlanId is always present on organization objects
rg "billingPlanId" src/lib --type ts -B 2 -A 2 | head -50Repository: appwrite/console
Length of output: 1454
🏁 Script executed:
#!/antml/bin/bash
# Check the Models.Organization type to see if billingPlanId is required
rg -A 20 "interface Organization" src --type ts | head -40Repository: appwrite/console
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check the complete project layout to understand organization loading flow
cat -n src/routes/\(console\)/project-\[region\]-\[project\]/+layout.ts | tail -40Repository: appwrite/console
Length of output: 1319
Use optional chaining to safely access billingPlanId on organization.
{@const isStarter = isStarterPlan($organization.billingPlanId)} should guard against missing billingPlanId to match the defensive pattern used throughout the codebase. The billing store explicitly checks for billingPlanId existence, indicating it's not guaranteed.
🔧 Suggested guard
- {`@const` isStarter = isStarterPlan($organization?.billingPlanId)}
+ {`@const` isStarter = $organization?.billingPlanId
+ ? isStarterPlan($organization.billingPlanId)
+ : false}🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/sites/site-[site]/settings/updateResourceLimits.svelte
around lines 88 - 94, Replace the direct access to billingPlanId when computing
isStarter so missing organization billing data is safe: update the isStarter
computation (symbol isStarter and function isStarterPlan) to pass the
organization billingPlanId using optional chaining (i.e., guard the access to
billingPlanId on $organization so undefined is passed to isStarterPlan when
absent) so the check matches the defensive pattern used elsewhere.
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
New Features
Bug Fixes & Improvements
Refactor
Removed
✏️ Tip: You can customize this high-level summary in your review settings.