Modernize tracking instrumentation events #179
Conversation
There was a problem hiding this comment.
Thanks @bormind had a quick look now and I think it looks good! I've just marked it as requesting changes so that we can approve once we have the event name that you're waiting on for trackDeclineFriendFollowAll.
Library/Koala/Koala.swift
Outdated
| @@ -1300,31 +1300,57 @@ public final class Koala { | |||
| } | |||
|
|
|||
| public func trackDeclineFriendFollowAll(source: FriendsSource) { | |||
| // deprecated | |||
| // self.track(event: "Facebook Friend Decline Follow All", properties: deprecatedProps) | |||
There was a problem hiding this comment.
Let's wait until we have the name before merging 👍
|
|
||
| XCTAssertEqual([nil, nil, 2], | ||
| self.trackingClient.properties.map { $0["page_count"] as! Int? }) | ||
|
|
There was a problem hiding this comment.
Can delete this new line ☝️
Library/Koala/Koala.swift
Outdated
| self.track(event: "Facebook Friend Decline Follow All", properties: ["source": source.trackingString]) | ||
| } | ||
|
|
||
| public func trackFacebookConnect(source: FriendsSource) { | ||
| self.track(event: "Facebook Connect", properties: ["source": source.trackingString]) | ||
| // deprecated | ||
| self.track(event: "Facebook Connect", properties: deprecatedProps) |
There was a problem hiding this comment.
Since we tracked the "source" property in the deprecated event, we still want to keep passing that property. Thus you could do something like this:
let props: [String:Any] = ["source": source.trackingString]
self.track(event: "Facebook Connect", properties: props.withAllValuesFrom(deprecatedProps))
self.track(event: "Connected Facebook", properties: props)
Library/Koala/Koala.swift
Outdated
| } | ||
|
|
||
| public func trackFacebookConnectError(source: FriendsSource) { | ||
| self.track(event: "Facebook Connect Error", properties: ["source": source.trackingString]) | ||
| // deprecated | ||
| self.track(event: "Facebook Connect Error", properties: deprecatedProps) |
There was a problem hiding this comment.
Same as above comment. Add the "source" prop here.
Library/Koala/Koala.swift
Outdated
| } | ||
|
|
||
| public func trackFindFriendsView(source: FriendsSource) { | ||
| self.track(event: "Find Friends View", properties: ["source": source.trackingString]) | ||
| // deprecated | ||
| self.track(event: "Find Friends View", properties: deprecatedProps) |
There was a problem hiding this comment.
Also add the "source" prop here.
Library/Koala/Koala.swift
Outdated
| } | ||
|
|
||
| public func trackFriendFollow(source: FriendsSource) { | ||
| self.track(event: "Facebook Friend Follow", properties: ["source": source.trackingString]) | ||
| // deprecated | ||
| self.track(event: "Facebook Friend Follow", properties: deprecatedProps) |
There was a problem hiding this comment.
Also add the "source" prop here.
Library/Koala/Koala.swift
Outdated
| } | ||
|
|
||
| public func trackFriendFollowAll(source: FriendsSource) { | ||
| self.track(event: "Facebook Friend Follow All", properties: ["source": source.trackingString]) | ||
| // deprecated | ||
| self.track(event: "Facebook Friend Follow All", properties: deprecatedProps) |
There was a problem hiding this comment.
Also add the "source" prop here. :)
Library/Koala/Koala.swift
Outdated
| } | ||
|
|
||
| public func trackFriendUnfollow(source: FriendsSource) { | ||
| self.track(event: "Facebook Friend Unfollow", properties: ["source": source.trackingString]) | ||
| // deprecated | ||
| self.track(event: "Facebook Friend Unfollow", properties: deprecatedProps) |
There was a problem hiding this comment.
Also add the "source" prop here. :)
| XCTAssertEqual(["Facebook Friend Follow"], self.trackingClient.events) | ||
| XCTAssertEqual(["activity"], self.trackingClient.properties(forKey: "source", as: String.self)) | ||
| XCTAssertEqual(["Facebook Friend Follow", "Followed Facebook Friend"], self.trackingClient.events) | ||
| XCTAssertEqual([nil, "activity"], self.trackingClient.properties(forKey: "source", as: String.self)) | ||
| } |
There was a problem hiding this comment.
these tests should be tracking the "source" property once you add it back in.
|
|
||
| source | ||
| .takePairWhen(pageCount.skip(first: 1).filter { $0 > 1 }) | ||
| .observeValues { AppEnvironment.current.koala.loadedMoreFriends(source: $0, pageCount: $1) } |
|
@theginbin ah of course re |
…rnized version of "Declined Follow All Facebook Friends" event
|
@theginbin, @justinswart, thank you for the review!! |
|
merge away @bormind! |
|
Great thanks @theginbin, @justinswart for helping me through! |
What
Why
As a part of an effort of renaming (modernizing) tracking events and adding missing once.
How
See 👀
Trello: Modernize following instrumentation
TODO
We probably have to come up with a new name for "Facebook Friend Decline Follow All" event and deprecate the current one