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

remove background push and fix push response handling #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sample/Sample.iOS/AppDelegate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public override void FailedToRegisterForRemoteNotifications(UIApplication applic
public override async void DidReceiveRemoteNotification(UIApplication application, NSDictionary userInfo, Action<UIBackgroundFetchResult> completionHandler)
#pragma warning restore RECS0165 // Asynchronous methods should return a Task instead of void
{
await NotifoIO.Current.DidReceiveMessageAsync(userInfo);
NotifoIO.Current.DidReceiveMessageAsync(userInfo);
completionHandler(UIBackgroundFetchResult.NewData);
}

Expand Down
6 changes: 4 additions & 2 deletions sample/Sample.iOS/Sample.iOS.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,13 @@
<WarningLevel>4</WarningLevel>
<MtouchArch>ARM64</MtouchArch>
<CodesignKey>iPhone Developer</CodesignKey>
<MtouchDebug>true</MtouchDebug>
<CodesignEntitlements>Entitlements.plist</CodesignEntitlements>
<MtouchLink>None</MtouchLink>
<MtouchInterpreter>-all</MtouchInterpreter>
<CodesignProvision>
</CodesignProvision>
<MtouchDebug>true</MtouchDebug>
<MtouchNoSymbolStrip>true</MtouchNoSymbolStrip>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|iPhone' ">
<DebugType>none</DebugType>
Expand All @@ -64,8 +65,9 @@
<MtouchArch>ARM64</MtouchArch>
<CodesignKey>iPhone Developer</CodesignKey>
<CodesignEntitlements>Entitlements.plist</CodesignEntitlements>
<MtouchDebug>true</MtouchDebug>
<CodesignProvision>VS: io.notifo.xamarin.sample Development</CodesignProvision>
<DeviceSpecificBuild>false</DeviceSpecificBuild>
<MtouchUseLlvm>true</MtouchUseLlvm>
</PropertyGroup>
<PropertyGroup>
<CodeAnalysisRuleSet>..\..\Notifo.ruleset</CodeAnalysisRuleSet>
Expand Down
2 changes: 1 addition & 1 deletion sample/Sample/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ public static class Constants

public const string ApiUrl = "https://notifo-dev.easierlife.com";

public const string UserApiKey = "1zkry2syrttdxjqbs1zoj7dg9o45cixfuxhblhh49lcx";
public const string UserApiKey = "jp1poshmqt9cadv3znndjm3whrhlqxfdxedwd2snwxyx";
}
}
14 changes: 11 additions & 3 deletions sample/SampleNotificationServiceExtension/NotificationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,25 @@ public override async void DidReceiveNotificationRequest(UNNotificationRequest r
//Save the notification and create a mutable copy
BestAttemptContent = (UNMutableNotificationContent)request.Content.MutableCopy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Somehow the formatting is really fucked up here with tabs and spaces.


NotifoIO.Current
var notifo = NotifoIO.Current
.SetSharedName("group.io.notifo.xamarin.sample")
.SetNotificationHandler(new NotificationHandler());

await NotifoIO.Current.DidReceiveNotificationRequestAsync(request, BestAttemptContent);
notifo.OnLog += On_Log;


await NotifoIO.Current.DidReceiveNotificationRequestAsync(request, BestAttemptContent);

// Display the notification.
ContentHandler(BestAttemptContent);
}

public override void TimeWillExpire()
private static void On_Log(object source, NotificationLogEventArgs e)
{
Console.WriteLine($"DEBUG: Log {e.Message} Message Args: {e.MessageArgs}", e.Message, e.MessageArgs);
}

public override void TimeWillExpire()
{
// Called just before the extension will be terminated by the system.
// Use this as an opportunity to deliver your "best attempt" at modified content, otherwise the original push payload will be used.
Expand Down
18 changes: 3 additions & 15 deletions sdk/Notifo.SDK.FirebasePlugin/NotifoMobilePushExtensions.ios.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,22 +51,10 @@ public static INotifoMobilePush UseFirebasePluginEventsProvider(this INotifoMobi
/// <param name="notifo">The <see cref="INotifoMobilePush"/> instance.</param>
/// <param name="data">The notification data dictionary.</param>
/// <returns>A <see cref="Task"/> representing the result of the asynchronous operation.</returns>
public static async Task DidReceiveMessageAsync(this INotifoMobilePush notifo, NSDictionary data)
public static void DidReceiveMessageAsync(this INotifoMobilePush notifo, NSDictionary data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not be called "Async", if it is not async.

{
notifo.RaiseDebug(Strings.ReceivedNotification, null, data);

static bool ContainsPullRefreshRequest(NSDictionary data)
{
var aps = data?.ObjectForKey(new NSString(Constants.ApsKey)) as NSDictionary;

return aps?.ContainsKey(new NSString(Constants.ContentAvailableKey)) == true;
}

if (ContainsPullRefreshRequest(data))
{
await notifo.DidReceivePullRefreshRequestAsync();
}


FirebasePushNotificationManager.DidReceiveMessage(data);
}

Expand Down Expand Up @@ -99,7 +87,7 @@ public static void RemoteNotificationRegistrationFailed(this INotifoMobilePush n
/// <param name="completionHandler">The action to execute when you have finished processing the user's response.</param>
public static void DidReceiveNotificationResponse(this INotifoMobilePush notifo, UNUserNotificationCenter center, UNNotificationResponse response, Action completionHandler)
{
notifo.DidReceiveNotificationResponse(center, response, completionHandler);
notifo.DidReceiveNotificationResponse(response);

if (CrossFirebasePushNotification.Current is IUNUserNotificationCenterDelegate notificationDelegate)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,16 @@ public async Task DidReceiveNotificationRequestAsync(UNNotificationRequest reque
// 3. Enrich with images, which need to be downloaded.
await EnrichImagesAsync(content, notification);

await HandlePendingNotificationsAsync();

// 4. Custom enrichment code (could be potentially be expensive).
EnrichWithCustomCode(content, notification);
}

/// <inheritdoc />
public async Task DidReceivePullRefreshRequestAsync(PullRefreshOptions? options = null)
public async Task HandlePendingNotificationsAsync()
{
options ??= new PullRefreshOptions();
var options = new PullRefreshOptions();
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes the full options useless.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah that's right. Could be removed completely but what do you think in general of removing the pullRefresh?

Copy link
Contributor

Choose a reason for hiding this comment

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

In general I tried not to have some non-configurable values, but it is weird, that these options are configured over the method. So we should just add a method to INotifoPush like

NotifoPush.Current.SetPullOptions(...);

Copy link
Contributor

Choose a reason for hiding this comment

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

I have introduced this change in the base branch


// iOS does not maintain a queue of undelivered notifications, therefore we have to query here.
var notifications = await GetPendingNotificationsAsync(options.Take, options.Period, default);
Expand Down