-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NT-1194] Migrate Optimizely events to send to the Data Lake #1177
[NT-1194] Migrate Optimizely events to send to the Data Lake #1177
Conversation
…os-oss into NT-1174-add-opty-to-events
…starter/ios-oss into NT-1194-migrate-opty-events # Conflicts: # Library/OptimizelyClientType.swift
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool cool, just few q's
@@ -417,11 +409,11 @@ final class AppDelegateViewModelTests: TestCase { | |||
XCTAssertEqual(["App Open", "Opened App"], trackingClient.events) | |||
|
|||
self.vm.inputs.applicationDidEnterBackground() | |||
XCTAssertEqual(["App Open", "Opened App", "App Close", "Closed App"], trackingClient.events) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to not track App Close
anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's right.
Library/Tracking/Koala.swift
Outdated
@@ -57,11 +64,14 @@ public final class Koala { | |||
/// Determines the screen from which the event is sent. | |||
public enum LocationContext: String { | |||
case activities = "activity_feed_screen" // ActivitiesViewController | |||
case campaign = "campaign_screen" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the // <view controller>
for this one?
let attributes = optimizelyUserAttributes(refTag: refTag) | ||
|
||
try? AppEnvironment.current.optimizelyClient?.track( | ||
eventKey: "Editorial Card Clicked", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed because the campaign is over right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR removes all events previously tracked on the Optimizely SDK, because those events have been migrated to the data lake. An upcoming PR will add the necessary Optimizely properties to the required events.
|
||
try? AppEnvironment.current.optimizelyClient? | ||
.track( | ||
eventKey: "App Completed Checkout", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understood that we still wanted this event.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, the backend will be sending this.
Generated by 🚫 Danger |
* [NT-1162] Add OptimizelyProperties (#1172) * Add new optimizely props * optimizelyProps tests * Change to “unknown” * Update test name * Use fake experiments strings for tests * Simplyfing allExperiments code * [NT-1194] Migrate Optimizely events to send to the Data Lake (#1177) * Add new optimizely props * optimizelyProps tests * Change to “unknown” * Update test name * Use fake experiments strings for tests * Add missing whitelisted data lake events * Add new whitelisted events * Add carousel swiped event * Onboarding tracking events & tests * Remove “App Completed Checkout” * Updating tests * Simplyfing allExperiments code * Tracking tests * Removing optimizely tracking references * Cleaning up * [NT-1174] Add Optimizely Properties to relevant events (#1180) * Add new optimizely props * optimizelyProps tests * Change to “unknown” * Update test name * Use fake experiments strings for tests * Add missing whitelisted data lake events * Add new whitelisted events * Add carousel swiped event * Onboarding tracking events & tests * Remove “App Completed Checkout” * Updating tests * Simplyfing allExperiments code * Tracking tests * Removing optimizely tracking references * Add optimizely properties to creator details viewed and campaign button clicked * Add optimizely properties to relevant events * Formatting * Cleaning up * Cleaning up test name * Coalesce to an empty dictionary for opty props * Fixing merge conflict issues
📲 What
Migrates some events that were previously being sent to Optimizely via the Optimizely SDK, to be sent instead to the data lake. This will allow us to send events directly to both Optimizely and Amplitude through data lake lambdas.
🤔 Why
In preparation for migrating all our Optimizely events to send from data lake lambdas, we need to make sure that these events are flowing through the data lake. Moving forward, all Optimizely events will flow through the data lake, so we no longer need to track anything through the Optimizely SDK.
🛠 How
track
function from theOptimizelyClientType
protocoltrack
on the Optimizely SDKKoala
client on theAppEnvironment
Note that adding the necessary Optimizely properties to the event property groups will be done in a follow-up PR.
✅ Acceptance criteria
Test that the follow events are being sent through the Data Lake, instead of through the Optimizely SDK:
context_location
should belanding_page
)context_location
should beonboarding
)context_location
should beonboarding
)