Track Push Notification Opt-In Status #14579

roryabraham · 2023-01-26T00:29:09Z

HOLD on https://github.com/Expensify/Web-Expensify/pull/36368

Note that a require cycle was recently added by a different PR unrelated to this one.

Details

This PR starts tracking whether a user is opted-in to push notifications on a per-device basis.

Relevant comment: urbanairship/react-native-airship#427 (comment)

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/209233

Tests / QA Steps

Web

Run the web app, and search console logs for deviceID.

Verify that you see a log line like:

[info] Got new deviceID - "def9ade3-7e2c-ecfe-ccb2-8941203716b2"

Refresh the page. Verify that you see a log line like so (with a matching GUID):
```
[info] Found existing deviceID - "def9ade3-7e2c-ecfe-ccb2-8941203716b2"
```
Close the tab, then open it again. Verify that you see a log line like so:
```
[info] Found existing deviceID - "def9ade3-7e2c-ecfe-ccb2-8941203716b2"
```

Log out then refresh the page. Verify that you see a log line like so:

[info] Found existing deviceID - "def9ade3-7e2c-ecfe-ccb2-8941203716b2"

Desktop

Run the desktop app, and search console logs for deviceID. You can open the dev console in the desktop app with CMD + Option + I

Verify that you see a log line like:

[info] Got new deviceID - "18632275959b5bef058ee4aa7fc70069d4ba54604c30986c4f1e3c09e287b521"

Close the app and reopen it. Verify that you see a log line like:

[info] Found existing deviceID - "18632275959b5bef058ee4aa7fc70069d4ba54604c30986c4f1e3c09e287b521"

Log out, then close the app and reopen it. Verify that you see that same log line.
Completely uninstall and reinstall the app, then open the app. Verify that you see the same log line.

iOS

Sign into the app.
When prompted, accept push notifications.
Look at the value of the pushNotificationsEnabled NVP, either with a CQ:
```
SELECT value
FROM nameValuePairs nvp
INNER JOIN accounts a ON a.accountID = nvp.accountID
WHERE a.email = '<email_you_used_to_login>'
  AND nvp.name = 'pushNotificationsEnabled';
```
Or by signing into the same account on https://staging.expensify.com, then running NVP.get('pushNotificationsEnabled') in the console.
Verify that the NVP shows that you opted in to push notifications for the current device. It should look like this:
```
{"iPhone15,2_90CD2827-B91E-45E5-A63E-76ADC67F47F0":[{"isEnabled":true,"timestamp":"2023-01-27 02:17:44"}]}
```
Where iPhone15,2_90CD2827-B91E-45E5-A63E-76ADC67F47F0 is the deviceID, the most recent value of the NVP is at the end of the array, and isEnabled should be true.
Sign out and sign back in with a different account, then accept notifications when prompted.
Look at the NVP again for the new account and verify that it shows the same deviceID w/ isEnabled: true.
Sign out and sign back in with the first account.
Look at the NVP again for the first account and verify that it hasn't changed.
Completely uninstall and reinstall the app.
Sign in to the same account and accept push notifications when prompted.
Look at the NVP again, and verify that it hasn't changed.
Completely uninstall and reinstall the app.
Sign in to the same account, but this time reject push notifications when prompted.
Look at the NVP again and verify that there's a new entry at the end of the array that shows isEnabled: false.
Now open settings, scroll down to New Expensify, and enable notifications. Then come back to the app.
Look at the NVP again and verify that there's a new entry at the end of the array that shows isEnabled: true.
Now open settings, scroll down to New Expensify, and disable notifications. Then come back to the app.
Look at the NVP again and verify that there's a new entry at the end of the array that shows isEnabled: false.

Android

Sign into the app on Android.
Look at the value of the pushNotificationsEnabled NVP , either with a CQ:
```
SELECT value
FROM nameValuePairs nvp
INNER JOIN accounts a ON a.accountID = nvp.accountID
WHERE a.email = '<email_you_used_to_login>'
  AND nvp.name = 'pushNotificationsEnabled';
```
Or by signing into the same account on https://staging.expensify.com, then running NVP.get('pushNotificationsEnabled') in the console.
Verify that the NVP shows that you opted in to push notifications for the current device.
Sign out, then sign back in with a different account.
Look at the NVP again for the different account and verify that you see the same deviceID with isEnabled: true.
Sign out, then sign back in with the first account.
Look at the NVP again for the first account and verify that it hasn't changed.
Close the app and reopen it.
Look at the NVP again and verify that it hasn't changed.
Now go into settings and disable notifications for New Expensify, then open the app again.
Look at the NVP again and verify that there's a new entry showing isEnabled: false
Close the app and reopen it.
Look at the NVP again and verify that it hasn't changed.
Now go into settings and re-enable notifications for New Expensify, then open the app again.
Look at the NVP again and verify that there's a new entry showing isEnabled: true.

Verify that no errors appear in the JS console

Offline tests

None.

PR Author Checklist

Screenshots/Videos

Web

Mobile Web - Chrome

No tests or QA for mobile web.

Mobile Web - Safari

No tests or QA for mobile web.

Desktop

iOS

https://www.youtube.com/watch?v=WAVT4B_ph9w

Android

Android.mov

…ication opt-in status

…dule

roryabraham · 2023-02-08T18:51:37Z

I can't test for the native platforms since it needs to check the NVP result

@mollfpr You should be able to check the NVP result by:

log into https://staging.expensify.com (OldDot) with the same account
open the JS console
Run NVP.get('pushNotificationsEnabled')

roryabraham · 2023-02-08T18:52:05Z

I got the above error when trying to run the ios app.

@mollfpr Not able to reproduce this. Did you run npm i && cd ios && pod install && cd .. ?

# Conflicts: # tests/unit/NetworkTest.js

roryabraham · 2023-02-09T06:08:46Z

Merge conflicts resolved (the only conflict was in a test)

mollfpr · 2023-02-09T17:38:08Z

Thanks @roryabraham I'll test it again this morning 🙏

mollfpr · 2023-02-10T07:56:54Z

@roryabraham I kept getting the airshipOptIn value false. I tried on different simulators iPhone 13 iOS 15.0, iPhone 14 iOS 16.2, and iPhone 14 Pro iOS 16.2, and I still got the same result. Any idea?

roryabraham · 2023-02-10T19:59:28Z

@mollfpr are you testing with an iOS simulator? Simulators can't receive push notifications so I wonder if that's the cause. Did you opt-in to push notifications when prompted?

mollfpr · 2023-02-10T23:42:00Z

@roryabraham Yes, I opted in when it prompted. I’ll try it on the physical device then.

mollfpr · 2023-02-16T15:46:25Z

@roryabraham I ask on Slack how to run the dev app on the iPhone physical device, but there's not possible for the contributors right now. Any suggestion?

Julesssss · 2023-02-16T15:49:08Z

@roryabraham I ask on Slack how to run the dev app on the iPhone physical device, but there's not possible for the contributors right now. Any suggestion?

Myself, @arosiclair, or @stitesExpensify can test iOS. I can't do this today, but should be able to find time tomorrow.

# Conflicts: # src/libs/Notification/PushNotification/index.native.js

roryabraham · 2023-02-16T19:28:45Z

Conflicts resolved

arosiclair · 2023-02-17T00:07:52Z

Working on the checklist right now

arosiclair · 2023-02-17T00:14:31Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

N/A

Mobile Web - Safari

N/A

Desktop

iOS

Android

arosiclair

Tests well!

OSBotify · 2023-02-17T01:36:51Z

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

roryabraham · 2023-02-17T01:39:09Z

Thanks for taking this over the finish line @arosiclair 🚀

github-actions · 2023-02-17T02:06:41Z

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	740.774 ms → 772.385 ms (+31.611 ms, +4.3%)
App start runJsBundle	202.161 ms → 208.500 ms (+6.339 ms, +3.1%)
App start nativeLaunch	20.344 ms → 20.700 ms (+0.356 ms, +1.8%)
App start regularAppStart	0.018 ms → 0.025 ms (+0.006 ms, +35.2%) 🟡
Open Search Page TTI	596.662 ms → 592.341 ms (-4.321 ms, -0.7%)

Show details

Name	Duration
App start TTI	Baseline Mean: 740.774 ms Stdev: 26.523 ms (3.6%) Runs: 681.1160509996116 696.5818959996104 701.8031329996884 704.798204000108 706.5195220001042 719.4570369999856 721.6416840003803 722.5148000000045 723.2312759999186 725.9954589996487 728.4180239997804 731.6230760002509 736.628971000202 737.7696059998125 739.1221610000357 744.0042310003191 748.6621139999479 748.756958999671 749.1122420001775 749.1192190004513 751.2448450000957 753.1716790003702 754.4436640003696 757.5738340001553 763.3494990002364 766.2256239997223 766.6921380003914 773.2468219995499 784.1421670001 784.4782229997218 792.5498350001872 Current Mean: 772.385 ms Stdev: 14.233 ms (1.8%) Runs: 737.417843000032 742.1717830002308 747.3804129995406 758.532360999845 763.6343000000343 764.4451440004632 764.827561000362 766.5370079996064 768.7244159998372 768.7365720001981 768.7587660001591 771.2379630003124 772.0408680001274 772.4897239999846 775.372673000209 778.8864249996841 779.6680100001395 783.8083739997819 784.7031939998269 784.9992709998041 785.2380769997835 785.3872210001573 785.5979089997709 788.3792300000787 791.4836280001327 791.5396020002663
App start runJsBundle	Baseline Mean: 202.161 ms Stdev: 17.237 ms (8.5%) Runs: 169 175 178 180 182 184 192 193 194 194 195 195 197 198 200 201 203 204 205 206 206 207 212 212 213 213 223 226 233 238 239 Current Mean: 208.500 ms Stdev: 16.282 ms (7.8%) Runs: 183 189 190 190 192 194 196 198 198 200 200 200 203 207 207 208 209 209 209 210 210 211 219 220 223 224 225 231 240 260
App start nativeLaunch	Baseline Mean: 20.344 ms Stdev: 1.632 ms (8.0%) Runs: 18 18 19 19 19 19 19 19 19 19 19 19 19 19 20 20 20 20 20 21 21 21 22 22 22 22 22 22 22 22 23 25 Current Mean: 20.700 ms Stdev: 1.882 ms (9.1%) Runs: 18 18 18 19 19 19 19 20 20 20 20 20 20 20 20 20 20 20 21 21 21 22 22 22 22 22 24 24 25 25
App start regularAppStart	Baseline Mean: 0.018 ms Stdev: 0.002 ms (9.8%) Runs: 0.015463000163435936 0.01574699953198433 0.015909000299870968 0.015991000458598137 0.016073000617325306 0.01631700061261654 0.0163569999858737 0.017090000212192535 0.017455999739468098 0.017455999739468098 0.01769999973475933 0.01769999973475933 0.017862999811768532 0.01794400066137314 0.018066000193357468 0.018432999961078167 0.01847400050610304 0.0185139998793602 0.01855500042438507 0.018757999874651432 0.018798000179231167 0.018920999951660633 0.018920999951660633 0.01896199956536293 0.019205999560654163 0.020060000009834766 0.020060000009834766 0.020548000000417233 0.020669999532401562 0.02148399967700243 0.021688000299036503 0.02286700066179037 Current Mean: 0.025 ms Stdev: 0.001 ms (5.3%) Runs: 0.02217600028961897 0.022583000361919403 0.0226239999756217 0.02274599950760603 0.023111999966204166 0.0233559999614954 0.023763000033795834 0.023763000033795834 0.023926000110805035 0.024007000029087067 0.02437400072813034 0.024617999792099 0.024821000173687935 0.025024999864399433 0.025024999864399433 0.025065000168979168 0.025065000168979168 0.025350000709295273 0.025390999391674995 0.025471999309957027 0.025511999614536762 0.025553000159561634 0.025756999850273132 0.025879000313580036 0.025960000231862068 0.02600100077688694 0.026245000772178173 0.0264079999178648 0.026693000458180904 0.02685500029474497 0.026895999908447266
Open Search Page TTI	Baseline Mean: 596.662 ms Stdev: 30.482 ms (5.1%) Runs: 558.0205079996958 558.1735840002075 562.1898600002751 562.309081999585 568.9816490001976 570.0254720002413 571.4995929999277 571.6765540000051 573.181519000791 574.0026859994978 575.5443930001929 575.8717450005934 575.95727599971 576.2661139992997 581.2214359994978 582.4971920000389 589.3858239995316 593.240885999985 597.6812740005553 601.1280519999564 601.4424640005454 604.862467000261 605.6511639999226 611.8449710002169 616.0598149998114 624.4444180000573 625.2630619993433 626.3937590001151 637.2588709993288 643.0720220003277 644.7494310000911 647.9996339995414 681.9414880005643 Current Mean: 592.341 ms Stdev: 24.522 ms (4.1%) Runs: 553.1792810000479 554.4200029997155 562.5511069996282 564.8460289994255 566.3236899999902 566.9390870006755 575.043335000053 575.1090500000864 575.2269289996475 575.5389000000432 578.369384999387 582.1022950001061 584.9219159996137 585.2823490006849 585.459188000299 589.265625 591.1930339997634 591.2557779997587 591.8753669997677 594.1554370000958 595.912882999517 596.0571699999273 596.1448569996282 598.4966640006751 599.0275480002165 601.8671059999615 612.9134940002114 615.925578000024 621.3954269997776 624.1845300002024 639.3376059997827 648.7664790004492 654.1678060004488

OSBotify · 2023-02-17T21:24:12Z

🚀 Deployed to staging by https://github.com/arosiclair in version: 1.2.74-0 🚀

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

mvtglobally · 2023-02-21T04:12:44Z

@roryabraham, we are struggling with this PR.

~~On Web and Desktop logins, the Info is not displayed in console at all. I can only see if if I run the NVP.get('pushNotificationsEnabled') and login with mobile device.~~ Its working
When I login on few mobile devices, 2nd device shows as NULL

Let me know if this is expected or should be logged as a separate issue
Title:
[ND] PR 14579 - 2nd device is displayed as NULL

Action Performed:

Sign into the app.
When prompted, accept push notifications.
Look at the value of the pushNotificationsEnabled NVP, by signing into the same account on https://staging.expensify.com, then running NVP.get('pushNotificationsEnabled') in the console.
Verify that the NVP shows that you opted in to push notifications for the current device
Navigate to the same account on another device
Verify that the NVP shows that you opted in to push notifications for new current device

Expected Result:
User can see only original device

Actual Result:
User can see the 1st device, but 2nd device displayed as NULL array

Attachment

src/libs/actions/Device/generateDeviceID/index.android.js

OSBotify · 2023-02-21T20:34:55Z

🚀 Deployed to production by https://github.com/chiragsalian in version: 1.2.74-0 🚀

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

roryabraham added 5 commits January 25, 2023 16:17

Install react-native-device-info

2b5d385

Upgrade urbanairship-react-native

b6e2b21

Add mock for react-native-device-info

4fc6f26

Update urbanairship-react-native mock

efc4490

Update CustomNotificationProvider

599be59

roryabraham self-assigned this Jan 26, 2023

roryabraham added 5 commits January 25, 2023 16:35

Make PushNotification lib have same exports on web and native

cce0558

Create User actions to opt in and out of push notifications

51116a7

Fix ternary

e158dbd

Use only one user action instead of two

6ddd67c

Hook up listeners in the PushNotification library to track push notif…

735e5a9

…ication opt-in status

roryabraham changed the title ~~Track Push Notification Opt-In Status~~ [HOLD] Track Push Notification Opt-In Status Jan 26, 2023

Fix JS style

7e0a54a

roryabraham added the InternalQA This pull request required internal QA label Jan 26, 2023

roryabraham added 16 commits January 25, 2023 17:34

Fix Onyx.connect callback to handle updated NVP shape

fa2fe5e

Merge branch 'main' into Rory-PushNotificationOptInStatus

72fea38

Fix Onyx.connect to be keyed by deviceID

8941f11

Consolidate Onyx connection into PushNotification lib

01048e3

Fix require cycle

6f89dc0

Use Google Play-compliant deviceID alternative

29f15e0

Move Device to its own action

973b551

Fix require cycle by moving push notification tracking to separate mo…

1513b20

…dule

Fix permission tracker export

ac8badf

Fix require cycle by moving setPushNotificationOptInStatus out of User

3b13de7

Remove awkward newline

6a7c215

Clarify comment

6621f1e

Simplify Onyx data to a boolean

4f2ca7b

Fix use of PermissionsTracker

77f029d

Remove permissionTracker entirely

fb853a3

Improve device comment again

1470b01

Merge branch 'main' into Rory-PushNotificationOptInStatus

99d799d

# Conflicts: # tests/unit/NetworkTest.js

roryabraham dismissed stale reviews from arosiclair and Julesssss via 99d799d February 9, 2023 06:07

roryabraham requested review from Julesssss and arosiclair February 9, 2023 06:08

Merge branch 'main' into Rory-PushNotificationOptInStatus

5af1d26

# Conflicts: # src/libs/Notification/PushNotification/index.native.js

Expensify deleted a comment from mollfpr Feb 17, 2023

arosiclair approved these changes Feb 17, 2023

View reviewed changes

arosiclair merged commit b6d2197 into main Feb 17, 2023

arosiclair deleted the Rory-PushNotificationOptInStatus branch February 17, 2023 01:32

OSBotify mentioned this pull request Feb 17, 2023

Deploy Checklist: New Expensify 2023-02-17 #15260

Closed

17 tasks

roryabraham commented Feb 21, 2023

View reviewed changes

src/libs/actions/Device/generateDeviceID/index.android.js Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track Push Notification Opt-In Status #14579

Track Push Notification Opt-In Status #14579

roryabraham commented Jan 26, 2023 •

edited

Loading

roryabraham commented Feb 8, 2023

roryabraham commented Feb 8, 2023

roryabraham commented Feb 9, 2023

mollfpr commented Feb 9, 2023

mollfpr commented Feb 10, 2023

roryabraham commented Feb 10, 2023

mollfpr commented Feb 10, 2023

mollfpr commented Feb 16, 2023

Julesssss commented Feb 16, 2023

roryabraham commented Feb 16, 2023

arosiclair commented Feb 17, 2023

arosiclair commented Feb 17, 2023 •

edited

Loading

arosiclair left a comment

OSBotify commented Feb 17, 2023

roryabraham commented Feb 17, 2023

github-actions bot commented Feb 17, 2023

OSBotify commented Feb 17, 2023

mvtglobally commented Feb 21, 2023 •

edited

Loading

OSBotify commented Feb 21, 2023

Track Push Notification Opt-In Status #14579

Track Push Notification Opt-In Status #14579

Conversation

roryabraham commented Jan 26, 2023 • edited Loading

Details

Fixed Issues

Tests / QA Steps

Web

Desktop

iOS

Android

Offline tests

PR Author Checklist

Screenshots/Videos

roryabraham commented Feb 8, 2023

roryabraham commented Feb 8, 2023

roryabraham commented Feb 9, 2023

mollfpr commented Feb 9, 2023

mollfpr commented Feb 10, 2023

roryabraham commented Feb 10, 2023

mollfpr commented Feb 10, 2023

mollfpr commented Feb 16, 2023

Julesssss commented Feb 16, 2023

roryabraham commented Feb 16, 2023

arosiclair commented Feb 17, 2023

arosiclair commented Feb 17, 2023 • edited Loading

Reviewer Checklist

Screenshots/Videos

arosiclair left a comment

Choose a reason for hiding this comment

OSBotify commented Feb 17, 2023

roryabraham commented Feb 17, 2023

github-actions bot commented Feb 17, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

OSBotify commented Feb 17, 2023

mvtglobally commented Feb 21, 2023 • edited Loading

OSBotify commented Feb 21, 2023

roryabraham commented Jan 26, 2023 •

edited

Loading

arosiclair commented Feb 17, 2023 •

edited

Loading

mvtglobally commented Feb 21, 2023 •

edited

Loading