fix: use plan capabilities instead of hardcoded plan checks for backup policy #2631
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Console (appwrite/console)Project ID: Tip GraphQL API works alongside REST and WebSocket protocols |
Contributor
WalkthroughThis PR updates three Svelte UI files to replace direct billing-plan checks with a new Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte (1)
23-24: Plan-capability checks via$currentPlan.backupPolicieslook correct; clarify edge cases and comment
- Importing
currentPlanand driving behavior off$currentPlan?.backupPolicies(1 → Pro “daily-only” flow, >1 → Scale+ full presets/custom) achieves the goal of removing hardcoded BillingPlan checks and aligns with how the UI is structured here.- The reactive pre-check for Pro (
if ($currentPlan?.backupPolicies === 1 && isFromBackupsTab)) is consistent with the Pro path below, but the comment “pre-check the hourly if on pro plan” no longer matches the implementation (it checkspolicy.label === 'Daily'). Consider updating the comment to avoid confusion.- For plans where
backupPoliciesmight be0orundefinedbut backups are still technically enabled, this component will fall into the “Scale+” ({:else}) branch and show the full custom-policy UI. If such configurations are possible, it may be worth explicitly handling0/undefined(e.g., by gating onbackupPolicies >= 2or by avoiding rendering untilcurrentPlanis fully resolved) so we don’t accidentally expose UI that the backend rejects.Also applies to: 139-160, 178-195, 197-239
src/routes/(console)/project-[region]-[project]/databases/create.svelte (1)
10-11: Policy-creation error handling and$currentPlan.backupsEnabledgating are solid; consider initial-plan state
- The inner
try { await createPolicies(databaseId); } catch (policyError) { ... }is a good refinement: database creation no longer fails if policy creation does, but users still get a clear warning and error telemetry viatrackError. The fallback message also makes it explicit that the database was created successfully.- Switching the Cloud-only backups UI to
{#if !$currentPlan?.backupsEnabled} … {:else} <CreatePolicy … /> {/if}correctly ties visibility to plan capabilities rather than BillingPlan constants, andcreatePolicies’ early-return on an emptytotalPolicieskeeps the unconditional call safe.- One subtle edge case: when the component first renders and
$currentPlanis stillundefined(if the store is hydrated asynchronously),!$currentPlan?.backupsEnabledevaluates totrue, so paying users might briefly see the “This database won't be backed up” warning before the store updates. If that’s observable in practice, consider guarding on a dedicated “plan loaded” flag or a defaultcurrentPlanvalue so the alert only appears once the plan is known.Also applies to: 80-101, 143-158
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/routes/(console)/project-[region]-[project]/databases/create.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/containerHeader.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/containerHeader.svelte
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/containerHeader.sveltesrc/routes/(console)/project-[region]-[project]/databases/create.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 (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/containerHeader.svelte (1)
51-67: UsingmaxPolicies === 1as the switch for Tag vs Badge looks consistentThe new condition that treats
maxPolicies === 1as the “single-policy plan” path and falls back to the Badge for all other values keeps the UI behavior simple and decouples it from specific plan enums. As long as parents reliably passmaxPoliciesaccording to the plan capabilities (1 for Pro, >1 for Scale, etc.), this should behave identically to the previous plan-based logic.
ItzNotABug
requested changes
Nov 22, 2025
Comment on lines
93
to
100
| addNotification({ | ||
| type: 'warning', | ||
| message: | ||
| policyError.message || | ||
| 'Failed to create backup policies. The database was created successfully.' | ||
| }); | ||
| trackError(policyError, Submit.DatabaseCreate); | ||
| } |
Member
There was a problem hiding this comment.
I doubt this would if proper plan based are already added above.
ItzNotABug
approved these changes
Nov 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.