feat(app): add support to handle background notification when a native notification activity is visible and react app is dead #5228

danwhite-ipc · 2021-04-28T14:51:36Z

Description

Allow for configuration of background native activities.

Related issues

Release Summary

Allow for configuration of background native activities to allow for the handling of background messages when a native activity is visible and the react native app is dead.

Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
- Yes
My change supports the following platforms;
- Android
- iOS
My change includes tests;
- e2e tests added or updated in packages/\*\*/e2e
- jest tests added or updated in packages/\*\*/__tests__
I have updated TypeScript types that are affected by my change.
This is a breaking change;
- Yes
- No

Test Plan

Think react-native-firebase is great? Please consider supporting the project with any of the below:

👉 Star this repo on GitHub ⭐️
👉 Follow React Native Firebase and Invertase on Twitter

vercel · 2021-04-28T14:51:41Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/F3iv5wfVSCKFviT1KRgpNs87UTza
✅ Preview: https://react-native-f-git-fork-ipcortex-feature-backgroundactiv-f67c10.vercel.app

CLAassistant · 2021-04-28T14:51:42Z

All committers have signed the CLA.

codecov · 2021-04-28T15:33:48Z

Codecov Report

Merging #5228 (9f5c3bf) into master (574f691) will decrease coverage by 52.30%.
The diff coverage is n/a.

❗ Current head 9f5c3bf differs from pull request most recent head 6a2cf5d. Consider uploading reports for the commit 6a2cf5d to get more accurate results

@@             Coverage Diff             @@
##           master    #5228       +/-   ##
===========================================
- Coverage   88.86%   36.57%   -52.29%     
===========================================
  Files         109       51       -58     
  Lines        3743     1518     -2225     
  Branches      360      360               
===========================================
- Hits         3326      555     -2771     
- Misses        370      733      +363     
- Partials       47      230      +183

mikehardy

Hey @danwhite-ipc thanks for the PR! This looks good in general - as in, it appears like it would work on API21+ devices and accomplish the task

I left a batch of nitpicks on spacing and wording and such but the only substantive review comment is about the API21 method. Can you have a think on that and adjust as needed?

Then I'll be happy to merge this and get a release out!

packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java

packages/app/firebase-schema.json

...s/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseJSON.java

mikehardy · 2021-04-28T15:48:43Z

packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java

+      ArrayList<String> backgroundActivities = json.getArrayValue("background_activity_names");
+
+      if(backgroundActivities.size() != 0) {
+        List<ActivityManager.AppTask> taskInfo = activityManager.getAppTasks();


This is an API21 call and we are still minSdkVersion 16 https://developer.android.com/reference/android/app/ActivityManager#getAppTasks()

Is there a way to accomplish this using APIs available on 16? If not it's safe to say the audience for API<21 is vanishingly small (RN0.64 bumps minSdkVersion to 21) so I'd accept an SDK version check to gate the API usage and make it safe, and then you would likely need to put a "NewApi" suppression in there to pass the lint check I just installed to guard against this #5208 after one slipped through recently

I have added support for API<21

…e/common/SharedUtils.java Co-authored-by: Mike Hardy <[email protected]>

Co-authored-by: Mike Hardy <[email protected]>

…e/common/ReactNativeFirebaseJSON.java Co-authored-by: Mike Hardy <[email protected]>

mikehardy

@danwhite-ipc This looks great, thank you for handling the old API!
I had a few trivial nitpicks with exception of an important suggestion on the Activity name comparison to make it work better (I think)

If everything looks good to you, accept those and once CI passes I should be able to release this - I have a release preparing right now

packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java

mikehardy · 2021-04-29T16:43:19Z

packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java

+          }
+        }
+
+        if (currentActivity != "" && backgroundActivities.contains(currentActivity)) {


🤔 two issues here -

1- using != on java strings asks if they are the exact same in-memory location, not if the strings are equal, you have to use String.equals() for string equality. In Javascript even it should be !== vs !=.
2- NullPointerExceptions on string comparison are a classic Java issue, so if you use Java String.equals() the safe way (to protect against nulls) is to reverse them and use the known-good string first, e.g. !"".equals(currentActivity)

Suggested change

if (currentActivity != "" && backgroundActivities.contains(currentActivity)) {

if (!"".equals(currentActivity) && backgroundActivities.contains(currentActivity)) {

Happy with the suggestion but it looks like your missing a ) at the end of your equals(currentActivity) check

🤦 yes indeed. The perils of coding in a web text box - good spot - updated - hopefully better?

packages/app/firebase-schema.json

…e/common/SharedUtils.java Co-authored-by: Mike Hardy <[email protected]>

Co-authored-by: Mike Hardy <[email protected]>

mikehardy

only rejecting so I can commit the suggestion / then approve / then merge
as mentioned I'm preparing a release literally this hour so I'm just trying to hustle it up

packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java

mikehardy · 2021-04-29T17:10:59Z

Really strange! Sorry this is likely generating notification spam for you, but I really wanted to be able to commit the change. GitHub normally offers me that button as an option but it did not this time. At any rate, if you submit a correct version of that string compare this can go out the door this afternoon...

…e/common/SharedUtils.java Co-authored-by: Mike Hardy <[email protected]>

danwhite-ipc · 2021-04-29T18:27:12Z

Sorry I signed off for the night so only just seen your message and jumped back on. I committed your update and hopefully all is fine now.

mikehardy · 2021-04-29T18:46:30Z

Ah no worries - still prime time for me ;-). I pulled your fork as a remote locally and fixed one small thing, I'll shepherd it to merge + release today. This is a cool feature, I'm excited to see it go live - thanks for contributing it!

mikehardy · 2021-04-29T18:46:43Z

Superceded by #5235

danwhite-ipc · 2021-04-29T19:15:32Z

@mikehardy No worries, always good to contribute

mikehardy · 2021-04-29T22:03:26Z

Release 11.4.1 just went out with the change related to this among other fixes + features. Enjoy! 🚀

Check json config for background activities

e915c52

vercel bot deployed to Preview April 28, 2021 14:51 View deployment

mikehardy requested changes Apr 28, 2021

View reviewed changes

mikehardy added the Workflow: Needs Review Pending feedback or review from a maintainer. label Apr 28, 2021

Update packages/app/android/src/reactnative/java/io/invertase/firebas…

b4a4292

…e/common/SharedUtils.java Co-authored-by: Mike Hardy <[email protected]>

vercel bot deployed to Preview April 28, 2021 15:54 View deployment

danwhite-ipc and others added 6 commits April 28, 2021 16:54

Update packages/app/android/src/reactnative/java/io/invertase/firebas…

04c4116

…e/common/SharedUtils.java Co-authored-by: Mike Hardy <[email protected]>

Update packages/app/android/src/reactnative/java/io/invertase/firebas…

883e466

…e/common/SharedUtils.java Co-authored-by: Mike Hardy <[email protected]>

Update packages/app/android/src/reactnative/java/io/invertase/firebas…

66950f7

…e/common/SharedUtils.java Co-authored-by: Mike Hardy <[email protected]>

Update packages/app/firebase-schema.json

e2a0e55

Co-authored-by: Mike Hardy <[email protected]>

Update packages/app/firebase-schema.json

d447b8c

Co-authored-by: Mike Hardy <[email protected]>

Update packages/app/android/src/reactnative/java/io/invertase/firebas…

68fe5bb

…e/common/ReactNativeFirebaseJSON.java Co-authored-by: Mike Hardy <[email protected]>

vercel bot deployed to Preview April 28, 2021 16:00 View deployment

Update config key

714c256

vercel bot deployed to Preview April 28, 2021 16:06 View deployment

mikehardy added Workflow: Waiting for User Response Blocked waiting for user response. and removed Workflow: Needs Review Pending feedback or review from a maintainer. labels Apr 28, 2021

Added background activity check support for API version < 21

b794c12

vercel bot deployed to Preview April 29, 2021 09:19 View deployment

danwhite-ipc changed the title ~~Feat(app): add support to handle background notification when a native notification activity is visible and react app is dead~~ feat(app): add support to handle background notification when a native notification activity is visible and react app is dead Apr 29, 2021

danwhite-ipc requested a review from mikehardy April 29, 2021 09:31

mikehardy requested changes Apr 29, 2021

View reviewed changes

mikehardy added the Workflow: Pending Merge Waiting on CI or similar label Apr 29, 2021

Update packages/app/android/src/reactnative/java/io/invertase/firebas…

e05f360

…e/common/SharedUtils.java Co-authored-by: Mike Hardy <[email protected]>

vercel bot deployed to Preview April 29, 2021 16:50 View deployment

danwhite-ipc and others added 2 commits April 29, 2021 17:50

Update packages/app/android/src/reactnative/java/io/invertase/firebas…

a3232bf

…e/common/SharedUtils.java Co-authored-by: Mike Hardy <[email protected]>

Update packages/app/firebase-schema.json

a219498

Co-authored-by: Mike Hardy <[email protected]>

vercel bot deployed to Preview April 29, 2021 16:58 View deployment

mikehardy requested changes Apr 29, 2021

View reviewed changes

mikehardy reviewed Apr 29, 2021

View reviewed changes

packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java Outdated Show resolved Hide resolved

Update packages/app/android/src/reactnative/java/io/invertase/firebas…

6a2cf5d

…e/common/SharedUtils.java Co-authored-by: Mike Hardy <[email protected]>

vercel bot deployed to Preview April 29, 2021 18:24 View deployment

mikehardy mentioned this pull request Apr 29, 2021

feat(app, android): support list of Activities to ignore when detecting AppState #5235

Merged

8 tasks

mikehardy closed this Apr 29, 2021

mikehardy removed Workflow: Pending Merge Waiting on CI or similar Workflow: Waiting for User Response Blocked waiting for user response. labels Apr 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(app): add support to handle background notification when a native notification activity is visible and react app is dead #5228

feat(app): add support to handle background notification when a native notification activity is visible and react app is dead #5228

danwhite-ipc commented Apr 28, 2021 •

edited

Loading

vercel bot commented Apr 28, 2021 •

edited

Loading

CLAassistant commented Apr 28, 2021 •

edited

Loading

codecov bot commented Apr 28, 2021 •

edited

Loading

mikehardy left a comment

mikehardy Apr 28, 2021

danwhite-ipc Apr 29, 2021

mikehardy left a comment

mikehardy Apr 29, 2021 •

edited

Loading

danwhite-ipc Apr 29, 2021

mikehardy Apr 29, 2021

mikehardy left a comment

mikehardy commented Apr 29, 2021

danwhite-ipc commented Apr 29, 2021

mikehardy commented Apr 29, 2021

mikehardy commented Apr 29, 2021

danwhite-ipc commented Apr 29, 2021

mikehardy commented Apr 29, 2021

	if (currentActivity != "" && backgroundActivities.contains(currentActivity)) {
	if (!"".equals(currentActivity) && backgroundActivities.contains(currentActivity)) {

feat(app): add support to handle background notification when a native notification activity is visible and react app is dead #5228

feat(app): add support to handle background notification when a native notification activity is visible and react app is dead #5228

Conversation

danwhite-ipc commented Apr 28, 2021 • edited Loading

Description

Related issues

Release Summary

Checklist

Test Plan

vercel bot commented Apr 28, 2021 • edited Loading

CLAassistant commented Apr 28, 2021 • edited Loading

codecov bot commented Apr 28, 2021 • edited Loading

Codecov Report

mikehardy left a comment

Choose a reason for hiding this comment

mikehardy Apr 28, 2021

Choose a reason for hiding this comment

danwhite-ipc Apr 29, 2021

Choose a reason for hiding this comment

mikehardy left a comment

Choose a reason for hiding this comment

mikehardy Apr 29, 2021 • edited Loading

Choose a reason for hiding this comment

danwhite-ipc Apr 29, 2021

Choose a reason for hiding this comment

mikehardy Apr 29, 2021

Choose a reason for hiding this comment

mikehardy left a comment

Choose a reason for hiding this comment

mikehardy commented Apr 29, 2021

danwhite-ipc commented Apr 29, 2021

mikehardy commented Apr 29, 2021

mikehardy commented Apr 29, 2021

danwhite-ipc commented Apr 29, 2021

mikehardy commented Apr 29, 2021

danwhite-ipc commented Apr 28, 2021 •

edited

Loading

vercel bot commented Apr 28, 2021 •

edited

Loading

CLAassistant commented Apr 28, 2021 •

edited

Loading

codecov bot commented Apr 28, 2021 •

edited

Loading

mikehardy Apr 29, 2021 •

edited

Loading