-
Notifications
You must be signed in to change notification settings - Fork 198
Fix: refresh when organization dependency change #2588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
before: Invalidating organization was not refreshing the page because it kept local copy of organization now: Invalidating organization correctly refreshes the page
Console (appwrite/console)Project ID: Tip Roll back Sites deployments instantly by switching between versions |
WalkthroughA single file was modified to make the organization variable reactive using Svelte's reactive statement syntax ( Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/routes/(console)/organization-[organization]/billing/+page.svelte (1)
147-154: Consider using the reactiveorganizationvariable for consistency.These child components receive
data?.organizationinstead of the reactiveorganizationvariable defined at line 30. While functionally equivalent (both are reactive), usingorganizationconsistently throughout the component would improve code clarity.Apply this diff to use the reactive variable consistently:
- <PaymentMethods organization={data?.organization} methods={data?.paymentMethods} /> + <PaymentMethods {organization} methods={data?.paymentMethods} /> <BillingAddress - organization={data?.organization} + {organization} billingAddress={data?.billingAddress} locale={data?.locale} countryList={data?.countryList} /> <TaxId /> - <BudgetCap organization={data?.organization} currentPlan={data?.currentPlan} /> + <BudgetCap {organization} currentPlan={data?.currentPlan} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/organization-[organization]/billing/+page.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte:54-58
Timestamp: 2025-10-13T05:16:07.656Z
Learning: In SvelteKit apps, shared layout components (like headers) that use `$derived(page.data.*)` should use optional chaining when accessing properties that may not be present on all routes. During page transitions, reactive statements can briefly evaluate with different page.data structures, so optional chaining prevents runtime errors when navigating between routes with different data shapes (e.g., between `/databases` and `/databases/database-[database]`).
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/organization-[organization]/billing/+page.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: e2e
- GitHub Check: build
🔇 Additional comments (2)
src/routes/(console)/organization-[organization]/billing/+page.svelte (2)
29-30: LGTM! Correctly fixes the reactivity issue.The reactive statement ensures
organizationupdates wheneverdata.organizationchanges, which will cascade to dependent reactive properties likedefaultPaymentMethodandbackupPaymentMethod. This correctly addresses the issue where invalidating the organization dependency didn't refresh the page.
32-39: The comment now has clear context.The comment questioning why these are reactive is now answered by the change at line 30. These reactive statements need to be reactive because they depend on
organization.paymentMethodIdandorganization.backupPaymentMethodId, andorganizationitself is now reactive. Whendata.organizationchanges, these derived values will correctly recompute.

What does this PR do?
Fix: refresh billing page when organization dependency change
before: Invalidating organization was not refreshing the page because it kept local copy of organization
now: Invalidating organization correctly refreshes the page
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?
YES
Summary by CodeRabbit