[firebase_messaging] Use UNUserNotificationCenter for ios 10+ #121

bparrishMines · 2019-09-09T23:15:56Z

Description

~~Switch to use UNUserNotificationCenter once flutter/engine#9864 lands in flutter master.~~ Landed

Follows instructions from: https://firebase.google.com/docs/cloud-messaging/ios/client#register_for_remote_notifications

This includes a test that can run locally on Android and iOS. We can't run it on our CI because it requires a valid Firebase project and a server token.

Related Issues

Should fix flutter/flutter#22099

Possibly Related Issues
flutter/flutter#35375
flutter/flutter#36201

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

bparrishMines · 2019-09-12T18:54:10Z

@collinjackson @kroikie This PR should hopefully help deal with the recent problems with the plugin. Although, I'm not sure how to test it since my provisioning profile doesn't support Push Notifications or include aps-environment entitlement?

error: Provisioning profile "iOS Team Provisioning Profile: *" doesn't support the Push Notifications capability. (in target 'Runner')
    error: Provisioning profile "iOS Team Provisioning Profile: *" doesn't include the aps-environment entitlement. (in target 'Runner')

Are the test driver tests on Cirrus sufficient?

collinjackson · 2019-09-23T18:48:58Z

The messaging v1 API might be a good option for integration testing: https://firebase.google.com/docs/reference/fcm/rest/v1/projects.messages

genert · 2019-09-25T08:01:08Z

I think the README should also contain code snippet for those who use Swift when it comes to delegating UNUserNotificationCenter.

Therefore, Swift users must add following line within override func application method in AppDelegate.swift file:

if #available(iOS 10.0, *) {
    UNUserNotificationCenter.current().delegate = self
}

tvolkert · 2019-10-10T14:53:10Z

It looks like flutter/engine#9864 landed a while back - is this PR ready to resume?

bparrishMines · 2019-10-15T00:32:35Z

@tvolkert I'm still working on adding tests for this.

minh98 · 2019-10-19T08:45:14Z

any update ? :D

kroikie · 2019-10-30T17:21:32Z

packages/firebase_messaging/README.md

@@ -143,6 +143,15 @@ To integrate your plugin into the iOS part of your app, follow these steps:

 1. Follow the steps in the "[Upload your APNs certificate](https://firebase.google.com/docs/cloud-messaging/ios/client#upload_your_apns_certificate)" section of the Firebase docs.

+1. Add the following lines to the `(BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions`
+method In the `AppDelegate.m` of your iOS project.


nit: s/In/in

kroikie · 2019-10-30T17:32:28Z

packages/firebase_messaging/example/test_driver/firebase_messaging.dart

+    // We skip this test because it requires a valid server token specified at
+    // the top of this file uncommenting `setUpAll`, and requires agreeing to
+    // receive messages by hand on ios.
+    test('onMessage', () async {
+      await http.post(
+        'https://fcm.googleapis.com/fcm/send',
+        headers: <String, String>{
+          'Content-Type': 'application/json',
+          'Authorization': 'key=$serverToken',
+        },
+        body: jsonEncode(
+          <String, dynamic>{
+            'notification': <String, dynamic>{
+              'body': 'this is a body',
+              'title': 'this is a title'
+            },
+            'priority': 'high',
+            'data': <String, dynamic>{
+              'click_action': 'FLUTTER_NOTIFICATION_CLICK',
+              'id': '1',
+              'status': 'done'
+            },
+            'to': await firebaseMessaging.getToken(),
+          },
+        ),
+      );
+
+      final Completer<Map<String, dynamic>> completer =
+          Completer<Map<String, dynamic>>();
+
+      firebaseMessaging.configure(
+        onMessage: (Map<String, dynamic> message) async {
+          completer.complete(message);
+        },
+      );
+
+      final Map<String, dynamic> message = await completer.future;
+      expect(
+        message,
+        containsPair('notification', containsPair('title', 'this is a title')),
+      );
+      expect(
+        message,
+        containsPair('notification', containsPair('body', 'this is a body')),
+      );
+    }, timeout: const Timeout(Duration(seconds: 5)), skip: true);


Since this is skipped for the reasons you identified, could we remove it till we have a good way to run it. Maybe even include it in the README as an option test for developers?

collinjackson

Feel free to file an issue and assign to me to get this test running on CI -- I think that the XCUITest stuff I'm doing should make it possible.

reecepm · 2019-11-01T02:59:18Z

As mentioned before, also needs support for swift. I believe it's something like this:

    if #available(iOS 10.0, *) {
        UNUserNotificationCenter.current().delegate = self as? UNUserNotificationCenterDelegate
    }

but not 100% on that

EDIT: I implemented your change and used this line above in my AppDelegate.swift and it worked for me.

reecepm · 2019-11-01T15:57:01Z

packages/firebase_messaging/README.md

+  [UNUserNotificationCenter currentNotificationCenter].delegate = (id<UNUserNotificationCenterDelegate>) self;
+}
+```
+


Alternatively, if you are using Swift over Objective C, add the following lines to the

application( _ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? ) -> Bool

method in the AppDelegate.swift of your iOS project.

if #available(iOS 10.0, *) { UNUserNotificationCenter.current().delegate = self as? UNUserNotificationCenterDelegate }

Thanks for pointing this out! I added a swift example.

njovy · 2019-11-06T02:17:14Z

How long would it take till it’s merged to master?

* bump core version to 0.4.1+4 (firebase#1395) * Remove deprecated firebase-core dependency (firebase#1417) * Remove deprecated firebase-core dependency * [firebase_auth] getIdToken use actual refresh value instead of checking if object exists (firebase#1334) * getIdToken use actual refresh value instead of checking if object exists * getIdToken refresh integration test * update xcode to v11 (firebase#1420) * Update FirebaseApp creation in tests (firebase#1406) * [firebase_database] Support v2 android embedder. (firebase#287) * [firebase_remote_config] Support v2 android embedder. (firebase#282) * [CI] Add version to example apps (firebase#1426) * Add hardcode version to example apps * [firebase_core] Add platform interface (firebase#1324) * Move firebase_core to firebase_core/firebase_core * Add firebase_core_platform_interface package * Update cirrus scripts and documentation * [firebase_dynamic_links] Add support of expanding from short links (firebase#1381) * Fixes strict compilation errors. (firebase#1331) * Upgrade crashlytics to v2 plugin API (firebase#1370) * Upgraded Crashlytics to v2 of Flutter Plugins. * Format documentation lists (See also & Errors) correctly (firebase#298) Fix documentation list format. * [firebase_auth] Fix NoSuchMethodError exception in `reauthenticateWithCredential` (firebase#237) * [firebase_auth] Fix NoSuchMethodError exception Fix NoSuchMethodError exception when calling FirebaseUser.reauthenticateWithCredential * add integration test * Fix analyzer warnings Co-Authored-By: Collin Jackson <[email protected]> * [cloud_firestore] [firebase_performance] fix analyzer warnings (firebase#1453) * fix analyzer warnings * [CI] Skip flaky firebase_performance driver tests (firebase#1452) * Skip flaky firebase_performance driver tests * [firebase_auth] Added missing Exception to the reauthenticateWithCredential docs (firebase#1448) (firebase#1449) * Added missing Exception to the reauthenticateWithCredential docs (firebase#1448) * [firebase_remote_config] Bumps Android Firebase dependency to 19.0.3 (firebase#1443) * Bump AGP, Gradle & Google Services Plugin * Bumps dependency to Firebase Config 19.0.3 * Replaces a deprecated method usage with the updated version * [firebase_messaging] Use UNUserNotificationCenter for ios 10+ (firebase#121) * Use UNUserNotificationCenter for ios 10+ * Add swift documentation * Move http test to README * Update CONTRIBUTING.md (firebase#1473) Removes obsolete recommendations about removing mockito (which is now used in testing federated plugins). * [firebase_core] Migrate to platform_interface (firebase#1472) * [firebase_messaging] Add an ArgumentError when passing invalid background message (firebase#252) * [firebase_dynamic_links] support v2 embedding (firebase#1372) * [cloud_firestore] Fix test that used FirebaseApp.channel (firebase#1476) * [cloud_firestore] Fix test that used FirebaseApp.channel * dartfmt * bump version * Fixed dynamic issue * Fixed dynamic issue * FIxed Object to list of object mapping * Fixed text result expectation * Reverted some changes * Satisfying formatter * Added android x to pass the test * Fixed changed log auto formatting

njovy · 2020-01-02T05:19:15Z

@kroikie @bparrishMines There are many same or similar issues posted that the latest fcm for flutter doesn't work properly on iOS. Please take a look.

rscsam · 2020-01-02T13:34:42Z

I am still experiencing issues where my onResume and onMessage callbacks are not firing properly. I made no code changes that I can think to be relevant.

njovy · 2020-01-03T04:39:49Z

@rscsam The latest library doesn't work on iOS. This isn't about wrong configuration. The library itself just doesn't work at the moment. I guess this repo isn't maintained by any Google developers because I have never had this kind of issue with Google products before. #1644 This is the one I opened up 3 weeks ago and it hasn't been fixed.

rscsam · 2020-01-04T03:54:13Z

@njovy This isn't a "solution" per se, but removing
if #available(iOS 10.0, *) { UNUserNotificationCenter.current().delegate = self as? UNUserNotificationCenterDelegate }
from my AppDelegate.swift fixed this problem for me.

njovy · 2020-01-04T09:41:09Z

@rscsam Getting rid of that line is a solution!!

kekedaine · 2020-01-08T11:28:55Z

@njovy This isn't a "solution" per se, but removing
if #available(iOS 10.0, *) { UNUserNotificationCenter.current().delegate = self as? UNUserNotificationCenterDelegate }
from my AppDelegate.swift fixed this problem for me.

work fine for me.
Flutter (Channel stable, v1.12.13+hotfix.5, on Mac OS X 10.15.1 19B88

Fixes #121

bparrishMines requested review from collinjackson and kroikie as code owners September 9, 2019 23:15

bparrishMines added the type: bug label Sep 9, 2019

bparrishMines changed the title ~~[firebase_messaging] Use UNUserNotificationCenter for ios 10+~~ [WIP] [firebase_messaging] Use UNUserNotificationCenter for ios 10+ Sep 10, 2019

collinjackson removed the type: bug label Sep 10, 2019

googlebot added the cla: yes label Sep 12, 2019

bparrishMines changed the title ~~[WIP] [firebase_messaging] Use UNUserNotificationCenter for ios 10+~~ [firebase_messaging] Use UNUserNotificationCenter for ios 10+ Sep 12, 2019

bparrishMines added the impact: critical label Sep 12, 2019

MaikuB mentioned this pull request Sep 14, 2019

firebase_messaging not working if include flutter_local_notifications package on iOS MaikuB/flutter_local_notifications#111

Closed

bparrishMines mentioned this pull request Oct 16, 2019

Add capability to add AppDelegate as UNUserNotificationCenterDelegate flutter/engine#9864

Merged

reecepm mentioned this pull request Oct 29, 2019

Firebase messaging onResume and onMessage broken on iOS since Flutter 1.7.4 #1056

Closed

kroikie approved these changes Oct 30, 2019

View reviewed changes

collinjackson approved these changes Oct 30, 2019

View reviewed changes

kroikie mentioned this pull request Oct 30, 2019

Fixes strict compilation errors. #1331

Merged

13 tasks

reecepm reviewed Nov 1, 2019

View reviewed changes

bparrishMines added 7 commits November 19, 2019 16:46

Use UNUserNotificationCenter for ios 10+

07ace4e

Formatting

bc913a5

Return success of retrieving permissions

a55059a

more formatting

e2d83b4

Remove setting delegate to registrar

aeba679

Set UNUserNotificationCenterDelegate in AppDelegate.m

3f99508

Update README

a9ba344

bparrishMines added 7 commits November 19, 2019 16:46

formatting

8a2d1cf

Add onMessage Test

ba23ed8

Skip test for CI

490b3ba

Rearrange code

c03d69b

Comment out setupall

71be475

Fix case and add swift documentation

37043b8

Move http test to README

b9c8112

kroikie force-pushed the almost_there branch from 96b567c to b9c8112 Compare November 20, 2019 00:48

kroikie merged commit 891ea57 into firebase:master Nov 20, 2019

NickM-27 mentioned this pull request Dec 12, 2019

[firebase_messaging] MissingPluginException when building for macOS #1608

Closed

njovy mentioned this pull request Jan 4, 2020

[firebase_messaging] Dart side callbacks for FCM are not called on iOS if a message has a notification field #1644

Closed

bparrishMines deleted the almost_there branch March 17, 2020 17:17

besidesordinary mentioned this pull request May 5, 2020

flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java uses unchecked or unsafe operations. Note: Recompile with -Xlint:unchecked for details. #2490

Closed

firebase locked and limited conversation to collaborators Aug 19, 2020

Ehesp pushed a commit that referenced this pull request Oct 6, 2020

Correctly cancel subscription in database (#198)

925c318

Fixes #121

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[firebase_messaging] Use UNUserNotificationCenter for ios 10+ #121

[firebase_messaging] Use UNUserNotificationCenter for ios 10+ #121

bparrishMines commented Sep 9, 2019 •

edited

Loading

bparrishMines commented Sep 12, 2019 •

edited

Loading

collinjackson commented Sep 23, 2019

genert commented Sep 25, 2019 •

edited

Loading

tvolkert commented Oct 10, 2019

bparrishMines commented Oct 15, 2019

minh98 commented Oct 19, 2019

kroikie Oct 30, 2019

kroikie Oct 30, 2019

collinjackson left a comment

reecepm commented Nov 1, 2019 •

edited

Loading

reecepm Nov 1, 2019 •

edited

Loading

bparrishMines Nov 20, 2019

njovy commented Nov 6, 2019

njovy commented Jan 2, 2020 •

edited

Loading

rscsam commented Jan 2, 2020

njovy commented Jan 3, 2020

rscsam commented Jan 4, 2020

njovy commented Jan 4, 2020

kekedaine commented Jan 8, 2020

[firebase_messaging] Use UNUserNotificationCenter for ios 10+ #121

[firebase_messaging] Use UNUserNotificationCenter for ios 10+ #121

Conversation

bparrishMines commented Sep 9, 2019 • edited Loading

Description

Related Issues

Checklist

Breaking Change

bparrishMines commented Sep 12, 2019 • edited Loading

collinjackson commented Sep 23, 2019

genert commented Sep 25, 2019 • edited Loading

tvolkert commented Oct 10, 2019

bparrishMines commented Oct 15, 2019

minh98 commented Oct 19, 2019

kroikie Oct 30, 2019

Choose a reason for hiding this comment

kroikie Oct 30, 2019

Choose a reason for hiding this comment

collinjackson left a comment

Choose a reason for hiding this comment

reecepm commented Nov 1, 2019 • edited Loading

reecepm Nov 1, 2019 • edited Loading

Choose a reason for hiding this comment

bparrishMines Nov 20, 2019

Choose a reason for hiding this comment

njovy commented Nov 6, 2019

njovy commented Jan 2, 2020 • edited Loading

rscsam commented Jan 2, 2020

njovy commented Jan 3, 2020

rscsam commented Jan 4, 2020

njovy commented Jan 4, 2020

kekedaine commented Jan 8, 2020

bparrishMines commented Sep 9, 2019 •

edited

Loading

bparrishMines commented Sep 12, 2019 •

edited

Loading

genert commented Sep 25, 2019 •

edited

Loading

reecepm commented Nov 1, 2019 •

edited

Loading

reecepm Nov 1, 2019 •

edited

Loading

njovy commented Jan 2, 2020 •

edited

Loading