Subscribe and unsubscribe push notifications on sign-in and sign-out

[arosiclair]

Details

These changes fix issues with push notifications being unsubscribed when putting the app in the background using the back button.

Fixed Issues

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

Tests

1. Sign into NewDot on Android
2. Send a message to your user
3. Verify a push notification displays for the message after a moment
4. Use the back button (or swipe from edge if gestures are enabled) to put the app in the background
5. Send another message to your user
6. Verify a push notification displays for the message after a moment
7. Sign out
8. Send another message to your user
9. Verify a notification is not received

• Verify that no errors appear in the JS console

Offline tests

None

QA Steps

Same as Tests

• Verify that no errors appear in the JS console

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover 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 tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (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 followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• 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
• I verified that if a function's arguments changed that all usages have also been updated correctly
• 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 author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

None

Mobile Web - Chrome

None

Mobile Web - Safari

None

Desktop

None

iOS

Android

[melvin-bot]

@Santhosh-Sellavel @francoisl One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[marcaaron]

Looks like there is a conflict (left a couple more notes in the thread above).

[arosiclair]

Taking this off hold since we have a hold on the other PR now. Also assigning a C+

[melvin-bot]

@sobitneupane @amyevans One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

suggestions were added

[Julesssss]

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

[Julesssss]

@arosiclair should we add tests for web/Desktop, to ensure they still receive notifications? I'm not seeing them on your branch, but maybe this is expected?

[Julesssss]

I ran out of time and still need to verify iOS on a physical device. Will continue tomorrow.

[arosiclair]

@arosiclair should we add tests for web/Desktop, to ensure they still receive notifications? I'm not seeing them on your branch, but maybe this is expected?

In dev I couldn't get web notifications on localhost but I was able to on 127.0.0.1.

[Julesssss]

I finished testing iOS this morning, and it tested well. There's just the one outstanding comment remaining now.

[OSBotify]

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

[github-actions]

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start nativeLaunch	9.813 ms → 19.581 ms (+9.768 ms, +99.5%) 🟡
App start TTI	674.240 ms → 681.786 ms (+7.546 ms, +1.1%)
App start runJsBundle	185.594 ms → 190.969 ms (+5.375 ms, +2.9%)
Open Search Page TTI	596.514 ms → 599.152 ms (+2.638 ms, ±0.0%)
App start regularAppStart	0.014 ms → 0.020 ms (+0.006 ms, +46.0%) 🟡

Show details

Name	Duration
App start nativeLaunch	Baseline Mean: 9.813 ms Stdev: 1.570 ms (16.0%) Runs: 7 8 8 8 8 8 8 8 9 9 9 9 9 9 9 9 10 10 10 11 11 11 11 11 11 11 11 11 12 12 13 13 Current Mean: 19.581 ms Stdev: 1.931 ms (9.9%) Runs: 17 17 18 18 18 18 18 18 18 18 18 18 18 19 19 19 19 20 20 20 20 20 20 21 21 22 22 22 23 24 24
App start TTI	Baseline Mean: 674.240 ms Stdev: 35.021 ms (5.2%) Runs: 616.4288329998963 619.4695460000075 619.9127930002287 620.9256549999118 621.1044680001214 628.4285419997759 637.6263230000623 638.6130550000817 650.5051819998771 652.678499000147 655.2393820001744 668.3976509999484 678.3512450000271 678.8686640001833 679.756182000041 681.4689989998005 683.9271909999661 687.4141339999624 688.1504660001956 689.288395000156 690.4500389997847 693.3025699998252 696.9403889998794 699.8727409997955 700.9779480001889 707.5513610001653 711.512616999913 716.0801140000112 719.3783249999397 728.1595040000975 740.6698540002108 Current Mean: 681.786 ms Stdev: 42.031 ms (6.2%) Runs: 628.0427779997699 634.1476150001399 638.1322460002266 639.9362320001237 640.5114400000311 641.6416219999082 642.1059179999866 642.5745879998431 643.2826490001753 644.3130160002038 648.3188180001453 659.2257500002161 661.0375109999441 665.8362590000033 669.1655760002322 671.7532890001312 674.3992349999025 688.5554499998689 689.7486569997855 696.2205039998516 696.5191390002146 696.9193489998579 697.6907739997841 700.8040720000863 704.2247290001251 707.6846610000357 708.459007000085 714.4110770002007 740.5501589998603 754.3054590001702 784.0105559998192 792.6227620001882
App start runJsBundle	Baseline Mean: 185.594 ms Stdev: 22.374 ms (12.1%) Runs: 152 153 157 162 163 163 164 165 166 167 167 169 170 172 173 184 187 192 192 194 196 200 201 203 210 210 212 212 216 218 223 226 Current Mean: 190.969 ms Stdev: 24.676 ms (12.9%) Runs: 153 158 161 163 163 169 172 174 175 175 177 180 181 183 184 184 184 184 186 186 195 198 203 208 210 214 214 222 231 237 242 245
Open Search Page TTI	Baseline Mean: 596.514 ms Stdev: 21.842 ms (3.7%) Runs: 552.728800999932 562.6963709997945 571.987548999954 572.9753829999827 575.0734459999949 578.5496419998817 580.5414230003953 580.7078859996982 580.7443849998526 581.9493409995921 585.4402270000428 586.5671390001662 592.7127689998597 592.7423910000362 595.1411949996836 596.0332029997371 596.3048510001972 597.3213300001808 598.1587319998071 598.818847999908 598.8457849998958 598.8760170000605 601.3842779998668 605.1321610002778 607.9612630000338 608.85884599993 615.3303630002774 615.4206550000235 631.043171999976 634.3291429998353 636.7832439998165 657.2876389999874 Current Mean: 599.152 ms Stdev: 18.472 ms (3.1%) Runs: 552.9267579996958 567.250692000147 571.7560629998334 574.8865970000625 577.4002689998597 580.1929930001497 580.8131100004539 584.0734459999949 586.1813149997033 590.0552570000291 590.8159580002539 593.2201330000535 593.8774409997277 594.0198969999328 598.180989000015 599.1859539998695 601.766235999763 603.4160150000826 604.902912999969 606.040161000099 607.9549559997395 608.8258460001089 608.9204919999465 609.4746099999174 612.0101319998503 614.452961999923 616.2576099997386 616.4377439999953 621.4051520000212 622.519164999947 624.0389000000432 628.4672849997878 630.2886149999686
App start regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.001 ms (9.5%) Runs: 0.011760000139474869 0.01220700005069375 0.012246999889612198 0.012409999966621399 0.012492000125348568 0.012736000120639801 0.012899000197649002 0.012940000277012587 0.012979999650269747 0.013141999952495098 0.013224999886006117 0.013671999797224998 0.01375299971550703 0.013753000181168318 0.013875999953597784 0.013915999792516232 0.01407900033518672 0.01411899970844388 0.014403999783098698 0.014525999780744314 0.01460800040513277 0.014771000016480684 0.0148930000141263 0.015299999620765448 0.015339999925345182 0.015503000002354383 0.01676500029861927 0.01729400036856532 Current Mean: 0.020 ms Stdev: 0.001 ms (7.1%) Runs: 0.018187999725341797 0.018350999802350998 0.018351000268012285 0.0185139998793602 0.018636000342667103 0.018798999954015017 0.019530999939888716 0.0197350000962615 0.019816000014543533 0.019898000173270702 0.019938999786973 0.02010100008919835 0.02010100008919835 0.020142000168561935 0.020182999782264233 0.020426999777555466 0.020507999695837498 0.020548999775201082 0.020548999775201082 0.020589000079780817 0.0206709997728467 0.02083300007507205 0.020955000072717667 0.02119999984279275 0.021809999831020832 0.022745000198483467 0.022745999973267317 0.024821000173687935

[OSBotify]

🚀 Deployed to staging by @Julesssss in version: 1.2.53-0 🚀

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

[Julesssss]

Verified on staging 👍

[OSBotify]

🚀 Deployed to production by @Julesssss in version: 1.2.53-0 🚀

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