Skip to content

Added support for TntId setter API#116

Merged
swarna04 merged 14 commits intoadobe:dev-v3.2.0from
swarna04:MOB-16757
Jul 25, 2022
Merged

Added support for TntId setter API#116
swarna04 merged 14 commits intoadobe:dev-v3.2.0from
swarna04:MOB-16757

Conversation

@swarna04
Copy link
Copy Markdown
Contributor

@swarna04 swarna04 commented Jul 20, 2022

Description

[Ref: MOB-16757]

  • Added support for an API to set tntId in the SDK
    func setTntId(_ id: String?)
  • Added functionality to derive Edge host value from the tntId location hint.
  • Added unit, functional and integration tests

Swift testapp

ObjectiveC testapp

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.

@swarna04 swarna04 changed the base branch from dev-v3.2.0 to MOB-16753-4 July 20, 2022 03:24
@swarna04 swarna04 changed the title Added support for setter for TntId Added support for TntId setter Jul 20, 2022
@swarna04 swarna04 changed the title Added support for TntId setter Added support for TntId setter API Jul 20, 2022
@swarna04 swarna04 requested a review from sbenedicadb July 20, 2022 15:51
Copy link
Copy Markdown
Member

@sbenedicadb sbenedicadb left a comment

Choose a reason for hiding this comment

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

looks good with a couple of nit items.

curious why we are storing "empty" tntId as empty string, but "empty" edgeHost as nil

}
}

/// Sets the Test and Target user identifier.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think we should remove "Test" from this summary. it's just "Target" these days, or maybe "Adobe Target"?

///
/// - Parameter id: a string containing the value of the Tnt Id to be set in the SDK.
static func setTntId(_ id: String?) {
let eventData = [TargetConstants.EventDataKeys.TNT_ID: id ?? ""]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

since it's an optional param, it's good to also indicate in the doc what happens when id is nil

/// and is removed at uninstall, upon privacy status update to opt out or when AEPTarget.resetExperience is called.
///
/// - Parameter id: a string containing the value of the Tnt Id to be set in the SDK.
static func setTntId(_ id: String?) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we need to expose this for objc?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is visible in objc, I've updated the objc testapp as well.

if tntIdValuesAreEqual(newTntId: tntId, oldTntId: targetState.tntId) {
Log.debug(label: Target.LOG_TAG, "setTntId - New tntId value is same as the existing tntId \(String(describing: targetState.tntId)).")
if tntId == targetState.tntId {
Log.debug(label: Target.LOG_TAG, "setTntIdInternal - New tntId value is same as the existing tntId \(String(describing: tntId)).")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: some of these debug logs could be moved to trace

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe we need some of these as debug logs to interpret the different scenarios. Added a trace log as well.

@swarna04 swarna04 changed the base branch from MOB-16753-4 to dev-v3.2.0 July 22, 2022 00:57
@swarna04 swarna04 changed the base branch from dev-v3.2.0 to MOB-16753-4 July 22, 2022 00:58
/// The provided Tnt Id is persisted in the SDK and attached to all subsequent Target requests. It is used to
/// derive the edge host value in the SDK, which is also persisted and used in future Target requests.
///
/// If the provided Tnt Id is nil or empty, the SDK will remove the Tnt Id and edge host values from persistence.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@swarna04 swarna04 changed the base branch from MOB-16753-4 to dev-v3.2.0 July 25, 2022 17:06
… into MOB-16757

# Conflicts:
#	AEPTarget.xcodeproj/project.pbxproj
#	AEPTarget/Sources/Target+PublicAPI.swift
#	AEPTarget/Sources/Target.swift
#	AEPTarget/Sources/TargetConstants.swift
#	AEPTarget/Tests/IntegrationTests/TargetIntegrationTests.swift
#	AEPTarget/Tests/UnitTests/Target+PublicAPITests.swift
#	AEPTargetDemoApp/ContentView.swift
@swarna04 swarna04 merged commit e0bf7e2 into adobe:dev-v3.2.0 Jul 25, 2022
swarna04 added a commit that referenced this pull request Jul 29, 2022
* Added support for Target session Id setter & getter (#115)

* Added support for Target session Id setter/ getter

* code cleanup

* Fixed formatting issue

* Incorporated feedback

* Incorporated feedback

* Added support for TntId setter API (#116)

* Added support for Target session Id setter/ getter

* code cleanup

* Fixed formatting issue

* Added support for Target tntId setter

* code cleanup

* fixed formatting

* fixed definition conflict test issue

* Incorporated feedback

* Incorporated feedback

* Using distinct Event name

* More testapp updates

* Release prep v3.2.0 (#117)

* release prep v3.2.0

* Feedback updates

* Dependency and doc updates

* Incorporated feedback

* Fixed issue: Persisted edge host value was not used for Target request issued upon app close & relaunch (#119)

* Fixed an issue where persisted edge host value was not used for Target request issued upon app close & relaunch

* minor doc update

* Target doc updates (#120)

* Target doc updates

* docs cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants