From ddf8c36d197bf43dcefb2e5c76a659a259863785 Mon Sep 17 00:00:00 2001 From: Ioannis J Date: Tue, 12 Nov 2024 09:51:53 +0200 Subject: [PATCH 1/9] fix: allow set person properties after identify --- PostHog/PostHogSDK.swift | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/PostHog/PostHogSDK.swift b/PostHog/PostHogSDK.swift index 7c2df7509..6092cd700 100644 --- a/PostHog/PostHogSDK.swift +++ b/PostHog/PostHogSDK.swift @@ -468,7 +468,10 @@ let maxRetryDelay = 30.0 let isIdentified = storageManager.isIdentified() - if distinctId != oldDistinctId, !isIdentified { + let hasDistinctIdChanges = distinctId != oldDistinctId && !isIdentified + let hasUserPropertyChanges = !(userProperties?.isEmpty ?? true) || !(userPropertiesSetOnce?.isEmpty ?? true) + + if hasDistinctIdChanges { // We keep the AnonymousId to be used by decide calls and identify to link the previousId storageManager.setAnonymousId(oldDistinctId) storageManager.setDistinctId(distinctId) @@ -490,6 +493,11 @@ let maxRetryDelay = 30.0 if shouldReloadFlagsForTesting { reloadFeatureFlags() } + } else if hasUserPropertyChanges { + capture("$set", + distinctId: distinctId, + userProperties: userProperties, + userPropertiesSetOnce: userPropertiesSetOnce) } else { hedgeLog("already identified with id: \(oldDistinctId)") } From b6b6f7ebec8f2e2f2abd877bfabf29b0837d28c6 Mon Sep 17 00:00:00 2001 From: Ioannis J Date: Tue, 12 Nov 2024 10:00:55 +0200 Subject: [PATCH 2/9] chore: update CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56a197e3a..b668e7bc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Next +- fix: allow changing person properties after identify ([#249](https://github.com/PostHog/posthog-ios/pull/249)) + ## 3.15.0 - 2024-11-11 - add autocapture support for UIKit ([#224](https://github.com/PostHog/posthog-ios/pull/224)) From 55790341a56c6268d6dcd5c7bc4fcad4d4b2e6fa Mon Sep 17 00:00:00 2001 From: Ioannis J Date: Tue, 12 Nov 2024 11:41:44 +0200 Subject: [PATCH 3/9] add note Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- PostHog/PostHogSDK.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PostHog/PostHogSDK.swift b/PostHog/PostHogSDK.swift index 6092cd700..1e483754c 100644 --- a/PostHog/PostHogSDK.swift +++ b/PostHog/PostHogSDK.swift @@ -498,6 +498,8 @@ let maxRetryDelay = 30.0 distinctId: distinctId, userProperties: userProperties, userPropertiesSetOnce: userPropertiesSetOnce) + + // Note we don't reload flags on property changes as these get processed async } else { hedgeLog("already identified with id: \(oldDistinctId)") } From d3378deb3529a55a7c9ecc6d47b664941ce7b47d Mon Sep 17 00:00:00 2001 From: Ioannis J Date: Tue, 12 Nov 2024 11:42:58 +0200 Subject: [PATCH 4/9] fix: checks --- PostHog/PostHogSDK.swift | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/PostHog/PostHogSDK.swift b/PostHog/PostHogSDK.swift index 1e483754c..1dadcf4c9 100644 --- a/PostHog/PostHogSDK.swift +++ b/PostHog/PostHogSDK.swift @@ -468,10 +468,7 @@ let maxRetryDelay = 30.0 let isIdentified = storageManager.isIdentified() - let hasDistinctIdChanges = distinctId != oldDistinctId && !isIdentified - let hasUserPropertyChanges = !(userProperties?.isEmpty ?? true) || !(userPropertiesSetOnce?.isEmpty ?? true) - - if hasDistinctIdChanges { + if distinctId != oldDistinctId && !isIdentified { // We keep the AnonymousId to be used by decide calls and identify to link the previousId storageManager.setAnonymousId(oldDistinctId) storageManager.setDistinctId(distinctId) @@ -493,7 +490,7 @@ let maxRetryDelay = 30.0 if shouldReloadFlagsForTesting { reloadFeatureFlags() } - } else if hasUserPropertyChanges { + } else if !(userProperties?.isEmpty ?? true) || !(userPropertiesSetOnce?.isEmpty ?? true) { capture("$set", distinctId: distinctId, userProperties: userProperties, From 31104871e130f803ee664274b5a10029006b9912 Mon Sep 17 00:00:00 2001 From: Ioannis J Date: Tue, 12 Nov 2024 11:45:48 +0200 Subject: [PATCH 5/9] fix: lint --- PostHog/PostHogSDK.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PostHog/PostHogSDK.swift b/PostHog/PostHogSDK.swift index 1dadcf4c9..bb4f7ce73 100644 --- a/PostHog/PostHogSDK.swift +++ b/PostHog/PostHogSDK.swift @@ -495,7 +495,7 @@ let maxRetryDelay = 30.0 distinctId: distinctId, userProperties: userProperties, userPropertiesSetOnce: userPropertiesSetOnce) - + // Note we don't reload flags on property changes as these get processed async } else { hedgeLog("already identified with id: \(oldDistinctId)") From fedc1ab57be56b74967549a95a6ee58ebdb4bf85 Mon Sep 17 00:00:00 2001 From: Ioannis J Date: Tue, 12 Nov 2024 11:58:56 +0200 Subject: [PATCH 6/9] feat: unit test --- PostHogTests/PostHogSDKTest.swift | 66 +++++++++++++++++++++++++------ 1 file changed, 55 insertions(+), 11 deletions(-) diff --git a/PostHogTests/PostHogSDKTest.swift b/PostHogTests/PostHogSDKTest.swift index b06fd768d..7acd42a35 100644 --- a/PostHogTests/PostHogSDKTest.swift +++ b/PostHogTests/PostHogSDKTest.swift @@ -156,7 +156,43 @@ class PostHogSDKTest: QuickSpec { } it("does not capture identify event if already identified") { - let sut = self.getSut() + let sut = self.getSut( + flushAt: 2 + ) + + sut.identify("distinctId", + userProperties: ["userProp": "value"], + userPropertiesSetOnce: ["userPropOnce": "value"]) + + sut.identify("distinctId") + sut.capture("satisfy_queue") + + let events = getBatchedEvents(server) + + expect(events.count) == 2 + + expect(events[0].event) == "$identify" + expect(events[1].event) == "satisfy_queue" + + expect(events[0].distinctId) == "distinctId" + let anonId = sut.getAnonymousId() + expect(events[0].properties["$anon_distinct_id"] as? String) == anonId + expect(events[0].properties["$is_identified"] as? Bool) == true + + let set = events[0].properties["$set"] as? [String: Any] ?? [:] + expect(set["userProp"] as? String) == "value" + + let setOnce = events[0].properties["$set_once"] as? [String: Any] ?? [:] + expect(setOnce["userPropOnce"] as? String) == "value" + + sut.reset() + sut.close() + } + + it("capture an identify event if already identified but user properties are set") { + let sut = self.getSut( + flushAt: 2 + ) sut.identify("distinctId", userProperties: ["userProp": "value"], @@ -168,21 +204,29 @@ class PostHogSDKTest: QuickSpec { let events = getBatchedEvents(server) - expect(events.count) == 1 + expect(events.count) == 2 - let event = events.first! - expect(event.event) == "$identify" + expect(events[0].event) == "$identify" + expect(events[1].event) == "$set" + + expect(events[0].distinctId) == "distinctId" + expect(events[1].distinctId) == events[0].distinctId - expect(event.distinctId) == "distinctId" let anonId = sut.getAnonymousId() - expect(event.properties["$anon_distinct_id"] as? String) == anonId - expect(event.properties["$is_identified"] as? Bool) == true + expect(events[0].properties["$anon_distinct_id"] as? String) == anonId + expect(events[0].properties["$is_identified"] as? Bool) == true - let set = event.properties["$set"] as? [String: Any] ?? [:] - expect(set["userProp"] as? String) == "value" + let set0 = events[0].properties["$set"] as? [String: Any] ?? [:] + expect(set0["userProp"] as? String) == "value" - let setOnce = event.properties["$set_once"] as? [String: Any] ?? [:] - expect(setOnce["userPropOnce"] as? String) == "value" + let set1 = events[1].properties["$set"] as? [String: Any] ?? [:] + expect(set1["userProp2"] as? String) == "value2" + + let setOnce0 = events[0].properties["$set_once"] as? [String: Any] ?? [:] + expect(setOnce0["userPropOnce"] as? String) == "value" + + let setOnce1 = events[1].properties["$set_once"] as? [String: Any] ?? [:] + expect(setOnce1["userPropOnce2"] as? String) == "value2" sut.reset() sut.close() From 37a356f336973f0b3aad9f0385188b8b8f8d035c Mon Sep 17 00:00:00 2001 From: Ioannis J Date: Tue, 12 Nov 2024 13:44:42 +0200 Subject: [PATCH 7/9] fix: check if distinctid has changed for identified users --- PostHog/PostHogSDK.swift | 59 +++++++++++++++++-------------- PostHogTests/PostHogSDKTest.swift | 39 +++++++++++++++++++- 2 files changed, 71 insertions(+), 27 deletions(-) diff --git a/PostHog/PostHogSDK.swift b/PostHog/PostHogSDK.swift index bb4f7ce73..25df23ede 100644 --- a/PostHog/PostHogSDK.swift +++ b/PostHog/PostHogSDK.swift @@ -468,37 +468,44 @@ let maxRetryDelay = 30.0 let isIdentified = storageManager.isIdentified() - if distinctId != oldDistinctId && !isIdentified { - // We keep the AnonymousId to be used by decide calls and identify to link the previousId - storageManager.setAnonymousId(oldDistinctId) - storageManager.setDistinctId(distinctId) - - storageManager.setIdentified(true) - - let properties = buildProperties(distinctId: distinctId, properties: [ - "distinct_id": distinctId, - "$anon_distinct_id": oldDistinctId, - ], userProperties: sanitizeDicionary(userProperties), userPropertiesSetOnce: sanitizeDicionary(userPropertiesSetOnce)) - let sanitizedProperties = sanitizeProperties(properties) + // if identified -> distinctId == oldDistinctId + // if !identified -> distinctId != oldDistinctId + if (distinctId == oldDistinctId) == isIdentified { + if !isIdentified { + // We keep the AnonymousId to be used by decide calls and identify to link the previousId + storageManager.setAnonymousId(oldDistinctId) + storageManager.setDistinctId(distinctId) + + storageManager.setIdentified(true) + + let properties = buildProperties(distinctId: distinctId, properties: [ + "distinct_id": distinctId, + "$anon_distinct_id": oldDistinctId, + ], userProperties: sanitizeDicionary(userProperties), userPropertiesSetOnce: sanitizeDicionary(userPropertiesSetOnce)) + let sanitizedProperties = sanitizeProperties(properties) + + queue.add(PostHogEvent( + event: "$identify", + distinctId: distinctId, + properties: sanitizedProperties + )) - queue.add(PostHogEvent( - event: "$identify", - distinctId: distinctId, - properties: sanitizedProperties - )) + if shouldReloadFlagsForTesting { + reloadFeatureFlags() + } + } else if !(userProperties?.isEmpty ?? true) || !(userPropertiesSetOnce?.isEmpty ?? true) { + capture("$set", + distinctId: distinctId, + userProperties: userProperties, + userPropertiesSetOnce: userPropertiesSetOnce) - if shouldReloadFlagsForTesting { - reloadFeatureFlags() + // Note we don't reload flags on property changes as these get processed async + } else { + hedgeLog("already identified with id: \(oldDistinctId)") } - } else if !(userProperties?.isEmpty ?? true) || !(userPropertiesSetOnce?.isEmpty ?? true) { - capture("$set", - distinctId: distinctId, - userProperties: userProperties, - userPropertiesSetOnce: userPropertiesSetOnce) - // Note we don't reload flags on property changes as these get processed async } else { - hedgeLog("already identified with id: \(oldDistinctId)") + hedgeLog("identified with a different id: \(oldDistinctId). Consider calling reset() before identifying as a new user") } } diff --git a/PostHogTests/PostHogSDKTest.swift b/PostHogTests/PostHogSDKTest.swift index 7acd42a35..39d7665c5 100644 --- a/PostHogTests/PostHogSDKTest.swift +++ b/PostHogTests/PostHogSDKTest.swift @@ -189,7 +189,7 @@ class PostHogSDKTest: QuickSpec { sut.close() } - it("capture an identify event if already identified but user properties are set") { + it("updates user props if already identified but user properties are set") { let sut = self.getSut( flushAt: 2 ) @@ -232,6 +232,43 @@ class PostHogSDKTest: QuickSpec { sut.close() } + it("does not capture user props for another distinctId even if user properties are set") { + let sut = self.getSut( + flushAt: 2 + ) + + sut.identify("distinctId", + userProperties: ["userProp": "value"], + userPropertiesSetOnce: ["userPropOnce": "value"]) + + sut.identify("distinctId2", + userProperties: ["userProp2": "value2"], + userPropertiesSetOnce: ["userPropOnce2": "value2"]) + + sut.capture("satisfy_queue") + + let events = getBatchedEvents(server) + + expect(events.count) == 2 + + expect(events[0].event) == "$identify" + expect(events[1].event) == "satisfy_queue" + + expect(events[0].distinctId) == "distinctId" + let anonId = sut.getAnonymousId() + expect(events[0].properties["$anon_distinct_id"] as? String) == anonId + expect(events[0].properties["$is_identified"] as? Bool) == true + + let set = events[0].properties["$set"] as? [String: Any] ?? [:] + expect(set["userProp"] as? String) == "value" + + let setOnce = events[0].properties["$set_once"] as? [String: Any] ?? [:] + expect(setOnce["userPropOnce"] as? String) == "value" + + sut.reset() + sut.close() + } + it("captures an alias event") { let sut = self.getSut() From 3dd2c79c095d78e84bf3118031c1bb342322f97d Mon Sep 17 00:00:00 2001 From: Ioannis J Date: Tue, 12 Nov 2024 13:58:08 +0200 Subject: [PATCH 8/9] fix: refactor checks --- PostHog/PostHogSDK.swift | 62 +++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 33 deletions(-) diff --git a/PostHog/PostHogSDK.swift b/PostHog/PostHogSDK.swift index 25df23ede..2a30f5783 100644 --- a/PostHog/PostHogSDK.swift +++ b/PostHog/PostHogSDK.swift @@ -468,44 +468,40 @@ let maxRetryDelay = 30.0 let isIdentified = storageManager.isIdentified() - // if identified -> distinctId == oldDistinctId - // if !identified -> distinctId != oldDistinctId - if (distinctId == oldDistinctId) == isIdentified { - if !isIdentified { - // We keep the AnonymousId to be used by decide calls and identify to link the previousId - storageManager.setAnonymousId(oldDistinctId) - storageManager.setDistinctId(distinctId) - - storageManager.setIdentified(true) - - let properties = buildProperties(distinctId: distinctId, properties: [ - "distinct_id": distinctId, - "$anon_distinct_id": oldDistinctId, - ], userProperties: sanitizeDicionary(userProperties), userPropertiesSetOnce: sanitizeDicionary(userPropertiesSetOnce)) - let sanitizedProperties = sanitizeProperties(properties) - - queue.add(PostHogEvent( - event: "$identify", - distinctId: distinctId, - properties: sanitizedProperties - )) + let hasDifferentDistinctId = distinctId != oldDistinctId - if shouldReloadFlagsForTesting { - reloadFeatureFlags() - } - } else if !(userProperties?.isEmpty ?? true) || !(userPropertiesSetOnce?.isEmpty ?? true) { - capture("$set", - distinctId: distinctId, - userProperties: userProperties, - userPropertiesSetOnce: userPropertiesSetOnce) + if hasDifferentDistinctId, !isIdentified { + // We keep the AnonymousId to be used by decide calls and identify to link the previousId + storageManager.setAnonymousId(oldDistinctId) + storageManager.setDistinctId(distinctId) - // Note we don't reload flags on property changes as these get processed async - } else { - hedgeLog("already identified with id: \(oldDistinctId)") + storageManager.setIdentified(true) + + let properties = buildProperties(distinctId: distinctId, properties: [ + "distinct_id": distinctId, + "$anon_distinct_id": oldDistinctId, + ], userProperties: sanitizeDicionary(userProperties), userPropertiesSetOnce: sanitizeDicionary(userPropertiesSetOnce)) + let sanitizedProperties = sanitizeProperties(properties) + + queue.add(PostHogEvent( + event: "$identify", + distinctId: distinctId, + properties: sanitizedProperties + )) + + if shouldReloadFlagsForTesting { + reloadFeatureFlags() } + } else if !hasDifferentDistinctId, !(userProperties?.isEmpty ?? true) || !(userPropertiesSetOnce?.isEmpty ?? true) { + capture("$set", + distinctId: distinctId, + userProperties: userProperties, + userPropertiesSetOnce: userPropertiesSetOnce) + + // Note we don't reload flags on property changes as these get processed async } else { - hedgeLog("identified with a different id: \(oldDistinctId). Consider calling reset() before identifying as a new user") + hedgeLog("already identified with id: \(oldDistinctId)") } } From bd8ec601a5214777f01ca9f96dcf83a064337275 Mon Sep 17 00:00:00 2001 From: Ioannis J Date: Tue, 12 Nov 2024 14:04:14 +0200 Subject: [PATCH 9/9] feat: added a comment --- PostHog/PostHogSDK.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PostHog/PostHogSDK.swift b/PostHog/PostHogSDK.swift index 2a30f5783..e3be74d32 100644 --- a/PostHog/PostHogSDK.swift +++ b/PostHog/PostHogSDK.swift @@ -492,6 +492,8 @@ let maxRetryDelay = 30.0 if shouldReloadFlagsForTesting { reloadFeatureFlags() } + // 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?.isEmpty ?? true) || !(userPropertiesSetOnce?.isEmpty ?? true) { capture("$set", distinctId: distinctId,