Issue 2585 - Alllowing to clear notification when the app is not running

[wolfenrain]

NOTE: iOS hasn't been tested yet because of #2617. I am delivering the PR nonetheless for discussions and possible changes if needed. When I am able to do the usecase described in #2617 I will be able to test iOS and fix any mistakes if needed.

Description

I added an extra optional key to the payload clearNotification, this key can be used to clear notificitations that have said notId.
For example:

// Send first notificitation
firstPayload = {
        'title': 'Clear notification',
        'body': 'Test body',
        'notId': '1337'
};
// Send second notification to clear it
secondPayload = {
        'title': 'Clear notification',
        'body': 'Test body',
        'clearNotification': '1337'
};

The changes are quite simple, for Android I added an extra check if the clearNotification key is available, if so then we simple remove the notification with that ID. I did change the clearNotification() method. I move the functionality to a static method. That way anyone that has acces can call the method while the cordova bridge is still intact.

For iOS basically the same thing except that I moved the clearNotification() functionality to clearRealNotification() so it can be used by anyone that has acccsm, and still keeping the cordova bridge intact.

Related Issue

#2585

Motivation and Context

This allows developers to remove notification from the notification tray without their user having the app open/running.

And here are some use-cases for this functionality describe by an other user.

How Has This Been Tested?

I tested this by sending above payload through FCM towards my devices. Android functionality hasn't changed much, I tested the push and the cordova bridge and both are still working as expected.

iOS testing HAS NOT BEEN DONE YET because of #2617. Because the clearNotification is in the same method as the notId, I haven't been able to test it yet.

Documentation is also not done because I wasn't sure where to put the new information, if anyone can point me in that direction, much appreciated.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[fredgalvao]

The way this is right now, and considering the example you used on the OP, I can still use the clearNotification attribute on the payload to clear said id if the application is in foreground, however the scenario on this line won't trigger and it'll fall through to the last else {, which will consider foreground:false even though we're in foreground, therefore creating a very inconsistent payload for the js to handle.

So, we have a few options:

• we can consider that the clearNotification attribute represents a separate and independent feature on the payload, and a single payload can be {an actual notification with all the other payload attributes available} and {a request to clear a specific notification} at the same time. This would need the first if statement here to be isolated as to not race against the existing ones. Check the clearBadge feature, as that's the closest we have to this option.
• we can consider that the clearNotification attribute represents the whole feature, and whenever it is present it will skip all other cases (if/else statements). This would need the first if statement to actually return from the method, or make the last else statement do nothing if it did clear a notification before. This would also make the example used on the OP invalid, as the title and body attributes on the payload mean nothing. This would also make it useful to move this feature to as early as possible on the method to avoid useless work (if we're gonna clear a notification and return, why waste resources preping the extras?) This would also make it needed to solve the question "who has the highest priority on the feature order: clearBadge or clearNotification?".

[wolfenrain]

Will take a look at it tonight or tomorrow! Thanks for taking the time! Will also update the docs :-)

[wolfenrain]

I would consider going for the first option, I would consider that the most likely use-case for people. And that way you don't force people. Will change and test it :-)

[fredgalvao]

Thanks for this!

I left a beefy comment on the code review, btw.
Also, we're gonna need the docs to be updated for this after that comment is resolved.