Add support for push notifications #1091
Conversation
klochowicz
requested review from
luckysori,
holzeis and
bonomat
and removed request for
holzeis
klochowicz
force-pushed
the
feat/add-firebase
branch
from
e53d81d
to
529938b
Compare
|
I closed this PR early due to a GitGuardian warning that we're leaking API keys. After researching different approaches for a bit (env variables, cloud secret service, firebase shared config) I double-checked that indeed this key is supposed to be public and it's a well known false positive that they just don't care to fix 😖 GitGuardian/APISecurityBestPractices#14 PR is open again, if someone has Android it would be much appreciated to help testing it with me :) |
| static const FirebaseOptions android = FirebaseOptions( | ||
| apiKey: 'AIzaSyCDI7arcBp4uriwfLySgqrchkNVDVW94uk', | ||
| appId: '1:237191932527:android:651be56a933e8bda36bb9a', | ||
| messagingSenderId: '237191932527', | ||
| projectId: 'project-3195624368411891722', | ||
| storageBucket: 'project-3195624368411891722.appspot.com', | ||
| ); | ||
|
|
||
| static const FirebaseOptions ios = FirebaseOptions( | ||
| apiKey: 'AIzaSyD1FiZUHkX-rdHUtAXU12ku2e7zmBHqLSQ', | ||
| appId: '1:237191932527:ios:ed5586aaa838be1e36bb9a', | ||
| messagingSenderId: '237191932527', | ||
| projectId: 'project-3195624368411891722', | ||
| storageBucket: 'project-3195624368411891722.appspot.com', | ||
| iosClientId: '237191932527-70uoogr06k3rrrmslr5u1r6i4st9ch39.apps.googleusercontent.com', | ||
| iosBundleId: 'finance.get10101.app', | ||
| ); |
There was a problem hiding this comment.
Do we need to differentiate between mainnet and regtest?
// Edit // comment should be one line above at the iosBundleId because for regtest it will be different.
There was a problem hiding this comment.
Do you think it's worth differentiating between mainnet and regtest? I would rather assume that the tokens (or registration emails) should filter out our testing from other data, so we have clear data for analytics.
note that the file is autogenerated and should not be tempered with.
| Future<void> requestNotificationPermission() async { | ||
| FirebaseMessaging messaging = FirebaseMessaging.instance; | ||
| final token = await messaging.getToken(); | ||
| FLog.info(text: "Firebase token: $token"); |
There was a problem hiding this comment.
Nit: I don't think this deserves an info log level.
There was a problem hiding this comment.
Guide for Android testers: @bonomat and @Restioson - please DM me your firebase tokens for your Android devices when you deploy, and then I can send you some notifications before the release.
You can do the same from the Firebase Console as well yourselves if you prefer to. I will send a guide today.
I've one, what's the test scenario you want me to tets? |
I can also test if given instructions :) |
| case TargetPlatform.linux: | ||
| throw UnsupportedError( | ||
| 'DefaultFirebaseOptions have not been configured for linux - ' | ||
| 'you can reconfigure this by running the FlutterFire CLI again.', | ||
| ); |
There was a problem hiding this comment.
❓So we will throw exceptions every time we send a notification to a test app running on desktop, right? I assume that's fine.
There was a problem hiding this comment.
every time we send a notification to a test app running on linux desktop, macOS is actually supported!
this part was autogenerated by flutterfire :)
if it ever becomes a problem we can detect the platform, and skip subscribing to topics on Linux.
PS have you tried running this code on Linux? if it crashes, I'm happy to pair up to ensure it still works well!
There was a problem hiding this comment.
Haven't tried it yet, but I will!
Enable the following modes for the needs of FCM:
- background_fetch,
- remote_notifications.
Try to start the backend 3 times before giving up.
Makes it clear that one cannot deploy to device and hope that local regtest will
work (the backend crashes).
Without this, we'd miss the notifications sent via FCM if the user had the app open. note: The android part has not been tested as I don't have Android.
klochowicz
force-pushed
the
feat/add-firebase
branch
from
e53d81d
to
ca1edd1
Compare
Notifications are sent via FCM.
Both background and foreground notifications are supported.
Tested on iOS.
foreground
background