feat: messaging migration #146
Conversation
|
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 messaging support across the migration system by introducing four new resource types (provider, topic, subscriber, message) and corresponding Resource classes and exporters. Transfer gains a GROUP_MESSAGING and resource lists. Source interfaces add exportGroupMessaging; Appwrite implements listing, reporting, exporting, and importing for messaging (provider creation, topic/subscriber/message handling, scheduling, and target resolution). Local and Mock adapters declare messaging support; CSV/JSON/Firebase/NHost stubs throw Not Implemented. Tests and mocks updated to include messaging resource types. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
No actionable comments were generated in the recent review. 🎉 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}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 1867-1884: The resolveTargetId(Subscriber $resource) currently
returns the source-side target ID when no matching providerType is found, which
is invalid on the destination; change resolveTargetId to signal a failure
instead of returning the stale ID (e.g., throw a descriptive exception like
InvalidArgumentException or return null/a named sentinel) and then update the
caller (createSubscriber) to detect this signal and handle it (skip creation,
log/collect the error, or recreate the target on destination) so an invalid
target ID is never passed to createSubscriber.
- Around line 1557-1590: The switch in importMessagingResource lacks a default
branch so unknown resource names fall through and get marked STATUS_SUCCESS; add
a default case that handles unexpected Resource::getName() values by logging the
unexpected type (use whatever logger is available in this class) and setting
$resource->setStatus(Resource::STATUS_FAILED) (or throw an
InvalidArgumentException) before returning the resource, ensuring
importMessagingResource properly reports failures for unrecognized
Resource::TYPE_* values.
- Around line 1838-1865: resolveMessageTargets currently calls
$this->users->listTargets($userId) and will throw an AppwriteException if the
user is missing, aborting the whole message migration; wrap the listTargets call
inside a try/catch that catches AppwriteException (or a generic exception), skip
adding any targets for that user when an exception occurs, optionally log the
userId and exception, and continue processing other targets; apply the same
pattern to resolveTargetId where $this->users->... is called so individual
resolution failures are skipped instead of failing the entire resource.
- Around line 1767-1831: In createScheduledMessage, the messaging->createPush
call passes null for the $contentAvailable, $critical and $priority parameters
which must be type-appropriate for Appwrite SDK v19; update the createPush
invocation in createScheduledMessage to pass false for $contentAvailable, false
for $critical, and MessagePriority::NORMAL() for $priority (use the SDK enum
reference) instead of null so the call matches the SDK's expected defaults.
In `@src/Migration/Sources/Appwrite.php`:
- Around line 1960-1975: buildTargetUserMap currently calls
$this->users->listTargets($user->getId()) once and only processes the first
page; change it to paginate through all pages for each user by repeatedly
calling Users::listTargets (same method) with the appropriate pagination params
(e.g. limit + offset or cursor/next token depending on the SDK) and appending
each page's $response['targets'] entries into $map[$target['$id']] =
$user->getId() until no further pages remain; ensure the pagination loop is
inside buildTargetUserMap so every user's full target list is resolved.
🧹 Nitpick comments (1)
src/Migration/Sources/Firebase.php (1)
811-815: Nit: Inconsistent exception message casing.This stub uses
'Not implemented'(lowercase 'i') while the other source stubs (CSV, NHost, JSON) andexportGroupFunctionsabove use'Not Implemented'(uppercase 'I').Suggested fix
protected function exportGroupMessaging(int $batchSize, array $resources): void { - throw new \Exception('Not implemented'); + throw new \Exception('Not Implemented'); }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 1723-1743: The createMessage method currently constructs DateTime
directly from Message::getScheduledAt and can throw on malformed input; wrap the
DateTime parsing/comparison in a try/catch inside createMessage (around the new
\DateTime($scheduledAt) > new \DateTime() check) so that any exception when
parsing $scheduledAt is caught and treated as a missing/invalid scheduled time
(set $status = 'draft' and continue), and then call
createScheduledMessage($resource, $resolvedTargets) only when parsing succeeds
and the scheduled time is in the future.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 1784-1798: In the Appwrite migration code (class Appwrite) adjust
the arguments passed to the SDK methods to match their typed signatures: for
createEmail replace any nulls for $cc, $bcc, and $attachments with empty arrays
([]) and ensure $html is a boolean (use false if no HTML), and for createPush
pass an empty array instead of null for the $data parameter; update the call
sites in the match branch where createEmail and createPush are invoked (use
$data['cc'] ?? [], $data['bcc'] ?? [], $data['attachments'] ?? [],
(bool)($data['html'] ?? false), and $data['data'] ?? []) so all parameters
conform to array/bool types expected by the Appwrite SDK.
1 participant
Summary
Changes
importMessagingResource,createProvider(all 11 types),createMessage,createScheduledMessage,resolveTargetId,resolveMessageTargetsexportGroupMessaging,exportProviders,exportTopics,exportSubscribers,exportMessages,buildTargetUserMap,reportMessagingtargetUserMapfor target ID resolutionexportGroupMessagingmethodexportGroupMessagingstubsSummary by CodeRabbit
New Features
Tests