-
Notifications
You must be signed in to change notification settings - Fork 33
SDK-307 Fix custom actions not working in background when SDK is not initialized #975
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: master
Are you sure you want to change the base?
Conversation
When a push action with openApp=false is received before SDK initialization, the custom action handler was never invoked because getMainActivityContext() returned null. This fix: 1. Stores the application context early in handlePushAction() so getMainActivityContext() returns non-null even before initialize() is called 2. Only clears pendingAction if the action was actually handled, allowing deferred processing when SDK is initialized later with a customActionHandler This enables "snooze" type flows where a push button action with openApp=false can trigger a journey without requiring the full app to open.
| // This allows the action to be processed later when SDK is fully initialized | ||
| // (e.g., when customActionHandler becomes available after initialize() is called). | ||
| if (handled) { | ||
| pendingAction = null; |
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 if it does not manage to execute the action? Can't this pendingAction become residual?
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.
Good catch! I've updated the code to clear any previous unhandled pending action when a new push action comes in. This prevents residual actions from accumulating if they were never handled. The logic now clears pendingAction at the start of handlePushAction() before setting up the new action.
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.
as stated in the other comment, that should be defined by code here, if we should have a pendingAction after not being able to handle it that is fine, but i feel like if this was not handled here it won't be in the next time if nothing changes, so we can just be calling processPendingAction indefinitely with nothing changing
| // Store the application context if not already set, so custom actions can be | ||
| // processed even when the SDK hasn't been fully initialized (e.g., openApp=false) | ||
| if (IterableApi.sharedInstance._applicationContext == null && context != null) { | ||
| IterableApi.sharedInstance._applicationContext = context.getApplicationContext(); |
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.
Can we create a method for initializing it so we don't call _applicationContext directly, if something else is necessary for pushes to work we can add it there
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.
Done! I've created a new initializeForPush(Context context) method in IterableApi that encapsulates the context initialization. This method:
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.
- Only sets the context if it hasn't been set already
- Includes proper null checking and logging
- Makes it easy to add additional push-related initialization in the future
|
Thanks for the review Franco! I've addressed both of your comments: I've also added new unit tests:
|
| // (e.g., if the SDK was never initialized or the action handler wasn't available). | ||
| if (pendingAction != null) { | ||
| IterableLogger.d(TAG, "Clearing previous unhandled pending action"); | ||
| pendingAction = null; |
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.
Is this necessary? On line 97 we are already setting the pending action to something else. Is there a place this can be used accidentally that i am missing?
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.
not really necessary not it was just responding to your previous comment and being extra safe for avoiding future bugs
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.
yeah, but i feel like this is just adding code that virtually does nothing. My comment was more regarding the pending action that would be as left over on that method, if we are just not processing it and that is the correct behavior that's fine, but if we should do something to the pending action if it was not handled that should be clear
What
Fixes custom actions not working when SDK is not initialized and
openApp=false. This enables "snooze" type push notification flows where a button action can trigger a journey without opening the app.Jira ticket: SDK-307
Changes
handlePushAction()sogetMainActivityContext()returns non-null even beforeinitialize()is calledpendingActionif the action was actually handled, allowing deferred processing when SDK is initialized later with acustomActionHandlerImpact
Testing
How to test:
openApp=falseand a custom action typeUnit tests added:
testBackgroundCustomActionWithNonInitializedSDK- verifies context is stored even without SDK inittestBackgroundCustomActionProcessedAfterSDKInit- verifies pending actions are processed after initialization