Background push notification processing #3877

roryabraham · 2021-07-02T23:26:03Z

Details

This pull request makes it so that push notifications received when the Android app is closed will update Onyx, within the bounds of certain OS restrictions. This is important to achieve a background data trickle, and update the mobile app with chat data as frequently as possible.

On Android, push notifications received when the app is closed will wake a headless JS process (that lives in the UrbanAirship RN sdk) and execute a JS callback. However, it will not mount any RN components, so the push notification callbacks must be registered outside of any React lifecycle. Because the callback is dependent on the Onyx store, we must also initialize that outside of any React lifecycle. Fortunately, that works just fine for our application 😁

Fixed Issues

$ (partial) https://github.com/Expensify/Expensify/issues/158830

Tests

This is a bit of a doozy to test. It will be native-only, with only regression testing on other platforms.

Get a prod build running – Android

Follow the instructions in this SO to generate a signing key and provide it to the app through build.gradle.
Open android/app/src/main/assets/airshipconfig.properties. Swap out the productionAppKey and productionAppSecret with the developmentAppKey and developmentAppSecret.
Run npm i
Run react-native start --reset-cache. Kill this command once you see the cache has been cleared.
Run cd android && ./gradlew clean
Open your Android emulator and make sure to fully uninstall any Expensify.cash app on there.
Run npm run android -- --variant=release.
Once that runs, verify that you do not see any logs in the metro window that opens. Kill the metro window, and verify that you can still view the app. Close the app (hit the square button and swipe up), then reopen it. It should still work.

Get a prod build running – iOS

This requires a physical iPhone for testing.
Run through the steps in this SO to get set up to run the app on a physical iPhone device.
Run npm i -g ios-deploy
Plug your device into your computer. Click "trust" when prompted.
Open XCode, go to the Expensify.cash project configurations, click Sigining and Capabilities, and click "Enable Automatic". This will make some temporary changes that must not be committed, but for me are always necessary to get the app to launch on a physical device.
Finally, run npm run ios -- --configuration Release --device "Your Device Name"

Test push notifications (both platforms)

Tail your VM logs for [Report] Handled event sent by Airship. This will indicate that the push notification callback executed.
Run npm run web (you pick)
Sign into account A on the mobile app. Sign into account B on the web app. These should be accounts on your VM, not prod accounts.
Send a message from account B to account A. It should arrive normally.
Send a reply from account A to account B. It should arrive normally.
As user A, open a different chat, and background the app.
Send a message from B to A. A push notification should appear, and a log line demonstrating that the callback occurred should also appear.
Put the emulator/device on airplane mode.
Double-tap the notification. It should open E.cash to the correct chat, and the message should be available.
Take the emulator/device off airplane mode.
(android only) Open the emulator settings, then go to Security -> Screen Lock -> Swipe.
Hit the power button on the emulator menu (or lock button on the iPhone) to lock the emulator/device:
Send a message from B to A. A push notification should appear, and a log line demonstrating that the callback occurred should also appear.
Put the emulator/device on airplane mode.
Double-tap the notification. It should open E.cash to the correct chat, and the new message should be available.
Hit the square button and swipe up to close E.cash entirely.
Send a message from B to A. A push notification should appear, and a log line demonstrating that the callback occurred should also appear.
Put the emulator/device on airplane mode.
Double-tap the notification. It should open E.cash to the correct chat, and the new message should be available.

QA Steps

Sign into account A on the Android app. Sign into account B on the web app.
Send a message from account B to account A. It should arrive normally.
Send a reply from account A to account B. It should arrive normally.
As user A, open a different chat, and background the app.
Send a message from B to A. A push notification should appear.
After about 10 seconds, put the emulator on airplane mode.
Double-tap the notification. It should open E.cash to the correct chat, and the message should be available.
Take the emulator off airplane mode.
Open the android settings, then go to Security -> Screen Lock -> Swipe (doesn't have to be swipe – just make sure you can see a lock screen on the device).
Lock the Android device.
Send a message from B to A. A push notification should appear.
After about 10 seconds, put the emulator on airplane mode.
Double-tap the notification. It should open E.cash to the correct chat, and the new message should be available.
Hit the square button and swipe up to close E.cash entirely.
Send a message from B to A. A push notification should appear, and a log line demonstrating that the callback occurred should also appear.
After about 10 seconds, put the emulator on airplane mode.
Double-tap the notification. It should open E.cash to the correct chat, and the new message should be available.

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

....Will need to gather screen recordings after the long weekend.

roryabraham · 2021-07-02T23:27:41Z

Going to need to HOLD on a Web-E PR to make sure that we're passing 'delivery_priority' => 'high' along with Android push notifications (only for report comments for now).

nickmurray47 · 2021-07-06T21:17:37Z

@roryabraham did you want an early review on this or should I wait until the HOLD comes off?

roryabraham · 2021-07-07T01:12:51Z

@nickmurray47 Feel free to review when you're ready! Just holding on the deployment of the API changes.

roryabraham · 2021-07-07T01:56:21Z

Not sure why PullerBear got a bit trigger-happy there @Luke9389, feel free to ignore.

roryabraham · 2021-07-07T21:51:21Z

Hooray, I was able to observe the iOS app being awoken and updating Onyx when the app was completely closed 🎉. It certainly does not happen consistently, even when the app is only backgrounded and not closed, but there is nothing we can do to side-step the OS limitations on content_available push notifications.

nickmurray47 · 2021-07-08T00:33:47Z

Going through the testing steps now

nickmurray47

Had a couple questions, but the code LGTM as far as I can tell. Still running through the testing steps.

src/libs/Notification/PushNotification/index.native.js

src/libs/actions/Report.js

roryabraham · 2021-07-08T23:33:42Z

@nickmurray47 Let me know if you have any more questions here

roryabraham · 2021-07-09T00:58:42Z

@nickmurray47 If you are running through the testing steps, you'll need to checkout this PR on Web-E for it to work.

roryabraham · 2021-07-09T01:07:32Z

Also, @nickmurray47 I added some testing steps for iOS, since this fixes some push notification issues on iOS as well.

nickmurray47 · 2021-07-12T18:14:09Z

If you are running through the testing steps, you'll need to checkout this PR on Web-E for it to work.

Figured I was missing something like this! Will test the new iOS steps today

roryabraham · 2021-07-12T21:04:30Z

cc @AndrewGable if you're interested

# Conflicts: # ios/Podfile.lock

roryabraham · 2021-08-30T22:53:02Z

Sorry to muddy up the git history, those last changes were meant to go to another PR. They've been removed from this one.

Julesssss · 2021-08-31T09:39:26Z

Sorry to hold up this PR -- I would like to review and test this properly but have just not had the time to get to additional reviews today.

nickmurray47 · 2021-08-31T16:58:07Z

Re-ran through all the test steps and now navigating to the exact chat from a notification works great!

One thing I just noticed (and not sure if this is by design) but it seems like the notification only pops up when I've fully closed the app. If I just switch to a different chat, background it, and receive a new message then no notification comes up.

marcaaron · 2021-08-31T17:24:58Z

One thing I just noticed (and not sure if this is by design) but it seems like the notification only pops up when I've fully closed the app. If I just switch to a different chat, background it, and receive a new message then no notification comes up.

I experienced something similar but it was more like... a lag or something where it just took several seconds for the notification to appear.

nickmurray47 · 2021-08-31T17:50:46Z

I experienced something similar but it was more like... a lag or something where it just took several seconds for the notification to appear.

Cool, wanted to make a note of that. The code looks good to me, very nicely done.

roryabraham · 2021-08-31T18:26:24Z

One thing I just noticed (and not sure if this is by design) but it seems like the notification only pops up when I've fully closed the app. If I just switch to a different chat, background it, and receive a new message then no notification comes up.

Thanks for retesting @nickmurray47! This sounds like it could be a separate issue ... unless you change some of the code, Web-Expensify will send a silent push notification if it still thinks you're connected via pusher. Might be addressed over here.

Julesssss · 2021-09-01T16:23:28Z

This sounds like it could be a separate issue ... unless you change some of the code, Web-Expensify will send a silent push notification if it still thinks you're connected via pusher. Might be addressed over here.

Yeah, that will be solved by this issue Rory linked.

Julesssss · 2021-09-01T16:29:39Z

.eslintrc.js

+    globals: {
+        __DEV__: 'readonly',
+    },


What is this for?

__DEV__ is a global variable in RN, and this tells ESLint that so that we can remove some unnecessary ESLint suppressions. Not too closely related to this PR, just a bit of housekeeping.

Julesssss · 2021-09-01T16:32:09Z

ios/NewExpensify/Info.plist

@@ -26,6 +26,7 @@
 			<key>CFBundleURLSchemes</key>
 			<array>
 				<string>expensify-cash</string>


Do we still need the old URLScheme? Are you aware of any code that still uses this?

I'm not aware of any code that still uses this, but I know we have some code that links back-and-forth between OldDot and NewDot and I don't want to break anything in this PR by removing it.

We can create a FP follow-up to remove this and any usages across all our repos (probably just E/App and Web-Expensify).

https://github.com/Expensify/Expensify/issues/176081

Julesssss

Cool, this is working really well! (I tested an Android physical device)

I'm not sure if this is helpful at all, but saw some comments about headless tasks... the whole headless JS thing is necessary because Android is so protective of UI threads. The only way to render anything on Android is via a UI thread, and ignoring some exceptions we are not granted access to these threads outside of an Activity. Services allow us to run code in the background but don't provide access to the UI thread and are restricted by the OS, hence why ReactNative Headless tasks extend from Service, doesn't have access to the App context, and cannot make any UI changes.

OSBotify · 2021-09-01T17:35:26Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2021-09-02T01:02:32Z

🚀 Deployed to staging by @roryabraham in version: 1.0.91-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

isagoico · 2021-09-02T18:08:34Z

This issue is failing this PR #5022

roryabraham · 2021-09-02T22:50:53Z

@isagoico Can we please retest this PR with the following testing steps:

Sign into account A on the Android app. Sign into account B on the web app.
Send a message from account B to account A. It should arrive normally.
Send a reply from account A to account B. It should arrive normally.
As user A, open a different chat, and background the app.
Wait for 5 minutes.
Send a message from B to A. A push notification should appear.
After about 10 seconds, put the emulator on airplane mode.
Double-tap the notification. It should open E.cash to the correct chat, and the message should be available.
Take the emulator off airplane mode.
Open the android settings, then go to Security -> Screen Lock -> Swipe (doesn't have to be swipe – just make sure you can see a lock screen on the device).
Lock the Android device.
Wait for 5 minutes.
Send a message from B to A. A push notification should appear.
After about 10 seconds, put the emulator on airplane mode.
Double-tap the notification. It should open E.cash to the correct chat, and the new message should be available.
Hit the square button and swipe up to close E.cash entirely.
Wait for 5 minutes.
Send a message from B to A. A push notification should appear, and a log line demonstrating that the callback occurred should also appear.
After about 10 seconds, put the emulator on airplane mode.
Double-tap the notification. It should open E.cash to the correct chat, and the new message should be available.

isagoico · 2021-09-03T00:50:59Z

Just tested this on Android with the new steps and it was a pass 🎉 Checking it off

OSBotify · 2021-09-03T01:30:01Z

🚀 Deployed to production by @roryabraham in version: 1.0.92-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

roryabraham · 2021-12-02T18:27:09Z

@isagoico, when you get a chance can you please confirm that the testing steps from this PR are included in our regression suite?

roryabraham added 5 commits July 2, 2021 15:52

Upgrade UA to 11.0.2

da2100e

Create a separate init function for push notification callbacks

3352b90

Give default notification preference and use const

ccb1461

Move report comment push notification callbacks to their own function

ea21c60

Initialize onyx and push notifications outside of react lifecycle

9675d72

roryabraham requested a review from a team July 2, 2021 23:26

roryabraham self-assigned this Jul 2, 2021

roryabraham changed the title ~~Background push notifications v2~~ Android background push notifications Jul 2, 2021

MelvinBot requested review from nickmurray47 and removed request for a team July 2, 2021 23:26

roryabraham changed the title ~~Android background push notifications~~ [HOLD] Android background push notifications Jul 2, 2021

roryabraham changed the title ~~[HOLD] Android background push notifications~~ [HOLD] Background push notification processing Jul 6, 2021

Update airship pod

e52d86d

roryabraham requested a review from a team as a code owner July 7, 2021 01:12

MelvinBot requested review from Luke9389 and removed request for a team July 7, 2021 01:12

roryabraham removed the request for review from Luke9389 July 7, 2021 01:55

nickmurray47 previously approved these changes Jul 8, 2021

View reviewed changes

src/libs/Notification/PushNotification/index.native.js Outdated Show resolved Hide resolved

src/libs/actions/Report.js Show resolved Hide resolved

Merge branch 'main' into Rory-BackgroundPushNotificationsV2

c2d1d48

# Conflicts: # ios/Podfile.lock

Make dummy change in README

fe6829c

roryabraham dismissed marcaaron’s stale review via fe6829c August 30, 2021 22:23

roryabraham added 2 commits August 30, 2021 15:24

Undo dummy change

da6e525

Fully undo dummy change

52a4908

Merge branch 'main' into Rory-BackgroundPushNotificationsV2

58a5b20

nickmurray47 approved these changes Aug 31, 2021

View reviewed changes

roryabraham mentioned this pull request Aug 31, 2021

onReady={() => {}} callback called before navigationRef is set. (getCurrentRoute() undefined onReady) react-navigation/react-navigation#9170

Closed

Julesssss reviewed Sep 1, 2021

View reviewed changes

Julesssss approved these changes Sep 1, 2021

View reviewed changes

roryabraham merged commit 4e8b6bf into main Sep 1, 2021

roryabraham deleted the Rory-BackgroundPushNotificationsV2 branch September 1, 2021 17:34

OSBotify mentioned this pull request Sep 1, 2021

Deploy Checklist: New Expensify 2021-09-01 #5004

Closed

55 tasks

Julesssss mentioned this pull request Sep 2, 2021

[HOLD][Expensify/#219254][$2000] Implement periodic background updates on mobile, keeping Onyx in sync #4264

Closed

5 tasks

isagoico mentioned this pull request Sep 2, 2021

No notification displayed when app is backgrounded or killed #5022

Closed

roryabraham mentioned this pull request Dec 2, 2021

[HOLD for payment 2022-02-02][$8000] - iOS - User stuck on Splash screen when launching the app #5620

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Background push notification processing #3877

Background push notification processing #3877

roryabraham commented Jul 2, 2021 •

edited

Loading

roryabraham commented Jul 2, 2021

nickmurray47 commented Jul 6, 2021

roryabraham commented Jul 7, 2021

roryabraham commented Jul 7, 2021

roryabraham commented Jul 7, 2021

nickmurray47 commented Jul 8, 2021

nickmurray47 left a comment

roryabraham commented Jul 8, 2021

roryabraham commented Jul 9, 2021

roryabraham commented Jul 9, 2021

nickmurray47 commented Jul 12, 2021

roryabraham commented Jul 12, 2021

roryabraham commented Aug 30, 2021

Julesssss commented Aug 31, 2021

nickmurray47 commented Aug 31, 2021

marcaaron commented Aug 31, 2021

nickmurray47 commented Aug 31, 2021

roryabraham commented Aug 31, 2021 •

edited

Loading

Julesssss commented Sep 1, 2021

Julesssss Sep 1, 2021

roryabraham Sep 1, 2021

Julesssss Sep 1, 2021 •

edited

Loading

roryabraham Sep 1, 2021

roryabraham Sep 1, 2021

roryabraham Sep 1, 2021

roryabraham Sep 1, 2021

Julesssss left a comment

OSBotify commented Sep 1, 2021

OSBotify commented Sep 2, 2021

isagoico commented Sep 2, 2021

roryabraham commented Sep 2, 2021 •

edited

Loading

isagoico commented Sep 3, 2021

OSBotify commented Sep 3, 2021

roryabraham commented Dec 2, 2021

Background push notification processing #3877

Background push notification processing #3877

Conversation

roryabraham commented Jul 2, 2021 • edited Loading

Details

Fixed Issues

Tests

Get a prod build running – Android

Get a prod build running – iOS

Test push notifications (both platforms)

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

roryabraham commented Jul 2, 2021

nickmurray47 commented Jul 6, 2021

roryabraham commented Jul 7, 2021

roryabraham commented Jul 7, 2021

roryabraham commented Jul 7, 2021

nickmurray47 commented Jul 8, 2021

nickmurray47 left a comment

Choose a reason for hiding this comment

roryabraham commented Jul 8, 2021

roryabraham commented Jul 9, 2021

roryabraham commented Jul 9, 2021

nickmurray47 commented Jul 12, 2021

roryabraham commented Jul 12, 2021

roryabraham commented Aug 30, 2021

Julesssss commented Aug 31, 2021

nickmurray47 commented Aug 31, 2021

marcaaron commented Aug 31, 2021

nickmurray47 commented Aug 31, 2021

roryabraham commented Aug 31, 2021 • edited Loading

Julesssss commented Sep 1, 2021

Julesssss Sep 1, 2021

Choose a reason for hiding this comment

roryabraham Sep 1, 2021

Choose a reason for hiding this comment

Julesssss Sep 1, 2021 • edited Loading

Choose a reason for hiding this comment

roryabraham Sep 1, 2021

Choose a reason for hiding this comment

roryabraham Sep 1, 2021

Choose a reason for hiding this comment

roryabraham Sep 1, 2021

Choose a reason for hiding this comment

roryabraham Sep 1, 2021

Choose a reason for hiding this comment

Julesssss left a comment

Choose a reason for hiding this comment

OSBotify commented Sep 1, 2021

OSBotify commented Sep 2, 2021

isagoico commented Sep 2, 2021

roryabraham commented Sep 2, 2021 • edited Loading

isagoico commented Sep 3, 2021

OSBotify commented Sep 3, 2021

roryabraham commented Dec 2, 2021

roryabraham commented Jul 2, 2021 •

edited

Loading

roryabraham commented Aug 31, 2021 •

edited

Loading

Julesssss Sep 1, 2021 •

edited

Loading

roryabraham commented Sep 2, 2021 •

edited

Loading