remove the usage of CustomIdentity #41

yangyansong-adbe · 2021-02-26T16:42:55Z

removes the hard dependency on Identity
#11 (comment)

AEPTarget/Sources/TargetConstants.swift

swarna04 · 2021-02-26T19:57:05Z

AEPTarget/Sources/DeliveryRequestBuilder.swift

+    ///   - thirdPartyId: `String` third party id
+    ///   - identitySharedState: `Identity` context  data
+    /// - Returns: `TargetIDs` object
+    static func generateTargetIDsBy(tntid: String?, thirdPartyId: String?, identitySharedState: [String: Any]?) -> TargetIDs? {


Let's rename this to getTargetIDs

I'd prefer not to use getTargetIDs here as it'd be reserved by setter/getter functions and apparently this is not a getter function. We can use create/generate/buildTargetIDs. Thoughts? @swarna04

Lets decide on one of the prefix and use it for everything as i did convert all generates to get already in my pr

I don't find the getter/ setter argument very compelling here as it's an internal static function namespaced by the enclosing enum. I do agree with @ravjain-adb that we should have consistency in naming here as it makes the code easier to read, maintain and debug. You guys can decide on this and update the methods here.

DeliveryRequestBuilder. getTargetIDs will let me think there is a DeliveryRequestBuilder. setTargetIDs at somewhere else. For me, it's not easy to understand what happened inside this function. But it's not a big deal, you guys can decide which name is a good fit here.

hmm thanks for pointing it out, agree! The reason I pointed it out in the first place was because of generateXXXBy(...). Either we follow a clear builder-like nomenclature (some non-object types like APIs do use it for easier customization) or steer away from it to prevent confusion.

AEPTarget/Tests/UnitTests/DeliveryRequestBuilderTests.swift

* integration_test: Removing unnecessary github actions Remove invalid string from log. add onshowfailure to fullscreenmessage delegate extension update createFullscreenMessage api calls from dev. Make sure preview token is not empty Fix typos and change log tag Set up changes with UIService, mockable ui elements. Add comments for ui delegate extensions Swift format Fix spacing Add comment for preview manager protocol Preview manager test progress. Need to update once uiutil protocol is set up. Create swift.yml

ravjain-adb

LGTM other than minor comment

swarna04 · 2021-03-04T23:13:38Z

AEPTarget/Sources/DeliveryRequestBuilder.swift

+                   let code = visitorId[TargetConstants.Identity.SharedState.Keys.VISITORID_TYPE] as? String,
+                   let authenticatedState = visitorId[TargetConstants.Identity.SharedState.Keys.VISITORID_AUTHENTICATION_STATE] as? Int
+                {
+                    if customerIds == nil { customerIds = [CustomerID]() }


Note: Look for a better fix here.

I have marked this comment in the issue #39 , I will check it later and see if we have a better fix for it.

* Project skeleton (#8) * cleanup project (#9) cleanup project * Swift Class placeholder (#10) * Placeholder for Swift classes * review feedback * disable Github action for now * Circleci project setup (#23) * Add .circleci/config.yml * set up CircleCI * no message * no message * no message * Target "prefetchContent" API (#11) * wip: skeleton setup for prefetch API * more prefetch API details * clean up code * rearrange code * AEPError -> Error * fix test fails * review feedback * fix review comments * add more docs * format * update CI scripts * disable Github Action * review feedback * review feedback * wip: skeleton setup for prefetch API * more prefetch API details * clean up code * fix test fails * review feedback * fix review comments * add more docs * update CI scripts * disable Github Action * review feedback * display notification tracking api * Updating with Notification clicked api. * Rebasing from prefetch branch * Removing AEPIdentity from podspec * Fixing compile time errors * Changes based on comments * fixes based on review comments * Adding public api tests for displayed and clicked location * Create swift.yml * Preview manager test progress. Need to update once uiutil protocol is set up. * Add comment for preview manager protocol * Fix spacing * Swift format * Add comments for ui delegate extensions * Set up changes with UIService, mockable ui elements. * Fix typos and change log tag * Make sure preview token is not empty * update createFullscreenMessage api calls from dev. * add onshowfailure to fullscreenmessage delegate extension * Remove invalid string from log. * Removing unnecessary github actions * Reset Experience API * Updating the pr based on review comments * remove the usage of CustomIdentity (#41) * remove usage of CustomIdentity * add function docs * cleanup code * fix review comments * remove AEPIdentity dependency for AEPTarget target * use the latest valid configuration (#46) * Adding configuration response handling * Updating the tests * Fixes based on comments for tests * Batch API feature * Adding pair id for getting the response back to customer * Adding A4T functionality * Adding few unit tests * - Update api to handle response content event - Updating unit tests * Updating based on previous pr comments * Updating based on comments * Using thread safe dictionary * Fixing the tests * Adding Objective c app * ObjC App UI * attempt to fix UI mishap * objc app and changes in sdk for it * Updates based on comments * Adding clear prefetch cache implementation * Removing signal * add release scripts * add integration test - prefetch (#49) * add integration test - prefetch * clean up * Adding Assurance to target demo apps * release automation (#56) * Data migration (#52) * migration stored V5 data to new format * no message * fix review comments * Preview manager test progress. Need to update once uiutil protocol is set up. * Setup preview manager target class usage. * wip: skeleton setup for prefetch API * more prefetch API details * clean up code * fix test fails * review feedback * fix review comments * add more docs * Mock previewmanager and initial test for Target preview events * Updated * Removed extra files * Updating tests * Updating tests * Updating preview api * Updating demo app to send a generic os event * Rebasing after conflicts * Updating tests * add tests for prefetch (#61) * add tests * add tests and fix format * Seperating functioonal tests with unit tests * clean up project (#67) * retrieve configuration and update target state (#54) * retrieve configuration and update target state (option 1) * lint * store configuration shared state in TargetState object * fix tests fail * fix lint * add tests for TargetState * remove duplicated function * improve the readability of code (#73) * Add functional test and updates on the request processing (#74) * Adding more functional tests * Adding more tests * Add more integration tests (#69) * more tests * enable "testRetrieveLocationContent" * not use empty target server url (#75) * Adding more tests * using env id from configuration (#76) * Checking objc annotation for all the public apis * Refactor the code to update session timestamp in one place (#77) * refactoring * increase the timeout * adding missed objc annotation * V4 data migration (#63) * migration stored V5 data to new format * no message * data migration for V4 * manually merges the missing code * revert changes * no message * fix review comments * add some trace logs * increase timeout in tests * clean up Prefetch code and add more tests (#79) * not allow dispatching multiple error response events for prefetch API * add missing key ("prefetchresult") in prefetch response event * Update AEPTarget.podspec * Update Makefile * Updating the test app to use analytics * clean up tests code (#83) * Speed up CI times (#84) * no message * no message * no message * no message * no message * no message * no message * no message * add functional tests for loadRequest (#85) * Extension class should not call MobileCore.dispatch (#86) * Add docs for public APIs (#62) * add docs for prefetch API * revert changes for the demo apps * add syntax * Update AEPTarget.md * Update AEPTarget.md * add docs for public APIs * revert changes for demo apps * update readme * no message * no message * Update LICENSE * Update COPYRIGHT * add API description for "Viaual preview" * Migrates all functional test cases from ACPTarget project (prefetch/retrieve) (#87) * add functional tests for loadRequest * no message * no message * add tests for prefetch API * import SwiftJSON library to the test code * Migrates all functional test cases from ACPTarget project (session/edgeHost/reset ...) (#88) * add functional tests for loadRequest * no message * no message * add tests for prefetch API * import SwiftJSON library to the test code * more tests * no message * allow set nil for public API - setThirdPartyId (#89) * add tests for "LocationClicked" API (#90) * add tests for "LocationDisplayed" API (#91) * add more tests (#92) * GA release (#81) * update podspec for GA release * clean up project * revert format * fix format * update public API signature (#93) * update public API signature * update API comments * review comments * update test app * update all public APIs * doc * update tests * clean up API docs * Update codecov.yml Co-authored-by: Christopher Hoffmann <cdhoffmann@users.noreply.github.com> Co-authored-by: ravinsingh jain <ravjain@adobe.com> Co-authored-by: ravjain-adb <61328814+ravjain-adb@users.noreply.github.com> Co-authored-by: Christopher Hoffmann <choffman@adobe.com> Co-authored-by: pravin <pravinpk0707@gmail.com>

yangyansong-adbe added 2 commits February 26, 2021 10:42

remove usage of CustomIdentity

fe5ff22

add function docs

18b8106

yangyansong-adbe linked an issue Feb 26, 2021 that may be closed by this pull request

[prefetch] remove hard dependency to AEPIdentity #38

Closed

yangyansong-adbe changed the title ~~remove usage of CustomIdentity~~ remove the usage of CustomIdentity Feb 26, 2021

yangyansong-adbe requested a review from swarna04 February 26, 2021 17:01

cleanup code

5656fc9

swarna04 requested changes Feb 26, 2021

View reviewed changes

swarna04 requested a review from ravjain-adb February 26, 2021 20:26

yangyansong-adbe added 3 commits March 4, 2021 12:28

fix review comments

f4a4631

remove AEPIdentity dependency for AEPTarget target

2e0a8b0

ravjain-adb approved these changes Mar 4, 2021

View reviewed changes

swarna04 reviewed Mar 4, 2021

View reviewed changes

swarna04 approved these changes Mar 4, 2021

View reviewed changes

yangyansong-adbe mentioned this pull request Mar 5, 2021

[Discussion] several review comments for Prefetch code #39

Closed

yangyansong-adbe merged commit 9adc9d8 into adobe:dev Mar 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove the usage of CustomIdentity #41

remove the usage of CustomIdentity #41

yangyansong-adbe commented Feb 26, 2021 •

edited

Loading

swarna04 Feb 26, 2021

yangyansong-adbe Feb 26, 2021 •

edited

Loading

ravjain-adb Mar 4, 2021

swarna04 Mar 4, 2021 •

edited

Loading

yangyansong-adbe Mar 4, 2021

swarna04 Mar 5, 2021

ravjain-adb left a comment

swarna04 Mar 4, 2021

yangyansong-adbe Mar 5, 2021

remove the usage of CustomIdentity #41

remove the usage of CustomIdentity #41

Conversation

yangyansong-adbe commented Feb 26, 2021 • edited Loading

swarna04 Feb 26, 2021

Choose a reason for hiding this comment

yangyansong-adbe Feb 26, 2021 • edited Loading

Choose a reason for hiding this comment

ravjain-adb Mar 4, 2021

Choose a reason for hiding this comment

swarna04 Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

yangyansong-adbe Mar 4, 2021

Choose a reason for hiding this comment

swarna04 Mar 5, 2021

Choose a reason for hiding this comment

ravjain-adb left a comment

Choose a reason for hiding this comment

swarna04 Mar 4, 2021

Choose a reason for hiding this comment

yangyansong-adbe Mar 5, 2021

Choose a reason for hiding this comment

yangyansong-adbe commented Feb 26, 2021 •

edited

Loading

yangyansong-adbe Feb 26, 2021 •

edited

Loading

swarna04 Mar 4, 2021 •

edited

Loading