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 JWT tokens let functions act on behalf of users while preserving their permissions |
WalkthroughThe PR migrates billing and organization code 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; adds utilities (billingIdToPlan, planHasGroup, getBasePlanFromGroup, getNextTierBillingPlan, makePlansMap, IconsMap); standardizes routing with resolve/route helpers; updates Stripe/account payment flows and confirmPayment signature; and converts many components, stores, routes, and props to use Models. types. 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. 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: 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.
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.
src/routes/(console)/organization-[organization]/billing/budgetAlert.svelte
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/auth/templates/emailSignature.svelte
Outdated
Show resolved
Hide resolved
...console)/project-[region]-[project]/databases/database-[database]/backups/upgradeCard.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte
Outdated
Show resolved
Hide resolved
.../(console)/project-[region]-[project]/sites/site-[site]/settings/updateResourceLimits.svelte
Show resolved
Hide resolved
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/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/containerHeader.svelte (1)
72-79: Grammar issue: "policy" should be pluralized conditionally.Line 73 uses singular "policy" regardless of
maxPoliciesvalue. WhenmaxPolicies > 1, the text should say "policies".Proposed fix
<slot name="tooltip"> <span> - You are limited to {maxPolicies} policy on your plan. + You are limited to {maxPolicies} {maxPolicies === 1 ? 'policy' : 'policies'} on your plan. <button class="u-underline" on:click={() => { showDropdown = !showDropdown; goto(getChangePlanUrl(project.teamId)); }}>Upgrade your plan</button> to add customized backup policies. </span> </slot>src/routes/(console)/project-[region]-[project]/auth/templates/emailSessionAlertTemplate.svelte (1)
47-54: Typo in template variable values.Lines 52-53 have malformed template variables with missing closing braces:
value={'{{ipAddress'}→ should bevalue={'{{ipAddress}}'}value={'{{country'}→ should bevalue={'{{country}}'}This causes the copied value to be incomplete when users click to copy.
🐛 Proposed fix
<Id value={'{{device}}'}>{'{{device}}'}</Id> - <Id value={'{{ipAddress'}>{'{{ipAddress}}'}</Id> - <Id value={'{{country'}>{'{{country}}'}</Id> + <Id value={'{{ipAddress}}'}>{'{{ipAddress}}'}</Id> + <Id value={'{{country}}'}>{'{{country}}'}</Id>
🤖 Fix all issues with AI agents
In `@src/routes/`(console)/project-[region]-[project]/settings/smtp/+page.svelte:
- Around line 45-60: The template accesses billing-related properties on
$currentPlan (e.g., currentPlan.customSmtp) without guarding for undefined which
can crash in self-hosted or loading states; update the derived/computed logic
(e.g., isButtonDisabled) and any places referencing currentPlan.customSmtp to
use optional chaining and safe defaults (for example currentPlan?.customSmtp ??
false) or guard with an isCloud check, and consider exposing a small derived
boolean like hasCustomSmtp for reuse in the template to keep checks consistent
and avoid runtime errors.
- Line 136: The getChangePlanUrl function currently declares a parameter named
organizationId but callers (e.g., href={getChangePlanUrl(project.teamId)} in
+page.svelte) pass project.teamId, causing a semantic mismatch; either rename
the function parameter organizationId to teamId in getChangePlanUrl (and update
its internal references) to reflect the actual value being passed, or, if
project.teamId truly represents an organization ID, update all callers to pass a
properly named organization ID and adjust the function's docs/comments; update
the signature in src/lib/stores/billing.ts (getChangePlanUrl) and every call
site (including the +page.svelte usage) to keep the parameter name consistent
and clear.
🧹 Nitpick comments (14)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/+layout.svelte (1)
48-170: Consider extracting the repeated base URL pattern to reduce duplication.The URL pattern
${base}/project-${page.params.region}-${page.params.project}/functions/function-${$func.$id}is repeated 10 times. Extracting it into a derived variable would improve maintainability and reduce the chance of inconsistencies.♻️ Suggested refactor
+ $: functionBase = `${base}/project-${page.params.region}-${page.params.project}/functions/function-${$func.$id}`; + $: $registerCommands([ { label: 'Create deployment', async callback() { if (!page.url.pathname.endsWith($func.$id)) { await goto( - `${base}/project-${page.params.region}-${page.params.project}/functions/function-${$func.$id}` + functionBase ); } showCreateDeployment.set(true); }, // ... and so on for other commands },Then update subsequent usages:
functionBasefor deployments`${functionBase}/settings#permissions`for permissions`${functionBase}/settings#events`for events`${functionBase}/settings#variables`for variables`${functionBase}/settings#timeout`for timeout`${functionBase}/settings#schedule`for schedule`${functionBase}/usage`for usage`${functionBase}/executions`for executions`${functionBase}/settings`for settingssrc/routes/(console)/project-[region]-[project]/databases/+layout.svelte (1)
39-41: Consider consolidating effects.This effect has no reactive dependencies in its body, meaning it will re-run on every state change. If the intent is to set ranks once when the layout mounts, this could be combined with the command registration effect above to reduce redundant executions.
♻️ Optional: Consolidate into single effect
$effect(() => { $registerCommands([ { label: 'Go to usage', callback: () => { goto(withPath(databasesProjectRoute, 'usage')); }, keys: ['g', 'u'], disabled: page.url.pathname.includes('usage') || page.url.pathname.includes('database-'), group: 'databases' }, { label: 'Find databases', callback: () => { addSubPanel(DatabasesPanel); }, group: 'databases', rank: -1 } ]); + $updateCommandGroupRanks({ databases: 200, navigation: 100 }); }); - - $effect(() => { - $updateCommandGroupRanks({ databases: 200, navigation: 100 }); - });src/routes/(console)/project-[region]-[project]/auth/user-[user]/deleteUser.svelte (2)
25-25: Minor inconsistency: fallback pattern differs from template.Line 25 uses a ternary (
user.name ? user.name : 'User') while line 42 uses the OR operator (user.name || 'User'). Consider using the same pattern for consistency.🔧 Suggested fix
- message: `${user.name ? user.name : 'User'} has been deleted` + message: `${user.name || 'User'} has been deleted`
33-36: Consider adding type narrowing for the caught exception.The caught exception
eis of typeunknown. Accessinge.messagedirectly may cause TypeScript errors in strict mode.🔧 Suggested fix
} catch (e) { - error = e.message; + error = e instanceof Error ? e.message : 'An unknown error occurred'; trackError(e, Submit.UserDelete); }src/routes/(console)/project-[region]-[project]/auth/breadcrumbs.svelte (1)
7-20: Inconsistent Svelte version syntax: use$derivedinstead of$:.The file imports
pagefrom$app/state(Svelte 5) but uses$:reactive statement (Svelte 4). For consistency with Svelte 5 patterns used elsewhere in this PR, consider using$derived.♻️ Suggested refactor
- $: breadcrumbs = [ + const breadcrumbs = $derived([ { href: `${base}/organization-${$organization?.$id}`, title: $organization?.name }, { href: `${base}/project-${page.params.region}-${page.params.project}`, title: page.data?.project?.name }, { href: `${base}/project-${page.params.region}-${page.params.project}/auth`, title: 'Auth' } - ]; + ]);As per coding guidelines: "Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development".
src/routes/(console)/project-[region]-[project]/settings/webhooks/+page.svelte (1)
106-113: Consider migrating remaining URLs to useresolve()for consistency.The Empty component's
on:clickhandler (line 111-112) still uses template literal URL construction while the newwebhooksCreateUrlusesresolve(). Same applies to line 66. Consider migrating these for consistency with the new pattern.♻️ Suggested refactor
{:else} <Empty single href="https://appwrite.io/docs/advanced/platform/webhooks" target="webhook" - on:click={() => - goto( - `${base}/project-${page.params.region}-${page.params.project}/settings/webhooks/create` - )} /> + on:click={() => goto(webhooksCreateUrl)} /> {/if}src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/upgradeCard.svelte (1)
83-95: Deduplicate upgrade URL computation.The same
hreflogic is repeated for mobile and desktop buttons. Consider computing it once in the script and reusing it.♻️ Proposed refactor
const themeClass = isDark ? 'u-only-dark' : 'u-only-light'; const mobileImg = isDark ? EmptyDarkMobile : EmptyLightMobile; const desktopImg = isDark ? EmptyDark : EmptyLight; const tabletImg = isDark ? EmptyDarkTablet : EmptyLightTablet; +const upgradeHref = isCloud + ? getChangePlanUrl(project.teamId) + : 'https://cloud.appwrite.io/register';- <Button - external={!isCloud} - href={isCloud - ? getChangePlanUrl(project.teamId) - : 'https://cloud.appwrite.io/register'}> + <Button + external={!isCloud} + href={upgradeHref}> {isCloud ? 'Upgrade plan' : 'Sign up'} </Button>- <Button - external={!isCloud} - href={isCloud - ? getChangePlanUrl(project.teamId) - : 'https://cloud.appwrite.io/register'}> + <Button + external={!isCloud} + href={upgradeHref}> {isCloud ? 'Upgrade plan' : 'Sign up'} </Button>src/routes/(console)/project-[region]-[project]/auth/security/updateSessionsLimit.svelte (1)
22-25: Minor inconsistency in optional chaining usage.Line 23 accesses
project.$idwithout optional chaining, while lines 18 and 58 useproject?.authSessionsLimit. Sinceprojectis passed from the page'sdata.project, it should always be defined, so the optional chaining on lines 18/58 is defensive but unnecessary. Consider using consistent access patterns throughout.src/routes/(console)/project-[region]-[project]/auth/security/updateSessionLength.svelte (1)
19-20: Inconsistent optional chaining between derived values and button disabled check.Line 19 uses
project?.authDurationbut line 55 usesproject.authDurationwithout optional chaining. Consider making this consistent - either guard all accesses or none (since the prop should always be defined when coming from page data).src/routes/(console)/project-[region]-[project]/auth/security/updateMockNumbers.svelte (1)
27-30: Minor inconsistency in optional chaining.Line 27 uses
project?.authMockNumbers, line 28 usesproject?.$id, but line 30 accessesproject.authMockNumbersdirectly without optional chaining. Consider making this consistent to avoid potential runtime errors ifprojectis unexpectedly undefined.♻️ Suggested consistency fix
- const initialNumbers = $derived(project.authMockNumbers?.map((n) => ({ ...n })) ?? []); + const initialNumbers = $derived(project?.authMockNumbers?.map((n) => ({ ...n })) ?? []);src/routes/(console)/project-[region]-[project]/auth/settings/+page.svelte (2)
40-46: Consider adding error type guard.The caught
erroris typedunknown, buterror.messageis accessed directly. This could fail if a non-Error value is thrown.Suggested improvement
} catch (error) { box.value = !box.value; addNotification({ type: 'error', - message: error.message + message: error instanceof Error ? error.message : 'An error occurred' }); trackError(error, Submit.AuthStatusUpdate); }
18-19: Consider using$derivedfor explicit data dependency.While the current pattern works correctly (SvelteKit re-renders the component when props change, reassigning the const), using
$derivedmakes the dependency relationship more explicit and aligns with Svelte 5 best practices:let { data }: PageProps = $props(); -const { project } = data; +const project = $derived(data.project);This clarifies that
projectis a reactive derivation ofdata.project, which is especially useful since the$effecton line 50 depends on this value.src/routes/(console)/project-[region]-[project]/auth/templates/localeOptions.svelte (1)
6-11: Type and optional chaining inconsistency.The
localeCodesprop is typed asModels.LocaleCode[](non-optional), but line 8 uses optional chaining (localeCodes?.map). This is inconsistent:
- If
localeCodesis always provided, remove the optional chaining.- If it can be undefined, either add a default value (
= []) or update the type toModels.LocaleCode[] | undefined.Additionally,
optionsis computed once at component initialization. IflocaleCodeschanges after mount,optionswon't update. Consider using$derivedfor reactivity.♻️ Suggested fix using $derived
export let value = 'en'; export let localeCodes: Models.LocaleCode[]; -const options = localeCodes?.map((code) => ({ +const options = $derived(localeCodes?.map((code) => ({ label: code.name, value: code.code -})); +})) ?? []);src/routes/(console)/project-[region]-[project]/settings/smtp/+page.svelte (1)
62-75: Avoid clearing SMTP fields on toggle; it drops state before submit.Clearing values whenever
enabledis false makes toggling back on in the same session require re-entering all fields. If the goal is to avoid sending SMTP data when disabled, gate the payload byenabledinstead.♻️ Suggested refactor
async function updateSmtp() { try { await sdk.forConsole.projects.updateSMTP({ projectId: project.$id, enabled, - senderName: senderName || undefined, - senderEmail: senderEmail || undefined, - replyTo: replyTo || undefined, - host: host || undefined, - port: port || undefined, - username: username || undefined, - password: password || undefined, - secure: secure ? (secure as SMTPSecure) : undefined + senderName: enabled ? senderName || undefined : undefined, + senderEmail: enabled ? senderEmail || undefined : undefined, + replyTo: enabled ? replyTo || undefined : undefined, + host: enabled ? host || undefined : undefined, + port: enabled ? port || undefined : undefined, + username: enabled ? username || undefined : undefined, + password: enabled ? password || undefined : undefined, + secure: enabled && secure ? (secure as SMTPSecure) : undefined });- $effect(() => { - if (!enabled) { - senderName = undefined; - senderEmail = undefined; - replyTo = undefined; - host = undefined; - port = undefined; - username = undefined; - password = undefined; - secure = undefined; - } - });Also applies to: 104-115
src/routes/(console)/project-[region]-[project]/settings/smtp/+page.svelte
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: 2
🤖 Fix all issues with AI agents
In `@src/routes/`(console)/project-[region]-[project]/functions/+page.svelte:
- Around line 51-64: The code uses templateConfig directly in the query which
can be null or contain unsafe characters; update the navigation logic around
template/templateConfig: read templateConfig from
page.url.searchParams.get('templateConfig'), and when building the URL for
goto(resolveRoute(...)) use encodeURIComponent(templateConfig) and only append
`?templateConfig=...` when templateConfig is truthy (non-null/non-empty); adjust
the call that creates templateUrl/withPath so it passes the encoded query string
conditionally and leave goto(createFunctionsUrl) unchanged.
- Around line 36-43: createFunctionsUrl is initialized once and becomes stale
when navigating between projects; make it reactive by deriving it from the page
params. Replace the constant init using resolveRoute with a derived store that
depends on page (or page.params) — e.g., use the existing $derived helper or
svelte's derived(page, p => ...) to compute
resolveRoute('/(console)/project-[region]-[project]/functions/create-function',
{ ...p.params }) so createFunctionsUrl updates whenever page.params changes.
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/overview/platforms/action.svelte (1)
36-42: Remove redundant$canWritePlatformscheck.The
{#if$canWritePlatforms}guard on line 37 is redundant since this entire block is already inside the{#if$canWritePlatforms}check from line 21.Suggested fix
{:else} <Popover let:toggle padding="none" placement="bottom-end"> - {`#if` $canWritePlatforms} - <Button on:click={toggle}> - <Icon icon={IconPlus} slot="start" /> - <span class="text">Add platform</span> - </Button> - {/if} + <Button on:click={toggle}> + <Icon icon={IconPlus} slot="start" /> + <span class="text">Add platform</span> + </Button> <div style:min-width="232px" slot="tooltip">src/routes/(console)/project-[region]-[project]/updateVariables.svelte (1)
234-240: Inconsistent route resolution pattern.This
gotocall still uses string interpolation while lines 278-283 and 333-338 now useresolveRoute. Consider aligning for consistency.♻️ Suggested fix
buttons: [ { method: () => goto( - `${base}/project-${page.params.region}-${page.params.project}/settings` + resolveRoute('/(console)/project-[region]-[project]/settings', { + ...page.params + }) ), name: 'Go to settings' } ]
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.