From a761215e314e704e6d0a0c28c5f6e1f377f2c9ca Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 6 May 2024 19:06:54 +0200 Subject: [PATCH] Add support for Verification state analytics --- appnav/build.gradle.kts | 1 + .../loggedin/AnalyticsVerificationStateExt.kt | 59 +++++++++ .../appnav/loggedin/LoggedInPresenter.kt | 33 +++++ .../appnav/loggedin/LoggedInPresenterTest.kt | 4 + gradle/libs.versions.toml | 2 +- .../posthog/build.gradle.kts | 6 + .../posthog/PosthogAnalyticsProvider.kt | 33 +++-- .../posthog/PosthogAnalyticsProviderTest.kt | 116 ++++++++++++++++++ 8 files changed, 245 insertions(+), 9 deletions(-) create mode 100644 appnav/src/main/kotlin/io/element/android/appnav/loggedin/AnalyticsVerificationStateExt.kt create mode 100644 services/analyticsproviders/posthog/src/test/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProviderTest.kt diff --git a/appnav/build.gradle.kts b/appnav/build.gradle.kts index ab215f8394..c24e544240 100644 --- a/appnav/build.gradle.kts +++ b/appnav/build.gradle.kts @@ -72,6 +72,7 @@ dependencies { testImplementation(projects.features.rageshake.test) testImplementation(projects.features.rageshake.impl) testImplementation(projects.services.appnavstate.test) + testImplementation(projects.services.analytics.test) testImplementation(libs.test.appyx.junit) testImplementation(libs.test.arch.core) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/AnalyticsVerificationStateExt.kt b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/AnalyticsVerificationStateExt.kt new file mode 100644 index 0000000000..2afa0e40f2 --- /dev/null +++ b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/AnalyticsVerificationStateExt.kt @@ -0,0 +1,59 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.appnav.loggedin + +import im.vector.app.features.analytics.plan.CryptoSessionStateChange +import im.vector.app.features.analytics.plan.UserProperties +import io.element.android.libraries.matrix.api.encryption.RecoveryState +import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus + +fun SessionVerifiedStatus.toAnalyticsUserPropertyValue(): UserProperties.VerificationState? { + return when (this) { + // we don't need to report transient states + SessionVerifiedStatus.Unknown -> null + SessionVerifiedStatus.NotVerified -> UserProperties.VerificationState.NotVerified + SessionVerifiedStatus.Verified -> UserProperties.VerificationState.Verified + } +} + +fun RecoveryState.toAnalyticsUserPropertyValue(): UserProperties.RecoveryState? { + return when (this) { + RecoveryState.ENABLED -> UserProperties.RecoveryState.Enabled + RecoveryState.DISABLED -> UserProperties.RecoveryState.Disabled + RecoveryState.INCOMPLETE -> UserProperties.RecoveryState.Incomplete + // we don't need to report transient states + else -> null + } +} +fun SessionVerifiedStatus.toAnalyticsStateChangeValue(): CryptoSessionStateChange.VerificationState? { + return when (this) { + // we don't need to report transient states + SessionVerifiedStatus.Unknown -> null + SessionVerifiedStatus.NotVerified -> CryptoSessionStateChange.VerificationState.NotVerified + SessionVerifiedStatus.Verified -> CryptoSessionStateChange.VerificationState.Verified + } +} + +fun RecoveryState.toAnalyticsStateChangeValue(): CryptoSessionStateChange.RecoveryState? { + return when (this) { + RecoveryState.ENABLED -> CryptoSessionStateChange.RecoveryState.Enabled + RecoveryState.DISABLED -> CryptoSessionStateChange.RecoveryState.Disabled + RecoveryState.INCOMPLETE -> CryptoSessionStateChange.RecoveryState.Incomplete + // we don't need to report transient states + else -> null + } +} diff --git a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt index 7cb1d634b8..0feaf47029 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt @@ -22,14 +22,19 @@ import androidx.compose.runtime.collectAsState import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.remember +import im.vector.app.features.analytics.plan.CryptoSessionStateChange +import im.vector.app.features.analytics.plan.UserProperties import io.element.android.features.networkmonitor.api.NetworkMonitor import io.element.android.features.networkmonitor.api.NetworkStatus import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.encryption.EncryptionService +import io.element.android.libraries.matrix.api.encryption.RecoveryState import io.element.android.libraries.matrix.api.roomlist.RoomListService import io.element.android.libraries.matrix.api.verification.SessionVerificationService import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import io.element.android.libraries.push.api.PushService +import io.element.android.services.analytics.api.AnalyticsService import kotlinx.coroutines.flow.map import javax.inject.Inject @@ -38,6 +43,8 @@ class LoggedInPresenter @Inject constructor( private val networkMonitor: NetworkMonitor, private val pushService: PushService, private val sessionVerificationService: SessionVerificationService, + private val analyticsService: AnalyticsService, + private val encryptionService: EncryptionService, ) : Presenter { @Composable override fun present(): LoggedInState { @@ -62,8 +69,34 @@ class LoggedInPresenter @Inject constructor( networkStatus == NetworkStatus.Online && syncIndicator == RoomListService.SyncIndicator.Show } } + val verificationState by sessionVerificationService.sessionVerifiedStatus.collectAsState() + val recoveryState by encryptionService.recoveryStateStateFlow.collectAsState() + reportCryptoStatusToAnalytics(verificationState, recoveryState) + return LoggedInState( showSyncSpinner = showSyncSpinner, ) } + + private fun reportCryptoStatusToAnalytics(verificationState: SessionVerifiedStatus, recoveryState: RecoveryState) { + // Update first the user property, to store the current status for that posthog user + val userVerificationState = verificationState.toAnalyticsUserPropertyValue() + val userRecoveryState = recoveryState.toAnalyticsUserPropertyValue() + if (userRecoveryState != null && userVerificationState != null) { + // we want to report when both value are known (if one is unknown we wait until we have them both) + analyticsService.updateUserProperties( + UserProperties( + verificationState = userVerificationState, + recoveryState = userRecoveryState + ) + ) + } + + // Also report when there is a change in the state, to be able to track the changes + val changeVerificationState = verificationState.toAnalyticsStateChangeValue() + val changeRecoveryState = recoveryState.toAnalyticsStateChangeValue() + if (changeVerificationState != null && changeRecoveryState != null) { + analyticsService.capture(CryptoSessionStateChange(changeRecoveryState, changeVerificationState)) + } + } } diff --git a/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt index df17053512..d1b89fca77 100644 --- a/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt +++ b/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt @@ -24,9 +24,11 @@ import io.element.android.features.networkmonitor.api.NetworkStatus import io.element.android.features.networkmonitor.test.FakeNetworkMonitor import io.element.android.libraries.matrix.api.roomlist.RoomListService import io.element.android.libraries.matrix.test.FakeMatrixClient +import io.element.android.libraries.matrix.test.encryption.FakeEncryptionService import io.element.android.libraries.matrix.test.roomlist.FakeRoomListService import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService import io.element.android.libraries.push.test.FakePushService +import io.element.android.services.analytics.test.FakeAnalyticsService import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.consumeItemsUntilPredicate import kotlinx.coroutines.test.runTest @@ -73,6 +75,8 @@ class LoggedInPresenterTest { networkMonitor = FakeNetworkMonitor(networkStatus), pushService = FakePushService(), sessionVerificationService = FakeSessionVerificationService(), + analyticsService = FakeAnalyticsService(), + encryptionService = FakeEncryptionService() ) } } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index db8cfe6b6a..1d5ee6c65b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -183,7 +183,7 @@ kotlinpoet = "com.squareup:kotlinpoet:1.16.0" posthog = "com.posthog:posthog-android:3.1.18" sentry = "io.sentry:sentry-android:7.8.0" # main branch can be tested replacing the version with main-SNAPSHOT -matrix_analytics_events = "com.github.matrix-org:matrix-analytics-events:0.20.0" +matrix_analytics_events = "com.github.matrix-org:matrix-analytics-events:0.21.0" # Emojibase matrix_emojibase_bindings = "io.element.android:emojibase-bindings:1.1.3" diff --git a/services/analyticsproviders/posthog/build.gradle.kts b/services/analyticsproviders/posthog/build.gradle.kts index c9049af237..8a923d4918 100644 --- a/services/analyticsproviders/posthog/build.gradle.kts +++ b/services/analyticsproviders/posthog/build.gradle.kts @@ -34,4 +34,10 @@ dependencies { implementation(projects.libraries.core) implementation(projects.libraries.di) implementation(projects.services.analyticsproviders.api) + + testImplementation(libs.coroutines.test) + testImplementation(libs.test.truth) + testImplementation(libs.test.junit) + testImplementation(projects.tests.testutils) + testImplementation(libs.test.mockk) } diff --git a/services/analyticsproviders/posthog/src/main/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProvider.kt b/services/analyticsproviders/posthog/src/main/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProvider.kt index fb542b7f28..b594dcbee4 100644 --- a/services/analyticsproviders/posthog/src/main/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProvider.kt +++ b/services/analyticsproviders/posthog/src/main/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProvider.kt @@ -40,6 +40,10 @@ class PosthogAnalyticsProvider @Inject constructor( private var posthog: PostHogInterface? = null private var analyticsId: String? = null + private var pendingUserProperties: MutableMap? = null + + private val userPropertiesLock = Any() + override fun init() { posthog = createPosthog() posthog?.optIn() @@ -57,10 +61,14 @@ class PosthogAnalyticsProvider @Inject constructor( } override fun capture(event: VectorAnalyticsEvent) { - posthog?.capture( - event = event.getName(), - properties = event.getProperties()?.keepOnlyNonNullValues().withExtraProperties(), - ) + synchronized(userPropertiesLock) { + posthog?.capture( + event = event.getName(), + properties = event.getProperties()?.keepOnlyNonNullValues().withExtraProperties(), + userProperties = pendingUserProperties, + ) + pendingUserProperties = null + } } override fun screen(screen: VectorAnalyticsScreen) { @@ -71,10 +79,19 @@ class PosthogAnalyticsProvider @Inject constructor( } override fun updateUserProperties(userProperties: UserProperties) { -// posthog?.identify( -// REUSE_EXISTING_ID, userProperties.getProperties()?.toPostHogUserProperties(), -// IGNORED_OPTIONS -// ) + synchronized(userPropertiesLock) { + // The pending properties will be sent with the following capture call + if (pendingUserProperties == null) { + pendingUserProperties = HashMap() + } + userProperties.getProperties()?.let { + pendingUserProperties?.putAll(it) + } + // We are not currently using `identify` in EIX, if it was the case + // we could have called identify to update the user properties. + // For now, we have to store them, and they will be updated when the next call + // to capture will happen. + } } override fun trackError(throwable: Throwable) { diff --git a/services/analyticsproviders/posthog/src/test/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProviderTest.kt b/services/analyticsproviders/posthog/src/test/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProviderTest.kt new file mode 100644 index 0000000000..9032ea5718 --- /dev/null +++ b/services/analyticsproviders/posthog/src/test/kotlin/io/element/android/services/analyticsproviders/posthog/PosthogAnalyticsProviderTest.kt @@ -0,0 +1,116 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.services.analyticsproviders.posthog + +import com.google.common.truth.Truth.assertThat +import com.posthog.PostHogInterface +import im.vector.app.features.analytics.itf.VectorAnalyticsEvent +import im.vector.app.features.analytics.plan.UserProperties +import io.element.android.tests.testutils.WarmUpRule +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.runs +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.junit.Rule +import org.junit.Test + +class PosthogAnalyticsProviderTest { + @get:Rule + val warmUpRule = WarmUpRule() + + @Test + fun `Posthog - Test user properties`() = runTest { + val mockPosthog = mockk().also { + every { it.optIn() } just runs + every { it.capture(any(), any(), any(), any(), any(), any()) } just runs + } + val mockPosthogFactory = mockk() + every { mockPosthogFactory.createPosthog() } returns mockPosthog + + val analyticsProvider = PosthogAnalyticsProvider(mockPosthogFactory) + analyticsProvider.init() + + val testUserProperties = UserProperties( + verificationState = UserProperties.VerificationState.Verified, + recoveryState = UserProperties.RecoveryState.Incomplete, + ) + analyticsProvider.updateUserProperties(testUserProperties) + + // report mock event + val mockEvent = mockk().also { + every { + it.getProperties() + } returns emptyMap() + every { it.getName() } returns "MockEventName" + } + analyticsProvider.capture(mockEvent) + val userPropertiesSlot = slot>() + + verify { mockPosthog.capture(event = "MockEventName", any(), any(), userProperties = capture(userPropertiesSlot)) } + + assertThat(userPropertiesSlot.captured).isNotNull() + assertThat(userPropertiesSlot.captured["verificationState"] as String).isEqualTo(testUserProperties.verificationState?.name) + assertThat(userPropertiesSlot.captured["recoveryState"] as String).isEqualTo(testUserProperties.recoveryState?.name) + + // Should only be reported once when the next event is sent + // Try another capture to check + analyticsProvider.capture(mockEvent) + verify { mockPosthog.capture(any(), any(), any(), userProperties = null) } + } + + @Test + fun `Posthog - Test accumulate user properties until next capture call`() = runTest { + val mockPosthog = mockk().also { + every { it.optIn() } just runs + every { it.capture(any(), any(), any(), any(), any(), any()) } just runs + } + val mockPosthogFactory = mockk() + every { mockPosthogFactory.createPosthog() } returns mockPosthog + + val analyticsProvider = PosthogAnalyticsProvider(mockPosthogFactory) + analyticsProvider.init() + + val testUserProperties = UserProperties( + verificationState = UserProperties.VerificationState.NotVerified, + ) + analyticsProvider.updateUserProperties(testUserProperties) + + // Update again + val testUserPropertiesUpdate = UserProperties( + verificationState = UserProperties.VerificationState.Verified, + ) + analyticsProvider.updateUserProperties(testUserPropertiesUpdate) + + // report mock event + val mockEvent = mockk().also { + every { + it.getProperties() + } returns emptyMap() + every { it.getName() } returns "MockEventName" + } + analyticsProvider.capture(mockEvent) + val userPropertiesSlot = slot>() + + verify { mockPosthog.capture(event = "MockEventName", any(), any(), userProperties = capture(userPropertiesSlot)) } + + assertThat(userPropertiesSlot.captured).isNotNull() + assertThat(userPropertiesSlot.captured["verificationState"] as String).isEqualTo(testUserPropertiesUpdate.verificationState?.name) + } +}