[FIX #4327] Migrate to react-native-firebase #4446

georgiemathews · 2018-05-25T01:14:11Z

Fixes #4327

Summary:

Migrates Notifications to use invertase/react-native-firebase

Review notes (optional):

Testing notes (optional):

Steps to test:

Open Status
Accept Notifications Request
See if notifications are received

status: ready

pedropombeiro · 2018-05-25T05:41:48Z

Thanks for the PR @georgiemathews!

The build is currently failing in Jenkins:

jeluard

Lots of changes look unrelated with the issue. Could we reduce the scope of this PR?

jeluard · 2018-05-25T05:50:53Z

android/app/build.gradle

@@ -207,7 +208,7 @@ dependencies {
    implementation "com.android.support:appcompat-v7:26.1.0"
    // Force using exact RN version instead of relying on gradle dependency resolution
    // https://docs.gradle.org/current/userguide/introduction_dependency_management.html#sec:dependency_resolution
-    compile ("com.facebook.react:react-native:0.53.3") { force = true }  // From node_modules
+    implementation ("com.facebook.react:react-native:0.53.3") { force = true }  // From node_modules


Why this change?

@dmitryn WDYT? Looks like it's reverting your recent change?

jeluard · 2018-05-25T05:52:36Z

android/app/build.gradle

+    implementation 'com.google.firebase:firebase-core:15.0.2' //this decides your firebase SDK version
+    implementation "com.google.firebase:firebase-messaging:15.0.2"
+    implementation 'me.leolin:ShortcutBadger:1.1.21@aar'
+    implementation project(':react-native-testfairy')


Please comment why we need all those.

shortcut badger allows for badges on android

badges? Don't think we want that.

okay @jeluard, will revert that as well.

jeluard · 2018-05-25T05:52:52Z

android/app/src/main/AndroidManifest.xml

@@ -9,6 +9,8 @@
    <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
    <uses-permission android:name="android.permission.ACCESS_WIFI_STATE" />
    <uses-permission android:name="android.permission.READ_PROFILE" />
+    <uses-permission android:name="android.permission.RECEIVE_BOOT_COMPLETED" />


This require comment too.

This is required for the notifications

jeluard · 2018-05-25T05:53:10Z

android/app/src/main/AndroidManifest.xml

        <activity
                android:name=".MainActivity"
                android:label="@string/app_name"
                android:screenOrientation="portrait"
                android:windowSoftInputMode="adjustResize"
                android:configChanges="keyboard|keyboardHidden|orientation|screenSize"
                android:resizeableActivity="false"
-                android:launchMode="singleTask">
+                android:launchMode="singleTop">


What is this?

This was recommended in the react-native-firebase, but I've looked into it and singleTask will functionally work as well. I'll revert this

jeluard · 2018-05-25T05:53:31Z

android/app/src/main/java/im/status/ethereum/MainActivity.java

-    @Override
-    public void onNewIntent(final Intent intent) {
-        if (intent.getDataString() != null && intent.getData().getScheme().startsWith("app-settings")) {
-          startActivity(createNotificationSettingsIntent());


Is this related?

this is from the react-native-fcm documentation

Well that's not the same thing you removed?

@jeluard thanks for the catch, I see this was implemented for easy access to the settings from profile. Thanks, will revert

jeluard · 2018-05-25T05:53:45Z

android/build.gradle

-        classpath 'com.android.tools.build:gradle:3.0.1'
-        classpath 'com.google.gms:google-services:3.0.0'
+        classpath 'com.android.tools.build:gradle:3.1.2'
+        classpath 'com.google.gms:google-services:3.2.1'


Is this required?

jeluard · 2018-05-25T05:53:53Z

android/build.gradle

@@ -25,5 +24,6 @@ allprojects {
        flatDir { dirs 'libs' }
        maven { url "http://139.162.11.12:8081/artifactory/libs-release-local" }
        maven { url "https://jitpack.io" }
+        google()


It was already there. Any reason to change its place?

Signed-off-by: Georgie Mathews <mathews.georgie@gmail.com>

jeluard · 2018-05-28T06:37:11Z

ios/Podfile

@@ -11,13 +11,14 @@
 #pod 'Firebase/Messaging', '3.17.0'

 target 'StatusIm' do
-  platform :ios, '8.0'
+  platform :ios, '9.0'


What does that mean with regards for supported mobiles?

jeluard · 2018-05-28T06:37:57Z

ios/Podfile.lock


-COCOAPODS: 1.3.1
+COCOAPODS: 1.5.2


Is this also required?

jeluard · 2018-05-28T06:38:15Z

package.json

    "react-native-fetch-polyfill": "1.1.2",
+    "react-native-firebase": "^4.2.0",


We want fixed versions.

pedropombeiro · 2018-05-30T11:16:40Z

android/app/src/main/AndroidManifest.xml

-              android:name="com.google.firebase.messaging.default_notification_icon"
-              android:resource="@drawable/ic_stat_status_notification" />
+        android:name="com.google.firebase.messaging.default_notification_icon"
+        android:resource="@drawable/ic_stat_ic_notification" />


I'm getting a build error regarding this resource:

AAPT: No resource found that matches the given name (at 'resource' with value '@drawable/ic_stat_ic_notification').

Is this change intentional?

pedropombeiro · 2018-05-30T11:38:45Z

@georgiemathews I'm seeing this when attempting to run your branch. Any ideas?

georgiemathews · 2018-05-31T17:26:45Z

@pombeirp should be fixed now!

pedropombeiro · 2018-06-04T15:34:57Z

Confirmed that app is able to send PNs on this branch. Could you please rebase on latest develop branch @georgiemathews?

Signed-off-by: Georgie Mathews <mathews.georgie@gmail.com>

…//github.com/georgiemathews/status-react into feature/react-native-firebase-migration-4327

Signed-off-by: Georgie Mathews <mathews.georgie@gmail.com>

pedropombeiro · 2018-06-04T18:03:56Z

@georgiemathews there must have been a problem in the new rebase, it rolls back the upgrade in a2762eb. I believe it also removed babel-polyfill from package-lock.json, not sure if that is intended.

jeluard suggested changes May 25, 2018

View reviewed changes

flexsurfer added the bounty label May 25, 2018

georgiemathews added 3 commits May 25, 2018 09:21

[FIX #4327] Migrate to react-native-firebase

e730e69

Signed-off-by: Georgie Mathews <mathews.georgie@gmail.com>

reverts requested changes

60d4156

removes android badge package

2257022

jeluard reviewed May 28, 2018

View reviewed changes

more requested changes

e492cae

pedropombeiro reviewed May 30, 2018

View reviewed changes

fixed android notification dependency

204a5f9

georgiemathews added 15 commits June 4, 2018 10:34

[FIX #4327] Migrate to react-native-firebase

50633a0

Signed-off-by: Georgie Mathews <mathews.georgie@gmail.com>

reverts requested changes

f9a5b5c

removes android badge package

2bd8b9f

more requested changes

8854114

fixed android notification dependency

8face67

Merge branch 'feature/react-native-firebase-migration-4327' of https:…

1332463

…//github.com/georgiemathews/status-react into feature/react-native-firebase-migration-4327

[FIX #4327] Migrate to react-native-firebase

017bb35

Signed-off-by: Georgie Mathews <mathews.georgie@gmail.com>

reverts requested changes

121cce1

removes android badge package

b6ec188

more requested changes

7fb8fc8

fixed android notification dependency

6a399f3

[FIX #4327] Migrate to react-native-firebase

e3a8161

Signed-off-by: Georgie Mathews <mathews.georgie@gmail.com>

reverts requested changes

549cab1

removes android badge package

d532078

more requested changes

7ee5079

georgiemathews added 2 commits June 4, 2018 10:51

fixed android notification dependency

5533250

rebased

c1d614b

rasom mentioned this pull request Jun 12, 2018

WIP - [FIX #4327] Migrate to react-native-firebase #4716

Closed

rasom closed this Aug 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FIX #4327] Migrate to react-native-firebase #4446

[FIX #4327] Migrate to react-native-firebase #4446

georgiemathews commented May 25, 2018

pedropombeiro commented May 25, 2018

jeluard left a comment

jeluard May 25, 2018

georgiemathews May 25, 2018 •

edited

Loading

jeluard May 25, 2018

jeluard May 25, 2018

georgiemathews May 25, 2018 •

edited

Loading

jeluard May 25, 2018

georgiemathews May 25, 2018

jeluard May 25, 2018

georgiemathews May 25, 2018

jeluard May 25, 2018

georgiemathews May 25, 2018

jeluard May 25, 2018

georgiemathews May 25, 2018 •

edited

Loading

jeluard May 25, 2018

georgiemathews May 25, 2018

jeluard May 25, 2018

georgiemathews May 25, 2018

jeluard May 25, 2018

jeluard May 25, 2018

georgiemathews May 25, 2018

jeluard May 28, 2018

jeluard May 28, 2018

jeluard May 28, 2018

pedropombeiro May 30, 2018

pedropombeiro commented May 30, 2018

georgiemathews commented May 31, 2018

pedropombeiro commented Jun 4, 2018

pedropombeiro commented Jun 4, 2018

		"react-native-fetch-polyfill": "1.1.2",
		"react-native-firebase": "^4.2.0",


		COCOAPODS: 1.3.1
		COCOAPODS: 1.5.2

[FIX #4327] Migrate to react-native-firebase #4446

[FIX #4327] Migrate to react-native-firebase #4446

Conversation

georgiemathews commented May 25, 2018

Summary:

Review notes (optional):

Testing notes (optional):

Steps to test:

pedropombeiro commented May 25, 2018

jeluard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgiemathews May 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgiemathews May 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgiemathews May 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedropombeiro commented May 30, 2018

georgiemathews commented May 31, 2018

pedropombeiro commented Jun 4, 2018

pedropombeiro commented Jun 4, 2018

georgiemathews May 25, 2018 •

edited

Loading

georgiemathews May 25, 2018 •

edited

Loading

georgiemathews May 25, 2018 •

edited

Loading