Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds parent-resource status checks in the Appwrite migration destination: before importing env vars, deployments, site env vars, and site deployments the code validates parent function/site status and throws exceptions when parent status is Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 |
Use the resources array to determine whether to export only the active deployment or all deployments. When deployment/site-deployment resource type is included, export all deployments instead of only the active one.
PHP booleans become '1'/'' in multipart/form-data but the API expects 'true'/'false' strings. The non-chunked function deployment path was already correct; this fixes the chunked function deployment and both site deployment paths.
…stination When a function or site already exists in the destination, the API returns 409 and the parent resource is marked as error. Previously, deployments for that parent would still be imported, creating duplicates. Now importDeployment and importSiteDeployment check the parent status and throw an error if it failed.
premtsd-code
force-pushed
the
fix-export-inactive-deployments
branch
from
589dd29 to
928e7c8
Compare
![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: 1
🤖 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/Migration/Sources/Appwrite.php`:
- Around line 1521-1522: $exportOnlyActive is being set when
Resource::TYPE_DEPLOYMENT is not requested but currently code falls back to
paginated listing and exports all deployments for resources (functions/sites)
that lack an active deployment; change the callers and exportDeployments logic
so that when $exportOnlyActive is true you only export deployments explicitly
referenced by an active deployment ID and never fall back to the full paginated
export: detect resources (functions/sites) with no activeDeploymentId and skip
exporting their deployments (do not call the paginated listing), and ensure the
same change is applied to the second occurrence around the lines mentioned (the
other $exportOnlyActive usage).
Comment on lines
+1521
to
+1522
| $exportOnlyActive = !\in_array(Resource::TYPE_DEPLOYMENT, $resources); | ||
| $this->exportDeployments($batchSize, $exportOnlyActive); |
Contributor
There was a problem hiding this comment.
$exportOnlyActive mode can still export all deployments when no active deployment exists.
When Line 1521 / Line 1551 set active-only mode, functions/sites without an active deployment ID currently fall through to paginated listing and export all deployments. That breaks the include-inactive contract for this edge case.
🔧 Proposed fix
--- a/src/Migration/Sources/Appwrite.php
+++ b/src/Migration/Sources/Appwrite.php
@@
- if ($exportOnlyActive && $func->getActiveDeployment()) {
- $deployment = $this->functions->getDeployment($func->getId(), $func->getActiveDeployment());
+ if ($exportOnlyActive) {
+ $activeDeploymentId = $func->getActiveDeployment();
+ if (empty($activeDeploymentId)) {
+ continue; // active-only mode: nothing to export for this function
+ }
+
+ $deployment = $this->functions->getDeployment($func->getId(), $activeDeploymentId);
@@
continue;
}
@@
- if ($exportOnlyActive && $site->getActiveDeployment()) {
- $deployment = $this->sites->getDeployment($site->getId(), $site->getActiveDeployment());
+ if ($exportOnlyActive) {
+ $activeDeploymentId = $site->getActiveDeployment();
+ if (empty($activeDeploymentId)) {
+ continue; // active-only mode: nothing to export for this site
+ }
+
+ $deployment = $this->sites->getDeployment($site->getId(), $activeDeploymentId);
@@
continue;
}Also applies to: 1551-1552
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Migration/Sources/Appwrite.php` around lines 1521 - 1522,
$exportOnlyActive is being set when Resource::TYPE_DEPLOYMENT is not requested
but currently code falls back to paginated listing and exports all deployments
for resources (functions/sites) that lack an active deployment; change the
callers and exportDeployments logic so that when $exportOnlyActive is true you
only export deployments explicitly referenced by an active deployment ID and
never fall back to the full paginated export: detect resources (functions/sites)
with no activeDeploymentId and skip exporting their deployments (do not call the
paginated listing), and ensure the same change is applied to the second
occurrence around the lines mentioned (the other $exportOnlyActive usage).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
activateparam to string'true'/'false'for multipart form-data requestsTest plan
Summary by CodeRabbit