-
Notifications
You must be signed in to change notification settings - Fork 39
[MOB-12263] android-add-ability-to-get placements #744
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/MOB-12261-add-embedded-manager-class
Are you sure you want to change the base?
Conversation
4 new issues
This is from Qlty Cloud, the successor to Code Climate Quality. Learn more. |
|
Diff Coverage: The code coverage on the diff in this pull request is 25.0%. Total Coverage: This PR will decrease coverage by 0.29%. File Coverage Changes
🛟 Help
This is from Qlty Cloud, the successor to Code Climate Quality. Learn more. |
…to loren/embedded/MOB-12263-android-add-ability-to-sync-embedded-messages
…terableAPIModuleImpl
android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java
Outdated
Show resolved
Hide resolved
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 embedded messaging placement functionality to the React Native Iterable SDK, allowing developers to retrieve placement IDs for embedded messages.
- Adds
getPlacementIds()method to the embedded manager for fetching placement IDs - Renames configuration property from
embeddedMessagingEnabledtoenableEmbeddedMessaging - Implements native Android support for embedded placement retrieval
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/embedded/classes/IterableEmbeddedManager.ts | Adds getPlacementIds method and imports IterableApi |
| src/core/classes/IterableConfig.ts | Renames embeddedMessagingEnabled to enableEmbeddedMessaging |
| src/core/classes/IterableApi.ts | Adds getEmbeddedPlacementIds static method with logging |
| src/api/NativeRNIterableAPI.ts | Defines native interface for getEmbeddedPlacementIds |
| example/src/hooks/useIterableApp.tsx | Enables embedded messaging in example app |
| example/src/components/Embedded/Embedded.tsx | Adds UI to test placement ID retrieval |
| android/src/oldarch/java/com/RNIterableAPIModule.java | Implements React method for old architecture |
| android/src/newarch/java/com/RNIterableAPIModule.java | Implements React method for new architecture |
| android/src/main/java/com/iterable/reactnative/Serialization.java | Adds config serialization for enableEmbeddedMessaging |
| android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java | Implements placement ID retrieval logic and embedded manager initialization |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // import com.iterable.iterableapi.IterableEmbeddedManager; | ||
| import com.iterable.iterableapi.IterableEmbeddedMessage; | ||
| // import com.iterable.iterableapi.IterableEmbeddedSession; | ||
| // import com.iterable.iterableapi.IterableEmbeddedUpdateHandler; |
Copilot
AI
Oct 14, 2025
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.
Remove commented-out import statements. These unused imports add clutter and should be cleaned up.
| // import com.iterable.iterableapi.IterableEmbeddedManager; | |
| import com.iterable.iterableapi.IterableEmbeddedMessage; | |
| // import com.iterable.iterableapi.IterableEmbeddedSession; | |
| // import com.iterable.iterableapi.IterableEmbeddedUpdateHandler; | |
| import com.iterable.iterableapi.IterableEmbeddedMessage; |
| configBuilder.setEnableEmbeddedMessaging(configReadableMap.getBoolean("enableEmbeddedMessaging")); | ||
| } |
Copilot
AI
Oct 14, 2025
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.
Inconsistent indentation. The closing brace should align with the if statement (2 spaces instead of 6).
| configBuilder.setEnableEmbeddedMessaging(configReadableMap.getBoolean("enableEmbeddedMessaging")); | |
| } | |
| configBuilder.setEnableEmbeddedMessaging(configReadableMap.getBoolean("enableEmbeddedMessaging")); | |
| } |
| WritableArray writableArray = Arguments.createArray(); | ||
| if (placementIds != null) { | ||
| for (Long placementId : placementIds) { | ||
| writableArray.pushDouble(placementId.doubleValue()); |
Copilot
AI
Oct 14, 2025
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.
Converting Long placement IDs to double may cause precision loss for large values. Consider using pushString() or ensure the native interface expects numbers instead of longs.
| writableArray.pushDouble(placementId.doubleValue()); | |
| writableArray.pushString(placementId.toString()); |
🔹 JIRA Ticket(s) if any
✏️ Description
Adds the ability to get placements