Skip to content

Conversation

@cacheung
Copy link
Contributor

@cacheung cacheung commented Sep 1, 2021

Add Edge Identity network extension package

Description

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.

yangyansong-adbe and others added 29 commits August 4, 2021 16:08
Init code for react native Edge
More updates
Update iOS implementation
Some more Android side change
React Native Edge extension
React Native AEPAssurance v3.0.0 package
Fix the build - Set right the unit tests
clean up files
Update iOS code and clean up files
Add unit test and update Readme
Cleanup few files
Update for review comments
Update package json for Edge package.
update readme file
This reverts commit 9f82a4a.
Add Edge Identity network extension package
import {Button, Platform, StyleSheet, Text, View, ScrollView, NativeModules} from 'react-native';
import {AEPCore, AEPLifecycle, AEPSignal, AEPMobileLogLevel, AEPMobilePrivacyStatus, AEPMobileVisitorAuthenticationState, AEPVisitorID, AEPExtensionEvent} from '@adobe/react-native-aepcore';
import {AEPIdentity as AEPIdentity, AEPCore, AEPLifecycle, AEPSignal, AEPMobileLogLevel, AEPMobilePrivacyStatus, AEPMobileVisitorAuthenticationState, AEPVisitorID, AEPExtensionEvent } from '@adobe/react-native-aepcore';
import {AEPIdentity as AEPEdgeIdentity } from '@adobe/react-native-aepedgeidentity';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add sample for core Identity and Edge Identity here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can have a separate view for Edge Identity.
It might be a good idea to make this sample app similar with the Swift Sample app in how the extensions are demoed and split in separate views.

Copy link
Contributor Author

@cacheung cacheung Sep 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a separate view for Edge Identity. But wondering if need to give an example how user can import when have both identities in one view.

Copy link
Contributor Author

@cacheung cacheung Sep 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iOS main view
mainView

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core API View
coreAPIView

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edge Identity View
edgeIdentityview

Copy link
Contributor

@emdobrin emdobrin Sep 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep an example with importing both, but the Edge Identity extension does not need to be included in Core.js. Maybe we can create a separate file for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed import Edge Identity from Core view.
Add an EdgeIdentityAndIdentity.js file for both Identities case.

Update with more review comments
@cacheung
Copy link
Contributor Author

cacheung commented Sep 2, 2021

@cacheung Can we remove RCTAEPEdgeIdentityArrayUtil.java and RCTAEPEdgeIdentityMapUtil.java in this PR? I don't see any usage.

@yangyansong-adbe removed.

PravinPK and others added 11 commits September 2, 2021 15:11
AEPAssurance 3.x package to Staging
Update tests and fix few bugs
Update Java Data Bridge for EdgeEventHandle payload and Readme
update for review comments
update identity register info
Update registering info for Readme
Update Readme initializing instruction
Update Readme and MainApplication.java
AEPIdentity.extensionVersion().then(version => console.log("AdobeExperienceSDK: AEPEdgeIdentity version: " + version));
```

#### In the case of AEPIdentities and Identity for Edge Network in the same app
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we put two identities case here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to include this in the readme of the Edge Identity extension, we can consider adding it later in the main readme if needed. We can also remove the example with two Identity above

MobileCore.setLogLevel(LoggingMode.DEBUG);
MobileCore.setWrapperType(WrapperType.REACT_NATIVE);

try {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have two identity registering info here?

Comment on lines 30 to 33
<Button
onPress={() => navigation.navigate('EdgeIdentityAndIdentity')}
title="EdgeIdentityAndIdentity"
/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this, we can keep only the Identity and EdgeIdentity views.

import {AEPIdentity as AEPIdentity, AEPCore} from '@adobe/react-native-aepcore';
import {AEPIdentity as AEPEdgeIdentity } from '@adobe/react-native-aepedgeidentity';

export default EdgeIdentityAndIdentity = ({ navigation }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to keep this example, let's remove.

isa = PBXGroup;
children = (
B3E7B5881CC2AC0600A0062D /* RCTAEPUserProfile.h */,
B3E7B5891CC2AC0600A0062D /* RCTAEPUserProfile.m */,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there seem to be some UserProfile related files here 😅 let's remove/update these to Edge Identity related ones.

GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
GCC_WARN_UNUSED_FUNCTION = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
IPHONEOS_DEPLOYMENT_TARGET = 8.0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be IPHONEOS_DEPLOYMENT_TARGET = 8.0; - same for all occurrences.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emdobrin Thanks for the review, I will update them. For this one, what is the difference IPHONEOS_DEPLOYMENT_TARGET = 8.0;?

134814211AA4EA7D00B7C361 /* Products */ = {
isa = PBXGroup;
children = (
134814201AA4EA6300B7C361 /* libRCTAEPUserProfile.a */,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incorrect product section, states user profile

*/
package com.adobe.marketing.mobile.reactnative.edgeidentity;

import com.adobe.marketing.mobile.AdobeCallback;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used, can be removed

AEPIdentity.extensionVersion().then(version => console.log("AdobeExperienceSDK: AEPEdgeIdentity version: " + version));
```

#### In the case of AEPIdentities and Identity for Edge Network in the same app
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to include this in the readme of the Edge Identity extension, we can consider adding it later in the main readme if needed. We can also remove the example with two Identity above

… project file

Remove EdgeIdentityAndIdentity view in sample app and fix typo in iOS project file
update project file target to IPHONEOS_DEPLOYMENT_TARGET = 10.0;
Update with review comments
@cacheung cacheung merged commit d2488ca into adobe:edgeIdentity Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants