Add xamarin way to use OSRemoteNotificationReceivedHandler

[tmijieux]

Description

One Line Summary

Add an easy way to use the OSRemoteNotificationReceivedHandler interface to replace old NotificationExtender in Xamarin .

Details

Usage in xamarin project:
In manifest:

    <meta-data
        android:name="com.onesignal.NotificationServiceExtension"
        android:value="com.my.app.MyOneSignalServiceExtension" />

using System;
using Android.Content;
using AndroidX.Core.App;
using Com.OneSignal.Android;

namespace MyApp.Droid
{
    public class MyExtender : NotificationExtenderBase
    {
        public override NotificationCompat.Builder Extend(NotificationCompat.Builder builder)
        {
            //do something with builder
            return builder
                /*.AddAction(new NotificationCompat.Action(null, "coucou", null))*/
                ;
        }
    }

    [NotificationExtension(Name = "com.my.app.MyOneSignalServiceExtension")]
    public class NotificationExtender : OSRemoteNotificationReceivedBase
    {
        public override void RemoteNotificationReceived(Context _ctx, OSNotificationReceivedEvent ev)
        {
            var notif = ev.Notification.MutableCopy();
            notif.SetExtender(new MyExtender());

            if (notif?.AdditionalData == null)
            {
                ev.Complete(notif);
                return;
            }



            bool hideNotif = false;
            try
            {
                var discard = notif.AdditionalData.Get("silent");
                if (discard != null)
                {
                    hideNotif = true;
                }
            }
            catch (Exception /*e*/)
            {
            }
            ev.Complete(hideNotif ? null : notif);
        }
    }
}

I have made the following modification on the native android project:
OneSignal/OneSignal-Android-SDK@main...tmijieux:OneSignal-Android-SDK:use_OSRemoveNotificationReceivedHandler_in_xamarin

I'm not 100% sure the java code is required. I think we need the abstract base classes because if we just use the interface directly from csharp i'm not sure if there will be a peer object /peer class in java world resolvable with introspection.
Maybe it works with just the interface ? i will try it tomorrow.

NOTE: to make the native android code compile i had to upgrade minSdkVersion to 19 , you should probably recompile the native code on your side with your requirements

Motivation

Since the update to 4.x it was unclear how to replace the NotificationExtenderService

Scope

This is only an extension, no existing feature should be affected.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
  • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
  • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
• I have reviewed this PR myself, ensuring it meets each checklist item

---

This change is 

[brismithers]

Overall this solution makes sense! Are those native Android changes required? If so, I would like to get that PR in as well. Also, note this change would be valuable to v5 of the SDK (which is already on monoandroid12.0). Thanks!

[tmijieux]

I just updated my commit to put the v12 in all of the three places you mentioned.

Are those native Android changes required ?

I just tested and It seems to work fine with the following code but it is probably a little bit harder/awkward to setup since because of the nested interface we are required to use, I think We can ditch the modification on the native code, but to keep the initial proposed usage with the simplicity of the single base class we can add these classes in the additions part of the android binding library. would that be ok ?

using System;
using System.Diagnostics;
using Android.Content;
using AndroidX.Core.App;
using Com.OneSignal.Android;
using MyApp;
using MyApp.Services;
using MyApp.Helpers;

using IExtender = AndroidX.Core.App.NotificationCompat.IExtender;

namespace MyApp.Droid
{
    public class MyExtender : Java.Lang.Object, IExtender
    {
        public  NotificationCompat.Builder Extend(NotificationCompat.Builder builder)
        {
            //do something with builder
            return builder
                //.SetContentTitle("mytest1")
                //.SetContentText("mytest2")
                //.AddAction(new NotificationCompat.Action(null, "mytest3", null))
                ;
        }
    }

    [NotificationExtension(Name = "com.my.app.MyOneSignalServiceExtension")]
    public class NotificationExtender : Java.Lang.Object, OneSignal.IOSRemoteNotificationReceivedHandler
    {
        public void RemoteNotificationReceived(Context _ctx, OSNotificationReceivedEvent ev)
        {
            Debug.WriteLine("In my extender!");
            var notif = ev.Notification.MutableCopy();
            notif.SetExtender(new MyExtender());

            if (notif?.AdditionalData == null)
            {
                Debug.WriteLine("regular notif");
                ev.Complete(notif);
                return;
            }

            bool hideNotif = false;
            try
            {
                var discard = notif.AdditionalData.Get("silent");
                if (discard != null)
                {
                    Debug.WriteLine("silent!");
                    hideNotif = true;
                }
            }
            catch (Exception /*e*/)
            {
            }
            ev.Complete(hideNotif ? null : notif);
        }
    }
}

[tmijieux]

The last version i pushed should work (tested on my side) and I removed the modification on the aar archive, with the same usage as initially proposed.

[brismithers]

what you did makes sense, thanks!

[brismithers]

Reviewed 2 of 5 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @emawby, @jinliu9508, @jkasten2, and @tmijieux)

[tmijieux]

Hi. Is this PR still in review ? gitlab still list this as "change requested" when i addressed everything .