-
Notifications
You must be signed in to change notification settings - Fork 42
[SDK-235] Add ability to sync and get messages #797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: loren/embedded/SDK-240-ios-add-ability-to-start-and-end-a-session
Are you sure you want to change the base?
Conversation
…ng embedded messages
… and improve message retrieval
|
Coverage Impact This PR will not change total coverage. 🚦 See full report on Qlty Cloud »🛟 Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds the ability to sync embedded messages and retrieve embedded messages with optional placement ID filtering for the iOS platform. The Android implementation already exists in the codebase.
Changes:
- Added iOS native implementation for
syncEmbeddedMessages()andgetEmbeddedMessages()methods - Added serialization support for
IterableEmbeddedMessageto convert to dictionary format - Simplified the example app usage to pass placement IDs directly instead of chaining promises
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ios/RNIterableAPI/Serialization.swift | Adds toDict() extension for IterableEmbeddedMessage to serialize messages for React Native |
| ios/RNIterableAPI/ReactIterableAPI.swift | Implements syncEmbeddedMessages() and getEmbeddedMessages() methods with placement ID filtering |
| ios/RNIterableAPI/RNIterableAPI.mm | Exports the new methods for both new and legacy React Native architectures |
| example/src/components/Embedded/Embedded.tsx | Simplifies the getEmbeddedMessages callback to accept placement IDs directly as a parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public func syncEmbeddedMessages() { | ||
| ITBInfo() | ||
| IterableAPI.embeddedManager.syncMessages { } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens when syncMessages fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, it would swallow it. I would love to add success/error callbacks, but unfortunately Android does not seem to expose them, so I would only be able to do so for iOS. As RN needs to have similar capabilities for both, I kind of have to forgo the error handling here as I can't seem to figure it out properly in Android.
@Ayyanchira -- please let me know if I misunderstood this!
| let placementMessages = IterableAPI.embeddedManager.getMessages( | ||
| for: placementId.intValue | ||
| ) | ||
| messages.append(contentsOf: placementMessages) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if placementMessages is nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would return an empty array
| ITBInfo() | ||
| var messages: [IterableEmbeddedMessage] = [] | ||
|
|
||
| if let placementIds = placementIds, !placementIds.isEmpty { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need a comment here to explain why if placementIds is nil or empty we just do IterableAPI.embeddedManager.getMessages(). Would the messages array be coherent in both cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be coherent for both. Added a comment regarding this, but not 100% sure if this is what you meant. Please let me know if it is not.
| // Serialize metadata (which is Codable) | ||
| if let metadataDict = SerializationUtil.encodableToDictionary(encodable: metadata) { | ||
| dict["metadata"] = metadataDict | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if serialization fails but the metadata is partially encodable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSONEnvoder is atomic -- If any property fails to encode, the entire encoding will fail. So there wouldn't be partial encoding. But the entire metadata would be silently omitted.
Updated this so that it would fail if the encoding fails so that the error isn't swallowed.
| if let elements = elements, | ||
| let elementsDict = SerializationUtil.encodableToDictionary(encodable: elements) { | ||
| dict["elements"] = elementsDict | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is serialization fails but the elementsDict is partially encodable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated this -- let me know if this works
sumeruchat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM

🔹 JIRA Ticket(s) if any
✏️ Description
Testing
Setup
yarn installyarn installbundle exec pod installwatchman watch-del-allyarn start --reset-cacheopen ReactNativeSdkExample.xcworkspace