-
Notifications
You must be signed in to change notification settings - Fork 79
CustomInAppDisplayProvider interface #839
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
Conversation
…plications to display CTInAppNotifications themselves..
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) ✅ code/snyk check is complete. No issues have been found. (View Details) |
WalkthroughA new extension point for custom in-app notification display is introduced. This involves adding the Changes
Sequence Diagram(s)sequenceDiagram
participant InAppController
participant Activity
participant CustomInAppDisplayProvider
participant User
InAppController->>Activity: Check if implements CustomInAppDisplayProvider
alt Activity implements provider and canDisplay
InAppController->>CustomInAppDisplayProvider: display(notification, config, host, callbacks)
CustomInAppDisplayProvider->>User: Display custom in-app notification
User->>CustomInAppDisplayProvider: Interact (action/button/dismiss/show)
CustomInAppDisplayProvider->>InAppController: Callbacks (onActionTriggered/onButtonClicked/onDismissed/onShown)
else Not handled by custom provider
InAppController->>InAppController: Proceed with default in-app display logic
end
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.java (1)
880-920: Consider adding error handling for robustness.The implementation correctly follows the interface contract and properly forwards callbacks to existing methods. However, consider adding error handling around the custom provider calls to prevent potential crashes from faulty implementations.
Consider wrapping the custom provider calls in try-catch blocks:
private boolean tryCustomShowInApp(CTInAppNotification notification) { Activity activity = CoreMetaData.getCurrentActivity(); if (activity == null || !(activity instanceof CustomInAppDisplayProvider) ) { return false; // not a CustomInAppDisplayProvider } CustomInAppDisplayProvider displayProvider = (CustomInAppDisplayProvider) activity; - if (!displayProvider.canDisplay(notification, config, activity)) { - return false; // not supported/wanted - } - displayProvider.display(notification, config, activity, new CustomInAppDisplayProvider.Callbacks() { + try { + if (!displayProvider.canDisplay(notification, config, activity)) { + return false; // not supported/wanted + } + displayProvider.display(notification, config, activity, new CustomInAppDisplayProvider.Callbacks() { @Override public Bundle onActionTriggered(@NonNull CTInAppNotification notification, @NonNull CTInAppAction action, @NonNull String callToAction, Bundle additionalData, Context activityContext) { return inAppNotificationActionTriggered(notification, action, callToAction, additionalData, activityContext); } @Override public void onButtonClicked(@NonNull CTInAppNotification notification, CTInAppNotificationButton button, Context activityContext) { inAppNotificationDidClick(notification, button, activityContext); } @Override public void onDismissed(@NonNull CTInAppNotification notification, Bundle formData) { inAppNotificationDidDismiss(notification, formData); } @Override public void onShown(@NonNull CTInAppNotification notification, Bundle formData) { inAppNotificationDidShow(notification, formData); } - }); + }); + } catch (Exception e) { + logger.debug(config.getAccountId(), "Error in custom in-app display provider: " + e.getMessage(), e); + return false; // fall back to standard display + } return true; // handled }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.java(3 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/interfaces/CustomInAppDisplayProvider.java(1 hunks)
🔇 Additional comments (3)
clevertap-core/src/main/java/com/clevertap/android/sdk/interfaces/CustomInAppDisplayProvider.java (1)
1-64: Excellent interface design and documentation!The
CustomInAppDisplayProviderinterface is well-designed with clear separation of concerns:
- The two-phase pattern (
canDisplay()→display()) allows providers to opt-in selectively- The
Callbacksinterface properly mirrors the existingInAppControllermethods for seamless integration- Comprehensive documentation explains the contract and requirements
- Consistent use of
@NonNullannotations ensures null safetyThis provides a clean extension point for custom in-app notification handling while maintaining compatibility with existing analytics and queue management.
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.java (2)
45-45: Clean integration with existing imports.The import is appropriately placed and follows the existing import organization.
798-800: Proper integration point with early return pattern.The integration correctly uses an early return pattern to bypass standard display logic when a custom provider handles the notification. This maintains backward compatibility while enabling the new functionality.
…rTap/clevertap-android-sdk into CustomInAppDisplayProvider
…tions (#24) Adds HtmlBannerInAppDisplayProvider to handle the display of custom-html header & footer banner-notifications. The normal CleverTap implementation relies on a FragmentActivity which unreal can't easily support. This functionality requires adding the CustomInAppDisplayProvider interface to the clevertap android sdk. CleverTap/clevertap-android-sdk#839 This PR includes (and defaults to) a prebuilt clevertap-android-sdk.aar with this pre-release functionality included. The AAR will be removed when the maven version has the needed features.
|
The changes in this PR are not needed anymore. See #841 |
Adds new interface
CustomInAppDisplayProviderwithInAppControllersupport to allow applications to displayCTInAppNotificationsthemselves.This allows the CleverTap Unreal Plugin to support the Custom-Html Header & Footer InApp notification types despite being unable to provide a
FragmentActivity:See the
HtmlBannerInAppDisplayProviderimplementation in the CleverTap Unreal SDK:CleverTap/clevertap-unreal-sdk#24
Summary by CodeRabbit