diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift index 31137a4fb..17e2b464d 100644 --- a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift +++ b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift @@ -6733,15 +6733,15 @@ class PHGPostHogMock: PHGPostHogProtocol { } //MARK: - capture - var capturePropertiesUnderlyingCallsCount = 0 - var capturePropertiesCallsCount: Int { + var capturePropertiesUserPropertiesUnderlyingCallsCount = 0 + var capturePropertiesUserPropertiesCallsCount: Int { get { if Thread.isMainThread { - return capturePropertiesUnderlyingCallsCount + return capturePropertiesUserPropertiesUnderlyingCallsCount } else { var returnValue: Int? = nil DispatchQueue.main.sync { - returnValue = capturePropertiesUnderlyingCallsCount + returnValue = capturePropertiesUserPropertiesUnderlyingCallsCount } return returnValue! @@ -6749,26 +6749,26 @@ class PHGPostHogMock: PHGPostHogProtocol { } set { if Thread.isMainThread { - capturePropertiesUnderlyingCallsCount = newValue + capturePropertiesUserPropertiesUnderlyingCallsCount = newValue } else { DispatchQueue.main.sync { - capturePropertiesUnderlyingCallsCount = newValue + capturePropertiesUserPropertiesUnderlyingCallsCount = newValue } } } } - var capturePropertiesCalled: Bool { - return capturePropertiesCallsCount > 0 + var capturePropertiesUserPropertiesCalled: Bool { + return capturePropertiesUserPropertiesCallsCount > 0 } - var capturePropertiesReceivedArguments: (event: String, properties: [String: Any]?)? - var capturePropertiesReceivedInvocations: [(event: String, properties: [String: Any]?)] = [] - var capturePropertiesClosure: ((String, [String: Any]?) -> Void)? + var capturePropertiesUserPropertiesReceivedArguments: (event: String, properties: [String: Any]?, userProperties: [String: Any]?)? + var capturePropertiesUserPropertiesReceivedInvocations: [(event: String, properties: [String: Any]?, userProperties: [String: Any]?)] = [] + var capturePropertiesUserPropertiesClosure: ((String, [String: Any]?, [String: Any]?) -> Void)? - func capture(_ event: String, properties: [String: Any]?) { - capturePropertiesCallsCount += 1 - capturePropertiesReceivedArguments = (event: event, properties: properties) - capturePropertiesReceivedInvocations.append((event: event, properties: properties)) - capturePropertiesClosure?(event, properties) + func capture(_ event: String, properties: [String: Any]?, userProperties: [String: Any]?) { + capturePropertiesUserPropertiesCallsCount += 1 + capturePropertiesUserPropertiesReceivedArguments = (event: event, properties: properties, userProperties: userProperties) + capturePropertiesUserPropertiesReceivedInvocations.append((event: event, properties: properties, userProperties: userProperties)) + capturePropertiesUserPropertiesClosure?(event, properties, userProperties) } //MARK: - screen diff --git a/ElementX/Sources/Services/Analytics/PHGPostHogProtocol.swift b/ElementX/Sources/Services/Analytics/PHGPostHogProtocol.swift index 5e4d77316..dbb0d120e 100644 --- a/ElementX/Sources/Services/Analytics/PHGPostHogProtocol.swift +++ b/ElementX/Sources/Services/Analytics/PHGPostHogProtocol.swift @@ -25,7 +25,7 @@ protocol PHGPostHogProtocol { func reset() - func capture(_ event: String, properties: [String: Any]?) + func capture(_ event: String, properties: [String: Any]?, userProperties: [String: Any]?) func screen(_ screenTitle: String, properties: [String: Any]?) } diff --git a/ElementX/Sources/Services/Analytics/PostHogAnalyticsClient.swift b/ElementX/Sources/Services/Analytics/PostHogAnalyticsClient.swift index 0fa3a0b1e..be7f9083f 100644 --- a/ElementX/Sources/Services/Analytics/PostHogAnalyticsClient.swift +++ b/ElementX/Sources/Services/Analytics/PostHogAnalyticsClient.swift @@ -69,12 +69,13 @@ class PostHogAnalyticsClient: AnalyticsClientProtocol { func capture(_ event: AnalyticsEventProtocol) { guard isRunning else { return } - postHog?.capture(event.eventName, properties: attachUserProperties(to: attachSuperProperties(to: event.properties))) + postHog?.capture(event.eventName, properties: attachSuperProperties(to: event.properties), userProperties: pendingUserProperties?.properties.compactMapValues { $0 }) + pendingUserProperties = nil } func screen(_ event: AnalyticsScreenProtocol) { guard isRunning else { return } - postHog?.screen(event.screenName.rawValue, properties: attachUserProperties(to: attachSuperProperties(to: event.properties))) + postHog?.screen(event.screenName.rawValue, properties: attachSuperProperties(to: event.properties)) } func updateUserProperties(_ userProperties: AnalyticsEvent.UserProperties) { @@ -106,21 +107,6 @@ class PostHogAnalyticsClient: AnalyticsClientProtocol { // MARK: - Private - /// Given a dictionary containing properties from an event, this method will return those properties - /// with any pending user properties included under the `$set` key. - /// - Parameter properties: A dictionary of properties from an event. - /// - Returns: The `properties` dictionary with any user properties included. - private func attachUserProperties(to properties: [String: Any]) -> [String: Any] { - guard isRunning, let userProperties = pendingUserProperties else { return properties } - - var properties = properties - - // As user properties overwrite old ones via $set, compactMap the dictionary to avoid resetting any missing properties - properties["$set"] = userProperties.properties.compactMapValues { $0 } - pendingUserProperties = nil - return properties - } - /// Attach super properties to events. /// If the property is already set on the event, the already set value will be kept. private func attachSuperProperties(to properties: [String: Any]) -> [String: Any] { diff --git a/UnitTests/Sources/AnalyticsTests.swift b/UnitTests/Sources/AnalyticsTests.swift index d35d02faf..1298df195 100644 --- a/UnitTests/Sources/AnalyticsTests.swift +++ b/UnitTests/Sources/AnalyticsTests.swift @@ -177,17 +177,35 @@ class AnalyticsTests: XCTestCase { func testSendingUserProperties() { // Given a client with user properties set - let client = PostHogAnalyticsClient() + + let client = PostHogAnalyticsClient(posthogFactory: MockPostHogFactory(mock: posthogMock)) + client.start(analyticsConfiguration: appSettings.analyticsConfiguration) + client.updateUserProperties(AnalyticsEvent.UserProperties(allChatsActiveFilter: nil, ftueUseCaseSelection: .PersonalMessaging, numFavouriteRooms: nil, numSpaces: nil)) - client.start(analyticsConfiguration: appSettings.analyticsConfiguration) XCTAssertNotNil(client.pendingUserProperties, "The user properties should be cached.") XCTAssertEqual(client.pendingUserProperties?.ftueUseCaseSelection, .PersonalMessaging, "The use case selection should match.") // When sending an event (tests run under Debug configuration so this is sent to the development instance) - client.screen(AnalyticsEvent.MobileScreen(durationMs: nil, screenName: .Home)) + let someEvent = AnalyticsEvent.Error(context: nil, + cryptoModule: .Rust, + cryptoSDK: .Rust, + domain: .E2EE, + eventLocalAgeMillis: nil, + isFederated: nil, + isMatrixDotOrg: nil, + name: .OlmKeysNotSentError, + timeToDecryptMillis: nil, + userTrustsOwnIdentity: nil, + wasVisibleToUser: nil) + client.capture(someEvent) + + let capturedEvent = posthogMock.capturePropertiesUserPropertiesReceivedArguments + + // The user properties should have been added + XCTAssertEqual(capturedEvent?.userProperties?["ftueUseCaseSelection"] as? String, AnalyticsEvent.UserProperties.FtueUseCaseSelection.PersonalMessaging.rawValue) // Then the properties should be cleared XCTAssertNil(client.pendingUserProperties, "The user properties should be cleared.") @@ -243,7 +261,7 @@ class AnalyticsTests: XCTestCase { wasVisibleToUser: nil) client.capture(someEvent) - let capturedEvent = posthogMock.capturePropertiesReceivedArguments + let capturedEvent = posthogMock.capturePropertiesUserPropertiesReceivedArguments // All the super properties should have been added XCTAssertEqual(capturedEvent?.properties?["cryptoSDK"] as? String, AnalyticsEvent.SuperProperties.CryptoSDK.Rust.rawValue) @@ -258,7 +276,7 @@ class AnalyticsTests: XCTestCase { ) client.capture(someEvent) - let capturedEvent2 = posthogMock.capturePropertiesReceivedArguments + let capturedEvent2 = posthogMock.capturePropertiesUserPropertiesReceivedArguments // All the super properties should have been added, with the one udpated XCTAssertEqual(capturedEvent2?.properties?["cryptoSDK"] as? String, AnalyticsEvent.SuperProperties.CryptoSDK.Rust.rawValue) @@ -290,13 +308,13 @@ class AnalyticsTests: XCTestCase { wasVisibleToUser: nil) client.capture(someEvent) - XCTAssertEqual(posthogMock.capturePropertiesCalled, false) + XCTAssertEqual(posthogMock.capturePropertiesUserPropertiesCalled, false) // start now client.start(analyticsConfiguration: appSettings.analyticsConfiguration) XCTAssertEqual(posthogMock.optInCalled, true) client.capture(someEvent) - XCTAssertEqual(posthogMock.capturePropertiesCalled, true) + XCTAssertEqual(posthogMock.capturePropertiesUserPropertiesCalled, true) } }