Feat: New Billing UI changes #2249
Conversation
…project-changes-2
…project-changes-2
ConsoleProject ID: Sites (2)
Note Cursor pagination performs better than offset pagination when loading further pages. |
…rojects-ui-change
…/github.com/appwrite/console into feat-SER-204-New-Archive-projects-ui-change
| organization: Models.Organization; | ||
| currentPlan: { | ||
| projects: number; | ||
| [key: string]: any; |
There was a problem hiding this comment.
don't use any, specify the type.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
src/lib/components/archiveProject.svelte (2)
244-249: Off-by-one: show “+N” when there are more than two platformsYou render two badges; the overflow badge should appear when length > 2.
- {#if platforms.length > 3} + {#if platforms.length > 2} <Badge variant="secondary" content={`+${platforms.length - 2}`} style="width: max-content;" /> {/if}
165-168: Runtime guard: project.platforms may be undefinedCalling .map() on possibly undefined will throw. Use a safe default before mapping.
- {@const platforms = filterPlatforms( - project.platforms.map((platform) => getPlatformInfo(platform.type)) - )} + {@const platforms = filterPlatforms( + (project.platforms ?? []).map((platform) => getPlatformInfo(platform.type)) + )}src/lib/components/organizationUsageLimits.svelte (6)
88-91: Analytics event name doesn’t match actionUsing OrganizationClickUpgrade for “Manage projects” will skew metrics. Switch to a dedicated “Manage projects” event.
- trackEvent(Click.OrganizationClickUpgrade, { source: 'usage_limits_manage_projects' }); + trackEvent(Click.OrganizationClickManageProjects, { source: 'usage_limits_manage_projects' });If the new constant doesn’t exist, add it to your analytics enum/definitions.
213-214: Pluralize “member(s)”Render correct singular/plural.
- <Typography.Text>{formatNumber(freePlanLimits.members)} member</Typography.Text> + <Typography.Text> + {formatNumber(freePlanLimits.members)} {freePlanLimits.members === 1 ? 'member' : 'members'} + </Typography.Text>
262-266: Hard-coded “two projects” → use dynamic limitReflect allowedProjectsToKeep and handle plural.
- <svelte:fragment slot="description"> - Choose which two projects to keep. Projects over the limit will be blocked after this date. - </svelte:fragment> + <svelte:fragment slot="description"> + Choose which {allowedProjectsToKeep} project{allowedProjectsToKeep === 1 ? '' : 's'} to keep. + Projects over the limit will be blocked after this date. + </svelte:fragment>
3-3: Free projects limit can be undefined → broken gatingUse getServiceLimit for projects (like members/storage). Depending on plansInfo at runtime risks undefined, leading to NaN displays and disabled Save forever.
-import { getServiceLimit, plansInfo } from '$lib/stores/billing'; +import { getServiceLimit } from '$lib/stores/billing'; @@ - let freePlanLimits = $derived({ - projects: $plansInfo?.get(BillingPlan.FREE)?.projects, - members: getServiceLimit('members', BillingPlan.FREE), - storage: getServiceLimit('storage', BillingPlan.FREE) - }); + let freePlanLimits = $derived({ + projects: getServiceLimit('projects', BillingPlan.FREE), + members: getServiceLimit('members', BillingPlan.FREE), + storage: getServiceLimit('storage', BillingPlan.FREE) + });Also applies to: 33-41
57-61: Excess projects miscalculatedYou’re not subtracting the free limit; UI overstates “Excess usage”.
- let excessUsage = $derived({ - projects: Math.max(0, currentUsage.projects), - members: Math.max(0, currentUsage.members - freePlanLimits.members), - storage: Math.max(0, storageUsageGB - freePlanLimits.storage) - }); + let excessUsage = $derived({ + projects: Math.max(0, currentUsage.projects - freePlanLimits.projects), + members: Math.max(0, currentUsage.members - freePlanLimits.members), + storage: Math.max(0, storageUsageGB - freePlanLimits.storage) + });
292-299: Unwrap Svelte store before formatting datebillingProjectsLimitDate is a store; pass $billingProjectsLimitDate to toLocaleDate.
- <Alert.Inline - status="warning" - title={`${projects.length - selectedProjects.length} projects will be archived on ${toLocaleDate(billingProjectsLimitDate)}`}> + <Alert.Inline + status="warning" + title={`${projects.length - selectedProjects.length} projects will be archived on ${toLocaleDate($billingProjectsLimitDate)}`}>
🧹 Nitpick comments (5)
src/lib/components/archiveProject.svelte (3)
130-136: Type-safe error handling in catchPrefer narrowing unknown and using instanceof Error to satisfy lint and clarity.
- } catch (error) { - const msg = - error && typeof error === 'object' && 'message' in error - ? String((error as { message: string }).message) - : 'Failed to unarchive project'; + } catch (error: unknown) { + const msg = + error instanceof Error + ? error.message + : (error && typeof error === 'object' && 'message' in error + ? String((error as { message?: unknown }).message) + : 'Failed to unarchive project'); addNotification({ type: 'error', message: msg }); }
54-56: Prevent double submit: add loading state to Unarchive flowDisable the Unarchive button while the request is in flight.
@@ - let showUnarchiveModal = $state(false); + let showUnarchiveModal = $state(false); + let unarchiving = $state(false); @@ - async function confirmUnarchive() { + async function confirmUnarchive() { if (!projectToUnarchive) return; - try { + try { + unarchiving = true; if (!organization) { addNotification({ type: 'error', message: 'Organization not found' }); return; } await sdk.forConsole.projects.updateStatus(projectToUnarchive.$id, Status.Active); await invalidate(Dependencies.ORGANIZATION); addNotification({ type: 'success', message: `${projectToUnarchive.name} has been unarchived` }); showUnarchiveModal = false; projectToUnarchive = null; } catch (error) { @@ - addNotification({ type: 'error', message: msg }); + addNotification({ type: 'error', message: msg }); } + finally { + unarchiving = false; + } } @@ - <Button on:click={confirmUnarchive}>Unarchive</Button> + <Button on:click={confirmUnarchive} disabled={unarchiving} aria-busy={unarchiving}> + Unarchive + </Button>Also applies to: 106-137, 270-274
176-205: Inline styles → utility classesMove inline width/whitespace/padding styles into CSS classes for consistency with the design system.
Example:
- style="white-space: nowrap;" on Tag
- style="width: max-content;" on Badge
- inline padding on DropList item
Also applies to: 234-242, 245-249
src/lib/components/organizationUsageLimits.svelte (2)
174-191: Defensive render: allowedProjectsToKeepIf you keep the current derivation, ensure the UI doesn’t show NaN when limits are not yet loaded.
- <Typography.Text - >{formatNumber(allowedProjectsToKeep)} projects</Typography.Text> + <Typography.Text> + {formatNumber(allowedProjectsToKeep ?? 0)} project{(allowedProjectsToKeep ?? 0) === 1 ? '' : 's'} + </Typography.Text>
306-347: z-index/pointer-events CSSIf these overrides are to work around Table layering, annotate the reason in a comment and scope more tightly (e.g., only when an alert is visible) to avoid future stacking issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/lib/components/archiveProject.svelte(1 hunks)src/lib/components/organizationUsageLimits.svelte(1 hunks)src/routes/(console)/organization-[organization]/+page.svelte(6 hunks)src/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(console)/organization-[organization]/+page.svelte
- src/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (14)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (14)
44-51: Fix 1000-based conversion in bandwidth formatter (use 1024³ and humanize max).Decimal bytes skew progress/labels; align with humanFileSize.
- const maxSize = humanFileSize(maxGB * 1000 * 1000 * 1000); - return `${currentSize.value} ${currentSize.unit} / ${maxSize.value} ${maxSize.unit}`; + const maxBytes = maxGB * 1024 * 1024 * 1024; + const maxSize = humanFileSize(maxBytes); + return `${currentSize.value} ${currentSize.unit} / ${maxSize.value} ${maxSize.unit}`;
58-86: Broaden createProgressData signature to accept undefined/null and guard currentValue.Call sites pass possibly-undefined limits; TS will complain and runtime may misbehave.
- function createProgressData( - currentValue: number, - maxValue: number | string - ): Array<{ size: number; color: string; tooltip?: { title: string; label: string } }> { + function createProgressData( + currentValue: number | null | undefined, + maxValue: number | string | null | undefined + ): Array<{ size: number; color: string; tooltip?: { title: string; label: string } }> { + if (currentValue === null || currentValue === undefined) return []; if ( maxValue === null || maxValue === undefined || (typeof maxValue === 'number' && maxValue <= 0) ) { return []; }
88-110: Storage progress: use 1024³ and consistent tooltip units.Prevents inaccurate percentages/labels.
- const maxBytes = maxGB * 1000 * 1000 * 1000; + const maxBytes = maxGB * 1024 * 1024 * 1024; const percentage = Math.min((currentBytes / maxBytes) * 100, 100); const progressColor = 'var(--bgcolor-neutral-invert)'; - const currentSize = humanFileSize(currentBytes); + const currentSize = humanFileSize(currentBytes); + const maxSize = humanFileSize(maxBytes); @@ - title: `${percentage.toFixed(0)}% used`, - label: `${currentSize.value} ${currentSize.unit} of ${maxGB} GB` + title: `${percentage.toFixed(0)}% used`, + label: `${currentSize.value} ${currentSize.unit} of ${maxSize.value} ${maxSize.unit}`
190-203: Null-safe resource access + 1024³ bytes and unlimited handling for bandwidth.Direct
.value/.amountderef can crash; also fix unit conversion and avoid 0 sentinel.- usage: `${formatBandwidthUsage(project.bandwidth.value, currentPlan?.bandwidth)}`, - price: formatCurrency(project.bandwidth.amount || 0) + usage: `${formatBandwidthUsage(project.bandwidth?.value ?? 0, currentPlan?.bandwidth)}`, + price: formatCurrency(project.bandwidth?.amount ?? 0) @@ - progressData: createStorageProgressData( - project.bandwidth.value || 0, - currentPlan?.bandwidth || 0 - ), - maxValue: currentPlan?.bandwidth - ? currentPlan.bandwidth * 1000 * 1000 * 1000 - : 0 + progressData: createStorageProgressData( + project.bandwidth?.value ?? 0, + currentPlan?.bandwidth ?? 0 + ), + maxValue: currentPlan?.bandwidth + ? currentPlan.bandwidth * 1024 * 1024 * 1024 + : nullApply the same null-safe pattern to other resources below (users/reads/writes/executions/storage/GB-hours/SMS).
205-213: Null-safe users row + permissive progress signature usage.- usage: `${formatNum(project.users.value || 0)} / ${currentPlan?.users ? formatNum(currentPlan.users) : 'Unlimited'}`, - price: formatCurrency(project.users.amount || 0) + usage: `${formatNum(project.users?.value ?? 0)} / ${currentPlan?.users ? formatNum(currentPlan.users) : 'Unlimited'}`, + price: formatCurrency(project.users?.amount ?? 0) @@ - progressData: createProgressData(project.users.value || 0, currentPlan?.users), + progressData: createProgressData(project.users?.value ?? 0, currentPlan?.users),
215-226: Null-safe database reads row.- usage: `${formatNum(project.databasesReads.value || 0)} / ${currentPlan?.databasesReads ? formatNum(currentPlan.databasesReads) : 'Unlimited'}`, - price: formatCurrency(project.databasesReads.amount || 0) + usage: `${formatNum(project.databasesReads?.value ?? 0)} / ${currentPlan?.databasesReads ? formatNum(currentPlan.databasesReads) : 'Unlimited'}`, + price: formatCurrency(project.databasesReads?.amount ?? 0) @@ - progressData: createProgressData( - project.databasesReads.value || 0, + progressData: createProgressData( + project.databasesReads?.value ?? 0, currentPlan?.databasesReads ),
228-239: Null-safe database writes row.- usage: `${formatNum(project.databasesWrites.value || 0)} / ${currentPlan?.databasesWrites ? formatNum(currentPlan.databasesWrites) : 'Unlimited'}`, - price: formatCurrency(project.databasesWrites.amount || 0) + usage: `${formatNum(project.databasesWrites?.value ?? 0)} / ${currentPlan?.databasesWrites ? formatNum(currentPlan.databasesWrites) : 'Unlimited'}`, + price: formatCurrency(project.databasesWrites?.amount ?? 0) @@ - progressData: createProgressData( - project.databasesWrites.value || 0, + progressData: createProgressData( + project.databasesWrites?.value ?? 0, currentPlan?.databasesWrites ),
241-252: Null-safe executions row.- usage: `${formatNum(project.executions.value || 0)} / ${currentPlan?.executions ? formatNum(currentPlan.executions) : 'Unlimited'}`, - price: formatCurrency(project.executions.amount || 0) + usage: `${formatNum(project.executions?.value ?? 0)} / ${currentPlan?.executions ? formatNum(currentPlan.executions) : 'Unlimited'}`, + price: formatCurrency(project.executions?.amount ?? 0) @@ - progressData: createProgressData( - project.executions.value || 0, + progressData: createProgressData( + project.executions?.value ?? 0, currentPlan?.executions ),
254-265: Storage row: null-safe, “Unlimited” label, 1024³, and skip progress when unlimited.- usage: `${formatHumanSize(project.storage.value || 0)} / ${currentPlan?.storage?.toString() || '0'} GB`, - price: formatCurrency(project.storage.amount || 0) + usage: `${formatHumanSize(project.storage?.value ?? 0)} / ${currentPlan?.storage ? `${currentPlan.storage} GB` : 'Unlimited'}`, + price: formatCurrency(project.storage?.amount ?? 0) @@ - progressData: createStorageProgressData( - project.storage.value || 0, - currentPlan?.storage || 0 - ), - maxValue: currentPlan?.storage ? currentPlan.storage * 1000 * 1000 * 1000 : 0 + progressData: currentPlan?.storage + ? createStorageProgressData(project.storage?.value ?? 0, currentPlan.storage) + : [], + maxValue: currentPlan?.storage ? currentPlan.storage * 1024 * 1024 * 1024 : null
267-277: GB-hours row: null-safe access.- usage: `${formatNum(project.gbHours.value || 0)} / ${currentPlan?.GBHours ? formatNum(currentPlan.GBHours) : 'Unlimited'}`, - price: formatCurrency(project.gbHours.amount || 0) + usage: `${formatNum(project.gbHours?.value ?? 0)} / ${currentPlan?.GBHours ? formatNum(currentPlan.GBHours) : 'Unlimited'}`, + price: formatCurrency(project.gbHours?.amount ?? 0) @@ - ? createProgressData(project.gbHours.value || 0, currentPlan.GBHours) + ? createProgressData(project.gbHours?.value ?? 0, currentPlan.GBHours)
279-285: SMS row: null-safe access.- usage: `${formatNum(project.authPhone.value || 0)} SMS messages`, - price: formatCurrency(project.authPhone.amount || 0) + usage: `${formatNum(project.authPhone?.value ?? 0)} SMS messages`, + price: formatCurrency(project.authPhone?.amount ?? 0)
287-294: Remove {@html} link injection; render anchors safely (XSS risk).Stop parsing HTML strings; pass href and render .
- { - id: `project-${project.projectId}-usage-details`, - cells: { - item: `<a href="/console/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`, - usage: '', - price: '' - } - } + { + id: `project-${project.projectId}-usage-details`, + cells: { item: 'Usage details', usage: '', price: '' }, + href: `${base}/project-${String(project.region ?? 'default')}-${project.projectId}/settings/usage` + }- {#if child.cells?.[col.id]?.includes('<a href=')} - {@html child.cells?.[col.id] ?? ''} + {#if col.id === 'item' && child.href} + <a href={child.href} style="text-decoration: underline; color: var(--fgcolor-accent-neutral);"> + Usage details + </a>Also applies to: 381-393
302-311: Prevent NaN when credits are undefined.Subtraction/min with undefined results in NaN; default to 0.
- $: totalAmount = Math.max( - (currentAggregation?.amount || currentPlan?.price || 0) - availableCredit, - 0 - ); + $: totalAmount = Math.max( + (currentAggregation?.amount || currentPlan?.price || 0) - (availableCredit ?? 0), + 0 + ); @@ - $: creditsApplied = Math.min( - currentAggregation?.amount || currentPlan?.price || 0, - availableCredit - ); + $: creditsApplied = Math.min( + currentAggregation?.amount || currentPlan?.price || 0, + availableCredit ?? 0 + );
441-480: Duplicate row id “total-row”. Use unique IDs.Duplicate IDs break ARIA/state; rename credits row.
- <ExpandableTable.Row {root} id="total-row" expandable={false}> + <ExpandableTable.Row {root} id="credits-row" expandable={false}>Also applies to: 482-512
🧹 Nitpick comments (2)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (2)
32-37: Confirm use of ‘fr’ widths for this table type.Earlier feedback suggested consistency with other tables; align with library defaults if needed.
329-335: Style nit: use toLocaleDate helper for both dates (consistency).- Current billing cycle ({new Date( - $organization?.billingCurrentInvoiceDate - ).toLocaleDateString('en', { day: 'numeric', month: 'short' })}-{new Date( - $organization?.billingNextInvoiceDate - ).toLocaleDateString('en', { day: 'numeric', month: 'short' })}) + Current billing cycle ({toLocaleDate($organization?.billingCurrentInvoiceDate, 'dd MMM')}-{toLocaleDate($organization?.billingNextInvoiceDate, 'dd MMM')})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
…s-ui-change' into feat-SER-204-New-Archive-projects-ui-change
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/routes/(console)/organization-[organization]/change-plan/+page.svelte (3)
65-65: Paginate projects; 1000 limit will truncate large orgsImplement offset-based pagination and keep total for downstream logic.
- let allProjects: { projects: Models.Project[] } | undefined; + let allProjects: { total: number; projects: Models.Project[] } | undefined;- try { - allProjects = await sdk.forConsole.projects.list([ - Query.equal('teamId', data.organization.$id), - Query.limit(1000) - ]); - } catch { - allProjects = { projects: [] }; - } + try { + const pageSize = 1000; + let offset = 0; + let acc: Models.Project[] = []; + let total = 0; + let guard = 0, MAX_PAGES = 100; // safety + do { + const pageResp = await sdk.forConsole.projects.list([ + Query.equal('teamId', data.organization.$id), + Query.limit(pageSize), + Query.offset(offset) + ]); + total = pageResp.total ?? pageResp.projects.length; + acc = acc.concat(pageResp.projects); + offset += pageResp.projects.length; + guard++; + } while (offset < total && guard < MAX_PAGES); + allProjects = { total, projects: acc }; + } catch { + allProjects = { total: 0, projects: [] }; + }Also applies to: 109-116
357-372: Extra members cost is wrong and label is hard-codedUses invited emails instead of actual member count, hard-codes “Pro”, and shows even when price is 0. Compute true extras and render dynamically; only show when charge > 0.
- {@const extraMembers = collaborators?.length ?? 0} - {@const price = formatCurrency( - extraMembers * - ($plansInfo?.get(selectedPlan)?.addons?.seats?.price ?? 0) - )} - {#if selectedPlan === BillingPlan.PRO} + {@const includedSeats = $plansInfo?.get(selectedPlan)?.addons?.seats?.included ?? 0} + {@const seatPrice = $plansInfo?.get(selectedPlan)?.addons?.seats?.price ?? 0} + {@const membersCount = data.members?.memberships?.length ?? 0} + {@const extraMembers = Math.max(0, membersCount - includedSeats)} + {@const price = formatCurrency(extraMembers * seatPrice)} + {#if extraMembers > 0 && seatPrice > 0} <Alert.Inline status="error"> <svelte:fragment slot="title"> - Your monthly payments will be adjusted for the Pro plan + Your monthly payments will be adjusted for the {tierToPlan(selectedPlan)?.name} plan </svelte:fragment> After switching plans, <b - >you will be charged {price} monthly for {extraMembers} team members.</b> + >you will be charged {price} monthly for {extraMembers} {extraMembers === 1 ? 'team member' : 'team members'}.</b> This will be reflected in your next invoice. </Alert.Inline> - {:else if selectedPlan === BillingPlan.FREE} + {:else if selectedPlan === BillingPlan.FREE}
64-66: Fix type: orgUsage can be undefinedYou assign undefined in the catch; make the type optional to satisfy TS.
- let orgUsage: OrganizationUsage; + let orgUsage: OrganizationUsage | undefined;
🧹 Nitpick comments (3)
src/routes/(console)/organization-[organization]/change-plan/+page.svelte (3)
49-51: Name the component ref type for reuseMinor readability: extract the inline structural type to a named alias.
- let usageLimitsComponent: - | { validateOrAlert: () => boolean; getSelectedProjects: () => string[] } - | undefined; + type UsageLimitsRef = { validateOrAlert: () => boolean; getSelectedProjects: () => string[] }; + let usageLimitsComponent: UsageLimitsRef | undefined;
373-383: FREE downgrade alert: guard missing date and severity levelFallback when billingNextInvoiceDate is absent and use “warning” severity instead of “error”.
- <Alert.Inline - status="error" - title={`Your organization will switch to ${tierToPlan(selectedPlan).name} plan on ${toLocaleDate( - $organization.billingNextInvoiceDate - )}`}> + <Alert.Inline + status="warning" + title={`Your organization will switch to ${tierToPlan(selectedPlan).name} plan on ${ + $organization.billingNextInvoiceDate + ? toLocaleDate($organization.billingNextInvoiceDate) + : 'the end of the current billing period' + }`}>
186-189: Surface project-selection failure to the userPlan gets updated even if project selection fails; warn the user, not just console.warn.
- } catch (projectError) { - console.warn('Project selection failed after plan update:', projectError); - } + } catch (projectError) { + addNotification({ + type: 'error', + message: + 'Plan updated, but applying selected projects failed. Please review your project selection.' + }); + trackError(projectError, Submit.OrganizationDowngrade); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/lib/components/organizationUsageLimits.svelte(1 hunks)src/routes/(console)/organization-[organization]/change-plan/+page.svelte(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/components/organizationUsageLimits.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (1)
src/routes/(console)/organization-[organization]/change-plan/+page.svelte (1)
386-391: LGTM: OrganizationUsageLimits integrationProps and bindings look correct; safe optional chaining for projects/members/storage.
| selectedPlan = | ||
| $currentPlan?.$id === BillingPlan.SCALE ? BillingPlan.SCALE : BillingPlan.PRO; | ||
|
|
There was a problem hiding this comment.
Don’t override URL-selected plan
This unconditionally resets selectedPlan and ignores ?plan=. Guard it as a fallback only.
- selectedPlan =
- $currentPlan?.$id === BillingPlan.SCALE ? BillingPlan.SCALE : BillingPlan.PRO;
+ if (!plan || !(plan in BillingPlan)) {
+ selectedPlan =
+ $currentPlan?.$id === BillingPlan.SCALE ? BillingPlan.SCALE : BillingPlan.PRO;
+ }📝 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.
| selectedPlan = | |
| $currentPlan?.$id === BillingPlan.SCALE ? BillingPlan.SCALE : BillingPlan.PRO; | |
| if (!plan || !(plan in BillingPlan)) { | |
| selectedPlan = | |
| $currentPlan?.$id === BillingPlan.SCALE ? BillingPlan.SCALE : BillingPlan.PRO; | |
| } |
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/change-plan/+page.svelte
around lines 100-102, the assignment currently unconditionally resets
selectedPlan based on $currentPlan; change it so this only runs as a fallback
when no plan was specified in the URL or already selected — i.e., check whether
the URL has a plan query param (or selectedPlan is already set) and skip
overriding if so, otherwise set selectedPlan to SCALE when $currentPlan?.$id ===
BillingPlan.SCALE else PRO.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/lib/components/billing/alerts/selectProjectCloud.svelte (3)
50-68: XSS via {@html} rendering of project namesProject names are user-controlled. Building an HTML string and injecting with {@html} enables XSS.
Apply this safe rendering (drop {@html} and render names in markup); remove the helper:
- function formatProjectsToArchive() { - let result = ''; - - projectsToArchive.forEach((project, index) => { - const text = `${index === 0 ? '' : ' '}<b>${project.name}</b> `; - result += text; - - if (index < projectsToArchive.length - 1) { - if (index == projectsToArchive.length - 2) { - result += 'and '; - } - if (index < projectsToArchive.length - 2) { - result += ', '; - } - } - }); - - return result; - } + // removed: render list in markup to avoid {@html}- <span> - {@html formatProjectsToArchive()} - will be archived. - </span> + <span> + {#each projectsToArchive as project, index} + {#if index > 0} + {#if index === projectsToArchive.length - 1} + and{' '} + {:else} + ,{' '} + {/if} + {/if} + <b>{project.name}</b> + {/each} + {' '}will be archived. + </span>Also applies to: 157-160
165-168: “Save” doesn’t submit the modalWithout
submit,onSubmit={updateSelected}won’t fire.- <Button.Button size="s" disabled={selectedProjects.length !== $currentPlan?.projects} - >Save</Button.Button> + <Button.Button size="s" submit disabled={selectedProjects.length !== $currentPlan?.projects}> + Save + </Button.Button>
34-37: Use organizationId instead of projects[0].teamIdAvoids crashes when the org has zero projects and removes hidden coupling.
- await sdk.forConsole.billing.updateSelectedProjects( - projects[0].teamId, - selectedProjects - ); + await sdk.forConsole.billing.updateSelectedProjects(organizationId, selectedProjects);src/lib/stores/billing.ts (1)
621-628: Unit mismatch in calculateExcess
- Bandwidth limit likely in GB, usage in bytes → convert limit to bytes.
- Executions are counts → remove 'GB' conversion.
export function calculateExcess(addon: AggregationTeam, plan: Plan) { return { - bandwidth: calculateResourceSurplus(addon.usageBandwidth, plan.bandwidth), + bandwidth: calculateResourceSurplus(addon.usageBandwidth, plan.bandwidth, 'GB'), storage: calculateResourceSurplus(addon.usageStorage, plan.storage, 'GB'), - executions: calculateResourceSurplus(addon.usageExecutions, plan.executions, 'GB'), + executions: calculateResourceSurplus(addon.usageExecutions, plan.executions), members: addon.additionalMembers }; }src/routes/(console)/organization-[organization]/change-plan/+page.svelte (2)
67-72: Declare and type paymentMethods/paymentMethodId (current code won’t compile).Reactive assignment to undeclared identifiers breaks TS/Svelte. Declare both with proper types and initialize once.
-$: paymentMethods = null; +let paymentMethods: { paymentMethods: PaymentMethodData[] } | null = null; + +$: paymentMethods; -$: paymentMethodId = +let paymentMethodId: string | null = null; +$: paymentMethodId = data.organization.paymentMethodId ?? paymentMethods?.paymentMethods?.find((method: PaymentMethodData) => !!method?.last4)?.$id;
273-278: Incorrect Stripe return_url for plan change
Route/console/change-plan?…doesn’t matchsrc/routes/(console)/organization-[organization]/change-plan; include the dynamic organization segment in the path:- await confirmPayment( - '', - clientSecret, - paymentMethodId, - '/console/change-plan?' + params.toString() - ); + await confirmPayment( + '', + clientSecret, + paymentMethodId, + `/console/organization-${organization.$id}/change-plan?${params.toString()}` + );
♻️ Duplicate comments (13)
src/lib/stores/billing.ts (2)
164-171: Unsafe access of plan.addons.seats.limit (can throw)
plan?.['addons']['seats']indexes into possibly-undefinedaddons. Prior review flagged this; issue persists.- // pro and scale plans have unlimited seats (per-project NEW pricing model) - const currentTier = tier ?? get(organization)?.billingPlan; - if (currentTier === BillingPlan.PRO || currentTier === BillingPlan.SCALE) { - return Infinity; // unlimited seats for Pro and Scale plans - } - // Free plan still has 1 member limit - return (plan?.['addons']['seats'] || [])['limit'] ?? 1; + // Pro/Scale: unlimited seats (new pricing model) + const currentTier = tier ?? get(organization)?.billingPlan; + if (currentTier === BillingPlan.PRO || currentTier === BillingPlan.SCALE) return Infinity; + // Free: seats addon limit (safe optional chaining) + return plan?.addons?.seats?.limit ?? 1;
341-343: Early returns disable Free-plan project limit checksThese short-circuit the alert for any org that already has projects. This was flagged previously.
- if (!org.projects) return; - if (org.projects.length > 0) return; + // Do not short-circuit based on org.projects; rely on orgProjectCount to enforce limitssrc/lib/components/archiveProject.svelte (2)
165-167: Guard project.platforms before mapping to prevent runtime crashproject.platforms may be undefined; .map() will throw.
- {@const platforms = filterPlatforms( - project.platforms.map((platform) => getPlatformInfo(platform.type)) - )} + {@const platforms = filterPlatforms( + (project.platforms ?? []).map((platform) => + getPlatformInfo(platform.type) + ) + )}
244-249: Off-by-one: show “+N” badge when there are more than two platformsWith > 3, exactly three platforms hides the overflow badge.
- {#if platforms.length > 3} + {#if platforms.length > 2} <Badge variant="secondary" content={`+${platforms.length - 2}`} style="width: max-content;" /> {/if}src/routes/(console)/organization-[organization]/+page.svelte (6)
212-217: Off-by-one: show overflow badge when > 2 platformsAlign with two visible badges.
- {#if platforms.length > 3} + {#if platforms.length > 2} <Badge variant="secondary" content={`+${platforms.length - 2}`} style="width: max-content;" /> {/if}
163-163: Pagination/total mismatch when client-filtering active projectsUsing activeProjects.length for total can desync with server limit/offset. Prefer server total or reset pagination when filtering.
Option A (keep server paging):
- total={activeProjects.length} + total={data.projects.total}And:
- total={activeProjects.length} /> + total={data.projects.total} />Option B (client paging): reset offset to 0 when selection/filter changes and compute totals consistently client-side.
Also applies to: 248-248
108-111: Declare reactive targets to avoid compile errorsprojectsToArchive and activeProjects are assigned reactively but never declared.
+ let projectsToArchive: Models.Project[] = []; + let activeProjects: Models.Project[] = []; $: projectsToArchive = data.projects.projects.filter((project) => project.status !== 'active'); $: activeProjects = data.projects.projects.filter((project) => project.status === 'active');
136-143: Null-guard organization.projects and fix message construction
- Accessing data.organization.projects.length can throw when projects is undefined.
- messagePrefix uses the array value instead of its length, yielding “[object Object] projects”.
- {#if isCloud && $currentPlan?.projects && $currentPlan?.projects > 0 && data.organization.projects.length > 0 && $canWriteProjects && (projectsToArchive.length > 0 || data.projects.total > $currentPlan.projects)} - {@const difference = projectsToArchive} - {@const messagePrefix = - difference.length > 1 ? `${difference} projects` : `${difference} project`} + {#if isCloud && $currentPlan?.projects && $currentPlan.projects > 0 && (data.organization.projects?.length ?? 0) > 0 && $canWriteProjects && (projectsToArchive.length > 0 || data.projects.total > $currentPlan.projects)} + {@const diffCount = projectsToArchive.length} + {@const messagePrefix = diffCount === 1 ? '1 project' : `${diffCount} projects`}
171-171: Use store value for viewport checkisSmallViewport is a store; without $ it’s always truthy, forcing 19-char truncation.
- ? formatNameHelper(project.name, isSmallViewport ? 19 : 25) + ? formatNameHelper(project.name, $isSmallViewport ? 19 : 25)
167-169: Guard project.platforms before mappingPrevent runtime errors when platforms is undefined.
- {@const platforms = filterPlatforms( - project.platforms.map((platform) => getPlatformInfo(platform.type)) - )} + {@const platforms = filterPlatforms( + (project.platforms ?? []).map((platform) => getPlatformInfo(platform.type)) + )}src/routes/(console)/organization-[organization]/change-plan/+page.svelte (3)
358-373: Fix extra seat pricing: use real member count, included seats, plan name; hide when zero.Current code counts invitees, hardcodes “Pro”, and always shows alert.
-{@const extraMembers = collaborators?.length ?? 0} -{@const price = formatCurrency( - extraMembers * - ($plansInfo?.get(selectedPlan)?.addons?.seats?.price ?? 0) -)} -{#if selectedPlan === BillingPlan.PRO} +{@const includedSeats = $plansInfo?.get(selectedPlan)?.addons?.seats?.included ?? 0} +{@const seatPrice = $plansInfo?.get(selectedPlan)?.addons?.seats?.price ?? 0} +{@const membersCount = data.members?.memberships?.length ?? 0} +{@const extraMembers = Math.max(0, membersCount - includedSeats)} +{@const price = formatCurrency(extraMembers * seatPrice)} +{#if extraMembers > 0 && seatPrice > 0} <Alert.Inline status="error"> <svelte:fragment slot="title"> - Your monthly payments will be adjusted for the Pro plan + Your monthly payments will be adjusted for the {tierToPlan(selectedPlan)?.name} plan </svelte:fragment> After switching plans, <b - >you will be charged {price} monthly for {extraMembers} team members.</b> + >you will be charged {price} monthly for {extraMembers} + {extraMembers === 1 ? ' team member' : ' team members'}.</b> This will be reflected in your next invoice. </Alert.Inline> -{:else if selectedPlan === BillingPlan.FREE} +{:else if selectedPlan === BillingPlan.FREE}
100-102: Don’t override URL-selected plan (?plan=).Preserve explicit user choice and only fallback when param is missing/invalid.
-selectedPlan = - $currentPlan?.$id === BillingPlan.SCALE ? BillingPlan.SCALE : BillingPlan.PRO; +if (!plan || !(plan in BillingPlan)) { + selectedPlan = + $currentPlan?.$id === BillingPlan.SCALE ? BillingPlan.SCALE : BillingPlan.PRO; +}
65-66: Paginate projects to avoid truncation and include total in shape.Single-page limit(1000) will drop overflow orgs. Aggregate pages and keep total.
-let allProjects: { projects: Models.Project[] } | undefined; +let allProjects: { total: number; projects: Models.Project[] } | undefined; @@ - try { - allProjects = await sdk.forConsole.projects.list([ - Query.equal('teamId', data.organization.$id), - Query.limit(1000) - ]); - } catch { - allProjects = { projects: [] }; - } + try { + let offset = 0; + const pageSize = 1000; + let acc: Models.Project[] = []; + let total = 0; + do { + const pageResp = await sdk.forConsole.projects.list([ + Query.equal('teamId', data.organization.$id), + Query.limit(pageSize), + Query.offset(offset) + ]); + total = pageResp.total ?? pageResp.projects.length; + acc = acc.concat(pageResp.projects); + offset += pageResp.projects.length; + } while (offset < total); + allProjects = { total, projects: acc }; + } catch { + allProjects = { total: 0, projects: [] }; + }Also applies to: 109-116
🧹 Nitpick comments (12)
src/routes/(console)/organization-[organization]/billing/cancelDowngradeModal.svelte (2)
14-14: Fix TS type for error bindingMake
errornullable to matchbind:error.- let error: string = null; + let error: string | null = null;
39-41: Guard against missing billingNextInvoiceDateAvoid runtime errors when org data hasn't loaded.
- plan on <strong> {toLocaleDate($organization.billingNextInvoiceDate)}</strong>. Are you sure + plan on <strong> {toLocaleDate($organization?.billingNextInvoiceDate)}</strong>. Are you suresrc/lib/components/billing/alerts/selectProjectCloud.svelte (1)
151-156: Hide “will be archived” alert when nothing will be archivedPrevent confusing “0 project(s) will be archived”.
- {#if selectedProjects.length === $currentPlan?.projects} + {#if selectedProjects.length === $currentPlan?.projects} {@const difference = projects.length - selectedProjects.length} {@const messagePrefix = difference > 1 ? `${difference} projects` : `${difference} project`} - <Alert.Inline + {#if difference > 0} + <Alert.Inline status="warning" - title={`${messagePrefix} will be archived on ${toLocaleDate($organization.billingNextInvoiceDate)}`}> + title={`${messagePrefix} will be archived on ${toLocaleDate($organization?.billingNextInvoiceDate)}`}> <span> - {@html formatProjectsToArchive()} - will be archived. + <!-- list is rendered in markup in the body --> </span> </Alert.Inline> + {/if} {/if}src/lib/components/billing/alerts/projectsLimit.svelte (1)
30-33: Guard missing invoice date to avoid “Invalid Date” in UIIf $organization.billingNextInvoiceDate is absent, toLocaleDate may render poorly. Add a safe fallback.
- Choose which projects to keep before {toLocaleDate( - $organization.billingNextInvoiceDate - )} or upgrade to Pro. Projects over the limit will be blocked after this date. + Choose which projects to keep before { $organization?.billingNextInvoiceDate + ? toLocaleDate($organization.billingNextInvoiceDate) + : 'your next invoice date' + } or upgrade to Pro. Projects over the limit will be blocked after this date.src/lib/components/archiveProject.svelte (2)
131-136: Type-safe error handling in catch blockAnnotate error as unknown and simplify extraction to satisfy eslint and avoid unsafe casts.
- } catch (error) { - const msg = - error && typeof error === 'object' && 'message' in error - ? String((error as { message: string }).message) - : 'Failed to unarchive project'; + } catch (error: unknown) { + const msg = + error instanceof Error + ? error.message + : (error && typeof error === 'object' && 'message' in error + ? String((error as { message?: unknown }).message) + : 'Failed to unarchive project'); addNotification({ type: 'error', message: msg }); }
181-193: Replace inline style with class for consistencyMove white-space: nowrap to a CSS class to align with the design system.
- <Tag - size="s" - style="white-space: nowrap;" + <Tag + size="s" + class="u-nowrap"And in <style>:
+ .u-nowrap { white-space: nowrap; }src/routes/(console)/organization-[organization]/+page.svelte (2)
221-223: Guard region accessfindRegion can return undefined; avoid reading name on undefined.
- <Typography.Text>{region.name}</Typography.Text> + <Typography.Text>{region?.name ?? 'Unknown region'}</Typography.Text>
105-106: Use Status enum for consistencyCompare against Status.Active instead of string literal for type safety and refactor resilience.
- return project.status !== 'active'; + return project.status !== Status.Active;src/routes/(console)/organization-[organization]/change-plan/+page.svelte (4)
387-393: Render usage limits UI only when needed (plans with project caps / FREE).Avoid noise for downgrades without project limits.
-<OrganizationUsageLimits - bind:this={usageLimitsComponent} - organization={data.organization} - projects={allProjects?.projects || []} - members={data.members?.memberships || []} - storageUsage={orgUsage?.storageTotal ?? 0} /> +{@const targetProjectsLimit = $plansInfo?.get(selectedPlan)?.projects ?? 0} +{#if targetProjectsLimit > 0 || selectedPlan === BillingPlan.FREE} + <OrganizationUsageLimits + bind:this={usageLimitsComponent} + organization={data.organization} + projects={allProjects?.projects || []} + members={data.members?.memberships || []} + storageUsage={orgUsage?.storageTotal ?? 0} /> +{/if}
232-237: Use the correct analytics event for upgrade failure.Likely should be OrganizationUpgrade, not OrganizationCreate.
- trackError(e, Submit.OrganizationCreate); + trackError(e, Submit.OrganizationUpgrade);
43-43: Type selectedCoupon to allow null.Prevents TS errors and matches initialization.
-let selectedCoupon: Partial<Coupon> = null; +let selectedCoupon: Partial<Coupon> | null = null;
310-315: Guard against undefined plansInfo/currentPlan to avoid runtime crashes.Safer reactive comparisons.
-$: isUpgrade = $plansInfo.get(selectedPlan).order > $currentPlan?.order; -$: isDowngrade = $plansInfo.get(selectedPlan).order < $currentPlan?.order; +$: isUpgrade = + ($plansInfo.get(selectedPlan)?.order ?? 0) > ($currentPlan?.order ?? 0); +$: isDowngrade = + ($plansInfo.get(selectedPlan)?.order ?? 0) < ($currentPlan?.order ?? 0);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
src/lib/components/archiveProject.svelte(1 hunks)src/lib/components/billing/alerts/projectsLimit.svelte(2 hunks)src/lib/components/billing/alerts/selectProjectCloud.svelte(2 hunks)src/lib/components/organizationUsageLimits.svelte(1 hunks)src/lib/stores/billing.ts(7 hunks)src/routes/(console)/organization-[organization]/+page.svelte(7 hunks)src/routes/(console)/organization-[organization]/billing/cancelDowngradeModal.svelte(1 hunks)src/routes/(console)/organization-[organization]/change-plan/+page.svelte(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/components/organizationUsageLimits.svelte
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/stores/billing.ts (2)
src/lib/stores/organization.ts (3)
organization(62-62)currentPlan(63-63)members(64-64)src/lib/sdk/billing.ts (2)
AggregationTeam(154-211)Plan(377-433)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (5)
src/routes/(console)/organization-[organization]/billing/cancelDowngradeModal.svelte (1)
31-36: Modal size prop addition looks goodSmall modal improves visual hierarchy and matches other billing modals.
src/lib/components/billing/alerts/selectProjectCloud.svelte (1)
10-10: Import switch to organization store is correctUsing
$organization.billingNextInvoiceDatecentralizes the date source.src/lib/stores/billing.ts (2)
242-242: Derived store useNewPricingModal: LGTMSimple, expressive toggle off
usagePerProject.
164-168: Confirm business rule: unlimited members on Pro/ScaleBefore merging, confirm backend semantics (e.g., whether “0 seats” denotes unlimited) to keep UI consistent with billing.
Would you like me to scan the codebase for any places that still assume numeric member limits on Pro/Scale and report them?
src/lib/components/billing/alerts/projectsLimit.svelte (1)
6-8: Import/source of billing data looks consistent with centralization to organization storeSwitch to organization store for invoice date and keeping only hideBillingHeaderRoutes/upgradeURL in billing store reads well. No concerns.
| let orgUsage: OrganizationUsage; | ||
| let allProjects: { projects: Models.Project[] } | undefined; | ||
|
|
There was a problem hiding this comment.
Fix orgUsage typing: it’s assigned undefined but typed non-optional.
Align declaration with usage.
-let orgUsage: OrganizationUsage;
+let orgUsage: OrganizationUsage | undefined;Also applies to: 103-107
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/change-plan/+page.svelte
around lines 64-66 and 103-107, the orgUsage variable is declared as a
non-optional OrganizationUsage but is assigned undefined in practice; change the
declaration to allow undefined (e.g., make it type OrganizationUsage |
undefined) or initialize it with a proper default value, and update any
downstream code to handle the possibly undefined value (null-checks or early
returns) so the typing matches actual usage.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (2)
88-110: Storage progress: use 1024³ and consistent tooltip unitsPrevents % drift and mixed units.
-const maxBytes = maxGB * 1000 * 1000 * 1000; +const maxBytes = maxGB * 1024 * 1024 * 1024; @@ -const currentSize = humanFileSize(currentBytes); +const currentSize = humanFileSize(currentBytes); +const maxSize = humanFileSize(maxBytes); @@ - label: `${currentSize.value} ${currentSize.unit} of ${maxGB} GB` + label: `${currentSize.value} ${currentSize.unit} of ${maxSize.value} ${maxSize.unit}`
44-51: Fix decimal vs binary conversion in bandwidth max; use 1024³humanFileSize is binary; using 1000³ skews values.
- const maxSize = humanFileSize(maxGB * 1000 * 1000 * 1000); + const maxSize = humanFileSize(maxGB * 1024 * 1024 * 1024);
🧹 Nitpick comments (5)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte (5)
32-37: Column widths: avoid 0fr; use auto/min-content to keep price visible0fr can collapse the price column and differs from other table configs; prefer 'auto' or 'min-content' and align with library conventions.
Apply:
-const columns = [ - { id: 'item', align: 'left' as const, width: '10fr' }, - { id: 'usage', align: 'left' as const, width: '20fr' }, - { id: 'price', align: 'right' as const, width: '0fr' } -]; +const columns = [ + { id: 'item', align: 'left' as const, width: '1fr' }, + { id: 'usage', align: 'left' as const, width: '2fr' }, + { id: 'price', align: 'right' as const, width: 'min-content' } +];
58-86: Type widen createProgressData params; you already guard null/undefinedSignature disallows values you handle at runtime.
-function createProgressData( - currentValue: number, - maxValue: number | string -): Array<{ size: number; color: string; tooltip?: { title: string; label: string } }> { +function createProgressData( + currentValue: number, + maxValue: number | string | null | undefined +): Array<{ size: number; color: string; tooltip?: { title: string; label: string } }> {
112-141: Type helpers to avoid implicit anyAdd explicit types for params/returns to satisfy TS/ESLint and improve safety.
-function getProjectsList(currentAggregation) { +function getProjectsList(currentAggregation: AggregationTeam | undefined) { @@ - function getBillingData(currentPlan, currentAggregation, isSmallViewport) { + function getBillingData( + currentPlan: Plan, + currentAggregation: AggregationTeam | undefined, + isSmallViewport: boolean + ) {Also applies to: 143-159
521-567: Minor: use consistent event name for Change planElsewhere you use Click enum; consider aligning for analytics consistency.
371-401: Progress bar render condition should treat 0/undefined uniformlyYou already return [] when max is invalid; checking child.maxValue is redundant and hides bars for small-but-valid 0 values.
-{#if child.progressData && child.progressData.length > 0 && child.maxValue} +{#if child.progressData && child.progressData.length > 0} <ProgressBar - maxSize={child.maxValue} + maxSize={child.maxValue} data={child.progressData} /> {/if}Also applies to: 402-427
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/routes/(console)/organization-[organization]/billing/planSummary.svelte(4 hunks)
| price: formatCurrency( | ||
| Math.max( | ||
| (currentAggregation?.amount ?? currentPlan?.price ?? 0) - availableCredit, | ||
| 0 | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Guard availableCredit; current math can yield NaN
Subtraction/min with undefined leads to NaN and broken totals.
- price: formatCurrency(
- Math.max(
- (currentAggregation?.amount ?? currentPlan?.price ?? 0) - availableCredit,
- 0
- )
- )
+ price: formatCurrency(
+ Math.max(
+ (currentAggregation?.amount ?? currentPlan?.price ?? 0) - (availableCredit ?? 0),
+ 0
+ )
+ )-$: totalAmount = Math.max(
- (currentAggregation?.amount ?? currentPlan?.price ?? 0) - availableCredit,
- 0
-);
+$: totalAmount = Math.max(
+ (currentAggregation?.amount ?? currentPlan?.price ?? 0) - (availableCredit ?? 0),
+ 0
+);
@@
-$: creditsApplied = Math.min(
- currentAggregation?.amount ?? currentPlan?.price ?? 0,
- availableCredit
-);
+$: creditsApplied = Math.min(
+ currentAggregation?.amount ?? currentPlan?.price ?? 0,
+ availableCredit ?? 0
+);Also applies to: 307-315
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/billing/planSummary.svelte
around lines 151-156 (also apply same fix at 307-315): the subtraction can
produce NaN when availableCredit is undefined; ensure all operands are numbers
by defaulting availableCredit to 0 (and/or coerce to Number) before performing
the math. Replace uses like (currentAggregation?.amount ?? currentPlan?.price ??
0) - availableCredit with a safe expression that uses
(currentAggregation?.amount ?? currentPlan?.price ?? 0) - (availableCredit ?? 0)
or Number(availableCredit ?? 0) so Math.max always receives numeric values.
| id: `project-${project.projectId}-bandwidth`, | ||
| cells: { | ||
| item: 'Bandwidth', | ||
| usage: `${formatBandwidthUsage(project.bandwidth.value, currentPlan?.bandwidth)}`, | ||
| price: formatCurrency(project.bandwidth.amount || 0) | ||
| }, | ||
| progressData: createStorageProgressData( | ||
| project.bandwidth.value || 0, | ||
| currentPlan?.bandwidth || 0 | ||
| ), | ||
| maxValue: currentPlan?.bandwidth | ||
| ? currentPlan.bandwidth * 1000 * 1000 * 1000 | ||
| : 0 |
There was a problem hiding this comment.
Null-safe resource access across all rows; potential runtime TypeError
Direct .value/.amount on missing resources will crash. Also fix 1024³ conversions and treat unlimited as null to hide progress.
-usage: `${formatBandwidthUsage(project.bandwidth.value, currentPlan?.bandwidth)}`,
-price: formatCurrency(project.bandwidth.amount || 0)
+usage: `${formatBandwidthUsage(project.bandwidth?.value ?? 0, currentPlan?.bandwidth)}`,
+price: formatCurrency(project.bandwidth?.amount ?? 0)
@@
-progressData: createStorageProgressData(
- project.bandwidth.value || 0,
- currentPlan?.bandwidth || 0
-),
-maxValue: currentPlan?.bandwidth
- ? currentPlan.bandwidth * 1000 * 1000 * 1000
- : 0
+progressData: createStorageProgressData(
+ project.bandwidth?.value ?? 0,
+ currentPlan?.bandwidth ?? 0
+),
+maxValue: currentPlan?.bandwidth
+ ? currentPlan.bandwidth * 1024 * 1024 * 1024
+ : null-usage: `${formatNum(project.users.value || 0)} / ${currentPlan?.users ? formatNum(currentPlan.users) : 'Unlimited'}`,
-price: formatCurrency(project.users.amount || 0)
+usage: `${formatNum(project.users?.value ?? 0)} / ${currentPlan?.users ? formatNum(currentPlan.users) : 'Unlimited'}`,
+price: formatCurrency(project.users?.amount ?? 0)
@@
-progressData: createProgressData(project.users.value || 0, currentPlan?.users),
+progressData: createProgressData(project.users?.value ?? 0, currentPlan?.users),-usage: `${formatNum(project.databasesReads.value || 0)} / ${currentPlan?.databasesReads ? formatNum(currentPlan.databasesReads) : 'Unlimited'}`,
-price: formatCurrency(project.databasesReads.amount || 0)
+usage: `${formatNum(project.databasesReads?.value ?? 0)} / ${currentPlan?.databasesReads ? formatNum(currentPlan.databasesReads) : 'Unlimited'}`,
+price: formatCurrency(project.databasesReads?.amount ?? 0)
@@
-progressData: createProgressData(
- project.databasesReads.value || 0,
- currentPlan?.databasesReads
-),
+progressData: createProgressData(
+ project.databasesReads?.value ?? 0,
+ currentPlan?.databasesReads
+),-usage: `${formatNum(project.databasesWrites.value || 0)} / ${currentPlan?.databasesWrites ? formatNum(currentPlan.databasesWrites) : 'Unlimited'}`,
-price: formatCurrency(project.databasesWrites.amount || 0)
+usage: `${formatNum(project.databasesWrites?.value ?? 0)} / ${currentPlan?.databasesWrites ? formatNum(currentPlan.databasesWrites) : 'Unlimited'}`,
+price: formatCurrency(project.databasesWrites?.amount ?? 0)
@@
-progressData: createProgressData(
- project.databasesWrites.value || 0,
- currentPlan?.databasesWrites
-),
+progressData: createProgressData(
+ project.databasesWrites?.value ?? 0,
+ currentPlan?.databasesWrites
+),-usage: `${formatNum(project.executions.value || 0)} / ${currentPlan?.executions ? formatNum(currentPlan.executions) : 'Unlimited'}`,
-price: formatCurrency(project.executions.amount || 0)
+usage: `${formatNum(project.executions?.value ?? 0)} / ${currentPlan?.executions ? formatNum(currentPlan.executions) : 'Unlimited'}`,
+price: formatCurrency(project.executions?.amount ?? 0)
@@
-progressData: createProgressData(
- project.executions.value || 0,
- currentPlan?.executions
-),
+progressData: createProgressData(
+ project.executions?.value ?? 0,
+ currentPlan?.executions
+),-usage: `${formatHumanSize(project.storage.value || 0)} / ${currentPlan?.storage?.toString() || '0'} GB`,
-price: formatCurrency(project.storage.amount || 0)
+usage: `${formatHumanSize(project.storage?.value ?? 0)} / ${currentPlan?.storage ? `${currentPlan.storage} GB` : 'Unlimited'}`,
+price: formatCurrency(project.storage?.amount ?? 0)
@@
-progressData: createStorageProgressData(
- project.storage.value || 0,
- currentPlan?.storage || 0
-),
-maxValue: currentPlan?.storage ? currentPlan.storage * 1000 * 1000 * 1000 : 0
+progressData: currentPlan?.storage
+ ? createStorageProgressData(project.storage?.value ?? 0, currentPlan.storage)
+ : [],
+maxValue: currentPlan?.storage
+ ? currentPlan.storage * 1024 * 1024 * 1024
+ : null-usage: `${formatNum(project.gbHours.value || 0)} / ${currentPlan?.GBHours ? formatNum(currentPlan.GBHours) : 'Unlimited'}`,
-price: formatCurrency(project.gbHours.amount || 0)
+usage: `${formatNum(project.gbHours?.value ?? 0)} / ${currentPlan?.GBHours ? formatNum(currentPlan.GBHours) : 'Unlimited'}`,
+price: formatCurrency(project.gbHours?.amount ?? 0)
@@
-? createProgressData(project.gbHours.value || 0, currentPlan.GBHours)
+? createProgressData(project.gbHours?.value ?? 0, currentPlan.GBHours)-usage: `${formatNum(project.authPhone.value || 0)} SMS messages`,
-price: formatCurrency(project.authPhone.amount || 0)
+usage: `${formatNum(project.authPhone?.value ?? 0)} SMS messages`,
+price: formatCurrency(project.authPhone?.amount ?? 0)Also applies to: 210-218, 220-231, 233-244, 246-257, 259-270, 272-282, 284-290
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/billing/planSummary.svelte
around lines 195-207 (and similarly for ranges 210-218, 220-231, 233-244,
246-257, 259-270, 272-282, 284-290), the code accesses resource properties like
.value and .amount directly which can throw when a resource is missing; also the
conversion uses 1000^3 instead of 1024^3 and unlimited should be treated as null
to hide progress. Fix by null-checking the resource object before reading
.value/.amount (use defaults like 0 or null), update unit conversions to use
1024 * 1024 * 1024, and treat an "unlimited" indicator (e.g., null/Infinity/ -1
depending on your model) as null so progress/progressData and maxValue are
omitted or set to 0/undefined to hide the progress bar; apply the same defensive
access and conversion changes to all listed line ranges.
| id: `project-${project.projectId}-usage-details`, | ||
| cells: { | ||
| item: `<a href="/console/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`, | ||
| usage: '', | ||
| price: '' | ||
| } | ||
| } |
There was a problem hiding this comment.
Stop injecting HTML with {@html}; render anchor safely (XSS risk)
Build href as data and render ; also use base for correct pathing.
-{
- id: `project-${project.projectId}-usage-details`,
- cells: {
- item: `<a href="/console/project-${String(project.region || 'default')}-${project.projectId}/settings/usage" style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">Usage details</a>`,
- usage: '',
- price: ''
- }
-}
+{
+ id: `project-${project.projectId}-usage-details`,
+ cells: { item: 'Usage details', usage: '', price: '' },
+ href: `${base}/project-${String(project.region || 'default')}-${project.projectId}/settings/usage`
+}-{#if child.cells?.[col.id]?.includes('<a href=')}
- {@html child.cells?.[col.id] ?? ''}
+{#if col.id === 'item' && child.href}
+ <a href={child.href} style="text-decoration: underline; color: var(--fgcolor-accent-neutral);">
+ Usage details
+ </a>Also applies to: 386-399
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/billing/planSummary.svelte
around lines 292-298 (and similarly lines 386-399), you're injecting an HTML
string into cells.item which is rendered with {@html} — replace this by building
a plain data href and label (e.g., { href:
`/console/project-${String(project.region||'default')}-${project.projectId}/settings/usage`,
label: 'Usage details' }) and stop using {@html}; in the template render a real
<a> element binding href to the computed path (prefixed with the app base from
$app/paths or a base variable) and use the label as textContent so no HTML is
injected, and update any consumer code that expects a string to consume the
href/label properties instead.
| <ExpandableTable.Row {root} id="total-row" expandable={false}> | ||
| <ExpandableTable.Cell | ||
| {root} | ||
| column="item" | ||
| expandable={false} | ||
| isOpen={false} | ||
| toggle={() => {}}> | ||
| <Layout.Stack | ||
| inline | ||
| direction="row" | ||
| gap="xxs" | ||
| alignItems="center" | ||
| alignContent="center"> | ||
| <Icon icon={IconTag} color="--fgcolor-success" size="s" /> | ||
|
|
||
| {#if currentPlan.supportsCredits && availableCredit > 0} | ||
| <Layout.Stack direction="row" justifyContent="space-between"> | ||
| <Layout.Stack direction="row" alignItems="center" gap="xxs"> | ||
| <Icon size="s" icon={IconTag} color="--fgcolor-success" /> | ||
| <Typography.Text color="--fgcolor-neutral-primary" | ||
| >Credits to be applied</Typography.Text> | ||
| >Credits</Typography.Text> | ||
| </Layout.Stack> | ||
| <Typography.Text color="--fgcolor-success"> | ||
| -{formatCurrency( | ||
| Math.min(availableCredit, currentInvoice?.amount ?? 0) | ||
| )} | ||
| </Typography.Text> | ||
| </Layout.Stack> | ||
| {/if} | ||
|
|
||
| {#if $organization?.billingPlan !== BillingPlan.FREE && $organization?.billingPlan !== BillingPlan.GITHUB_EDUCATION} | ||
| <Divider /> | ||
| <Layout.Stack direction="row" justifyContent="space-between"> | ||
| <Typography.Text color="--fgcolor-neutral-primary" variant="m-500"> | ||
| <Layout.Stack direction="row" alignItems="center" gap="s"> | ||
| Current total (USD) | ||
| <Tooltip> | ||
| <Icon icon={IconInfo} /> | ||
| <svelte:fragment slot="tooltip"> | ||
| Estimates are updated daily and may differ from your | ||
| final invoice. | ||
| </svelte:fragment> | ||
| </Tooltip> | ||
| </Layout.Stack> | ||
| </ExpandableTable.Cell> | ||
| <ExpandableTable.Cell | ||
| {root} | ||
| column="usage" | ||
| expandable={false} | ||
| isOpen={false} | ||
| toggle={() => {}}> | ||
| <Typography.Text variant="m-500" color="--fgcolor-neutral-primary"> | ||
| </Typography.Text> | ||
| <Typography.Text color="--fgcolor-neutral-primary" variant="m-500"> | ||
| {formatCurrency( | ||
| Math.max( | ||
| (currentInvoice?.amount ?? 0) - | ||
| Math.min(availableCredit, currentInvoice?.amount ?? 0), | ||
| 0 | ||
| ) | ||
| )} | ||
| </ExpandableTable.Cell> | ||
| <ExpandableTable.Cell | ||
| {root} | ||
| column="price" | ||
| expandable={false} | ||
| isOpen={false} | ||
| toggle={() => {}}> | ||
| <Typography.Text variant="m-500" color="--fgcolor-neutral-primary"> | ||
| -{formatCurrency(creditsApplied)} | ||
| </Typography.Text> | ||
| </Layout.Stack> | ||
| {/if} | ||
| </Layout.Stack> | ||
| </Card.Base> | ||
| </svelte:fragment> | ||
| <svelte:fragment slot="actions"> | ||
| </ExpandableTable.Cell> | ||
| </ExpandableTable.Row> | ||
| {/if} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate row id "total-row"; breaks ARIA/state
Give Credits and Total rows unique ids; also guard undefined credits in the condition.
-{#if availableCredit > 0}
- <ExpandableTable.Row {root} id="total-row" expandable={false}>
+{#if (availableCredit ?? 0) > 0}
+ <ExpandableTable.Row {root} id="credits-row" expandable={false}>
@@
-<ExpandableTable.Row {root} id="total-row" expandable={false}>
+<ExpandableTable.Row {root} id="total-row-final" expandable={false}>Also applies to: 487-517
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/billing/planSummary.svelte
around lines 446-485 (and the similar block at 487-517), the ExpandableTable.Row
uses a duplicate id "total-row" which breaks ARIA/state and the conditional
rendering doesn't guard against undefined credits; update the Rows to have
unique ids (e.g., "credits-row" for the Credits row and keep "total-row" for
Total) and ensure the Credits block only renders when creditsApplied is defined
(or truthy) by adding an explicit check (e.g., creditsApplied != null) before
rendering so you don't render an empty/invalid row; keep the rest of the props
the same to preserve behavior.
TorstenDittmann
deleted the
feat-SER-204-New-Archive-projects-ui-change
branch
TorstenDittmann
restored the
feat-SER-204-New-Archive-projects-ui-change
branch
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
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
Style
Chores