Add platform migration support#154
Merged
Merged
Conversation
Contributor
|
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:
WalkthroughAdds platform/integration support across the migration system. Introduces Resource::TYPE_PLATFORM and a new Platform resource class. Renames Transfer group GROUP_SETTINGS → GROUP_INTEGRATIONS and registers platform resources in public/root lists. Extends Source with getIntegrationsBatchSize and abstract exportGroupIntegrations; Appwrite Source adds console-key fetching/caching (setConsoleKey, getConsoleHeaders), reporting and export flows for platforms. Appwrite Destination accepts a dbForPlatform/projectInternalId, adds importIntegrationsResource and createPlatform (duplicate checks, timestamp preservation, cache purge). CSV, Firebase, JSON, NHost add stub exportGroupIntegrations methods. Tests/mocks updated. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
📝 Coding Plan
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.
🧹 Nitpick comments (3)
src/Migration/Sources/Firebase.php (1)
816-819: Minor: Inconsistent exception message casing.The exception message uses
'Not implemented'(lowercase 'i'), while other sources (CSV, JSON, NHost) use'Not Implemented'(uppercase 'I'). Consider standardizing for consistency.Proposed fix
protected function exportGroupIntegrations(int $batchSize, array $resources): void { - throw new \Exception('Not implemented'); + throw new \Exception('Not Implemented'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Migration/Sources/Firebase.php` around lines 816 - 819, The exception message in Firebase::exportGroupIntegrations currently uses 'Not implemented'; update the throw in the protected method exportGroupIntegrations(int $batchSize, array $resources) to throw the same canonical message 'Not Implemented' used by other sources so the exception casing is consistent across implementations (locate the throw new \Exception(...) inside exportGroupIntegrations and change the string).src/Migration/Sources/Appwrite.php (2)
2065-2073: Consider implementing pagination using$batchSizeparameter.The
$batchSizeparameter is unused andexportPlatformsfetches all platforms in a single API call. Other export methods implement pagination for scalability. If the platforms API supports pagination, consider adding it:-private function exportPlatforms(array $consoleHeaders): void +private function exportPlatforms(array $consoleHeaders, int $batchSize = 25): void { - $response = $this->call('GET', '/projects/' . $this->project . '/platforms', $consoleHeaders); + $lastPlatform = null; + + while (true) { + $queries = [Query::limit($batchSize)]; + if ($lastPlatform) { + $queries[] = Query::cursorAfter($lastPlatform); + } + + $response = $this->call('GET', '/projects/' . $this->project . '/platforms?' . http_build_query(['queries' => $queries]), $consoleHeaders); + // ... process platforms ... + + if (count($response['platforms']) < $batchSize) { + break; + } + }If the API doesn't support pagination or platform counts are expected to be small, consider documenting this or removing
$batchSizefrom the method signature for clarity.Also applies to: 2105-2129
🤖 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 2065 - 2073, exportGroupIntegrations currently ignores the $batchSize parameter and calls exportPlatforms which fetches all platforms at once; update exportPlatforms (and/or the call site in exportGroupIntegrations) to implement pageable fetching using $batchSize (e.g., loop with offset/page tokens until no more results) and yield/return batches compatible with exportWithConsoleHeaders, or if the Appwrite platforms API has no pagination, remove $batchSize from exportGroupIntegrations’ signature or add a code comment documenting the lack of pagination; reference the exportGroupIntegrations method, exportPlatforms function, and exportWithConsoleHeaders helper when making the change.
2043-2058: Consider using$resourceIdsparameter for consistency.The
$resourceIdsparameter is unused (as flagged by static analysis). Otherreport*methods likereportAuth,reportStorage, etc. use this parameter to filter results. For consistency, consider adding filtering support:private function reportIntegrations(array $resources, array &$report, array $resourceIds = []): void { if (\in_array(Resource::TYPE_PLATFORM, $resources)) { $consoleHeaders = $this->getConsoleHeaders(); if ($consoleHeaders === null) { return; } try { - $response = $this->call('GET', '/projects/' . $this->project . '/platforms', $consoleHeaders); + $queries = []; + if (!empty($resourceIds[Resource::TYPE_PLATFORM])) { + // Note: Add filtering if the API supports it + } + $response = $this->call('GET', '/projects/' . $this->project . '/platforms', $consoleHeaders); $report[Resource::TYPE_PLATFORM] = $response['total'] ?? 0;Alternatively, remove the unused parameter if filtering isn't applicable for platforms.
🤖 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 2043 - 2058, The reportIntegrations method currently ignores the $resourceIds parameter; either use it to filter the platforms count or remove the unused parameter. To fix, update reportIntegrations (method name: reportIntegrations) to apply $resourceIds when calling the platforms endpoint (e.g. include IDs or a filter in the GET to '/projects/{project}/platforms' and compute $report[Resource::TYPE_PLATFORM] from the filtered response) or delete the $resourceIds parameter from the method signature and all callers if platform filtering is not supported; ensure the try/catch and assignment to $report[Resource::TYPE_PLATFORM] remain consistent with other report* methods (e.g., reportAuth/reportStorage) when implementing the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Migration/Sources/Appwrite.php`:
- Around line 2065-2073: exportGroupIntegrations currently ignores the
$batchSize parameter and calls exportPlatforms which fetches all platforms at
once; update exportPlatforms (and/or the call site in exportGroupIntegrations)
to implement pageable fetching using $batchSize (e.g., loop with offset/page
tokens until no more results) and yield/return batches compatible with
exportWithConsoleHeaders, or if the Appwrite platforms API has no pagination,
remove $batchSize from exportGroupIntegrations’ signature or add a code comment
documenting the lack of pagination; reference the exportGroupIntegrations
method, exportPlatforms function, and exportWithConsoleHeaders helper when
making the change.
- Around line 2043-2058: The reportIntegrations method currently ignores the
$resourceIds parameter; either use it to filter the platforms count or remove
the unused parameter. To fix, update reportIntegrations (method name:
reportIntegrations) to apply $resourceIds when calling the platforms endpoint
(e.g. include IDs or a filter in the GET to '/projects/{project}/platforms' and
compute $report[Resource::TYPE_PLATFORM] from the filtered response) or delete
the $resourceIds parameter from the method signature and all callers if platform
filtering is not supported; ensure the try/catch and assignment to
$report[Resource::TYPE_PLATFORM] remain consistent with other report* methods
(e.g., reportAuth/reportStorage) when implementing the change.
In `@src/Migration/Sources/Firebase.php`:
- Around line 816-819: The exception message in
Firebase::exportGroupIntegrations currently uses 'Not implemented'; update the
throw in the protected method exportGroupIntegrations(int $batchSize, array
$resources) to throw the same canonical message 'Not Implemented' used by other
sources so the exception casing is consistent across implementations (locate the
throw new \Exception(...) inside exportGroupIntegrations and change the string).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/Migration/Destinations/Appwrite.phpsrc/Migration/Resource.phpsrc/Migration/Resources/Integrations/Platform.phpsrc/Migration/Source.phpsrc/Migration/Sources/Appwrite.phpsrc/Migration/Sources/CSV.phpsrc/Migration/Sources/Firebase.phpsrc/Migration/Sources/JSON.phpsrc/Migration/Sources/NHost.phpsrc/Migration/Transfer.phptests/Migration/Unit/Adapters/MockDestination.phptests/Migration/Unit/Adapters/MockSource.php
Contributor
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/Migration/Sources/Appwrite.php`:
- Around line 2043-2059: Ensure reportIntegrations always initializes
$report[Resource::TYPE_PLATFORM] to 0 before any early returns (e.g., when
getConsoleHeaders() returns null) and honor the optional $resourceIds filter:
after successfully calling $this->call('GET', '/projects/.../platforms',
$consoleHeaders) in reportIntegrations, if $resourceIds is non-empty compute the
count as the number of returned platform IDs that intersect $resourceIds
(otherwise use $response['total'] ?? 0), then assign that value to
$report[Resource::TYPE_PLATFORM]; keep the catch block to set it to 0 on
failure.
- Around line 2065-2074: The exportPlatforms call currently ignores pagination
and batchSize; change exportPlatforms to accept an int $batchSize parameter and
implement the same paginated loop pattern used by other exporters (e.g.,
exportSites): build a Query::limit($batchSize) and use
Query::cursorAfter($cursor) to iterate until no more results, accumulate/emit
each page via exportWithConsoleHeaders in exportGroupIntegrations, and honor
root filtering by checking $this->rootResourceId and $this->rootResourceType
(apply a query filter or skip non-matching platform records) so platforms are
fetched in pages and scoped to the requested root resource.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5286cc3-043e-4038-981d-5bf3f1e49d69
📒 Files selected for processing (1)
src/Migration/Sources/Appwrite.php
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Migration/Transfer.php (1)
61-95:⚠️ Potential issue | 🔴 CriticalResolve unresolved merge-conflict blocks before merge.
Lines 61, 95, 144, 150, and 410–418 still include git conflict markers, which makes this file unparsable and risks dropping either database/messaging groups or the integrations group depending on which side is kept.
🔧 Suggested merge direction
-<<<<<<< HEAD + public const GROUP_INTEGRATIONS_RESOURCES = [ + Resource::TYPE_PLATFORM, + ]; + public const GROUP_DOCUMENTSDB_RESOURCES = [ Resource::TYPE_DATABASE_DOCUMENTSDB, Resource::TYPE_COLLECTION, Resource::TYPE_INDEX, Resource::TYPE_DOCUMENT ]; public const GROUP_VECTORSDB_RESOURCES = [ Resource::TYPE_DATABASE_VECTORSDB, Resource::TYPE_COLLECTION, Resource::TYPE_ATTRIBUTE, Resource::TYPE_INDEX, Resource::TYPE_DOCUMENT ]; public const GROUP_DATABASES_RESOURCES = [ Resource::TYPE_DATABASE, Resource::TYPE_DATABASE_DOCUMENTSDB, Resource::TYPE_DATABASE_VECTORSDB, Resource::TYPE_TABLE, Resource::TYPE_INDEX, Resource::TYPE_COLUMN, Resource::TYPE_ROW, Resource::TYPE_DOCUMENT, Resource::TYPE_COLLECTION, Resource::TYPE_ATTRIBUTE ]; - - public const GROUP_SETTINGS_RESOURCES = []; ->>>>>>> origin/mainpublic const ROOT_RESOURCES = [ Resource::TYPE_BUCKET, Resource::TYPE_DATABASE, Resource::TYPE_DATABASE_DOCUMENTSDB, Resource::TYPE_DATABASE_VECTORSDB, Resource::TYPE_FUNCTION, Resource::TYPE_SITE, Resource::TYPE_USER, Resource::TYPE_TEAM, + Resource::TYPE_PLATFORM, + Resource::TYPE_PROVIDER, + Resource::TYPE_TOPIC, + Resource::TYPE_MESSAGE, ];$resources = match ($service) { self::GROUP_FUNCTIONS => array_merge($resources, self::GROUP_FUNCTIONS_RESOURCES), self::GROUP_SITES => array_merge($resources, self::GROUP_SITES_RESOURCES), self::GROUP_STORAGE => array_merge($resources, self::GROUP_STORAGE_RESOURCES), self::GROUP_GENERAL => array_merge($resources, []), self::GROUP_AUTH => array_merge($resources, self::GROUP_AUTH_RESOURCES), self::GROUP_DATABASES => array_merge($resources, self::GROUP_DATABASES_RESOURCES), + self::GROUP_DATABASES_TABLES_DB => array_merge($resources, self::GROUP_TABLESDB_RESOURCES), + self::GROUP_DATABASES_DOCUMENTS_DB => array_merge($resources, self::GROUP_DOCUMENTSDB_RESOURCES), + self::GROUP_DATABASES_VECTOR_DB => array_merge($resources, self::GROUP_VECTORSDB_RESOURCES), + self::GROUP_MESSAGING => array_merge($resources, self::GROUP_MESSAGING_RESOURCES), self::GROUP_INTEGRATIONS => array_merge($resources, self::GROUP_INTEGRATIONS_RESOURCES), default => throw new \Exception('No service group found'), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Migration/Transfer.php` around lines 61 - 95, This file still contains git conflict markers; remove the markers (<<<<<<<, =======, >>>>>>>) and properly merge the two sides so all intended resource group constants exist: keep GROUP_INTEGRATIONS_RESOURCES (Resource::TYPE_PLATFORM) and also retain the new GROUP_DOCUMENTSDB_RESOURCES, GROUP_VECTORSDB_RESOURCES, GROUP_DATABASES_RESOURCES and GROUP_SETTINGS_RESOURCES definitions, ensuring there are no duplicate constant names or syntax errors; also search for and remove conflict markers at the other reported locations (around the blocks that include lines ~144, ~150 and ~410–418) so the PHP parses cleanly and all groups are preserved.
🧹 Nitpick comments (1)
src/Migration/Destinations/Appwrite.php (1)
2222-2231: Duplicate detection query may have a race condition.The
findOnecheck followed bycreateDocumentis not atomic. In high-concurrency scenarios, two imports could pass the existence check simultaneously and both attempt to create the platform. TheDuplicateExceptioncatch on line 2250 handles this correctly as a fallback, so the logic is safe but could log misleading "already exists" messages for legitimate race conditions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Migration/Destinations/Appwrite.php` around lines 2222 - 2231, The duplicate-detection using $this->dbForPlatform->findOne(...) followed by an early return causes misleading "Platform already exists" status under race conditions; remove the early return and allow the subsequent createDocument call (and its DuplicateException handler) to determine the final outcome, or change the early-path to only short-circuit when you can guarantee exclusivity; specifically update the block around $this->dbForPlatform->findOne, Query::equal(...) and $resource->setStatus(Resource::STATUS_SKIPPED, ...) so it does not preemptively set SKIPPED on a non-atomic check but instead relies on the existing DuplicateException handling (the catch near the createDocument call) to set the SKIPPED status for real duplicates caused by race conditions.
🤖 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/Destinations/Appwrite.php`:
- Around line 2220-2258: The createPlatform method is missing its closing brace,
causing validateFieldsForIndexes to be parsed inside it; add a closing "}"
immediately after the final "return true;" in the createPlatform method (the
method that begins with "protected function createPlatform(Platform $resource):
bool") so that the subsequent "private function validateFieldsForIndexes(Index
$resource, UtopiaDocument $table, array &$lengths)" is defined as a separate
method; ensure braces/indentation for createPlatform are balanced.
- Line 2240: The project document access uses a non-existent method
getInternalId(); replace that call with getSequence() on the Document returned
by $this->dbForPlatform->getDocument('projects', $this->project) so the
assignment for 'projectInternalId' uses ->getSequence() instead of
->getInternalId(); update the expression where 'projectInternalId' is set (in
the Destinations\Appwrite migration code) to call getSequence().
In `@src/Migration/Resource.php`:
- Around line 114-121: Remove the unresolved merge conflict markers in the
ALL_RESOURCES array inside the Resource class and ensure the array contains all
resource type constants: retain self::TYPE_PLATFORM and add self::TYPE_PROVIDER,
self::TYPE_TOPIC, self::TYPE_SUBSCRIBER, and self::TYPE_MESSAGE; specifically
edit the ALL_RESOURCES definition to eliminate the <<<<<<<, =======, and >>>>>>>
lines and produce a clean array listing the full set of constants.
- Around line 74-81: The file contains unresolved git conflict markers; remove
the conflict markers (<<<<<<<, =======, >>>>>>>) and restore a single coherent
constants block that includes all required integration types by adding public
const TYPE_PLATFORM = 'platform'; public const TYPE_SUBSCRIBER = 'subscriber';
public const TYPE_MESSAGE = 'message'; (keep existing surrounding formatting and
visibility) so the class compiles cleanly and no markers remain.
In `@tests/Migration/Unit/Adapters/MockDestination.php`:
- Around line 54-61: Resolve the git merge conflict markers in
MockDestination::getSupportedResources(): remove the <<<<<<<, =======, and
>>>>>>> lines and return an array that includes both Resource::TYPE_PLATFORM and
the messaging types Resource::TYPE_PROVIDER, Resource::TYPE_TOPIC,
Resource::TYPE_SUBSCRIBER, and Resource::TYPE_MESSAGE so all expected resource
types are present.
In `@tests/Migration/Unit/Adapters/MockSource.php`:
- Around line 83-90: Resolve the unresolved git conflict markers inside the
MockSource class's getSupportedResources() method by removing the <<<<<<<,
=======, >>>>>>> lines and returning a single array that includes
Resource::TYPE_PLATFORM plus Resource::TYPE_PROVIDER, Resource::TYPE_TOPIC,
Resource::TYPE_SUBSCRIBER, and Resource::TYPE_MESSAGE so all supported resource
types are present.
---
Outside diff comments:
In `@src/Migration/Transfer.php`:
- Around line 61-95: This file still contains git conflict markers; remove the
markers (<<<<<<<, =======, >>>>>>>) and properly merge the two sides so all
intended resource group constants exist: keep GROUP_INTEGRATIONS_RESOURCES
(Resource::TYPE_PLATFORM) and also retain the new GROUP_DOCUMENTSDB_RESOURCES,
GROUP_VECTORSDB_RESOURCES, GROUP_DATABASES_RESOURCES and
GROUP_SETTINGS_RESOURCES definitions, ensuring there are no duplicate constant
names or syntax errors; also search for and remove conflict markers at the other
reported locations (around the blocks that include lines ~144, ~150 and
~410–418) so the PHP parses cleanly and all groups are preserved.
---
Nitpick comments:
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 2222-2231: The duplicate-detection using
$this->dbForPlatform->findOne(...) followed by an early return causes misleading
"Platform already exists" status under race conditions; remove the early return
and allow the subsequent createDocument call (and its DuplicateException
handler) to determine the final outcome, or change the early-path to only
short-circuit when you can guarantee exclusivity; specifically update the block
around $this->dbForPlatform->findOne, Query::equal(...) and
$resource->setStatus(Resource::STATUS_SKIPPED, ...) so it does not preemptively
set SKIPPED on a non-atomic check but instead relies on the existing
DuplicateException handling (the catch near the createDocument call) to set the
SKIPPED status for real duplicates caused by race conditions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0586d6ac-2bcc-411a-9dd6-aa0630e9e8d0
📒 Files selected for processing (11)
src/Migration/Destinations/Appwrite.phpsrc/Migration/Resource.phpsrc/Migration/Source.phpsrc/Migration/Sources/Appwrite.phpsrc/Migration/Sources/CSV.phpsrc/Migration/Sources/Firebase.phpsrc/Migration/Sources/JSON.phpsrc/Migration/Sources/NHost.phpsrc/Migration/Transfer.phptests/Migration/Unit/Adapters/MockDestination.phptests/Migration/Unit/Adapters/MockSource.php
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/Migration/Sources/Appwrite.php (2)
2235-2249:⚠️ Potential issue | 🟠 MajorInitialize platform report and honor
resourceIdsfiltering.On Line 2240, returning early leaves
\$report[Resource::TYPE_PLATFORM]unset. Also\$resourceIdsis currently unused, so filtered reports can be incorrect.💡 Suggested logic fix
private function reportIntegrations(array $resources, array &$report, array $resourceIds = []): void { - if (\in_array(Resource::TYPE_PLATFORM, $resources)) { - $consoleHeaders = $this->getConsoleHeaders(); + if (!\in_array(Resource::TYPE_PLATFORM, $resources, true)) { + return; + } + + $report[Resource::TYPE_PLATFORM] = 0; + $consoleHeaders = $this->getConsoleHeaders(); - if ($consoleHeaders === null) { - return; - } + if ($consoleHeaders === null) { + return; + } - try { - $response = $this->call('GET', '/projects/' . $this->project . '/platforms', $consoleHeaders); - $report[Resource::TYPE_PLATFORM] = $response['total'] ?? 0; - } catch (\Throwable) { - $report[Resource::TYPE_PLATFORM] = 0; - } - } + try { + $response = $this->call('GET', '/projects/' . $this->project . '/platforms', $consoleHeaders); + $platforms = $response['platforms'] ?? []; + if (!empty($resourceIds[Resource::TYPE_PLATFORM])) { + $allowed = \array_flip((array) $resourceIds[Resource::TYPE_PLATFORM]); + $platforms = \array_filter($platforms, fn (array $p): bool => isset($allowed[$p['$id'] ?? ''])); + $report[Resource::TYPE_PLATFORM] = \count($platforms); + return; + } + $report[Resource::TYPE_PLATFORM] = $response['total'] ?? \count($platforms); + } catch (\Throwable) { + // keep fallback 0 + } }🤖 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 2235 - 2249, The reportIntegrations method currently skips setting $report[Resource::TYPE_PLATFORM] when getConsoleHeaders() returns null and never respects the $resourceIds filter; update reportIntegrations to always initialize $report[Resource::TYPE_PLATFORM] (e.g., to 0) before the headers check and, when $resourceIds is non-empty, only populate the platform count if Resource::TYPE_PLATFORM is included in $resourceIds (otherwise leave it as 0 or skip); keep using getConsoleHeaders() and the existing call('GET', '/projects/'.$this->project.'/platforms', $consoleHeaders) inside the try/catch to set the count and preserve the catch behavior for exceptions.
2277-2284:⚠️ Potential issue | 🟠 MajorUse paginated export for platforms and thread
batchSizethrough.
exportGroupIntegrations()accepts\$batchSize, butexportPlatforms()currently performs a single non-paginated request and does not apply root-resource filtering. This can truncate exported platforms.📌 Directional refactor
protected function exportGroupIntegrations(int $batchSize, array $resources): void { - if (\in_array(Resource::TYPE_PLATFORM, $resources)) { + if (\in_array(Resource::TYPE_PLATFORM, $resources, true)) { $this->exportWithConsoleHeaders( Resource::TYPE_PLATFORM, Transfer::GROUP_INTEGRATIONS, - $this->exportPlatforms(...) + fn (array $consoleHeaders) => $this->exportPlatforms($consoleHeaders, $batchSize) ); } } -private function exportPlatforms(array $consoleHeaders): void +private function exportPlatforms(array $consoleHeaders, int $batchSize): void { - $response = $this->call('GET', '/projects/' . $this->project . '/platforms', $consoleHeaders); - ... + // paginate with limit + cursorAfter until page size < $batchSize + // and respect rootResourceId/rootResourceType when TYPE_PLATFORM is targeted }For Appwrite REST API v1 / PHP SDK v19, confirm whether `GET /projects/{projectId}/platforms` supports pagination (`limit`, `cursorAfter`) via `queries`, and provide the exact request/query format.Also applies to: 2343-2367
🤖 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 2277 - 2284, The exportPlatforms call inside exportGroupIntegrations must be converted to a paginated exporter that respects the passed $batchSize and filters by root resource: update exportPlatforms to accept (int $batchSize, ?string $rootResource = null) and implement paginated requests to Appwrite platforms using the SDK's queries (e.g. include limit=$batchSize and cursorAfter handling) so each page is yielded/exported; then change the exportGroupIntegrations invocation to pass $batchSize and the appropriate root resource when calling $this->exportPlatforms(...) so Transfer::GROUP_INTEGRATIONS export will iterate all pages rather than a single non-paginated request.
🤖 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 2235-2250: The file is missing closing braces for three methods
causing a parse error; add the missing closing brace "}" at the end of
reportIntegrations (method reportIntegrations), add the missing closing brace at
the end of exportWithConsoleHeaders (method exportWithConsoleHeaders), and add
the missing closing brace at the end of exportPlatforms (method
exportPlatforms). Also move any docblocks currently injected after those method
bodies (the malformed "@param string $databaseType" and other docblocks) back
above their respective function declarations or remove them so they are not
placed where the closing brace belongs, ensuring each method ends with a proper
'}' before the next docblock or method declaration.
---
Duplicate comments:
In `@src/Migration/Sources/Appwrite.php`:
- Around line 2235-2249: The reportIntegrations method currently skips setting
$report[Resource::TYPE_PLATFORM] when getConsoleHeaders() returns null and never
respects the $resourceIds filter; update reportIntegrations to always initialize
$report[Resource::TYPE_PLATFORM] (e.g., to 0) before the headers check and, when
$resourceIds is non-empty, only populate the platform count if
Resource::TYPE_PLATFORM is included in $resourceIds (otherwise leave it as 0 or
skip); keep using getConsoleHeaders() and the existing call('GET',
'/projects/'.$this->project.'/platforms', $consoleHeaders) inside the try/catch
to set the count and preserve the catch behavior for exceptions.
- Around line 2277-2284: The exportPlatforms call inside exportGroupIntegrations
must be converted to a paginated exporter that respects the passed $batchSize
and filters by root resource: update exportPlatforms to accept (int $batchSize,
?string $rootResource = null) and implement paginated requests to Appwrite
platforms using the SDK's queries (e.g. include limit=$batchSize and cursorAfter
handling) so each page is yielded/exported; then change the
exportGroupIntegrations invocation to pass $batchSize and the appropriate root
resource when calling $this->exportPlatforms(...) so
Transfer::GROUP_INTEGRATIONS export will iterate all pages rather than a single
non-paginated request.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ceee9396-ec71-4744-abb8-16c3f8f4611f
📒 Files selected for processing (5)
src/Migration/Resource.phpsrc/Migration/Sources/Appwrite.phpsrc/Migration/Transfer.phptests/Migration/Unit/Adapters/MockDestination.phptests/Migration/Unit/Adapters/MockSource.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Migration/Resource.php
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/Migration/Sources/Appwrite.php (2)
2235-2251:⚠️ Potential issue | 🟠 MajorInitialize platform report count and honor
resourceIdsin integrations report.
reportIntegrations()can return without setting\$report[Resource::TYPE_PLATFORM], and filtered reports ignore\$resourceIds, which can produce incorrect totals.💡 Proposed fix
private function reportIntegrations(array $resources, array &$report, array $resourceIds = []): void { - if (\in_array(Resource::TYPE_PLATFORM, $resources)) { - $consoleHeaders = $this->getConsoleHeaders(); - - if ($consoleHeaders === null) { - return; - } - - try { - $response = $this->call('GET', '/projects/' . $this->project . '/platforms', $consoleHeaders); - $report[Resource::TYPE_PLATFORM] = $response['total'] ?? 0; - } catch (\Throwable) { - $report[Resource::TYPE_PLATFORM] = 0; - } + if (!\in_array(Resource::TYPE_PLATFORM, $resources, true)) { + return; + } + + $report[Resource::TYPE_PLATFORM] = 0; + $consoleHeaders = $this->getConsoleHeaders(); + if ($consoleHeaders === null) { + return; + } + + try { + $response = $this->call('GET', '/projects/' . $this->project . '/platforms', $consoleHeaders); + $platforms = $response['platforms'] ?? []; + + if (!empty($resourceIds[Resource::TYPE_PLATFORM])) { + $allowed = \array_flip((array) $resourceIds[Resource::TYPE_PLATFORM]); + $platforms = \array_filter( + $platforms, + fn (array $platform): bool => isset($allowed[$platform['$id'] ?? '']) + ); + $report[Resource::TYPE_PLATFORM] = \count($platforms); + return; + } + + $report[Resource::TYPE_PLATFORM] = $response['total'] ?? \count($platforms); + } catch (\Throwable) { + // keep fallback 0 } }🤖 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 2235 - 2251, Initialize $report[Resource::TYPE_PLATFORM] = 0 at the start of reportIntegrations(), and when fetching platforms via call('GET', '/projects/' . $this->project . '/platforms', $consoleHeaders) honor the $resourceIds filter: if $resourceIds is non-empty, inspect $response['data'] (the returned platform items) and set $report[Resource::TYPE_PLATFORM] to the count of items whose '$id' (or 'id') is in $resourceIds; otherwise fall back to $response['total'] ?? 0. Keep the existing try/catch and ensure the catch leaves $report[Resource::TYPE_PLATFORM] as 0.
2281-2290:⚠️ Potential issue | 🟠 MajorUse
\$batchSizeand paginate platform export (current flow can truncate data).
exportGroupIntegrations()receives\$batchSize, butexportPlatforms()performs a single list call with no cursor loop and no root-platform filter handling. This can export only the first page and ignore targeted root exports.💡 Proposed fix
protected function exportGroupIntegrations(int $batchSize, array $resources): void { if (\in_array(Resource::TYPE_PLATFORM, $resources)) { $this->exportWithConsoleHeaders( Resource::TYPE_PLATFORM, Transfer::GROUP_INTEGRATIONS, - $this->exportPlatforms(...) + fn (array $consoleHeaders) => $this->exportPlatforms($consoleHeaders, $batchSize) ); } } /** * `@throws` AppwriteException */ -private function exportPlatforms(array $consoleHeaders): void +private function exportPlatforms(array $consoleHeaders, int $batchSize): void { - $response = $this->call('GET', '/projects/' . $this->project . '/platforms', $consoleHeaders); - - if (empty($response['platforms'])) { - return; - } - - $platforms = []; - - foreach ($response['platforms'] as $platform) { - $platforms[] = new Platform( - $platform['$id'] ?? '', - $platform['type'] ?? '', - $platform['name'] ?? '', - $platform['key'] ?? '', - $platform['store'] ?? '', - $platform['hostname'] ?? '', - createdAt: $platform['$createdAt'] ?? '', - updatedAt: $platform['$updatedAt'] ?? '', - ); - } - - $this->callback($platforms); + $cursor = null; + while (true) { + $queries = [Query::limit($batchSize)]; + + if ($this->rootResourceId !== '' && $this->rootResourceType === Resource::TYPE_PLATFORM) { + $queries[] = Query::equal('$id', $this->rootResourceId); + $queries[] = Query::limit(1); + } elseif ($cursor !== null) { + $queries[] = Query::cursorAfter($cursor); + } + + $response = $this->call( + 'GET', + '/projects/' . $this->project . '/platforms', + $consoleHeaders, + ['queries' => $queries] + ); + + $current = $response['platforms'] ?? []; + if (empty($current)) { + break; + } + + $platforms = []; + foreach ($current as $platform) { + $platforms[] = new Platform( + $platform['$id'] ?? '', + $platform['type'] ?? '', + $platform['name'] ?? '', + $platform['key'] ?? '', + $platform['store'] ?? '', + $platform['hostname'] ?? '', + createdAt: $platform['$createdAt'] ?? '', + updatedAt: $platform['$updatedAt'] ?? '', + ); + } + + $this->callback($platforms); + + if ($this->rootResourceId !== '' && $this->rootResourceType === Resource::TYPE_PLATFORM) { + break; + } + + $cursor = $current[\count($current) - 1]['$id'] ?? null; + if (\count($current) < $batchSize) { + break; + } + } }For Appwrite API/SDK v19, what is the exact supported pagination format for GET /v1/projects/{projectId}/platforms (queries[] with limit/cursorAfter), and how should those queries be encoded for REST requests?Also applies to: 2351-2375
🤖 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 2281 - 2290, exportGroupIntegrations currently passes $batchSize but exportPlatforms does a single list call; update exportPlatforms to loop using $batchSize and Appwrite v19 cursor pagination: call listPlatforms repeatedly with queries[] params "limit(<batchSize>)" and "cursorAfter(<lastId>)" (encoded as queries[]=limit(<n>)&queries[]=cursorAfter(<id>) in REST) until no more results, and honor any root-platform filter when building the queries; reference the exportGroupIntegrations and exportPlatforms methods and replace the single-call list with a cursor-based loop that updates cursorAfter from the last item each iteration.
🤖 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/Destinations/Appwrite.php`:
- Around line 2202-2216: The switch in importIntegrationsResource(Resource
$resource) only handles Resource::TYPE_PLATFORM and then unconditionally sets
status to STATUS_SUCCESS if not STATUS_SKIPPED, allowing unsupported types to be
marked success; add a default branch (or explicit check after the switch) that
fails fast for unknown resource names by setting the resource to a failure
status (or throwing an exception) and/or logging an error, referencing
importIntegrationsResource(), Resource::TYPE_PLATFORM, Resource::STATUS_SKIPPED
and Resource::STATUS_SUCCESS so unsupported integration types are never silently
marked successful.
---
Duplicate comments:
In `@src/Migration/Sources/Appwrite.php`:
- Around line 2235-2251: Initialize $report[Resource::TYPE_PLATFORM] = 0 at the
start of reportIntegrations(), and when fetching platforms via call('GET',
'/projects/' . $this->project . '/platforms', $consoleHeaders) honor the
$resourceIds filter: if $resourceIds is non-empty, inspect $response['data']
(the returned platform items) and set $report[Resource::TYPE_PLATFORM] to the
count of items whose '$id' (or 'id') is in $resourceIds; otherwise fall back to
$response['total'] ?? 0. Keep the existing try/catch and ensure the catch leaves
$report[Resource::TYPE_PLATFORM] as 0.
- Around line 2281-2290: exportGroupIntegrations currently passes $batchSize but
exportPlatforms does a single list call; update exportPlatforms to loop using
$batchSize and Appwrite v19 cursor pagination: call listPlatforms repeatedly
with queries[] params "limit(<batchSize>)" and "cursorAfter(<lastId>)" (encoded
as queries[]=limit(<n>)&queries[]=cursorAfter(<id>) in REST) until no more
results, and honor any root-platform filter when building the queries; reference
the exportGroupIntegrations and exportPlatforms methods and replace the
single-call list with a cursor-based loop that updates cursorAfter from the last
item each iteration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe7b7011-99c4-442c-a44c-5df899e85d2f
📒 Files selected for processing (2)
src/Migration/Destinations/Appwrite.phpsrc/Migration/Sources/Appwrite.php
# Conflicts: # src/Migration/Destinations/Appwrite.php # src/Migration/Source.php # src/Migration/Sources/Appwrite.php # src/Migration/Sources/CSV.php # src/Migration/Sources/Firebase.php # src/Migration/Sources/NHost.php # tests/Migration/Unit/Adapters/MockSource.php
Contributor
Greptile SummaryThis PR introduces platform integration as a migratable resource type, covering the full export → transfer → import pipeline across all source and destination adapters.
Confidence Score: 5/5The change is additive and well-contained; known open items (exception swallowing in reportIntegrations, onDuplicate not yet implemented) were already discussed in earlier review rounds and the developer has acknowledged them. No new unaddressed defects were found in this iteration. The pagination loop, batchSize forwarding, and duplicate-detection logic in createPlatform all work correctly. The two outstanding concerns from prior rounds are tracked and the developer is aware of them. src/Migration/Sources/Appwrite.php (reportIntegrations silently swallows exceptions) and src/Migration/Transfer.php (orphaned GROUP_SETTINGS_RESOURCES constant) are the only files with lingering known issues from prior review rounds. Important Files Changed
Reviews (7): Last reviewed commit: "Rename source project to projectId; hold..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
4 tasks
abnegate
reviewed
May 14, 2026
abnegate
reviewed
May 14, 2026
| $this->functions = new Functions($this->client); | ||
| $this->messaging = new Messaging($this->client); | ||
| $this->sites = new Sites($this->client); | ||
| $this->projectService = new Project($this->client); |
Member
There was a problem hiding this comment.
Let's instead change the existing property to projectId and name this project
abnegate
approved these changes
May 14, 2026
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
2 participants
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
Platformresource class underResources/Integrationswith support for all platform typesGROUP_INTEGRATIONSresource group andTYPE_PLATFORMconstant toResourceexportPlatforms()inSources/Appwriteusing console key authentication with cachingimportPlatformResource()inDestinations/AppwriteSummary by CodeRabbit
New Features
Tests
Chores