ushahidi / tenfour

API For TenFour
MIT License
7 stars 3 forks source link

Push Notifications Broken #172

Open CeciliaHinga opened 5 years ago

CeciliaHinga commented 5 years ago

From tenfour-archive created by mackers: ushahidi/tenfour-archive#1415

Expected Behaviour

Actual Behaviour

Steps to Reproduce

Environment

CeciliaHinga commented 5 years ago

See if Firebase upgrade fixes the issue

CeciliaHinga commented 5 years ago

@dalezak Something is happening on Android. The app receives the notification, but does not respond correctly:

Actual Results:

Expected Results:

CeciliaHinga commented 5 years ago

Hey @dalezak any progress on this or how can I help??

CeciliaHinga commented 5 years ago

Sadly, no progress on this yet @mackers, have been sidelined by Dispatcher prototype.

Hopefully I can get to this next week after we share the Internal Alpha of Dispatcher?

CeciliaHinga commented 5 years ago

@dalezak are you able to look at this in next cycle?

CeciliaHinga commented 5 years ago

Hmm, checking Firebase console it looks like there are notifications being sent.

unspecified-1

CeciliaHinga commented 5 years ago

Good news and bad news @mackers @caharding.

Good news, is the latest angularfire2 supports messaging, which means we don't need to hack together our our implementation for push notifications in the PWA.

Bad news, I tried upgrading angularfire2 but it requires Angular 6.0.0 or greater.

npm WARN @angular/fire@5.1.0 requires a peer of @angular/common@>=6.0.0 <8 but none was installed.
npm WARN @angular/fire@5.1.0 requires a peer of @angular/core@>=6.0.0 <8 but none was installed.
npm WARN @angular/fire@5.1.0 requires a peer of @angular/platform-browser@>=6.0.0 <8 but none was installed.
npm WARN @angular/fire@5.1.0 requires a peer of @angular/platform-browser-dynamic@>=6.0.0 <8 but none was installed.
npm WARN @angular/fire@5.1.0 requires a peer of rxjs@^6.0.0 but none was installed.
npm WARN ajv-keywords@2.1.1 requires a peer of ajv@^5.0.0 but none was installed.

And fortunately the latest stable Ionic 3.9.2 only supports Angular 5.2.11, after upgrading I'm getting errors launching the PWA.

TypeError: Object(...) is not a function at new AngularFireMessaging (http://localhost:8100/build/vendor.js:101742:86)

So unfortunately we won't be able to use this new version of angularfire2 at the moment anyways 😞

CeciliaHinga commented 5 years ago

So, I just sent a test from the Firebase Console, after some local changes to the app, the test notification is being received in the iOS app.

console.warn: FirebaseProvider onNotification
{"from":"804609537189","collapse_key":"com.ushahidi.tenfour","notification":{"title":"Title1","e":"1","body":"Message1"}}

However when I create a new checkin on https://dale.dev.tenfour.org/#/checkins, the app doesn't receive any notification.

@mackers are Push Notifications wired up to broadcast from dev.tenfour.org?

CeciliaHinga commented 5 years ago

Woah! When I switched the Sender ID to 804609537189, I was able to receive a notification in the PWA!

{"data":{"gcm.n.e":"1","google.c.a.ts":"1542406528","google.c.a.udt":"0","google.c.a.c_id":"6275608972089643762","google.c.a.e":"1"},"from":"804609537189","priority":"high","notification":{"title":"Title`","body":"Message1"}}

Ooooh, @mackers maybe 240600431570 is from the OLD Firebase project when we were hosting the PWA there? And 804609537189 is for the CURRENT Firebase project?

CeciliaHinga commented 5 years ago

Ya, I should have time in this next cycle @caharding to fix this 👍

CeciliaHinga commented 5 years ago

@dalezak this looks like it will have to wait until a push to production has happened? Please confirm.

CeciliaHinga commented 5 years ago

@dalezak Push notifications are coming through for iPhone but not on Android.

CeciliaHinga commented 5 years ago

Yes, backend uses FCM_SENDER_ID: 804609537189

CeciliaHinga commented 5 years ago

Hmm, another thing that's weird @mackers, our environment variables have:

firebaseSenderId: '240600431570'

However the Sender ID on the Firebase Console is 804609537189.

unspecified-2

Shouldn't these values be the same? Where did the 240600431570 come from?