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(android): Add user config. Add push notification bundle edit possibility before processing #2070

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

Conversation

sergey-kozel
Copy link

Sometimes it's impossible to change the payload of push notifications on backend, and sometimes we need to add custom logic to push notification processing regarding an app status. In these cases, we need some mechanism of preprocessing the push data before processing it by the module. But due to the fact that there are no ways to do this with RemoteMessage type itself (which is the only input parameter), it requires some additional logic from the module. This PR introduces a possibility to edit the push notification bundle before start processing and allows extending module behaviour with user-defined config in the future.

@Dallas62
Copy link
Collaborator

Hi,
I don't understand why we should add a process inside le library like this one.
As far as I remember, it's possible to chain the Listener to add custom logic if needed.
Regards

@Dallas62
Copy link
Collaborator

#1445

@sergey-kozel
Copy link
Author

Hello @Dallas62,
It is not about different listeners as in the attached PR. More precisely, it can be resolved with different listeners indeed but I will need to write all further logic of push handling which provides this awesome package by myself. What I need instead is to change a bit payload of the bundle and see how the library does its great job. Let's see my example:

public class MainPushNotificationsService extends FirebaseMessagingService {

    private RNPushNotificationListenerService rnPushNotificationListenerService;
    private NotificationsRepository notificationsRepository;

    @Override
    public void onCreate() {
        super.onCreate();
        this.init();
    }

    @Override
    public void onMessageReceived(RemoteMessage remoteMessage) {
        super.onMessageReceived(remoteMessage);
        boolean isHandled = false;
        if (this.rnPushNotificationListenerService == null) {
            this.init();
        }
        notificationsRepository.create(new NotificationEntity(remoteMessage.getData()));
        if (!isAppInForeground(getApplicationContext())) {
            isHandled = AppboyFirebaseMessagingService.handleBrazeRemoteMessage(this, remoteMessage);
        }
        if (!isHandled) {
            rnPushNotificationListenerService.onMessageReceived(remoteMessage);
        }
    }

    @Override
    public void onNewToken(String token) {
        Appboy.getInstance(getApplicationContext()).registerAppboyPushMessages(token);
    }

    private void init() {
        RNPushNotificationUserConfig userConfig = new RNPushNotificationUserConfig();
        userConfig.setPushNotificationBundleEditor(new BackendlessPushNotificationEditor());
        this.rnPushNotificationListenerService = new RNPushNotificationListenerService(this, userConfig);
        this.notificationsRepository = new NotificationsRepository(getApplicationContext());
    }

    private class BackendlessPushNotificationEditor implements IRNPushNotificationBundleEditor {
        @Override
        public void editBundle(Bundle bundle, RemoteMessage message) {
            if (!isAppInForeground(getApplicationContext())) {
                for(Map.Entry<String, String> entry : message.getData().entrySet()) {
                    bundle.putString(entry.getKey(), entry.getValue());
                }
            }
        }
    }
}

As you can see I only need to copy something from data to the root payload (it is "message" if you are interested in order to show notification automatically) if the app is not in the foreground. And as I said there are no ways to edit and "repack" RemoteMessage object to pass into the package already edited version.
And I believe it is no so rare case as you might think. In the real world, we have to maintain different versions of the app in production and a simple change of payload on the backend might be not allowed to not break the previous ones.
And I may imagine a lot of cases when this bundle editing extension might be required. For example colors of the notifications. Currently, the library takes the color either from a message payload or from resources. But what if someday customer decides to change color depending on the current app theme?
But I agree that it might look like a dangerous backdoor to the internal logic of the module and should be used only "at your own risk".

@Dallas62
Copy link
Collaborator

Hi @sergey-kozel
I understand your point, but I don't understand why you are not able to change the remote message (on backend) if you need advanced notification handling?
A good solution would be to send a data-only notification and trigger a localNotification based on your needs.
What about iOS?
Regards

@sergey-kozel
Copy link
Author

sergey-kozel commented Jul 1, 2021

It is already data-only notification. And you are right that I can trigger local notification but the thing is JS code will be executed only if an app in the background but not killed or was not ran at all. And the good solution might be using combined push (notification + data) but as I said we have to maintain the previous versions of the app and obviously they are will be broken with such serious push payload change.
IOS in its turn takes less control and is responsible for displaying notification itself. And actually, there is the possibility to modify the push notification payload out of the box with the notification service app extension. In a certain sense, my changes are equivalent to this extension.

@bruce-glazier
Copy link

@sergey-kozel 100% agree, currently switching to a "data-only" notification payload will break backward compatibility. If this works then it would be really helpful.

@Dallas62
Copy link
Collaborator

@sergey-kozel 100% agree, currently switching to a "data-only" notification payload will break backward compatibility. If this works then it would be really helpful.

You can make your change in your application, release it. Then wait one month to switch to data-only. It's just release management.

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.

3 participants