feat: add site and function migration support#2882
Conversation
Console (appwrite/console)Project ID: Tip Storage files get ClamAV malware scanning and encryption by default |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR replaces a single Resources enum with provider-specific migration enums (AppwriteMigrationResource, FirebaseMigrationResource, NHostMigrationResource, SupabaseMigrationResource), introduces a MigrationResource union and exported MigrationFormData, expands initialFormData (adds sites and users.teams), adds createMigrationFormStore and createMigrationProviderStore, updates conversion functions (migrationFormToResources, resourcesToMigrationForm) to use provider-specific resource types, adjusts UI components to render a new sites group and guard storage/functions rendering, updates a migrations query to order by createdAt, and bumps the Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Key observations
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR successfully adds site and function migration support by replacing the deprecated Key improvements:
Fixes from previous review:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User selects resources in form] --> B[MigrationFormData]
B --> |users.root + users.teams| C[User, Team, Membership]
B --> |databases.root + databases.rows| D[Database, Table, Column, Index, Row]
B --> |storage.root| E[Bucket, File]
B --> |functions.root + functions.deploymentInactive| F[Function, Environmentvariable, Deployment]
B --> |sites.root + sites.deploymentInactive| G[Site, Sitevariable, Sitedeployment]
C --> H[migrationFormToResources]
D --> H
E --> H
F --> H
G --> H
H --> I{Provider?}
I --> |appwrite| J[AppwriteMigrationResource array]
I --> |firebase| K[FirebaseMigrationResource array]
I --> |supabase| L[SupabaseMigrationResource array]
I --> |nhost| M[NHostMigrationResource array]
J --> N[createAppwriteMigration API]
K --> O[createFirebaseMigration API]
L --> P[createSupabaseMigration API]
M --> Q[createNHostMigration API]
style F fill:#e1f5ff
style G fill:#e1f5ff
style H fill:#fff4e1
Last reviewed commit: ed4b380 |
Additional Comments (1)
When Need to either:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/lib/stores/migration.ts`:
- Around line 167-178: The migration->form conversion currently maps
Function/Deployment/Site/SiteDeployment to formData.functions.root/inactive and
formData.sites.root/inactive but misses mapping EnvironmentVariable and
SiteVariable so the UI env toggles remain off; add checks for
AppwriteMigrationResource.EnvironmentVariable and
AppwriteMigrationResource.SiteVariable and set the corresponding formData
toggles (e.g. set formData.functions.env = true when EnvironmentVariable is
present and set formData.sites.env = true when SiteVariable is present)
alongside the existing mappings in the same block that references
AppwriteMigrationResource.Function/Deployment/Site/SiteDeployment.
- Around line 79-116: The code always adds environment variable resources when
functions.root or sites.root is true, ignoring the specific env toggles; update
the resource-building logic so MigrationResource.EnvironmentVariable is only
added when both formData.functions.root and formData.functions.env are true
(refer to formData.functions and MigrationResource.EnvironmentVariable), and
likewise only add MigrationResource.SiteVariable when both formData.sites.root
and formData.sites.env are true (refer to formData.sites and
MigrationResource.SiteVariable); keep the existing
providerResources[provider].includes(...) guard and use the same
addResource(...) helper.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.jsonsrc/lib/stores/migration.tssrc/routes/(console)/(migration-wizard)/resource-form.sveltesrc/routes/(console)/project-[region]-[project]/settings/migrations/(import)/importReport.svelte
Update ResourcesFriendly map keys and AppwriteMigrationResource enum references to use kebab-case values (environment-variable, site-deployment, site-variable) matching the utopia-migration library.
Additional Comments (1)
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/lib/stores/migration.ts (2)
167-177:⚠️ Potential issue | 🟠 MajorPopulate env toggles when converting resources back to form state.
EnvironmentvariableandSitevariableare not mapped toformData.functions.env/formData.sites.env, so UI toggles can render unchecked even when those resources are selected.🐛 Proposed fix
if (resources.includes(AppwriteMigrationResource.Function)) { formData.functions.root = true; } + if (resources.includes(AppwriteMigrationResource.Environmentvariable)) { + formData.functions.env = true; + } if (resources.includes(AppwriteMigrationResource.Deployment)) { formData.functions.inactive = true; } if (resources.includes(AppwriteMigrationResource.Site)) { formData.sites.root = true; } + if (resources.includes(AppwriteMigrationResource.Sitevariable)) { + formData.sites.env = true; + } if (resources.includes(AppwriteMigrationResource.Sitedeployment)) { formData.sites.inactive = true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/stores/migration.ts` around lines 167 - 177, When converting selected resources back into form state in migration.ts, the code sets formData.functions.root/inactive and formData.sites.root/inactive but omits mapping AppwriteMigrationResource.Environmentvariable and AppwriteMigrationResource.Sitevariable; add checks like if (resources.includes(AppwriteMigrationResource.Environmentvariable)) { formData.functions.env = true } and if (resources.includes(AppwriteMigrationResource.Sitevariable)) { formData.sites.env = true } so the UI toggles for functions.env and sites.env reflect selected resources.
103-113:⚠️ Potential issue | 🟠 MajorRespect
functions.envandsites.envwhen building selected resources.Line 105 and Line 112 currently add env resources whenever
rootis enabled, so env opt-outs are ignored.🐛 Proposed fix
if (formData.functions.root) { addResource(AppwriteMigrationResource.Function); - addResource(AppwriteMigrationResource.Environmentvariable); + } + if (formData.functions.root && formData.functions.env) { + addResource(AppwriteMigrationResource.Environmentvariable); } if (formData.functions.inactive) { addResource(AppwriteMigrationResource.Deployment); } if (formData.sites.root) { addResource(AppwriteMigrationResource.Site); - addResource(AppwriteMigrationResource.Sitevariable); + } + if (formData.sites.root && formData.sites.env) { + addResource(AppwriteMigrationResource.Sitevariable); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/stores/migration.ts` around lines 103 - 113, The migration resource builder currently adds Environmentvariable and Sitevariable unconditionally when formData.functions.root or formData.sites.root are true, ignoring the user's env opt-out; update the logic in the block that calls addResource (e.g., where formData.functions.root and formData.sites.root are checked) to also check formData.functions.env and formData.sites.env respectively before calling addResource(AppwriteMigrationResource.Environmentvariable) and addResource(AppwriteMigrationResource.Sitevariable), so env resources are only added when the corresponding .env flag is true.
🤖 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/lib/stores/migration.ts`:
- Around line 167-177: When converting selected resources back into form state
in migration.ts, the code sets formData.functions.root/inactive and
formData.sites.root/inactive but omits mapping
AppwriteMigrationResource.Environmentvariable and
AppwriteMigrationResource.Sitevariable; add checks like if
(resources.includes(AppwriteMigrationResource.Environmentvariable)) {
formData.functions.env = true } and if
(resources.includes(AppwriteMigrationResource.Sitevariable)) {
formData.sites.env = true } so the UI toggles for functions.env and sites.env
reflect selected resources.
- Around line 103-113: The migration resource builder currently adds
Environmentvariable and Sitevariable unconditionally when
formData.functions.root or formData.sites.root are true, ignoring the user's env
opt-out; update the logic in the block that calls addResource (e.g., where
formData.functions.root and formData.sites.root are checked) to also check
formData.functions.env and formData.sites.env respectively before calling
addResource(AppwriteMigrationResource.Environmentvariable) and
addResource(AppwriteMigrationResource.Sitevariable), so env resources are only
added when the corresponding .env flag is true.
This reverts commit f4470c6.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/lib/stores/migration.ts (2)
103-113:⚠️ Potential issue | 🟠 MajorRespect
functions.env/sites.envwhen adding env resources.
EnvironmentVariableandSiteVariableare currently added whenever root is on, so the env checkboxes don’t control output. This breaks user intent.Suggested fix
if (formData.functions.root) { addResource(AppwriteMigrationResource.Function); - addResource(AppwriteMigrationResource.EnvironmentVariable); + } + if (formData.functions.env) { + addResource(AppwriteMigrationResource.EnvironmentVariable); } if (formData.functions.inactive) { addResource(AppwriteMigrationResource.Deployment); } if (formData.sites.root) { addResource(AppwriteMigrationResource.Site); - addResource(AppwriteMigrationResource.SiteVariable); + } + if (formData.sites.env) { + addResource(AppwriteMigrationResource.SiteVariable); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/stores/migration.ts` around lines 103 - 113, The code currently unconditionally adds env resources when formData.functions.root or formData.sites.root are true; change the logic in the block that calls addResource (look for addResource and AppwriteMigrationResource.EnvironmentVariable / AppwriteMigrationResource.SiteVariable) to also check formData.functions.env and formData.sites.env respectively before adding EnvironmentVariable or SiteVariable so those variables are only added when both the root and the corresponding env checkbox are enabled; leave existing additions of Function, Deployment, and Site unchanged.
167-178:⚠️ Potential issue | 🟠 MajorMap env resources back into form env toggles.
When hydrating form state,
EnvironmentVariableandSiteVariableare not mapped, so env toggles won’t reflect selected resources.Suggested fix
if (resources.includes(AppwriteMigrationResource.Function)) { formData.functions.root = true; } + if (resources.includes(AppwriteMigrationResource.EnvironmentVariable)) { + formData.functions.env = true; + } if (resources.includes(AppwriteMigrationResource.Deployment)) { formData.functions.inactive = true; } if (resources.includes(AppwriteMigrationResource.Site)) { formData.sites.root = true; } + if (resources.includes(AppwriteMigrationResource.SiteVariable)) { + formData.sites.env = true; + } if (resources.includes(AppwriteMigrationResource.SiteDeployment)) { formData.sites.inactive = true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/stores/migration.ts` around lines 167 - 178, The hydration block currently maps Function/Deployment and Site/SiteDeployment but misses EnvironmentVariable and SiteVariable, so env toggles don't reflect selected resources; add checks for AppwriteMigrationResource.EnvironmentVariable and AppwriteMigrationResource.SiteVariable and set the corresponding formData env toggles (e.g., set formData.env.root = true for EnvironmentVariable and formData.env.inactive = true for SiteVariable) alongside the existing mappings in the same block so the form's env toggles reflect selected resources.
🤖 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/lib/stores/migration.ts`:
- Around line 103-113: The code currently unconditionally adds env resources
when formData.functions.root or formData.sites.root are true; change the logic
in the block that calls addResource (look for addResource and
AppwriteMigrationResource.EnvironmentVariable /
AppwriteMigrationResource.SiteVariable) to also check formData.functions.env and
formData.sites.env respectively before adding EnvironmentVariable or
SiteVariable so those variables are only added when both the root and the
corresponding env checkbox are enabled; leave existing additions of Function,
Deployment, and Site unchanged.
- Around line 167-178: The hydration block currently maps Function/Deployment
and Site/SiteDeployment but misses EnvironmentVariable and SiteVariable, so env
toggles don't reflect selected resources; add checks for
AppwriteMigrationResource.EnvironmentVariable and
AppwriteMigrationResource.SiteVariable and set the corresponding formData env
toggles (e.g., set formData.env.root = true for EnvironmentVariable and
formData.env.inactive = true for SiteVariable) alongside the existing mappings
in the same block so the form's env toggles reflect selected resources.
- Fix SDK enum casing (Environmentvariable, Sitedeployment, Sitevariable) - Use kebab-case keys in ResourcesFriendly to match backend values - Always include deployments when function/site root is checked - Remove environment variables checkbox from migration form - Add optional chaining for unknown resource types in details view - Sort migrations list by newest first
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/stores/migration.ts (1)
37-40:⚠️ Potential issue | 🟠 MajorAvoid shallow-copying
initialFormData(nested state gets mutated globally).Line 37 and Line 137 clone only the top-level object. Nested groups (
users,databases,functions,storage,sites) remain shared references, so writes inresourcesToMigrationFormcan mutate module defaults and breakreset()determinism.Proposed fix
+const cloneInitialFormData = (): MigrationFormData => ({ + users: { ...initialFormData.users }, + databases: { ...initialFormData.databases }, + functions: { ...initialFormData.functions }, + storage: { ...initialFormData.storage }, + sites: { ...initialFormData.sites } +}); + export const createMigrationFormStore = () => { - const store = writable({ ...initialFormData }); + const store = writable(cloneInitialFormData()); const reset = () => { - store.set({ ...initialFormData }); + store.set(cloneInitialFormData()); }; @@ export const resourcesToMigrationForm = (resources: MigrationResource[]): MigrationFormData => { - const formData = { ...initialFormData }; + const formData = cloneInitialFormData();Also applies to: 136-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/stores/migration.ts` around lines 37 - 40, The code shallow-copies initialFormData into the Svelte writable (store = writable({ ...initialFormData })) and in reset() (store.set({ ...initialFormData })), leaving nested objects (users, databases, functions, storage, sites) shared and vulnerable to mutation; replace the shallow clone with a deep clone (e.g., use structuredClone(initialFormData) or a deepClone utility) both when initializing store and inside reset(), and apply the same deep-clone fix to the other occurrence around resourcesToMigrationForm so nested state is isolated per-instance.
♻️ Duplicate comments (1)
src/lib/stores/migration.ts (1)
101-110:⚠️ Potential issue | 🟠 MajorInactive deployment toggles are currently ignored in serialization.
Line 104 and Line 109 always add deployment resources when root is enabled, but
formData.functions.inactive/formData.sites.inactiveare never read. This makes the inactive checkboxes effectively no-op.Proposed fix
if (formData.functions.root) { addResource(AppwriteMigrationResource.Function); addResource(AppwriteMigrationResource.Environmentvariable); - addResource(AppwriteMigrationResource.Deployment); + if (formData.functions.inactive) { + addResource(AppwriteMigrationResource.Deployment); + } } if (formData.sites.root) { addResource(AppwriteMigrationResource.Site); addResource(AppwriteMigrationResource.Sitevariable); - addResource(AppwriteMigrationResource.Sitedeployment); + if (formData.sites.inactive) { + addResource(AppwriteMigrationResource.Sitedeployment); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/stores/migration.ts` around lines 101 - 110, The code always adds Deployment and Sitedeployment when functions.root or sites.root is true, ignoring the inactive checkboxes; change the logic so addResource(AppwriteMigrationResource.Deployment) is only called when formData.functions.inactive is true (not just functions.root), and likewise call addResource(AppwriteMigrationResource.Sitedeployment) only when formData.sites.inactive is true; keep the existing additions for Function/Environmentvariable and Site/Sitevariable intact and only gate the deployment-related addResource calls on the corresponding .inactive flags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/lib/stores/migration.ts`:
- Around line 37-40: The code shallow-copies initialFormData into the Svelte
writable (store = writable({ ...initialFormData })) and in reset() (store.set({
...initialFormData })), leaving nested objects (users, databases, functions,
storage, sites) shared and vulnerable to mutation; replace the shallow clone
with a deep clone (e.g., use structuredClone(initialFormData) or a deepClone
utility) both when initializing store and inside reset(), and apply the same
deep-clone fix to the other occurrence around resourcesToMigrationForm so nested
state is isolated per-instance.
---
Duplicate comments:
In `@src/lib/stores/migration.ts`:
- Around line 101-110: The code always adds Deployment and Sitedeployment when
functions.root or sites.root is true, ignoring the inactive checkboxes; change
the logic so addResource(AppwriteMigrationResource.Deployment) is only called
when formData.functions.inactive is true (not just functions.root), and likewise
call addResource(AppwriteMigrationResource.Sitedeployment) only when
formData.sites.inactive is true; keep the existing additions for
Function/Environmentvariable and Site/Sitevariable intact and only gate the
deployment-related addResource calls on the corresponding .inactive flags.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lib/stores/migration.tssrc/routes/(console)/project-[region]-[project]/settings/migrations/(import)/importReport.sveltesrc/routes/(console)/project-[region]-[project]/settings/migrations/+page.tssrc/routes/(console)/project-[region]-[project]/settings/migrations/details.svelte
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/lib/stores/migration.ts (1)
113-121:⚠️ Potential issue | 🟠 MajorRespect function/site sub-resource toggles in both conversion directions.
migrationFormToResourcescurrently auto-adds env/deployment resources whenrootis enabled, andresourcesToMigrationFormdoes not restore env-specific selections. This collapses user intent and can mis-sync the wizard state.🐛 Proposed fix
const initialFormData = { users: { root: false, teams: false }, databases: { root: false, rows: false }, functions: { root: false, + env: false, inactive: false }, storage: { root: false }, sites: { root: false, + env: false, inactive: false } }; @@ if (formData.functions.root) { addResource(AppwriteMigrationResource.Function); + } + if (formData.functions.env) { addResource(AppwriteMigrationResource.Environmentvariable); + } + if (formData.functions.inactive) { addResource(AppwriteMigrationResource.Deployment); } if (formData.sites.root) { addResource(AppwriteMigrationResource.Site); + } + if (formData.sites.env) { addResource(AppwriteMigrationResource.Sitevariable); + } + if (formData.sites.inactive) { addResource(AppwriteMigrationResource.Sitedeployment); } @@ if (resources.includes(AppwriteMigrationResource.Function)) { formData.functions.root = true; } + if (resources.includes(AppwriteMigrationResource.Environmentvariable)) { + formData.functions.env = true; + } if (resources.includes(AppwriteMigrationResource.Deployment)) { formData.functions.inactive = true; } if (resources.includes(AppwriteMigrationResource.Site)) { formData.sites.root = true; } + if (resources.includes(AppwriteMigrationResource.Sitevariable)) { + formData.sites.env = true; + } if (resources.includes(AppwriteMigrationResource.Sitedeployment)) { formData.sites.inactive = true; }Also applies to: 173-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/stores/migration.ts` around lines 113 - 121, migrationFormToResources currently unconditionally adds Environmentvariable and Deployment (and Sitevariable/Sitedeployment) whenever functions.root or sites.root is true, and resourcesToMigrationForm doesn't restore those env-specific toggles; update migrationFormToResources to add AppwriteMigrationResource.Environmentvariable and AppwriteMigrationResource.Deployment only if the corresponding function-level sub-toggles (e.g., functions.env and functions.deployment) are true (similarly for sites.sitevariable/site deployment), and update resourcesToMigrationForm to map presence of AppwriteMigrationResource.Environmentvariable, AppwriteMigrationResource.Deployment, AppwriteMigrationResource.Sitevariable and AppwriteMigrationResource.Sitedeployment back into the migration form object so the sub-resource checkboxes are preserved when converting resources -> form (affecting both migrationFormToResources and resourcesToMigrationForm, also apply same logic to the other similar block referenced around the other occurrence).
🤖 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/lib/stores/migration.ts`:
- Around line 113-121: migrationFormToResources currently unconditionally adds
Environmentvariable and Deployment (and Sitevariable/Sitedeployment) whenever
functions.root or sites.root is true, and resourcesToMigrationForm doesn't
restore those env-specific toggles; update migrationFormToResources to add
AppwriteMigrationResource.Environmentvariable and
AppwriteMigrationResource.Deployment only if the corresponding function-level
sub-toggles (e.g., functions.env and functions.deployment) are true (similarly
for sites.sitevariable/site deployment), and update resourcesToMigrationForm to
map presence of AppwriteMigrationResource.Environmentvariable,
AppwriteMigrationResource.Deployment, AppwriteMigrationResource.Sitevariable and
AppwriteMigrationResource.Sitedeployment back into the migration form object so
the sub-resource checkboxes are preserved when converting resources -> form
(affecting both migrationFormToResources and resourcesToMigrationForm, also
apply same logic to the other similar block referenced around the other
occurrence).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
package.jsonsrc/lib/stores/migration.tssrc/routes/(console)/(migration-wizard)/resource-form.sveltesrc/routes/(console)/(migration-wizard)/wizard.sveltesrc/routes/(console)/project-[region]-[project]/settings/migrations/(import)/wizard.svelte
| Query.isNull('destination') | ||
| ]) | ||
| ]), | ||
| Query.orderDesc('$createdAt') |
There was a problem hiding this comment.
Let's use orderDesc() with no params instead. That will default to $sequence, which will be faster to order on. Do we have this orderDesc(createdAt) elsewhere?
| Query.isNull('destination') | ||
| ]) | ||
| ]), | ||
| Query.orderDesc('') |
There was a problem hiding this comment.
Empty string won't sort migrations. Should be '$createdAt' to show newest first
| Query.orderDesc('') | |
| Query.orderDesc('$createdAt') |
There was a problem hiding this comment.
Empty string will sort migrations by primary key.
Additional Comments (1)
|

Summary
Resourcesenum with provider-specific migration resource enums (AppwriteMigrationResource,FirebaseMigrationResource,NHostMigrationResource,SupabaseMigrationResource)Site,Sitedeployment,Sitevariable,Environmentvariablesitesform group with "Include inactive deployments" togglemigrationFormToResourcesto conditionally includeDeployment/Sitedeploymentresources only when "Include inactive deployments" is toggledusers.teamstoggle to actually map toTeam/MembershipresourcesScreencast.from.2026-03-03.23-01-04.webm