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

feat: multiple push providers #1445

Merged
merged 2 commits into from
May 19, 2020
Merged

Conversation

sdenaci
Copy link
Contributor

@sdenaci sdenaci commented May 18, 2020

this pr reimplements #1128 (see description below) on current development branch
I needed to use this library with multiple push providers on a project I was working on, and saw on the 1128 discussion that @shercoder and @Dallas62 planned to reimplement it on dev. So I tried to do that with minimal changing, and submit it back to library in case it is useful.

This change only impacts Android

The need for this change came because the project I work has a need to support multiple push providers. We have an internal provider where we manager our in-house push notifications and then there is a need for external/marketing push notifications that we depend on a third-party to deliver.

Since there can only be one FirebaseMessagingService with intent-filter of com.google.firebase.MESSAGING_EVENT, we cannot have a service from Localytics and RNPushNotificationListenerService declared in our AndroidManifest. Android by default will only pick the first declaration.

In order to support multiple providers, a developer can create a custom FirebaseMessagingService that distributes the push notification to providers. Please see this issue I created that explains the problem and the solution in more details #952

Basically this change now allows the developer in need to do the following in their custom FirebaseMessagingService:

private RNReceivedMessageHandler receivedMessageHandler = new RNReceivedMessageHandler(this);

...
...
...

@Override
    public void onMessageReceived(RemoteMessage remoteMessage) {
        Map<String, String> data = remoteMessage.getData();

        String from = remoteMessage.getFrom();
       
            try {
                if (from first provider) {
                    receivedMessageHandler.handleReceivedMessage(remoteMessage);
                } else {
                    // pass remote message to another provider
                }
            } catch (Exception e) {
                Log.e(e, "Failed to extract Push Message", remoteMessage.getMessageId());
            }
        
    }

NOTE: There will be no change in behavior in how the current version of this library works and the users who don't need the support for multiple push providers won't have to change anything in their code base and continue to use the RNPushNotificationListenerService as it is.

Testing

1. For single provider, you can just go through the normal setup as described.

2. For multiple providers, go through the normal setup for this library but instead use your customer `FirebaseMessagingService` instead of `RNPushNotificationListenerService` as mentioned above.

fix: fix naming

fix: more fixes

fix: more fixes

fix: add activity service

fix: update naming
@Dallas62
Copy link
Collaborator

Hi @sdenaci
Thanks for the PR, I'm not sure if onNewToken is still working.

@sdenaci
Copy link
Contributor Author

sdenaci commented May 18, 2020

hi @Dallas62 ! thanks for pointing that out. I have now added an explicit override in the listener file.

@Dallas62
Copy link
Collaborator

Thanks for the fast change!
Another question which comes into my mind. I don't know why exactly you need this features (use-case), but how did we know which provider give us the token ? (or notification)

@sdenaci
Copy link
Contributor Author

sdenaci commented May 18, 2020

For notifications: The other library I am using provides a method to check whether the push comes from their server. In the case that users are using other libraries that do not provide methods like this, the push would have to include some kind of key in push that listener could switch on in order to determine which handler should be invoked.
For token: User's custom listener implementation of onNewToken would have to use both libraries' on newToken method (because both libraries have to do whatever they have to do with the new token)

@Dallas62
Copy link
Collaborator

Ok, since onNewToken is not the initial request, and we should add the origin of the token (here Firebase).
I will merge and move the method, thanks again for the PR !

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.

None yet

2 participants