From 55a3c1d3316aed263863b5dd69f5c09648182ba0 Mon Sep 17 00:00:00 2001 From: Justin Swart Date: Thu, 29 Apr 2021 10:32:41 -0700 Subject: [PATCH] Prevent duplicate identity calls and only send deltas (#1452) --- Kickstarter.xcodeproj/project.pbxproj | 9 +- Library/KeyValueStoreType.swift | 13 +++ Library/Tracking/KSRAnalytics.swift | 18 ++-- .../Tracking/KSRAnalyticsIdentityData.swift | 81 ++++++++++++++++++ .../KSRAnalyticsIdentityDataTests.swift | 82 +++++++++++++++++++ Library/Tracking/KSRAnalyticsTests.swift | 71 ++++++++-------- 6 files changed, 231 insertions(+), 43 deletions(-) create mode 100644 Library/Tracking/KSRAnalyticsIdentityData.swift create mode 100644 Library/Tracking/KSRAnalyticsIdentityDataTests.swift diff --git a/Kickstarter.xcodeproj/project.pbxproj b/Kickstarter.xcodeproj/project.pbxproj index e6c1fbb87e..f5114cc409 100644 --- a/Kickstarter.xcodeproj/project.pbxproj +++ b/Kickstarter.xcodeproj/project.pbxproj @@ -460,6 +460,8 @@ 8A49396924B690E000C3C3CE /* RewardAddOnSelectionViewEnvelopeTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A49396824B690E000C3C3CE /* RewardAddOnSelectionViewEnvelopeTemplate.swift */; }; 8A49396B24B7735100C3C3CE /* RewardAddOnCardView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A49396A24B7735100C3C3CE /* RewardAddOnCardView.swift */; }; 8A49396F24B77B3500C3C3CE /* RewardAddOnCardViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A49396E24B77B3500C3C3CE /* RewardAddOnCardViewModel.swift */; }; + 8A4B8E90263A02EA00D92E4E /* KSRAnalyticsIdentityData.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A4B8E812639F8C700D92E4E /* KSRAnalyticsIdentityData.swift */; }; + 8A4B8E98263A153D00D92E4E /* KSRAnalyticsIdentityDataTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A4B8E97263A153D00D92E4E /* KSRAnalyticsIdentityDataTests.swift */; }; 8A4DDAB32373427000ADE31D /* PledgeStatusLabelView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A4DDAB22373427000ADE31D /* PledgeStatusLabelView.swift */; }; 8A4DDAB52373429300ADE31D /* PledgeStatusLabelViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A4DDAB42373429300ADE31D /* PledgeStatusLabelViewModel.swift */; }; 8A4DDAB72373A05000ADE31D /* PledgeStatusLabelViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A4DDAB62373A05000ADE31D /* PledgeStatusLabelViewModelTests.swift */; }; @@ -1997,6 +1999,8 @@ 8A49396824B690E000C3C3CE /* RewardAddOnSelectionViewEnvelopeTemplate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RewardAddOnSelectionViewEnvelopeTemplate.swift; sourceTree = ""; }; 8A49396A24B7735100C3C3CE /* RewardAddOnCardView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RewardAddOnCardView.swift; sourceTree = ""; }; 8A49396E24B77B3500C3C3CE /* RewardAddOnCardViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RewardAddOnCardViewModel.swift; sourceTree = ""; }; + 8A4B8E812639F8C700D92E4E /* KSRAnalyticsIdentityData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KSRAnalyticsIdentityData.swift; sourceTree = ""; }; + 8A4B8E97263A153D00D92E4E /* KSRAnalyticsIdentityDataTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KSRAnalyticsIdentityDataTests.swift; sourceTree = ""; }; 8A4DDAB22373427000ADE31D /* PledgeStatusLabelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PledgeStatusLabelView.swift; sourceTree = ""; }; 8A4DDAB42373429300ADE31D /* PledgeStatusLabelViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PledgeStatusLabelViewModel.swift; sourceTree = ""; }; 8A4DDAB62373A05000ADE31D /* PledgeStatusLabelViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PledgeStatusLabelViewModelTests.swift; sourceTree = ""; }; @@ -3178,6 +3182,8 @@ children = ( 8A23203025B0ECF700B940C3 /* IdentifyingTrackingClient.swift */, 8A213CE8239EAEA400BBB4C7 /* KSRAnalytics.swift */, + 8A4B8E812639F8C700D92E4E /* KSRAnalyticsIdentityData.swift */, + 8A4B8E97263A153D00D92E4E /* KSRAnalyticsIdentityDataTests.swift */, 8A213CE7239EAEA400BBB4C7 /* KSRAnalyticsTests.swift */, 8A213CE9239EAEA400BBB4C7 /* MockTrackingClient.swift */, 8A417DEF25AE37D200A2C406 /* Segment.swift */, @@ -5421,6 +5427,7 @@ D6089E8F209106CD0032CC99 /* PushNotificationDialog.swift in Sources */, A7CC13D51D00E6CF00035C52 /* FindFriendsFriendFollowCellViewModel.swift in Sources */, 598D96C51D42A3E3003F3F66 /* ActivitySampleFollowCellViewModel.swift in Sources */, + 8A4B8E90263A02EA00D92E4E /* KSRAnalyticsIdentityData.swift in Sources */, 598D96BA1D426FD8003F3F66 /* ActivitySampleBackingCellViewModel.swift in Sources */, 59E877381DC9419700BCD1F7 /* Newsletter.swift in Sources */, 8A142EBD23354BFD00FB43AB /* AddNewCardIntent.swift in Sources */, @@ -5686,6 +5693,7 @@ D04AACAC218BB72100CF713E /* SettingsNotificationCellViewModelTests.swift in Sources */, D04AACAA218BB72100CF713E /* MessageBannerViewModelTests.swift in Sources */, A7ED1FBC1E831C5C00BFFA01 /* FindFriendsFriendFollowCellViewModelTests.swift in Sources */, + 8A4B8E98263A153D00D92E4E /* KSRAnalyticsIdentityDataTests.swift in Sources */, A7ED1FDD1E831C5C00BFFA01 /* ShareViewModelTests.swift in Sources */, 37DEC2202257CA0A0051EF9B /* PledgeViewModelTests.swift in Sources */, A7ED20001E831C5C00BFFA01 /* SearchViewModelTests.swift in Sources */, @@ -6899,7 +6907,6 @@ ASSETCATALOG_COMPILER_APPICON_NAME = "app-icon-debug"; CLANG_ENABLE_MODULES = YES; CODE_SIGN_ENTITLEMENTS = "Kickstarter-iOS/Debug.entitlements"; - CODE_SIGN_IDENTITY = "iPhone Developer"; CODE_SIGN_IDENTITY = "Apple Development: Native Team (4BLNH33RY7)"; CODE_SIGN_STYLE = Manual; DEFINES_MODULE = YES; diff --git a/Library/KeyValueStoreType.swift b/Library/KeyValueStoreType.swift index dc2dc85346..707882e287 100644 --- a/Library/KeyValueStoreType.swift +++ b/Library/KeyValueStoreType.swift @@ -2,6 +2,7 @@ import Foundation public enum AppKeys: String { // swiftformat:disable wrap + case analyticsIdentityData = "com.kickstarter.KeyValueStoreType.analyticsIdentityData" case closedFacebookConnectInActivity = "com.kickstarter.KeyValueStoreType.closedFacebookConnectInActivity" case closedFindFriendsInActivity = "com.kickstarter.KeyValueStoreType.closedFindFriendsInActivity" case deniedNotificationContexts = "com.kickstarter.KeyValueStoreType.deniedNotificationContexts" @@ -33,6 +34,7 @@ public protocol KeyValueStoreType: AnyObject { func synchronize() -> Bool func removeObject(forKey defaultName: String) + var analyticsIdentityData: KSRAnalyticsIdentityData? { get set } var deniedNotificationContexts: [String] { get set } var favoriteCategoryIds: [Int] { get set } var hasClosedFacebookConnectInActivity: Bool { get set } @@ -50,6 +52,17 @@ public protocol KeyValueStoreType: AnyObject { } extension KeyValueStoreType { + public var analyticsIdentityData: KSRAnalyticsIdentityData? { + get { + guard let data = self.data(forKey: AppKeys.analyticsIdentityData.rawValue) else { return nil } + return try? JSONDecoder().decode(KSRAnalyticsIdentityData.self, from: data) + } + set { + let data = try? JSONEncoder().encode(newValue) + self.set(data, forKey: AppKeys.analyticsIdentityData.rawValue) + } + } + public var favoriteCategoryIds: [Int] { get { return self.object(forKey: AppKeys.favoriteCategoryIds.rawValue) as? [Int] ?? [] diff --git a/Library/Tracking/KSRAnalytics.swift b/Library/Tracking/KSRAnalytics.swift index e9218a391f..3b89e1fbf7 100644 --- a/Library/Tracking/KSRAnalytics.swift +++ b/Library/Tracking/KSRAnalytics.swift @@ -574,16 +574,18 @@ public final class KSRAnalytics { return } + let previousIdentityData = AppEnvironment.current.userDefaults.analyticsIdentityData + + let newData = KSRAnalyticsIdentityData(user) + + guard newData != previousIdentityData else { return } + self.segmentClient?.identify( - "\(user.id)", - traits: [ - "name": user.name, - "is_creator": user.isCreator, - "backed_projects_count": user.stats.backedProjectsCount ?? 0, - "created_projects_count": user.stats.createdProjectsCount ?? 0 - ] - .withAllValuesFrom(user.notifications.encode()) + "\(newData.userId)", + traits: newData.uniqueTraits(comparedTo: previousIdentityData) ) + + AppEnvironment.current.userDefaults.analyticsIdentityData = newData } private func updateAndObservePreferredContentSizeCategory() { diff --git a/Library/Tracking/KSRAnalyticsIdentityData.swift b/Library/Tracking/KSRAnalyticsIdentityData.swift new file mode 100644 index 0000000000..11ade8108d --- /dev/null +++ b/Library/Tracking/KSRAnalyticsIdentityData.swift @@ -0,0 +1,81 @@ +import Foundation +import KsApi + +private enum KSRAnalyticsIdentityTraitValue: Equatable { + case string(String) + case bool(Bool) + + var value: Any { + switch self { + case let .string(value): return value + case let .bool(value): return value + } + } +} + +public struct KSRAnalyticsIdentityData: Equatable { + public let userId: Int + private let name: String + private let notifications: User.Notifications + + init(_ user: User) { + self.userId = user.id + self.name = user.name + self.notifications = user.notifications + } + + public static func == (lhs: KSRAnalyticsIdentityData, rhs: KSRAnalyticsIdentityData) -> Bool { + let uniqueTraits = lhs.uniqueTraits(comparedTo: rhs) + + return uniqueTraits.isEmpty + } + + func uniqueTraits(comparedTo otherData: KSRAnalyticsIdentityData?) -> [String: Any] { + var newTraits: [String: Any] = [:] + let otherTraits = otherData?.traits ?? [:] + + for key in self.traits.keys where self.traits[key] != otherTraits[key] { + newTraits[key] = self.traits[key]?.value + } + + return newTraits + } + + fileprivate var traits: [String: KSRAnalyticsIdentityTraitValue] { + let notifications = self.notifications.encode() + .mapValues { ($0 as? Bool).flatMap(KSRAnalyticsIdentityTraitValue.bool) } + .compactMapValues { $0 } + + return [ + "name": .string(self.name) + ] + .withAllValuesFrom(notifications) + } +} + +extension KSRAnalyticsIdentityData: Codable { + private enum CodingKeys: String, CodingKey { + case userId + case name + case notifications + } + + public func encode(to encoder: Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(self.userId, forKey: .userId) + try container.encode(self.name, forKey: .name) + + let data = try JSONSerialization.data(withJSONObject: self.notifications.encode(), options: []) + try container.encode(data, forKey: .notifications) + } + + public init(from decoder: Decoder) throws { + let values = try decoder.container(keyedBy: CodingKeys.self) + + self.userId = try values.decode(Int.self, forKey: .userId) + self.name = try values.decode(String.self, forKey: .name) + + let data = try values.decode(Data.self, forKey: .notifications) + self.notifications = try JSONDecoder().decode(User.Notifications.self, from: data) + } +} diff --git a/Library/Tracking/KSRAnalyticsIdentityDataTests.swift b/Library/Tracking/KSRAnalyticsIdentityDataTests.swift new file mode 100644 index 0000000000..5c3c41ca4a --- /dev/null +++ b/Library/Tracking/KSRAnalyticsIdentityDataTests.swift @@ -0,0 +1,82 @@ +@testable import KsApi +@testable import Library +import Prelude +import XCTest + +final class KSRAnalyticsIdentityDataTests: XCTestCase { + func testInitialization() { + let user = User.template + |> User.lens.name .~ "Test User" + |> User.lens.notifications.mobileBackings .~ false + |> User.lens.notifications.messages .~ true + + let data = KSRAnalyticsIdentityData(user) + + XCTAssertEqual(data.userId, 1) + XCTAssertEqual(data.uniqueTraits(comparedTo: nil)["name"] as? String, "Test User") + XCTAssertEqual(data.uniqueTraits(comparedTo: nil)["notify_mobile_of_backings"] as? Bool, false) + XCTAssertEqual(data.uniqueTraits(comparedTo: nil)["notify_of_messages"] as? Bool, true) + } + + func testUniqueTraits() { + let user1 = User.template + |> User.lens.name .~ "Test User 1" + |> User.lens.notifications.mobileBackings .~ false + |> User.lens.notifications.messages .~ true + + let user2 = User.template + |> User.lens.name .~ "Test User 2" + |> User.lens.notifications.mobileBackings .~ false + |> User.lens.notifications.messages .~ true + |> User.lens.notifications.friendActivity .~ true + + let data1 = KSRAnalyticsIdentityData(user1) + let data2 = KSRAnalyticsIdentityData(user2) + + let uniqueTraits = data2.uniqueTraits(comparedTo: data1) + + XCTAssertEqual(uniqueTraits.keys.count, 2) + + XCTAssertEqual(uniqueTraits["name"] as? String, "Test User 2") + XCTAssertEqual(uniqueTraits["notify_of_friend_activity"] as? Bool, true) + } + + func testEquality() { + let user1 = User.template + |> User.lens.name .~ "Test User 1" + |> User.lens.notifications.mobileBackings .~ false + |> User.lens.notifications.messages .~ true + + let user2 = User.template + |> User.lens.name .~ "Test User 1" + |> User.lens.notifications.mobileBackings .~ false + |> User.lens.notifications.messages .~ true + + XCTAssertEqual(KSRAnalyticsIdentityData(user1), KSRAnalyticsIdentityData(user2)) + + let user3 = User.template + |> User.lens.name .~ "Test User 2" + |> User.lens.notifications.mobileBackings .~ false + |> User.lens.notifications.messages .~ true + + let user4 = User.template + |> User.lens.name .~ "Test User 3" + |> User.lens.notifications.mobileBackings .~ true + |> User.lens.notifications.messages .~ true + + XCTAssertNotEqual(KSRAnalyticsIdentityData(user3), KSRAnalyticsIdentityData(user4)) + } + + func testEncodingDecoding() { + let data1 = KSRAnalyticsIdentityData(.template) + + guard let encoded = try? JSONEncoder().encode(data1) else { + XCTFail("Failed to encode") + return + } + + let decoded = try? JSONDecoder().decode(KSRAnalyticsIdentityData.self, from: encoded) + + XCTAssertEqual(decoded, data1) + } +} diff --git a/Library/Tracking/KSRAnalyticsTests.swift b/Library/Tracking/KSRAnalyticsTests.swift index cd075bf378..6dd91a2147 100644 --- a/Library/Tracking/KSRAnalyticsTests.swift +++ b/Library/Tracking/KSRAnalyticsTests.swift @@ -2329,22 +2329,11 @@ final class KSRAnalyticsTests: TestCase { func testIdentifyingTrackingClient() { let user = User.template - |> User.lens.stats.backedProjectsCount .~ 2 - |> User.lens.stats.createdProjectsCount .~ 3 AppEnvironment.updateCurrentUser(user) XCTAssertEqual(self.segmentTrackingClient.userId, "\(user.id)") XCTAssertEqual(self.segmentTrackingClient.traits?["name"] as? String, user.name) - XCTAssertEqual(self.segmentTrackingClient.traits?["is_creator"] as? Bool, user.isCreator) - XCTAssertEqual( - self.segmentTrackingClient.traits?["backed_projects_count"] as? Int, - user.stats.backedProjectsCount - ) - XCTAssertEqual( - self.segmentTrackingClient.traits?["created_projects_count"] as? Int, - user.stats.createdProjectsCount - ) let notifications1 = user.notifications.encode() @@ -2352,36 +2341,50 @@ final class KSRAnalyticsTests: TestCase { XCTAssertEqual(notifications1[key] as? Bool, self.segmentTrackingClient.traits?[key] as? Bool) } - let user2 = user - |> User.lens.id .~ 9_999 - |> User.lens.name .~ "Another User" - |> User.lens.stats.backedProjectsCount .~ 4 - |> User.lens.stats.createdProjectsCount .~ 0 + AppEnvironment.logout() + + XCTAssertNil(self.segmentTrackingClient.userId) + XCTAssertNil(self.segmentTrackingClient.traits) + } - AppEnvironment.updateCurrentUser(user2) + func testIdentifyingTrackingClient_DoesNotRepeat() { + let mockKeyValueStore = MockKeyValueStore() - XCTAssertEqual(self.segmentTrackingClient.userId, "\(user2.id)") - XCTAssertEqual(self.segmentTrackingClient.traits?["name"] as? String, user2.name) - XCTAssertEqual(self.segmentTrackingClient.traits?["is_creator"] as? Bool, user2.isCreator) - XCTAssertEqual( - self.segmentTrackingClient.traits?["backed_projects_count"] as? Int, - user2.stats.backedProjectsCount - ) - XCTAssertEqual( - self.segmentTrackingClient.traits?["created_projects_count"] as? Int, - user2.stats.createdProjectsCount - ) + let user = User.template - let notifications2 = user.notifications.encode() + let data = KSRAnalyticsIdentityData(user) - for (key, _) in notifications2 { - XCTAssertEqual(notifications2[key] as? Bool, self.segmentTrackingClient.traits?[key] as? Bool) + mockKeyValueStore.analyticsIdentityData = data + + withEnvironment(userDefaults: mockKeyValueStore) { + AppEnvironment.updateCurrentUser(user) + + XCTAssertNil(self.segmentTrackingClient.userId) + XCTAssertNil(self.segmentTrackingClient.traits) } + } - AppEnvironment.logout() + func testIdentifyingTrackingClient_OnlySendsDeltas() { + let mockKeyValueStore = MockKeyValueStore() - XCTAssertNil(self.segmentTrackingClient.userId) - XCTAssertNil(self.segmentTrackingClient.traits) + let user = User.template + |> User.lens.notifications.mobileUpdates .~ true + |> User.lens.notifications.messages .~ true + + let data = KSRAnalyticsIdentityData(user) + + mockKeyValueStore.analyticsIdentityData = data + + withEnvironment(userDefaults: mockKeyValueStore) { + let updatedUser = User.template + |> User.lens.notifications.mobileUpdates .~ true + |> User.lens.notifications.messages .~ false + + AppEnvironment.updateCurrentUser(updatedUser) + + XCTAssertEqual(self.segmentTrackingClient.userId, "\(1)") + XCTAssertEqual(self.segmentTrackingClient.traits?["notify_of_messages"] as? Bool, false) + } } func testTrackAddOnsContinueButtonClicked() {