Skip to content

Add support for Verification state analytics #2806

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

Merged
merged 4 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions appnav/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
@@ -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

Check warning on line 37 in appnav/src/main/kotlin/io/element/android/appnav/loggedin/AnalyticsVerificationStateExt.kt

View check run for this annotation

Codecov / codecov/patch

appnav/src/main/kotlin/io/element/android/appnav/loggedin/AnalyticsVerificationStateExt.kt#L35-L37

Added lines #L35 - L37 were not covered by tests
// 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

Check warning on line 55 in appnav/src/main/kotlin/io/element/android/appnav/loggedin/AnalyticsVerificationStateExt.kt

View check run for this annotation

Codecov / codecov/patch

appnav/src/main/kotlin/io/element/android/appnav/loggedin/AnalyticsVerificationStateExt.kt#L53-L55

Added lines #L53 - L55 were not covered by tests
// we don't need to report transient states
else -> null
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,19 @@
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

Expand All @@ -38,6 +43,8 @@
private val networkMonitor: NetworkMonitor,
private val pushService: PushService,
private val sessionVerificationService: SessionVerificationService,
private val analyticsService: AnalyticsService,
private val encryptionService: EncryptionService,
) : Presenter<LoggedInState> {
@Composable
override fun present(): LoggedInState {
Expand All @@ -62,8 +69,34 @@
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

Check warning on line 90 in appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt

View check run for this annotation

Codecov / codecov/patch

appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt#L87-L90

Added lines #L87 - L90 were not covered by tests
)
)
}

// 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))

Check warning on line 99 in appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt

View check run for this annotation

Codecov / codecov/patch

appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt#L99

Added line #L99 was not covered by tests
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -73,6 +75,8 @@ class LoggedInPresenterTest {
networkMonitor = FakeNetworkMonitor(networkStatus),
pushService = FakePushService(),
sessionVerificationService = FakeSessionVerificationService(),
analyticsService = FakeAnalyticsService(),
encryptionService = FakeEncryptionService()
)
}
}
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 6 additions & 0 deletions services/analyticsproviders/posthog/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ class PosthogAnalyticsProvider @Inject constructor(
private var posthog: PostHogInterface? = null
private var analyticsId: String? = null

private var pendingUserProperties: MutableMap<String, Any>? = null

private val userPropertiesLock = Any()

override fun init() {
posthog = createPosthog()
posthog?.optIn()
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<PostHogInterface>().also {
every { it.optIn() } just runs
every { it.capture(any(), any(), any(), any(), any(), any()) } just runs
}
val mockPosthogFactory = mockk<PostHogFactory>()
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<VectorAnalyticsEvent>().also {
every {
it.getProperties()
} returns emptyMap()
every { it.getName() } returns "MockEventName"
}
analyticsProvider.capture(mockEvent)
val userPropertiesSlot = slot<Map<String, Any>>()

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<PostHogInterface>().also {
every { it.optIn() } just runs
every { it.capture(any(), any(), any(), any(), any(), any()) } just runs
}
val mockPosthogFactory = mockk<PostHogFactory>()
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<VectorAnalyticsEvent>().also {
every {
it.getProperties()
} returns emptyMap()
every { it.getName() } returns "MockEventName"
}
analyticsProvider.capture(mockEvent)
val userPropertiesSlot = slot<Map<String, Any>>()

verify { mockPosthog.capture(event = "MockEventName", any(), any(), userProperties = capture(userPropertiesSlot)) }

assertThat(userPropertiesSlot.captured).isNotNull()
assertThat(userPropertiesSlot.captured["verificationState"] as String).isEqualTo(testUserPropertiesUpdate.verificationState?.name)
}
}
Loading