Contains the React Native Griffon plugin, sample app and Unit tests. #1

shtomar-adb · 2020-05-28T16:20:40Z

Description

Contains the React Native Griffon plugin and sample app. There are few pending issues that I am working.

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

nporter-adbe

Some initial comments, can you remove Pods so its easier to review? Currently, my browser is struggling to load the diff due to the size.

nporter-adbe · 2020-05-28T16:26:48Z

RCTACPReactNativeAcpGriffon.podspec

@@ -0,0 +1,25 @@
+
+Pod::Spec.new do |s|
+  s.name         = "RCTACPReactNativeAcpGriffon"


Can you updated the podspec to be consistent with the other React Native podspecs we have (naming, summary, etc)
https://github.com/adobe/react-native-acpanalytics/blob/master/RCTACPAnalytics.podspec

Updated the Podspec.

nporter-adbe · 2020-05-28T16:31:26Z

README.md

+
+#### iOS
+
+1. In XCode, in the project navigator, right click `Libraries` ➜ `Add Files to [your project's name]`


We don't advertise manual installation, I'd recommend only providing instructions using react-native link and the auto-link feature introduced in RN 0.60+.

https://github.com/adobe/react-native-acpanalytics#2-install-javascript-packages

I'll update it once I update Readme.md

nporter-adbe · 2020-05-28T16:34:12Z

android/build.gradle

@@ -0,0 +1,39 @@
+


Update this file to use the same versions as used in other RN packages unless there is a specific need from Girffon to use different versions. https://raw.githubusercontent.com/adobe/react-native-acpanalytics/master/android/build.gradle

nporter-adbe · 2020-05-28T16:35:26Z

README.md

-# react-native-acpgriffon
-Adobe Experience Platform - Project Griffon plugin for React-Native apps
+
+# adobe-mobile-marketing-react-native-acp-griffon


Please update to use the format other RN packages are using for their readme: https://github.com/adobe/react-native-acpanalytics/blob/master/README.md

nporter-adbe · 2020-05-28T16:36:57Z

...n/java/com/adobe/marketing/mobile/reactnative/griffon/RCTACPReactNativeAcpGriffonModule.java

@@ -0,0 +1,43 @@
+


Missing the copyright header.

/* Copyright 2020 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */

Added the copyright header.

nporter-adbe · 2020-05-28T16:38:40Z

...n/java/com/adobe/marketing/mobile/reactnative/griffon/RCTACPReactNativeAcpGriffonModule.java

+
+    @ReactMethod
+    public void startSession(final String url) {
+        Griffon.startSession(url);


Handle the case where url could be null

nporter-adbe · 2020-05-28T16:39:02Z

.../java/com/adobe/marketing/mobile/reactnative/griffon/RCTACPReactNativeAcpGriffonPackage.java

@@ -0,0 +1,28 @@
+


Missing copyright header.

nporter-adbe · 2020-05-28T16:39:26Z

.../java/com/adobe/marketing/mobile/reactnative/griffon/RCTACPReactNativeAcpGriffonPackage.java

+import com.facebook.react.bridge.ReactApplicationContext;
+import com.facebook.react.uimanager.ViewManager;
+import com.facebook.react.bridge.JavaScriptModule;
+public class RCTACPReactNativeAcpGriffonPackage implements ReactPackage {


Same comment about naming conventions.

Changed as per convention.

nporter-adbe · 2020-05-28T16:41:26Z

index.js

@@ -0,0 +1,4 @@
+// const ACPGriffon = require('./js/ACPGriffon');


Few comments on this file:

Usually, we have our index.js file live in the js directory.

Implement to be consistent with our other index.js files: https://github.com/adobe/react-native-acpanalytics/blob/master/js/index.js

Add copyright header

Moved index.js to js and added copyright.

nporter-adbe · 2020-05-28T16:43:37Z

ios/Pods/DoubleConversion/LICENSE

@@ -0,0 +1,26 @@
+Copyright 2006-2011, the V8 project authors. All rights reserved.


We should not be checking in our pods, and additionally, we shouldn't be checking in an Xcode project which references pods. This will cause build issues for our customers and require them to download all the pods.

Remove pods and add them to git ignore

Before committing run pod deintegrate

nporter-adbe · 2020-05-28T16:53:07Z

ios/src/RCTACPReactNativeAcpGriffon.h

@@ -0,0 +1,10 @@
+


Copyright header.

nporter-adbe · 2020-05-28T16:54:33Z

ios/src/RCTACPReactNativeAcpGriffon.m

@@ -0,0 +1,29 @@
+


Copyright header

nporter-adbe · 2020-05-28T16:55:23Z

ios/src/RCTACPReactNativeAcpGriffon.m

+
+
+RCT_EXPORT_METHOD(startSession: (NSString *) url){
+    [ACPGriffon startSession:[[NSURL alloc] initWithString:url]];


Griffon's headers specifies that url should be Nonnull, we should do a check here to ensure it is not nil.

Added the nil check.

nporter-adbe · 2020-05-28T19:08:02Z

sample/GriffonSample/android/app/src/main/java/com/griffonsample/MainActivity.java

@@ -0,0 +1,15 @@
+package com.griffonsample;


Be sure to add the copyright header to all the files, including those in the sample app.

nporter-adbe · 2020-05-28T19:09:16Z

package.json

+  "author": "",
+  "license": "",
+  "peerDependencies": {
+    "react-native": ">=0.44.0"


Add "@adobe/react-native-acpcore": ">=1.0.0" as a peer dependency.

Added core as peer dependency.

nporter-adbe · 2020-05-28T19:10:18Z

package.json

@@ -0,0 +1,27 @@
+{


Be sure to copy the pre-publish script: https://github.com/adobe/react-native-acpanalytics/blob/master/package.json#L8

nporter-adbe · 2020-05-28T19:10:55Z

package.json

+{
+  "name": "@adobe/react-native-acpgriffon",
+  "version": "1.0.0",
+  "description": "",


Populate the metadata to be similar to that in other packages: https://github.com/adobe/react-native-acpanalytics/blob/master/package.json#L8

nporter-adbe · 2020-05-28T19:11:11Z

js/ACPGriffon.js

@@ -0,0 +1,19 @@
+


Copyright header

nporter-adbe · 2020-05-28T19:11:20Z

js/ACPGriffon.js

+Alert.alert("----- "+NativeModules);
+const ACPGriffon = NativeModules.ACPGriffon;
+
+// Alert.alert("{}{}{}{}"+JSON.stringify(NativeModules));


Unused code

nporter-adbe · 2020-05-28T19:11:29Z

js/ACPGriffon.js

@@ -0,0 +1,19 @@
+
+import { NativeModules, Alert } from 'react-native';
+Alert.alert("----- "+NativeModules);


Unused code, also remove the Alert import.

nporter-adbe · 2020-05-28T19:12:36Z

js/ACPGriffon.js

+        return Promise.resolve(ACPGriffon.getExtensionVersion());
+    },
+    registerExtension: () => {
+        return Promis.resolve(ACPGriffon.registerExtension());


Our other register extension APIs don't resolve a promise as it doesn't have a return value: https://github.com/adobe/react-native-acpanalytics/blob/master/js/ACPAnalytics.js#L32

Removed the Promise.

ravjain-adb · 2020-06-02T01:56:34Z

RCTACPGriffon.podspec

+  s.license      = "Apache 2.0 License"
+  s.platform     = :ios, "10.0"
+
+  s.source       = { :git => "https://github.com/adobe/react-native-acp-griffon", :tag => "#{s.version}" }


shouldnt this be react-native-acpgriffon

…android lib from sample app.

…dge class to set current activity to Griffon. This is to handle Griffon issue where Griffon fails to show PincodeNetryActivity for first time.

…griffon into acp-griffon Merged the remote branch.

…eed to be registered from Native code Application/AppDelegate in android and ios respectively which is recommended in RN.

…griffon into acp-griffon Merged remote branch.

nporter-adbe

looks really good just a few comments.

nporter-adbe · 2020-06-04T21:34:53Z

README.md

+
+###### **iOS**
+```objective-c
+#import <RCTACPAnalytics/ACPAnalytics.h>


Import wrong

Missed to change that. Fixed

nporter-adbe · 2020-06-04T21:35:07Z

README.md

+
+###### **Android:**
+```java
+import com.adobe.marketing.mobile.Analytics;


nporter-adbe · 2020-06-04T21:35:24Z

__tests__/ACPGriffon.test.js

@@ -0,0 +1,18 @@
+import { NativeModules } from 'react-native';


Add copyright header

nporter-adbe · 2020-06-04T21:37:54Z

ios/RCTACPGriffon.xcodeproj/project.pbxproj

+				3D83B4BD247E5608007E783A /* libACPGriffon_iOS.a */,
+				3D83B4BB247E55F5007E783A /* ACPGriffon.h */,
+				134814211AA4EA7D00B7C361 /* Products */,
+				BFD6951DDA7DCA909FD456A9 /* Pods */,


We should remove references to Pods in our project before commiting it. Try pod deintegrate, you may need to go into the project and delete the Pods folder manually. This can cause build issues in cusutomers apps.

Deintegrated the pods.

nporter-adbe · 2020-06-04T21:40:51Z

sample/ACPGriffonSample/android/app/src/main/java/com/griffonsample/MainApplication.java

+    super.onCreate();
+    SoLoader.init(this, /* native exopackage */ false);
+    MobileCore.setApplication(this); // add this line
+    MobileCore.configureWithAppID("94f571f308d5/e30a9514788b/launch-44fec1a705f1-development");


Don't commit app Ids to public git. Remove and add a place holder

Replaced with placeholders.

nporter-adbe · 2020-06-04T21:41:23Z

sample/ACPGriffonSample/ios/GriffonSample/AppDelegate.m

+  [self.window makeKeyAndVisible];
+
+  //Register ACP Core and Griffon Extensions.
+  [ACPCore configureWithAppId:@"94f571f308d5/e30a9514788b/launch-44fec1a705f1-development"];


Add placeholder for app id

nporter-adbe · 2020-06-04T21:41:39Z

sample/ACPGriffonSample/js/App.js

+const RNACPCore = require('@adobe/react-native-acpcore'); 
+const ACPCore = RNACPCore.ACPCore;
+// import { ACPGriffon } from '@adobe/react-native-acpgriffon';
+// const RNACPGriffon = require('@adobe/react-native-acpgriffon'); 


Can we clean up these imports?

…of memory.

Added other react native plugin info in Readme.

…griffon into acp-griffon Merge the remote branch.

nporter-adbe

nice!

Added module and sample app.

edf6ef8

shtomar-adb requested review from nporter-adbe, sbenedicadb and ravjain-adb May 28, 2020 16:21

nporter-adbe suggested changes May 28, 2020

View reviewed changes

Removed pods that got pushed.

383ed88

nporter-adbe reviewed May 28, 2020

View reviewed changes

Fixed code review comments.

f73cfcc

shtomar-adb changed the title ~~WIP: Contains the React Native Griffon plugin and sample app.~~ Contains the React Native Griffon plugin, sample app and Unit tests. Jun 1, 2020

Added Griffon registration settings in Main class.

2614305

ravjain-adb reviewed Jun 2, 2020

View reviewed changes

shtomar-adb and others added 10 commits June 2, 2020 00:49

Moved declaration of GriffonFullScreenTakeOverActivity to RN package …

ddaf03f

…android lib from sample app.

Deintegrated pods from RN package.

f436610

Added makefile and supported files.

b3cc6d6

Added Readme.md

d5e7831

Changes th JS API name from getVersion to extensionVersion. Added bri…

09dfb44

…dge class to set current activity to Griffon. This is to handle Griffon issue where Griffon fails to show PincodeNetryActivity for first time.

Added circle ci file.

2ecaed4

Merge branch 'acp-griffon' of github.com:shtomar-adb/react-native-acp…

d271785

…griffon into acp-griffon Merged the remote branch.

Formatting changes in ReadMe.

5c2778a

Removed the js API to register Griffon extension. Griffon extension n…

fc8c6d4

…eed to be registered from Native code Application/AppDelegate in android and ios respectively which is recommended in RN.

Merge branch 'acp-griffon' of github.com:shtomar-adb/react-native-acp…

f983937

…griffon into acp-griffon Merged remote branch.

nporter-adbe reviewed Jun 4, 2020

View reviewed changes

shtomar-adb and others added 28 commits June 8, 2020 20:32

Moved @bable/runtime from dev to dependencies.

a8fdf34

Added under LDL_FLAGS which was shown as the warning.

23e9a4f

Remove podlock, change the version of babel runtime to 7.0.0.

e3ba26a

Changed the jest version in circleci to keep it in sync wit package.json

d253a33

Fix the CI jobs: Unit test and build-sample-app-android

0378545

Updated the the scheme.

58d1926

Resolve the path issue in setting.

23d0aaf

Modify config file to test build-sample-app-ios

b8206ae

Updated Make to do pod install.

a983c01

deintegrated pods

9b6b00d

Added build-androiid to workflow

a0b51c7

Added info flags

aa58936

Minor change in copy-to-sample.ssh

da86bc9

Fixed typo error.

ccd999e

Updated xcode version to 11.3.1

4275990

Added the debug flag

65f90a5

Added max worker to 2, since android build is failing because of out …

fee034b

…of memory.

Changes for CI pipeline fix.

070b7b5

Update CI worklflow.

654bc27

Fix for Unit tests on CI pipeline.

32d63be

Fixed Android pipeline issues.

0711102

Removed the pods related info.

dea1b23

Removed unnecessary code form AppDelegate.

71e343e

Added user notifications framework.

fe5ef1a

Update README.md

9c915dc

Added other react native plugin info in Readme.

Changes in the ios package and sample app build settings.

7029999

Merge branch 'acp-griffon' of github.com:shtomar-adb/react-native-acp…

3b11498

…griffon into acp-griffon Merge the remote branch.

Bump up the React and react-native dev dependencies.

e6088e1

nporter-adbe approved these changes Jun 15, 2020

View reviewed changes

shtomar-adb merged commit fd725c2 into adobe:master Jun 15, 2020

		@@ -0,0 +1,25 @@

		Pod::Spec.new do \|s\|
		s.name = "RCTACPReactNativeAcpGriffon"


		#### iOS

		1. In XCode, in the project navigator, right click `Libraries` ➜ `Add Files to [your project's name]`

		@@ -0,0 +1,4 @@
		// const ACPGriffon = require('./js/ACPGriffon');

		@@ -0,0 +1,26 @@
		Copyright 2006-2011, the V8 project authors. All rights reserved.



		RCT_EXPORT_METHOD(startSession: (NSString *) url){
		[ACPGriffon startSession:[[NSURL alloc] initWithString:url]];

		@@ -0,0 +1,19 @@

		import { NativeModules, Alert } from 'react-native';
		Alert.alert("----- "+NativeModules);

		@@ -0,0 +1,18 @@
		import { NativeModules } from 'react-native';

Contains the React Native Griffon plugin, sample app and Unit tests. #1

Contains the React Native Griffon plugin, sample app and Unit tests. #1

Conversation

shtomar-adb commented May 28, 2020

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

nporter-adbe left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shtomar-adb Jun 1, 2020 • 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

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

nporter-adbe May 28, 2020 • 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

nporter-adbe left a 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

nporter-adbe Jun 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nporter-adbe left a comment

Choose a reason for hiding this comment

shtomar-adb Jun 1, 2020 •

edited

Loading

nporter-adbe May 28, 2020 •

edited

Loading

nporter-adbe Jun 4, 2020 •

edited

Loading