Skip to content

feat: add site and function migration support#2882

Merged
abnegate merged 15 commits intomainfrom
feat-func-site-migration
Mar 5, 2026
Merged

feat: add site and function migration support#2882
abnegate merged 15 commits intomainfrom
feat-func-site-migration

Conversation

@premtsd-code
Copy link
Contributor

@premtsd-code premtsd-code commented Feb 24, 2026

Summary

  • Replace deprecated Resources enum with provider-specific migration resource enums (AppwriteMigrationResource, FirebaseMigrationResource, NHostMigrationResource, SupabaseMigrationResource)
  • Add support for new migration resource types: Site, Sitedeployment, Sitevariable, Environmentvariable
  • Add sites form group with "Include inactive deployments" toggle
  • Fix migrationFormToResources to conditionally include Deployment/Sitedeployment resources only when "Include inactive deployments" is toggled
  • Wire up users.teams toggle to actually map to Team/Membership resources
  • Fix child checkbox state dispatch — child toggles now properly update the form store instead of silently mutating props
  • Fix parent/child checkbox behavior — unchecking parent unchecks all children, but toggling a child does not affect the parent
Screencast.from.2026-03-03.23-01-04.webm

@appwrite
Copy link

appwrite bot commented Feb 24, 2026

Console (appwrite/console)

Project ID: 688b7bf400350cbd60e9

Sites (1)
Site Status Logs Preview QR
 console-stage
688b7cf6003b1842c9dc
Ready Ready View Logs Preview URL QR Code

Tip

Storage files get ClamAV malware scanning and encryption by default

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 24, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This 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 @appwrite.io/console package ref in package.json.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Key observations

  • package.json: @appwrite.io/console version ref updated.
  • Types/API:
    • New union MigrationResource and exported MigrationFormData.
    • Public signatures changed for providerResources, migrationFormToResources, resourcesToMigrationForm.
    • New exports: createMigrationFormStore, createMigrationProviderStore.
  • Stores/data:
    • initialFormData expanded: sites (root, inactive) and users.teams flag.
    • ProviderResourceMap introduced to map providers to their MigrationResource variants.
    • Conversion logic extended to handle functions (Function, EnvironmentVariable, Deployment) and sites (Site, SiteVariable, SiteDeployment).
  • UI:
    • resource-form.svelte and import report/wizard components updated to use migration enums and render a sites group; storage and functions groups have additional presence guards.
  • Other:
    • migrations list query now includes Query.orderDesc('$createdAt').
    • details.svelte uses optional chaining/nullish coalescing when reading ResourcesFriendly labels.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: adding site and function migration support, which are the primary features introduced across the migration wizard, import report, and resource handling.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-func-site-migration

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 24, 2026

Greptile Summary

This PR successfully adds site and function migration support by replacing the deprecated Resources enum with provider-specific migration resource enums. The implementation properly handles new resource types (Site, Sitedeployment, Sitevariable, Environmentvariable) and fixes several previously reported issues.

Key improvements:

  • Inactive deployment toggles now properly gate Deployment and Sitedeployment resources in migrationFormToResources
  • Teams toggle correctly maps to Team and Membership resources
  • Child checkbox state properly dispatches updates instead of using problematic bind: directives
  • Provider-specific type casts ensure type safety when calling migration APIs

Fixes from previous review:

  • Lines 124-126 and 131-133 in migration.ts now correctly check deploymentInactive before adding deployment resources (previously flagged issue resolved)
  • Line 103-106 in migration.ts properly adds Team/Membership when teams toggle is checked (previously flagged issue resolved)
  • Lines 115-119 in importReport.svelte fix child checkbox dispatch pattern (previously flagged issue resolved)

Confidence Score: 4/5

  • This PR is safe to merge with minor considerations for production deployment
  • Score reflects that the PR successfully addresses all previously reported issues and implements the new functionality correctly. The inactive deployment toggles, teams mapping, and checkbox state management are all fixed. The implementation follows existing codebase patterns and maintains type safety. Deducted one point due to the GitHub branch dependency that should be replaced with a versioned release before production deployment (as noted in previous review).
  • No files require special attention - all previously flagged issues have been resolved

Important Files Changed

Filename Overview
package.json Updates @appwrite.io/console dependency to support new migration resource enums
src/lib/stores/migration.ts Adds sites and functions migration support, fixes teams mapping and inactive deployment toggles
src/routes/(console)/(migration-wizard)/resource-form.svelte Adds rendering logic for functions and sites form groups
src/routes/(console)/project-[region]-[project]/settings/migrations/(import)/importReport.svelte Fixes child checkbox state management and adds sites form group labels

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
Loading

Last reviewed commit: ed4b380

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, 6 comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 24, 2026

Additional Comments (1)

src/routes/(console)/(migration-wizard)/resource-form.svelte
checks if provider-specific resource arrays include AppwriteMigrationResource enum values

When $provider.provider is firebase, supabase, or nhost, the resources array will contain values from those respective enums (e.g., FirebaseMigrationResource.Bucket), not AppwriteMigrationResource.Bucket. The comparison will always fail for non-Appwrite providers.

Need to either:

  1. Use a provider-agnostic approach (check for string values like 'bucket', 'file')
  2. Map enum values across providers
  3. Use the provider-specific enum based on $provider.provider

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between a41047e and a638592.

📒 Files selected for processing (4)
  • package.json
  • src/lib/stores/migration.ts
  • src/routes/(console)/(migration-wizard)/resource-form.svelte
  • src/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.
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 25, 2026

Additional Comments (1)

src/routes/(console)/(migration-wizard)/resource-form.svelte
only checks AppwriteMigrationResource enum when resources could be from any provider

resources is assigned from providerResources[$provider.provider] which could be Firebase/Supabase/NHost enums. Checking against AppwriteMigrationResource.Bucket etc. will fail if provider enums are distinct instances.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
src/lib/stores/migration.ts (2)

167-177: ⚠️ Potential issue | 🟠 Major

Populate env toggles when converting resources back to form state.

Environmentvariable and Sitevariable are not mapped to formData.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 | 🟠 Major

Respect functions.env and sites.env when building selected resources.

Line 105 and Line 112 currently add env resources whenever root is 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.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a638592 and f4470c6.

📒 Files selected for processing (1)
  • src/lib/stores/migration.ts

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
src/lib/stores/migration.ts (2)

103-113: ⚠️ Potential issue | 🟠 Major

Respect functions.env / sites.env when adding env resources.

EnvironmentVariable and SiteVariable are 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 | 🟠 Major

Map env resources back into form env toggles.

When hydrating form state, EnvironmentVariable and SiteVariable are 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.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4470c6 and 267984c.

📒 Files selected for processing (1)
  • src/lib/stores/migration.ts

- 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
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 | 🟠 Major

Avoid 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 in resourcesToMigrationForm can mutate module defaults and break reset() 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 | 🟠 Major

Inactive 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.inactive are 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

📥 Commits

Reviewing files that changed from the base of the PR and between 267984c and 0e5a0b8.

📒 Files selected for processing (4)
  • src/lib/stores/migration.ts
  • src/routes/(console)/project-[region]-[project]/settings/migrations/(import)/importReport.svelte
  • src/routes/(console)/project-[region]-[project]/settings/migrations/+page.ts
  • src/routes/(console)/project-[region]-[project]/settings/migrations/details.svelte

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/lib/stores/migration.ts (1)

113-121: ⚠️ Potential issue | 🟠 Major

Respect function/site sub-resource toggles in both conversion directions.

migrationFormToResources currently auto-adds env/deployment resources when root is enabled, and resourcesToMigrationForm does 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0e5a0b8 and 0ffbbcc.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • package.json
  • src/lib/stores/migration.ts
  • src/routes/(console)/(migration-wizard)/resource-form.svelte
  • src/routes/(console)/(migration-wizard)/wizard.svelte
  • src/routes/(console)/project-[region]-[project]/settings/migrations/(import)/wizard.svelte

Query.isNull('destination')
])
]),
Query.orderDesc('$createdAt')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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('')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty string won't sort migrations. Should be '$createdAt' to show newest first

Suggested change
Query.orderDesc('')
Query.orderDesc('$createdAt')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty string will sort migrations by primary key.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 4, 2026

Additional Comments (1)

src/routes/(console)/project-[region]-[project]/settings/migrations/(import)/importReport.svelte
Button click handler and bind:checked both mutate prop directly. Remove bind:checked (like child checkboxes do) and remove button's on:click

        <Button extraCompact>
            <Layout.Stack direction="row" gap="s" alignItems="center">
                <Layout.Stack inline direction="row" gap="l" alignItems="flex-start">
                    <Selector.Checkbox
                        size="s"
                        checked={formGroup.root}
                        on:change={(event) => {
                            const updated = { ...formGroup, root: event.detail };
                            dispatch('updateFormGroup', updated);
                        }} />

@abnegate abnegate merged commit 6138b73 into main Mar 5, 2026
5 checks passed
@abnegate abnegate deleted the feat-func-site-migration branch March 5, 2026 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants