Skip to content
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

poc: Dynamic config #281

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

MAKARD
Copy link

@MAKARD MAKARD commented Dec 10, 2024

Case Number: 00249678

*/
if (configuredApiKey != null) {
// Delete previous push token to avoid receiving push notifications originating from previous instance
FirebaseMessaging.getInstance().deleteToken().await()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround 1:
During testing, it appears push notifications are still coming for the device with the new braze config (new API key), even though the campaign is configured for another application (API key)

}

@Throws
private suspend fun initialize(config: ConfigData) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SDK creates the instance regardless of configuration validity - i.e when braze.xml is absent and programmatic configuration is not executed yet, the SDK throws warnings to the console that API_KEY is empty

To prevent receiving the same push notification more than once,
it's required to unregister the previous device push token
*/
await UIApplication.shared.unregisterForRemoteNotifications()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround 2:
Sometimes, after switching to another configuration, the same push notification is coming twice.


// Remove references to the previous instance, so GC can deallocate it
brazeInstance = nil
BrazeReactBridge.deinitBraze()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround 3:
After switching to another configuration, the push notifications are not coming at all (with workaround 2 in place).

and before creating a new instance.
Otherwise, push notifications for the new instance won't work
*/
DispatchQueue.main.asyncAfter(deadline: .now() + delay) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround 4:
After switching to another configuration, the push notifications are not coming at all (with workarounds 2 and 3 in place).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason need to wait after de-initializing previous instance

Out of curiosity, did you experience this on both iOS and Android?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue was observed only on iOS

DispatchQueue.main.asyncAfter(deadline: .now() + delay) {

// A workaround for "Configuration" interfaces Swift<->Objective-C incompatibility
brazeInstance = BrazeInitWorkaround.initBraze(configuration)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround 5:
"Configuration" protocol does not conform to "BRZConfiguration".


#pragma mark - Setup

++(void)deinitBraze {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround 6:
Adding a patch file to enrich BrazeReactBridge with deinitBraze method which is required for Workaround 3

@sircelsius
Copy link
Collaborator

THANK YOU

This is awesome, thank you so much for taking the time to propose such a thorough, well documented contribution.

We are currently working in our private repositories on adding support for dynamically initializing the Braze SDK. We are still working on this, but we will have more to share soon.

We are currently not set up to accept external contributions of this magnitude, so we will not be merging this. However, we will do the following:

  1. Our engineering team will comment on your workarounds in this PR and provide as much clarity as we can. I can already confirm that for example the challenges you mention related to handling push notifications are a significant area of complexity.
  2. We will make sure to notify here as we release our version of this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants