From a5e6feb7e0a376f36c6b55986dea941188a8b5e1 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 12 Nov 2024 08:36:52 +0100 Subject: [PATCH 1/6] fix: allow changing person properties after identify --- CHANGELOG.md | 2 ++ posthog/src/main/java/com/posthog/PostHog.kt | 20 ++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 608067cf..c5af48d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Next +- fix: allow changing person properties after identify ([#204](https://github.com/PostHog/posthog-android/pull/204)) + ## 3.9.1 - 2024-11-11 - recording: fix observation on multiple threads in layout/draw is not supported for compose ([#204](https://github.com/PostHog/posthog-android/pull/204)) diff --git a/posthog/src/main/java/com/posthog/PostHog.kt b/posthog/src/main/java/com/posthog/PostHog.kt index 1ecc29ee..e2404aef 100644 --- a/posthog/src/main/java/com/posthog/PostHog.kt +++ b/posthog/src/main/java/com/posthog/PostHog.kt @@ -556,6 +556,8 @@ public class PostHog private constructor( config?.logger?.log("identify called with invalid anonymousId: $anonymousId.") } + var shouldReloadFlags = false + if (previousDistinctId != distinctId && !isIdentified) { // this has to be set before capture since this flag will be read during the event // capture @@ -578,14 +580,24 @@ public class PostHog private constructor( config?.logger?.log("identify called with invalid former distinctId: $previousDistinctId.") } this.distinctId = distinctId + shouldReloadFlags = true + } else if (userProperties?.isNotEmpty() == true || userPropertiesSetOnce?.isNotEmpty() == true) { + capture( + "\$set", + distinctId = distinctId, + userProperties = userProperties, + userPropertiesSetOnce = userPropertiesSetOnce, + ) - // only because of testing in isolation, this flag is always enabled - if (reloadFeatureFlags) { - reloadFeatureFlags() - } + shouldReloadFlags = true } else { config?.logger?.log("already identified with id: $distinctId.") } + + // only because of testing in isolation, this flag(reloadFeatureFlags) is always enabled + if (shouldReloadFlags && reloadFeatureFlags) { + reloadFeatureFlags() + } } private fun hasPersonProcessing(): Boolean { From 0a6ad6b63d8ee5a73410d9090b7beef86f8b99bd Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 12 Nov 2024 08:39:51 +0100 Subject: [PATCH 2/6] add note --- posthog/src/main/java/com/posthog/PostHog.kt | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/posthog/src/main/java/com/posthog/PostHog.kt b/posthog/src/main/java/com/posthog/PostHog.kt index e2404aef..aed23dec 100644 --- a/posthog/src/main/java/com/posthog/PostHog.kt +++ b/posthog/src/main/java/com/posthog/PostHog.kt @@ -556,8 +556,6 @@ public class PostHog private constructor( config?.logger?.log("identify called with invalid anonymousId: $anonymousId.") } - var shouldReloadFlags = false - if (previousDistinctId != distinctId && !isIdentified) { // this has to be set before capture since this flag will be read during the event // capture @@ -580,7 +578,11 @@ public class PostHog private constructor( config?.logger?.log("identify called with invalid former distinctId: $previousDistinctId.") } this.distinctId = distinctId - shouldReloadFlags = true + + // only because of testing in isolation, this flag is always enabled + if (reloadFeatureFlags) { + reloadFeatureFlags() + } } else if (userProperties?.isNotEmpty() == true || userPropertiesSetOnce?.isNotEmpty() == true) { capture( "\$set", @@ -588,16 +590,10 @@ public class PostHog private constructor( userProperties = userProperties, userPropertiesSetOnce = userPropertiesSetOnce, ) - - shouldReloadFlags = true + // Note we don't reload flags on property changes as these get processed async } else { config?.logger?.log("already identified with id: $distinctId.") } - - // only because of testing in isolation, this flag(reloadFeatureFlags) is always enabled - if (shouldReloadFlags && reloadFeatureFlags) { - reloadFeatureFlags() - } } private fun hasPersonProcessing(): Boolean { From 6f656bc6c6a1aecd4e6a2395769dca83ac1cbae2 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 12 Nov 2024 08:40:23 +0100 Subject: [PATCH 3/6] fix --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5af48d0..d7cd4932 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## Next -- fix: allow changing person properties after identify ([#204](https://github.com/PostHog/posthog-android/pull/204)) +- fix: allow changing person properties after identify ([#205](https://github.com/PostHog/posthog-android/pull/205)) ## 3.9.1 - 2024-11-11 From ef2e552e9318ba9c4a0ae17bdfe8789986871578 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 12 Nov 2024 12:15:29 +0100 Subject: [PATCH 4/6] add test --- posthog/src/main/java/com/posthog/PostHog.kt | 7 ++-- .../src/test/java/com/posthog/PostHogTest.kt | 33 ++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/posthog/src/main/java/com/posthog/PostHog.kt b/posthog/src/main/java/com/posthog/PostHog.kt index aed23dec..a554d65c 100644 --- a/posthog/src/main/java/com/posthog/PostHog.kt +++ b/posthog/src/main/java/com/posthog/PostHog.kt @@ -556,7 +556,8 @@ public class PostHog private constructor( config?.logger?.log("identify called with invalid anonymousId: $anonymousId.") } - if (previousDistinctId != distinctId && !isIdentified) { + val hasDifferentDistinctId = previousDistinctId != distinctId + if (hasDifferentDistinctId && !isIdentified) { // this has to be set before capture since this flag will be read during the event // capture synchronized(identifiedLock) { @@ -583,7 +584,9 @@ public class PostHog private constructor( if (reloadFeatureFlags) { reloadFeatureFlags() } - } else if (userProperties?.isNotEmpty() == true || userPropertiesSetOnce?.isNotEmpty() == true) { + // we need to make sure the user props update is for the same user + // otherwise they have to reset and identify again + } else if (!hasDifferentDistinctId && (userProperties?.isNotEmpty() == true || userPropertiesSetOnce?.isNotEmpty() == true)) { capture( "\$set", distinctId = distinctId, diff --git a/posthog/src/test/java/com/posthog/PostHogTest.kt b/posthog/src/test/java/com/posthog/PostHogTest.kt index 5db8741d..9fc0fe6a 100644 --- a/posthog/src/test/java/com/posthog/PostHogTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogTest.kt @@ -574,13 +574,44 @@ internal class PostHogTest { sut.identify( "anotherDistinctId", + ) + + queueExecutor.shutdownAndAwaitTermination() + + assertEquals(1, http.requestCount) + + sut.close() + } + + @Test + fun `captures a set event if identified`() { + val http = mockHttp() + val url = http.url("/") + + val sut = getSut(url.toString(), preloadFeatureFlags = false, reloadFeatureFlags = false, flushAt = 2) + + sut.identify( + DISTINCT_ID, + userProperties = userProps, + userPropertiesSetOnce = userPropsOnce, + ) + + sut.identify( + DISTINCT_ID, userProperties = userProps, userPropertiesSetOnce = userPropsOnce, ) queueExecutor.shutdownAndAwaitTermination() - assertEquals(1, http.requestCount) + val request = http.takeRequest() + + val content = request.body.unGzip() + val batch = serializer.deserialize(content.reader()) + + val theEvent = batch.batch.last() + + assertEquals("\$set", theEvent.event) sut.close() } From 683fad2747d031b9619694ab30f8a5b91e812fcc Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 12 Nov 2024 12:18:42 +0100 Subject: [PATCH 5/6] fix --- posthog/src/test/java/com/posthog/PostHogTest.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/posthog/src/test/java/com/posthog/PostHogTest.kt b/posthog/src/test/java/com/posthog/PostHogTest.kt index 9fc0fe6a..0ffc11d8 100644 --- a/posthog/src/test/java/com/posthog/PostHogTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogTest.kt @@ -596,6 +596,9 @@ internal class PostHogTest { userPropertiesSetOnce = userPropsOnce, ) + val userProps = mapOf("user1" to "theResult") + val userPropsOnce = mapOf("logged" to false) + sut.identify( DISTINCT_ID, userProperties = userProps, @@ -612,6 +615,8 @@ internal class PostHogTest { val theEvent = batch.batch.last() assertEquals("\$set", theEvent.event) + assertEquals(userProps, theEvent.properties!!["\$set"]) + assertEquals(userPropsOnce, theEvent.properties!!["\$set_once"]) sut.close() } From e9117dfe09e27d9b7ee4ddfd47516e70bfa8314d Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 12 Nov 2024 13:29:11 +0100 Subject: [PATCH 6/6] add new test --- .../src/test/java/com/posthog/PostHogTest.kt | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/posthog/src/test/java/com/posthog/PostHogTest.kt b/posthog/src/test/java/com/posthog/PostHogTest.kt index 0ffc11d8..af534383 100644 --- a/posthog/src/test/java/com/posthog/PostHogTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogTest.kt @@ -621,6 +621,45 @@ internal class PostHogTest { sut.close() } + @Test + fun `does not capture a set event if different user`() { + val http = mockHttp() + val url = http.url("/") + + val sut = getSut(url.toString(), preloadFeatureFlags = false, reloadFeatureFlags = false) + + sut.identify( + DISTINCT_ID, + userProperties = userProps, + userPropertiesSetOnce = userPropsOnce, + ) + + val userProps = mapOf("user1" to "theResult") + val userPropsOnce = mapOf("logged" to false) + + sut.identify( + "different user", + userProperties = userProps, + userPropertiesSetOnce = userPropsOnce, + ) + + queueExecutor.shutdownAndAwaitTermination() + + queueExecutor.shutdownAndAwaitTermination() + + val request = http.takeRequest() + + val content = request.body.unGzip() + val batch = serializer.deserialize(content.reader()) + + val theEvent = batch.batch.last() + + assertEquals(1, batch.batch.size) + assertEquals("\$identify", theEvent.event) + + sut.close() + } + @Test fun `captures an identify event post reset`() { val http = mockHttp()