Open
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
Comment on lines
+2043
to
+2059
| 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; | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Always initialize platform report count, and honor requested IDs.
When Resource::TYPE_PLATFORM is requested, reportIntegrations() can return without setting $report[Resource::TYPE_PLATFORM] (Line 2048-2050). Also, $resourceIds is currently unused, so filtered runs can report incorrect totals.
💡 Suggested 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 $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
+ }
}🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 2043-2043: Avoid unused parameters such as '$resourceIds'. (undefined)
(UnusedFormalParameter)
🤖 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 - 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.
Comment on lines
+2065
to
+2074
| 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(...) | ||
| ); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Appwrite API/SDK v19, does GET /projects/{projectId}/platforms support pagination via queries[]=limit(...) and queries[]=cursorAfter(...)? If yes, what is the exact query format?
💡 Result:
Yes — if you call GET /v1/projects/{projectId}/platforms with the queries array, you can paginate the same way as other list endpoints: limit + cursorAfter. The REST API expects each query as an escaped JSON query string passed via repeated queries[] parameters. (appwrite.io)
REST query format (exact)
GET /v1/projects/{projectId}/platforms?queries[]={"method":"limit","values":[25]}&queries[]={"method":"cursorAfter","values":["<LAST_PLATFORM_ID>"]}URL-encode the JSON when sending it over HTTP (spaces/quotes/braces). Appwrite documents this as “escaped JSON strings” passed as queries[] and shows cursorAfter using {"method":"cursorAfter","values":["rowId"]}. (appwrite.io)
SDK format (v19-style)
Pass an array of query strings generated by the SDK Query helper:
queries: [
Query.limit(25),
Query.cursorAfter('<LAST_PLATFORM_ID>')
](These helpers generate the same JSON query strings shown above.) (appwrite.io)
Citations:
- 1: https://appwrite.io/docs/apis/rest
- 2: https://appwrite.io/docs/apis/rest
- 3: https://appwrite.io/docs/products/databases/pagination?utm_source=openai
🏁 Script executed:
# First, let's check the file structure and read the relevant lines
wc -l src/Migration/Sources/Appwrite.phpRepository: utopia-php/migration
Length of output: 103
🏁 Script executed:
# Read the code around lines 2065-2074 and 2105-2129
sed -n '2065,2074p' src/Migration/Sources/Appwrite.php
echo "---"
sed -n '2105,2129p' src/Migration/Sources/Appwrite.phpRepository: utopia-php/migration
Length of output: 1238
🏁 Script executed:
# Find the exportPlatforms method to see the actual implementation
grep -n "protected function exportPlatforms\|private function exportPlatforms" src/Migration/Sources/Appwrite.phpRepository: utopia-php/migration
Length of output: 134
🏁 Script executed:
# Search for other export methods to see pagination patterns
grep -n "private function export" src/Migration/Sources/Appwrite.php | head -20Repository: utopia-php/migration
Length of output: 1299
🏁 Script executed:
# Look at one example exporter to see if it uses pagination
sed -n '1800,1900p' src/Migration/Sources/Appwrite.php | head -50Repository: utopia-php/migration
Length of output: 1549
🏁 Script executed:
# Search for a specific exporter method like exportServers or exportKeys
grep -A 20 "private function exportServers" src/Migration/Sources/Appwrite.php | head -30Repository: utopia-php/migration
Length of output: 46
🏁 Script executed:
# Search for rootResourceId usage to verify root resource handling claim
grep -n "rootResourceId\|rootResourceType" src/Migration/Sources/Appwrite.php | head -20Repository: utopia-php/migration
Length of output: 1678
exportPlatforms() does not paginate results and ignores the batch size parameter.
Unlike other resource exporters in this class, this method makes a single API call without pagination support. If the Appwrite API returns paginated results, platform data will be truncated. Additionally, it does not support filtering by root resource ID as other exporters do.
The method signature should accept int $batchSize, and the implementation should:
- Use
Query::limit($batchSize)andQuery::cursorAfter()pagination queries - Loop until all results are fetched
- Support root resource filtering via
$this->rootResourceIdand$this->rootResourceTypechecks
See other exporters in this file (e.g., exportSites at line 1827) for the established pagination pattern to follow.
🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 2065-2065: Avoid unused parameters such as '$batchSize'. (undefined)
(UnusedFormalParameter)
🤖 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 - 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.
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
| $this->dbForPlatform->createDocument('platforms', new UtopiaDocument([ | ||
| '$id' => ID::unique(), | ||
| '$permissions' => $resource->getPermissions(), | ||
| 'projectInternalId' => $this->dbForPlatform->getDocument('projects', $this->project)->getInternalId(), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that getInternalId() is used correctly for project documents
ast-grep --pattern $'$_->getDocument($_, $_)->getInternalId()'
rg -n 'getInternalId\(\)' --type=php -C2Repository: utopia-php/migration
Length of output: 768
🏁 Script executed:
# Check for Document class definition and getInternalId method
find . -type f -name "*.php" | xargs grep -l "class Document"Repository: utopia-php/migration
Length of output: 162
🏁 Script executed:
# Search for getInternalId method definition
rg -n "function getInternalId|public.*getInternalId" --type=phpRepository: utopia-php/migration
Length of output: 46
🏁 Script executed:
# Check for getSequence usage to understand when it's used
rg -n "getSequence\(\)" --type=php -C2 | head -50Repository: utopia-php/migration
Length of output: 2909
🏁 Script executed:
# Look at the dbForPlatform->getDocument usage in the same file
rg -n "getDocument\(" src/Migration/Destinations/Appwrite.php | head -20Repository: utopia-php/migration
Length of output: 1112
🏁 Script executed:
# Read the Document class to verify available methods
cat -n src/Migration/Resources/Database/Document.php | head -100Repository: utopia-php/migration
Length of output: 1712
🏁 Script executed:
# Check for all public methods in Document class
rg -n "public.*function" src/Migration/Resources/Database/Document.phpRepository: utopia-php/migration
Length of output: 170
🏁 Script executed:
# Read the Row class to check for getInternalId and other methods
cat -n src/Migration/Resources/Database/Row.phpRepository: utopia-php/migration
Length of output: 2991
🏁 Script executed:
# Read the Resource class to check for getInternalId and getSequence
cat -n src/Migration/Resource.phpRepository: utopia-php/migration
Length of output: 9627
🏁 Script executed:
# Check composer.json for dependencies
cat composer.json | grep -A 20 '"require"'Repository: utopia-php/migration
Length of output: 599
🏁 Script executed:
# Search for dbForPlatform initialization and type hints to understand what it returns
rg -n "dbForPlatform" src/Migration/Destinations/Appwrite.php -B2 -A2 | head -40Repository: utopia-php/migration
Length of output: 1552
🏁 Script executed:
# Search for other references to 'projects' document to understand its structure
rg -n "getDocument\('projects'" --type=php -A2 -B2Repository: utopia-php/migration
Length of output: 604
🏁 Script executed:
# Search for getInternalId in the entire repo to understand its usage pattern
rg -n "getInternalId" --type=php -B3 -A3Repository: utopia-php/migration
Length of output: 1505
🏁 Script executed:
# Check if there's a Project resource class that might have getInternalId
find . -type f -name "*.php" | xargs grep -l "class Project"Repository: utopia-php/migration
Length of output: 48
🏁 Script executed:
# Verify the line context around 2240 to understand what method should be used
sed -n '2230,2250p' src/Migration/Destinations/Appwrite.phpRepository: utopia-php/migration
Length of output: 1026
🏁 Script executed:
# Check if utopia-php/database Document class has getInternalId method by examining any documentation or type hints
rg -n "import.*Document|use.*Document" src/Migration/Destinations/Appwrite.phpRepository: utopia-php/migration
Length of output: 292
🏁 Script executed:
# Check if there are any other patterns where we retrieve and use project document properties
rg -n "projects.*project\)" src/Migration/Destinations/Appwrite.php -A3 -B1 | head -50Repository: utopia-php/migration
Length of output: 654
🏁 Script executed:
# Search for any documentation or comments about projectInternalId
rg -n "projectInternalId" --type=php -B3 -A3Repository: utopia-php/migration
Length of output: 833
🏁 Script executed:
# Check the git history or comments around line 2240 to understand intent
git log -p --follow -S "projectInternalId" -- src/Migration/Destinations/Appwrite.php 2>/dev/null | head -100Repository: utopia-php/migration
Length of output: 46
🏁 Script executed:
# Look at the broader context of the platforms document structure
rg -n "platforms.*createDocument" src/Migration/Destinations/Appwrite.php -B5 -A15Repository: utopia-php/migration
Length of output: 46
🏁 Script executed:
# Check the imports to understand what's available from utopia-php/database
grep -r "use Utopia" src/Migration/Destinations/Appwrite.php | head -20Repository: utopia-php/migration
Length of output: 944
🏁 Script executed:
# Look for any cached documents or other getDocument calls to see what methods are used on them
rg -n "getDocument\(" src/Migration/Destinations/Appwrite.php -A1 | grep -E "(getId|getSequence|getInternalId|get[A-Z])" | head -20Repository: utopia-php/migration
Length of output: 1349
Replace getInternalId() with getSequence() on the project document.
The code calls getInternalId() which does not exist on Utopia\Database\Document. The correct method is getSequence(), as consistently used throughout the codebase for retrieving internal/sequential IDs from documents.
Line 2240
'projectInternalId' => $this->dbForPlatform->getDocument('projects', $this->project)->getSequence(),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Migration/Destinations/Appwrite.php` at 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().
src/Migration/Resource.php
Outdated
Comment on lines
+114
to
+121
| <<<<<<< HEAD | ||
| self::TYPE_PLATFORM, | ||
| ======= | ||
| self::TYPE_PROVIDER, | ||
| self::TYPE_TOPIC, | ||
| self::TYPE_SUBSCRIBER, | ||
| self::TYPE_MESSAGE, | ||
| >>>>>>> origin/main |
Contributor
There was a problem hiding this comment.
Second merge conflict in ALL_RESOURCES array.
The ALL_RESOURCES array also has unresolved merge conflict markers. Both the messaging types and TYPE_PLATFORM should be included.
🔧 Proposed fix to resolve the merge conflict
-<<<<<<< HEAD
- self::TYPE_PLATFORM,
-=======
self::TYPE_PROVIDER,
self::TYPE_TOPIC,
self::TYPE_SUBSCRIBER,
self::TYPE_MESSAGE,
->>>>>>> origin/main
+ self::TYPE_PLATFORM,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <<<<<<< HEAD | |
| self::TYPE_PLATFORM, | |
| ======= | |
| self::TYPE_PROVIDER, | |
| self::TYPE_TOPIC, | |
| self::TYPE_SUBSCRIBER, | |
| self::TYPE_MESSAGE, | |
| >>>>>>> origin/main | |
| self::TYPE_PROVIDER, | |
| self::TYPE_TOPIC, | |
| self::TYPE_SUBSCRIBER, | |
| self::TYPE_MESSAGE, | |
| self::TYPE_PLATFORM, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Migration/Resource.php` around lines 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.
Comment on lines
+54
to
+61
| <<<<<<< HEAD | ||
| Resource::TYPE_PLATFORM, | ||
| ======= | ||
| Resource::TYPE_PROVIDER, | ||
| Resource::TYPE_TOPIC, | ||
| Resource::TYPE_SUBSCRIBER, | ||
| Resource::TYPE_MESSAGE, | ||
| >>>>>>> origin/main |
Contributor
There was a problem hiding this comment.
Unresolved merge conflict markers in test file.
The getSupportedResources() method contains git merge conflict markers that must be resolved. Both TYPE_PLATFORM and the messaging types should be included.
🔧 Proposed fix to resolve the merge conflict
Resource::TYPE_MEMBERSHIP,
-<<<<<<< HEAD
- Resource::TYPE_PLATFORM,
-=======
Resource::TYPE_PROVIDER,
Resource::TYPE_TOPIC,
Resource::TYPE_SUBSCRIBER,
Resource::TYPE_MESSAGE,
->>>>>>> origin/main
+ Resource::TYPE_PLATFORM,
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <<<<<<< HEAD | |
| Resource::TYPE_PLATFORM, | |
| ======= | |
| Resource::TYPE_PROVIDER, | |
| Resource::TYPE_TOPIC, | |
| Resource::TYPE_SUBSCRIBER, | |
| Resource::TYPE_MESSAGE, | |
| >>>>>>> origin/main | |
| Resource::TYPE_MEMBERSHIP, | |
| Resource::TYPE_PROVIDER, | |
| Resource::TYPE_TOPIC, | |
| Resource::TYPE_SUBSCRIBER, | |
| Resource::TYPE_MESSAGE, | |
| Resource::TYPE_PLATFORM, | |
| ]; |
🧰 Tools
🪛 GitHub Actions: CodeQL
[error] 54-54: PHPStan: Syntax error, unexpected T_SL, expecting ',' or ']' or ')' on line 54.
🪛 GitHub Actions: Tests
[error] 54-54: ParseError: syntax error, unexpected token '<<', expecting ']' (while running 'docker compose exec tests php vendor/bin/phpunit')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Migration/Unit/Adapters/MockDestination.php` around lines 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.
Comment on lines
+83
to
+90
| <<<<<<< HEAD | ||
| Resource::TYPE_PLATFORM, | ||
| ======= | ||
| Resource::TYPE_PROVIDER, | ||
| Resource::TYPE_TOPIC, | ||
| Resource::TYPE_SUBSCRIBER, | ||
| Resource::TYPE_MESSAGE, | ||
| >>>>>>> origin/main |
Contributor
There was a problem hiding this comment.
Unresolved merge conflict markers in test mock.
The getSupportedResources() method contains git merge conflict markers that must be resolved. Both TYPE_PLATFORM and the messaging types should be included.
🔧 Proposed fix to resolve the merge conflict
Resource::TYPE_MEMBERSHIP,
-<<<<<<< HEAD
- Resource::TYPE_PLATFORM,
-=======
Resource::TYPE_PROVIDER,
Resource::TYPE_TOPIC,
Resource::TYPE_SUBSCRIBER,
Resource::TYPE_MESSAGE,
->>>>>>> origin/main
+ Resource::TYPE_PLATFORM,
// legacy📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <<<<<<< HEAD | |
| Resource::TYPE_PLATFORM, | |
| ======= | |
| Resource::TYPE_PROVIDER, | |
| Resource::TYPE_TOPIC, | |
| Resource::TYPE_SUBSCRIBER, | |
| Resource::TYPE_MESSAGE, | |
| >>>>>>> origin/main | |
| Resource::TYPE_MEMBERSHIP, | |
| Resource::TYPE_PROVIDER, | |
| Resource::TYPE_TOPIC, | |
| Resource::TYPE_SUBSCRIBER, | |
| Resource::TYPE_MESSAGE, | |
| Resource::TYPE_PLATFORM, | |
| // legacy |
🧰 Tools
🪛 GitHub Actions: CodeQL
[error] 83-83: PHPStan: Syntax error, unexpected T_SL, expecting ',' or ']' or ')' on line 83.
🪛 GitHub Actions: Tests
[error] 83-83: ParseError: syntax error, unexpected token '<<', expecting ']' (while running 'docker compose exec tests php vendor/bin/phpunit')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Migration/Unit/Adapters/MockSource.php` around lines 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.
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
Comment on lines
+2202
to
+2216
| public function importIntegrationsResource(Resource $resource): Resource | ||
| { | ||
| switch ($resource->getName()) { | ||
| case Resource::TYPE_PLATFORM: | ||
| /** @var Platform $resource */ | ||
| $this->createPlatform($resource); | ||
| break; | ||
| } | ||
|
|
||
| if ($resource->getStatus() !== Resource::STATUS_SKIPPED) { | ||
| $resource->setStatus(Resource::STATUS_SUCCESS); | ||
| } | ||
|
|
||
| return $resource; | ||
| } |
Contributor
There was a problem hiding this comment.
Fail fast on unsupported integration resource types.
importIntegrationsResource() has no default branch, so unknown integration types can be marked STATUS_SUCCESS without any import action.
💡 Proposed fix
public function importIntegrationsResource(Resource $resource): Resource
{
switch ($resource->getName()) {
case Resource::TYPE_PLATFORM:
/** `@var` Platform $resource */
$this->createPlatform($resource);
break;
+ default:
+ throw new \Exception('Unknown integrations resource type: ' . $resource->getName(), Exception::CODE_VALIDATION);
}
if ($resource->getStatus() !== Resource::STATUS_SKIPPED) {
$resource->setStatus(Resource::STATUS_SUCCESS);
}
return $resource;
}🤖 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 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.
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
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