From 5de386c3c9a9ddb0249a3cad51590aa4a53f5457 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 30 Jan 2023 10:49:36 +0100 Subject: [PATCH 1/4] Ensure we never call `posthog.identify` if user did not consent, because it sends request `/decide/?v=2` to the analytic server. --- .../analytics/impl/DefaultVectorAnalytics.kt | 35 ++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt b/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt index ca7608166c..20712c81b6 100644 --- a/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt +++ b/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt @@ -31,6 +31,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import org.matrix.android.sdk.api.extensions.orFalse import timber.log.Timber import javax.inject.Inject import javax.inject.Singleton @@ -60,6 +61,9 @@ class DefaultVectorAnalytics @Inject constructor( private var userConsent: Boolean? = null private var analyticsId: String? = null + // Cache for the properties to send + private var pendingUserProperties: UserProperties? = null + override fun init() { observeUserConsent() observeAnalyticsId() @@ -112,6 +116,7 @@ class DefaultVectorAnalytics @Inject constructor( private suspend fun identifyPostHog() { val id = analyticsId ?: return + if (!userConsent.orFalse()) return if (id.isEmpty()) { Timber.tag(analyticsTag.value).d("reset") posthog?.reset() @@ -126,7 +131,7 @@ class DefaultVectorAnalytics @Inject constructor( .onEach { consent -> Timber.tag(analyticsTag.value).d("User consent updated to $consent") userConsent = consent - optOutPostHog() + initOrStopPostHog() initOrStopSentry() } .launchIn(globalScope) @@ -141,8 +146,20 @@ class DefaultVectorAnalytics @Inject constructor( } } - private fun optOutPostHog() { - userConsent?.let { posthog?.optOut(!it) } + private suspend fun initOrStopPostHog() { + userConsent?.let { _userConsent -> + when (_userConsent) { + true -> { + posthog?.optOut(false) + identifyPostHog() + pendingUserProperties?.let { doUpdateUserProperties(it) } + pendingUserProperties = null + } + false -> { + posthog?.optOut(true) + } + } + } } override fun capture(event: VectorAnalyticsEvent) { @@ -160,7 +177,17 @@ class DefaultVectorAnalytics @Inject constructor( } override fun updateUserProperties(userProperties: UserProperties) { - posthog?.identify(REUSE_EXISTING_ID, userProperties.getProperties()?.toPostHogUserProperties(), IGNORED_OPTIONS) + if (userConsent == true) { + doUpdateUserProperties(userProperties) + } else { + pendingUserProperties = userProperties + } + } + + private fun doUpdateUserProperties(userProperties: UserProperties) { + posthog + ?.takeIf { userConsent == true } + ?.identify(REUSE_EXISTING_ID, userProperties.getProperties()?.toPostHogUserProperties(), IGNORED_OPTIONS) } private fun Map?.toPostHogProperties(): Properties? { From 7bef90109d994ebf723cd531f98bca36029fa17e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 30 Jan 2023 12:22:05 +0100 Subject: [PATCH 2/4] Changelog --- changelog.d/8031.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8031.bugfix diff --git a/changelog.d/8031.bugfix b/changelog.d/8031.bugfix new file mode 100644 index 0000000000..0e7ff28509 --- /dev/null +++ b/changelog.d/8031.bugfix @@ -0,0 +1 @@ +Do not send any request to Posthog if no consent is provided. From c8277e2d4359db9498a53d8a84cdd192900318ca Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 30 Jan 2023 12:22:31 +0100 Subject: [PATCH 3/4] Posthog: flush queue before optin out. --- .../app/features/analytics/impl/DefaultVectorAnalytics.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt b/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt index 20712c81b6..917c3468c6 100644 --- a/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt +++ b/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt @@ -156,6 +156,8 @@ class DefaultVectorAnalytics @Inject constructor( pendingUserProperties = null } false -> { + // When opting out, ensure that the queue is flushed first, or it will be flushed later (after user has revoked consent) + posthog?.flush() posthog?.optOut(true) } } From 307ac4060e86d801a59fefca224d00806a8b0fee Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 30 Jan 2023 14:17:16 +0100 Subject: [PATCH 4/4] Posthog: fix test. User consent must be provided to touch Posthog API. --- .../app/features/analytics/impl/DefaultVectorAnalyticsTest.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vector/src/test/java/im/vector/app/features/analytics/impl/DefaultVectorAnalyticsTest.kt b/vector/src/test/java/im/vector/app/features/analytics/impl/DefaultVectorAnalyticsTest.kt index 3fd0528a19..084ee2410e 100644 --- a/vector/src/test/java/im/vector/app/features/analytics/impl/DefaultVectorAnalyticsTest.kt +++ b/vector/src/test/java/im/vector/app/features/analytics/impl/DefaultVectorAnalyticsTest.kt @@ -97,6 +97,7 @@ class DefaultVectorAnalyticsTest { @Test fun `given lateinit user properties when valid analytics id updates then identify with lateinit properties`() = runTest { fakeLateInitUserPropertiesFactory.givenCreatesProperties(A_LATE_INIT_USER_PROPERTIES) + fakeAnalyticsStore.givenUserContent(true) fakeAnalyticsStore.givenAnalyticsId(AN_ANALYTICS_ID) @@ -106,6 +107,7 @@ class DefaultVectorAnalyticsTest { @Test fun `when signing out then resets posthog and closes Sentry`() = runTest { fakeAnalyticsStore.allowSettingAnalyticsIdToCallBackingFlow() + fakeAnalyticsStore.givenUserContent(true) defaultVectorAnalytics.onSignOut()