fix: Disable billing plan actions during upgrade with tooltip on hover#2861
fix: Disable billing plan actions during upgrade with tooltip on hover#2861HarshMN2345 wants to merge 2 commits intomainfrom
Conversation
Console (appwrite/console)Project ID: Tip Custom domains work with both CNAME for subdomains and NS records for apex domains |
WalkthroughTwo billing plan summary Svelte components (planSummary.svelte and planSummaryOld.svelte) are updated to introduce a reactive Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![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
🧹 Nitpick comments (1)
src/routes/(console)/organization-[organization]/billing/planSummaryOld.svelte (1)
45-49:Layout.Stackwrapping a single<p>element is unnecessary.The stack adds a flex container around a single child, which provides no layout benefit. Unless there are plans to add siblings here, a bare
<p>would be simpler.Suggested simplification
- <Layout.Stack gap="xs"> - <p class="text u-bold"> - Due at: {toLocaleDate($organization?.billingNextInvoiceDate)} - </p> - </Layout.Stack> + <p class="text u-bold"> + Due at: {toLocaleDate($organization?.billingNextInvoiceDate)} + </p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(console)/organization-[organization]/billing/planSummaryOld.svelte around lines 45 - 49, The Layout.Stack wrapping a single <p> is unnecessary; remove the Layout.Stack component and render the paragraph directly by replacing the Layout.Stack block (where Layout.Stack wraps <p class="text u-bold">Due at: {toLocaleDate($organization?.billingNextInvoiceDate)}</p>) with the bare <p> element so the markup is simpler and avoids an extra flex container.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/`(console)/organization-[organization]/billing/planSummary.svelte:
- Around line 614-620: Replace the exact branch condition that currently checks
"$organization?.billingPlanDowngrade !== null" with a truthy guard and deletion
guard so the button only renders when billingPlanDowngrade is present and the
org is not marked for deletion (use "$organization?.billingPlanDowngrade &&
!$organization?.markedForDeletion"); also ensure the Button's disabled prop
includes the deletion guard (e.g., disabled={isUpgrading ||
$organization?.markedForDeletion}) and keep the on:click handler (showCancel)
as-is.
In
`@src/routes/`(console)/organization-[organization]/billing/planSummaryOld.svelte:
- Around line 186-192: The conditional currently uses a strict null check
($organization?.billingPlanDowngrade !== null) which treats undefined as valid;
change it to a loose null check ($organization?.billingPlanDowngrade != null) so
both null and undefined are treated the same as elsewhere in billing; update the
branch that renders the Cancel change Button (references:
$organization?.billingPlanDowngrade, isUpgrading, showCancel) to use != null for
consistency with +page.ts/+page.svelte checks.
---
Nitpick comments:
In
`@src/routes/`(console)/organization-[organization]/billing/planSummaryOld.svelte:
- Around line 45-49: The Layout.Stack wrapping a single <p> is unnecessary;
remove the Layout.Stack component and render the paragraph directly by replacing
the Layout.Stack block (where Layout.Stack wraps <p class="text u-bold">Due at:
{toLocaleDate($organization?.billingNextInvoiceDate)}</p>) with the bare <p>
element so the markup is simpler and avoids an extra flex container.
| {:else if $organization?.billingPlanDowngrade !== null} | ||
| <Button | ||
| text | ||
| disabled={isUpgrading} | ||
| on:click={() => (showCancel = true)}> | ||
| Cancel change | ||
| </Button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and check its size
wc -l src/routes/'(console)'/organization-'[organization]'/billing/planSummary.svelteRepository: appwrite/console
Length of output: 139
🏁 Script executed:
# Get the relevant section around lines 614-620
sed -n '590,650p' src/routes/'(console)'/organization-'[organization]'/billing/planSummary.svelteRepository: appwrite/console
Length of output: 3119
🏁 Script executed:
# Search for billingPlanDowngrade usage across the codebase
rg -n 'billingPlanDowngrade' --type=svelte --type=ts -C2Repository: appwrite/console
Length of output: 90
🏁 Script executed:
# Search for markedForDeletion checks in the same file and context
rg -n 'markedForDeletion' src/routes/'(console)'/organization-'[organization]'/billing/planSummary.svelte -C2Repository: appwrite/console
Length of output: 718
🏁 Script executed:
# Check organization type definition to understand billingPlanDowngrade field
fd -e ts -e d.ts | xargs rg -l 'billingPlanDowngrade' | head -5Repository: appwrite/console
Length of output: 125
🏁 Script executed:
# Search for organization type definition
fd -type f \( -name "*.d.ts" -o -name "*.ts" \) | xargs rg -l "billingPlanDowngrade" | head -10Repository: appwrite/console
Length of output: 623
🏁 Script executed:
# Also check for types in store definitions or models
fd -type f -path "*/models/*" -o -path "*/types/*" | xargs rg -l "billingPlanDowngrade" 2>/dev/null | head -10Repository: appwrite/console
Length of output: 623
🏁 Script executed:
# Look at the +page.ts file for this route to understand the data structure
cat -n src/routes/'(console)'/organization-'[organization]'/billing/+page.ts | head -100Repository: appwrite/console
Length of output: 4432
🏁 Script executed:
# Search for where billingPlanDowngrade comes from (API/store)
rg -n "billingPlanDowngrade.*=" --type ts -A2 | head -50Repository: appwrite/console
Length of output: 448
🏁 Script executed:
# Look for organization type definition in `@appwrite.io/console`
find . -name "*.d.ts" -o -name "*.ts" | xargs grep -l "interface.*Organization\|type.*Organization" | grep -E "(models|types)" | head -5Repository: appwrite/console
Length of output: 871
🏁 Script executed:
# Check the Models import to understand organization structure
rg -n "Models\.Organization" src/routes/'(console)'/organization-'[organization]'/billing/+page.ts -A5 -B5Repository: appwrite/console
Length of output: 1762
🏁 Script executed:
# Look at how billingPlanDowngrade is used in the template - get more context from planSummary.svelte
sed -n '600,630p' src/routes/'(console)'/organization-'[organization]'/billing/planSummary.svelteRepository: appwrite/console
Length of output: 1852
🏁 Script executed:
# Check if there are any type assertions or optional chaining patterns for billingPlanDowngrade
rg -n "billingPlanDowngrade" src/routes/'(console)'/organization-'[organization]'/billing/ -B2 -A2Repository: appwrite/console
Length of output: 4911
🏁 Script executed:
# Check what Models.Organization contains - look at node_modules
find node_modules -path "*console*" -name "*.d.ts" | xargs grep -l "billingPlanDowngrade" 2>/dev/null | head -3Repository: appwrite/console
Length of output: 42
🏁 Script executed:
# Look at cancelDowngradeModal to see how billingPlanDowngrade is used there
sed -n '1,50p' src/routes/'(console)'/organization-'[organization]'/billing/cancelDowngradeModal.svelteRepository: appwrite/console
Length of output: 1788
🏁 Script executed:
# Check if there's any validation or type narrowing for billingPlanDowngrade in the component
sed -n '1,100p' src/routes/'(console)'/organization-'[organization]'/billing/planSummary.svelte | grep -A5 -B5 "script"Repository: appwrite/console
Length of output: 369
🏁 Script executed:
# Let's verify the actual data type/pattern by checking how billingPlanDowngrade is set in organization store
rg -n "billingPlanDowngrade" src/lib/ -B2 -A2 | head -50Repository: appwrite/console
Length of output: 42
🏁 Script executed:
# Check the actual API contract - look for models in console package
find node_modules -path "*@appwrite.io/console*" -name "index.d.ts" 2>/dev/null | head -1 | xargs cat | grep -A 20 "billingPlanDowngrade" | head -30Repository: appwrite/console
Length of output: 42
🏁 Script executed:
# Let's look at the entire planSummary.svelte script section to understand the data flow
sed -n '1,150p' src/routes/'(console)'/organization-'[organization]'/billing/planSummary.svelteRepository: appwrite/console
Length of output: 5860
🏁 Script executed:
# Check if there's documentation on Organization type or if we can find the pattern
rg -n "organization\?\.billingPlan" src/routes/'(console)'/organization-'[organization]'/billing/ | head -20Repository: appwrite/console
Length of output: 1019
Add markedForDeletion guard and use truthy check for billingPlanDowngrade.
Two bugs in this branch:
-
undefined !== nullevaluates totrue, causing the "Cancel change" button to display incorrectly if the field is absent from the API response. The codebase elsewhere (e.g., +page.svelte) uses a truthy check instead:{#iforganization?.billingPlanDowngrade}. -
Missing
markedForDeletionguard — the other two button branches check$organization?.markedForDeletion, but this one does not, leaving the button enabled during organization deletion.
Proposed fix
- {:else if $organization?.billingPlanDowngrade !== null}
+ {:else if $organization?.billingPlanDowngrade}
<Button
text
- disabled={isUpgrading}
+ disabled={$organization?.markedForDeletion || isUpgrading}
on:click={() => (showCancel = true)}>
Cancel change
</Button>📝 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.
| {:else if $organization?.billingPlanDowngrade !== null} | |
| <Button | |
| text | |
| disabled={isUpgrading} | |
| on:click={() => (showCancel = true)}> | |
| Cancel change | |
| </Button> | |
| {:else if $organization?.billingPlanDowngrade} | |
| <Button | |
| text | |
| disabled={$organization?.markedForDeletion || isUpgrading} | |
| on:click={() => (showCancel = true)}> | |
| Cancel change | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(console)/organization-[organization]/billing/planSummary.svelte
around lines 614 - 620, Replace the exact branch condition that currently checks
"$organization?.billingPlanDowngrade !== null" with a truthy guard and deletion
guard so the button only renders when billingPlanDowngrade is present and the
org is not marked for deletion (use "$organization?.billingPlanDowngrade &&
!$organization?.markedForDeletion"); also ensure the Button's disabled prop
includes the deletion guard (e.g., disabled={isUpgrading ||
$organization?.markedForDeletion}) and keep the on:click handler (showCancel)
as-is.
| {:else if $organization?.billingPlanDowngrade !== null} | ||
| <Button | ||
| text | ||
| disabled={isUpgrading} | ||
| on:click={() => (showCancel = true)}> | ||
| Cancel change | ||
| </Button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check type definitions and usage of billingPlanDowngrade
echo "=== SDK type definition for billingPlanDowngrade ==="
rg -n 'billingPlanDowngrade' --type=ts -C3
echo ""
echo "=== Usage in Svelte files ==="
rg -n 'billingPlanDowngrade' --glob '*.svelte' -C3
echo ""
echo "=== Check in models/types files ==="
fd -e ts -e d.ts | xargs rg -l 'billingPlanDowngrade' | head -20Repository: appwrite/console
Length of output: 6644
🏁 Script executed:
#!/bin/bash
# Look for type definitions and models
echo "=== Search for organization type definition ==="
fd -e d.ts -e ts | xargs rg -l 'billingPlanDowngrade' | grep -E '(types|models|schema)' | head -20
echo ""
echo "=== Check store initialization ==="
rg -n 'organization.*=.*store' --type=ts -C2 | head -30
echo ""
echo "=== Look for organization model/interface definition ==="
rg -n 'interface.*Organization|type.*Organization' --type=ts -C3 | grep -A5 -B5 'billingPlan' | head -40
echo ""
echo "=== Check how organization store is populated ==="
rg -n 'billingPlanDowngrade' src -C5 | grep -E '^\s*(export|const|let|interface|type)' | head -20Repository: appwrite/console
Length of output: 252
🏁 Script executed:
#!/bin/bash
# Search for stores and organization definition
echo "=== Look for stores directory ==="
find . -name '*store*' -type f | grep -E '\.(ts|js)$' | head -20
echo ""
echo "=== Look for organization in lib files ==="
rg -n 'export.*organization' --type=ts src/lib | head -20
echo ""
echo "=== Check the actual SDK types from Appwrite ==="
rg -n 'Organization' --type=ts src | grep -E 'import|from.*appwrite' | head -20
echo ""
echo "=== Look at layout files to see how organization is passed ==="
rg -n 'organization' src/routes/\(console\)/layout.ts -C5 | head -50Repository: appwrite/console
Length of output: 3966
🏁 Script executed:
#!/bin/bash
# Read the organization store definition
echo "=== src/lib/stores/organization.ts ==="
cat -n src/lib/stores/organization.ts
echo ""
echo "=== src/routes/(console)/organization-[organization]/billing/store.ts ==="
cat -n src/routes/\(console\)/organization-\[organization\]/billing/store.ts
echo ""
echo "=== Check Models.Organization import ==="
rg -n 'Models\.Organization|from.*models|from.*appwrite' src/lib/stores/organization.ts src/lib/stores/billing.ts | head -20Repository: appwrite/console
Length of output: 5232
🏁 Script executed:
#!/bin/bash
# Check if other code uses strict !== null or loose checks for similar fields
echo "=== Check null checking patterns in billing files ==="
rg -n '!== null|== null' src/routes/\(console\)/organization-\[organization\]/billing/ --type=svelte | head -20
echo ""
echo "=== Check how billingPlanDowngrade is used after the check ==="
rg -n 'billingPlanDowngrade' src/routes/\(console\)/organization-\[organization\]/billing/planSummaryOld.svelte -A5 -B5
echo ""
echo "=== Look at how the field is accessed elsewhere ==="
rg -n 'billingPlanDowngrade' src/routes/\(console\)/organization-\[organization\]/billing/+page.svelte -A3 -B3Repository: appwrite/console
Length of output: 1392
🏁 Script executed:
#!/bin/bash
# Check how billingPlanDowngrade is checked in planSummary.svelte as well
echo "=== planSummary.svelte usage ==="
grep -n 'billingPlanDowngrade' src/routes/\(console\)/organization-\[organization\]/billing/planSummary.svelte | head -20
echo ""
echo "=== Check cancelDowngradeModal.svelte ==="
head -50 src/routes/\(console\)/organization-\[organization\]/billing/cancelDowngradeModal.svelte
echo ""
echo "=== Check if there's type definition or comments about billingPlanDowngrade ==="
rg -n 'billingPlanDowngrade' src -C2 | grep -E '(\/\/|\/\*|\*\/|type|interface|optional)' | head -20
echo ""
echo "=== Understand the ternary logic in +page.ts more closely ==="
sed -n '80,90p' src/routes/\(console\)/organization-\[organization\]/billing/+page.tsRepository: appwrite/console
Length of output: 2433
Use loose equality != null to match the intent of other billing components.
The strict check !== null incorrectly treats undefined as a valid downgrade state. Elsewhere in the billing section (+page.ts line 84, +page.svelte line 132), truthy checks are used consistently. Align this check with that pattern by using loose equality, which treats both null and undefined as falsy:
Recommended fix
- {:else if $organization?.billingPlanDowngrade !== null}
+ {:else if $organization?.billingPlanDowngrade != null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/routes/`(console)/organization-[organization]/billing/planSummaryOld.svelte
around lines 186 - 192, The conditional currently uses a strict null check
($organization?.billingPlanDowngrade !== null) which treats undefined as valid;
change it to a loose null check ($organization?.billingPlanDowngrade != null) so
both null and undefined are treated the same as elsewhere in billing; update the
branch that renders the Cancel change Button (references:
$organization?.billingPlanDowngrade, isUpgrading, showCancel) to use != null for
consistency with +page.ts/+page.svelte checks.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/routes/`(console)/organization-[organization]/billing/planSummary.svelte:
- Around line 614-620: The "Cancel change" conditional and disable logic are
wrong: replace the loose null check in the template that uses
$organization?.billingPlanDowngrade !== null with a truthy check (e.g. {`#if`
$organization?.billingPlanDowngrade}) and add the same deletion guard to the
Button's disabled expression so it disables when
$organization?.markedForDeletion || isUpgrading (same pattern used by the other
branches), keeping the on:click and showCancel behavior unchanged.
| wrap="wrap" | ||
| class="u-width-full-line actions-mobile"> | ||
| <Tooltip disabled={!isUpgrading}> | ||
| <div> |
There was a problem hiding this comment.
instead of usual label in button, may be the label should also change to Change in progress or Payment processing or Upgrade is progress ?
2 participants
What does this PR do?
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
UI Improvements