From ef67c0970c6f39b07e93ddbc99e093d7d8a9efa2 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Mon, 27 Apr 2026 12:37:43 -0300 Subject: [PATCH 1/6] fix(surveys): include dismissal responses --- .changeset/wise-dolphins-sing.md | 5 + .../surveys/PostHogSurveysIntegration.kt | 89 +++++---- .../PostHogSurveysDismissedEventTest.kt | 170 ++++++++++++++++++ 3 files changed, 233 insertions(+), 31 deletions(-) create mode 100644 .changeset/wise-dolphins-sing.md create mode 100644 posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt diff --git a/.changeset/wise-dolphins-sing.md b/.changeset/wise-dolphins-sing.md new file mode 100644 index 00000000..3cb8fc13 --- /dev/null +++ b/.changeset/wise-dolphins-sing.md @@ -0,0 +1,5 @@ +--- +"posthog-android": patch +--- + +Include survey responses on Android dismissal events, including question id based response keys and partial completion state. diff --git a/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt b/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt index 437db2c7..b0b433d2 100644 --- a/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt @@ -280,7 +280,10 @@ public class PostHogSurveysIntegration( val nextQuestion = getNextQuestion(originalSurvey, questionIndex, response) // Store the response for survey completion tracking - currentSurveyResponses[getResponseKey(questionIndex)] = response + currentSurveyResponses[getLegacyResponseKey(questionIndex)] = response + originalSurvey.questions.getOrNull(questionIndex)?.id?.takeIf { it.isNotEmpty() }?.let { questionId -> + currentSurveyResponses[getQuestionIdResponseKey(questionId)] = response + } // Check if survey is completed (needed on close event) activeSurveyCompleted = nextQuestion.isSurveyCompleted @@ -297,6 +300,7 @@ public class PostHogSurveysIntegration( val onSurveyClosed: OnPostHogSurveyClosed = onSurveyClosed@{ _ -> // Get current active survey and completion state val currentActiveSurvey = activeSurvey + val surveyResponses = currentSurveyResponses.toMap() // Validate that this survey matches the currently active survey if (currentActiveSurvey == null || originalSurvey.id != currentActiveSurvey.id) { @@ -306,7 +310,7 @@ public class PostHogSurveysIntegration( // Send survey dismissed event if survey was not completed if (!activeSurveyCompleted) { - sendSurveyDismissedEvent(originalSurvey) + sendSurveyDismissedEvent(originalSurvey, surveyResponses) } // Mark survey as seen @@ -652,26 +656,14 @@ public class PostHogSurveysIntegration( survey: Survey, responses: Map, ) { - val questionProperties = - mutableMapOf( - "\$survey_questions" to survey.questions.map { it.question }, - ) - - // Add survey interaction property for "responded" - questionProperties["\$set"] = - mapOf( - getSurveyInteractionProperty(survey, "responded") to true, - ) - - // Convert responses to simple values - val responsesProperties = - responses.mapNotNull { (key, response) -> - response.toResponseValue()?.let { value -> - key to value - } - }.toMap() - - val additionalProperties = questionProperties + responsesProperties + val additionalProperties = + buildSurveyResponseProperties(survey, responses) + + mapOf( + "\$set" to + mapOf( + getSurveyInteractionProperty(survey, "responded") to true, + ), + ) sendSurveyEvent( event = "survey sent", @@ -683,15 +675,19 @@ public class PostHogSurveysIntegration( /** * Sends a "survey dismissed" event to PostHog instance */ - private fun sendSurveyDismissedEvent(survey: Survey) { + private fun sendSurveyDismissedEvent( + survey: Survey, + responses: Map, + ) { val additionalProperties = - mapOf( - "\$survey_questions" to survey.questions.map { it.question }, - "\$set" to - mapOf( - getSurveyInteractionProperty(survey, "dismissed") to true, - ), - ) + buildSurveyResponseProperties(survey, responses) + + mapOf( + "\$survey_partially_completed" to surveyHasResponses(responses), + "\$set" to + mapOf( + getSurveyInteractionProperty(survey, "dismissed") to true, + ), + ) sendSurveyEvent( event = "survey dismissed", @@ -700,6 +696,29 @@ public class PostHogSurveysIntegration( ) } + private fun buildSurveyResponseProperties( + survey: Survey, + responses: Map, + ): Map { + val questionProperties = + mutableMapOf( + "\$survey_questions" to survey.questions.map { it.question }, + ) + + val responsesProperties = + responses.mapNotNull { (key, response) -> + response.toResponseValue()?.let { value -> + key to value + } + }.toMap() + + return questionProperties + responsesProperties + } + + private fun surveyHasResponses(responses: Map): Boolean { + return responses.values.any { it.toResponseValue() != null } + } + /** * Helper method to send survey events with consistent properties */ @@ -759,7 +778,7 @@ public class PostHogSurveysIntegration( * Generate the property key used to store a response for a given question index. * For index 0 returns "$survey_response", otherwise returns "$survey_response_". */ - private fun getResponseKey(index: Int): String { + private fun getLegacyResponseKey(index: Int): String { return if (index == 0) { "\$survey_response" } else { @@ -767,6 +786,14 @@ public class PostHogSurveysIntegration( } } + /** + * Generate the property key used to store a response for a given question id. + * Returns "$survey_response_". + */ + private fun getQuestionIdResponseKey(questionId: String): String { + return "\$survey_response_$questionId" + } + // Seen Survey Tracking Methods /** diff --git a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt new file mode 100644 index 00000000..1d36dcf8 --- /dev/null +++ b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt @@ -0,0 +1,170 @@ +package com.posthog.android.surveys + +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.posthog.PostHogConfig +import com.posthog.android.PostHogFake +import com.posthog.internal.PostHogSerializer +import com.posthog.surveys.OnPostHogSurveyClosed +import com.posthog.surveys.OnPostHogSurveyResponse +import com.posthog.surveys.OnPostHogSurveyShown +import com.posthog.surveys.PostHogDisplaySurvey +import com.posthog.surveys.PostHogSurveyResponse +import com.posthog.surveys.PostHogSurveysDelegate +import com.posthog.surveys.Survey +import com.posthog.surveys.SurveyQuestion +import com.posthog.surveys.SurveyType +import org.junit.runner.RunWith +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +@RunWith(AndroidJUnit4::class) +internal class PostHogSurveysDismissedEventTest { + private val context = ApplicationProvider.getApplicationContext() + private val serializer = PostHogSerializer(PostHogConfig("test-api-key")) + + private class RecordingDelegate : PostHogSurveysDelegate { + var shownSurvey: PostHogDisplaySurvey? = null + var onSurveyShown: OnPostHogSurveyShown? = null + var onSurveyResponse: OnPostHogSurveyResponse? = null + var onSurveyClosed: OnPostHogSurveyClosed? = null + + override fun renderSurvey( + survey: PostHogDisplaySurvey, + onSurveyShown: OnPostHogSurveyShown, + onSurveyResponse: OnPostHogSurveyResponse, + onSurveyClosed: OnPostHogSurveyClosed, + ) { + shownSurvey = survey + this.onSurveyShown = onSurveyShown + this.onSurveyResponse = onSurveyResponse + this.onSurveyClosed = onSurveyClosed + } + + override fun cleanupSurveys() {} + } + + private fun createIntegration(delegate: RecordingDelegate): Pair { + val config = + PostHogConfig("test-api-key").apply { + surveys = true + surveysConfig.surveysDelegate = delegate + } + + val integration = PostHogSurveysIntegration(context, config) + val postHog = PostHogFake() + integration.install(postHog) + + return integration to postHog + } + + private fun createQuestion( + id: String, + question: String, + ): SurveyQuestion { + return serializer.gson.fromJson( + """ + { + "id": "$id", + "type": "open", + "question": "$question", + "description": null, + "descriptionContentType": "text", + "optional": false, + "buttonText": null, + "branching": null + } + """.trimIndent(), + SurveyQuestion::class.java, + ) + } + + private fun createSurvey( + id: String = "test-survey-id", + name: String = "Test Survey", + currentIteration: Int? = null, + ): Survey { + return Survey( + id = id, + name = name, + type = SurveyType.POPOVER, + questions = + listOf( + createQuestion("question-1", "How satisfied are you?"), + createQuestion("question-2", "Any additional comments?"), + ), + description = null, + featureFlagKeys = null, + linkedFlagKey = null, + targetingFlagKey = null, + internalTargetingFlagKey = null, + conditions = null, + appearance = null, + currentIteration = currentIteration, + currentIterationStartDate = null, + startDate = java.util.Date(), + endDate = null, + schedule = null, + ) + } + + @Test + fun `survey dismissed includes responses and marks partial completion when there are answers`() { + val delegate = RecordingDelegate() + val (integration, postHog) = createIntegration(delegate) + val survey = createSurvey() + + integration.showSurvey(survey) + + val shownSurvey = assertNotNull(delegate.shownSurvey) + assertNotNull(delegate.onSurveyShown).invoke(shownSurvey) + assertNotNull(delegate.onSurveyResponse).invoke(shownSurvey, 0, PostHogSurveyResponse.Text("Great product!")) + assertNotNull(delegate.onSurveyClosed).invoke(shownSurvey) + + assertEquals("survey dismissed", postHog.event) + + val properties = assertNotNull(postHog.properties) + assertEquals("Test Survey", properties["\$survey_name"]) + assertEquals("test-survey-id", properties["\$survey_id"]) + assertEquals(true, properties["\$survey_partially_completed"]) + assertEquals("Great product!", properties["\$survey_response"]) + assertEquals("Great product!", properties["\$survey_response_question-1"]) + assertEquals( + listOf("How satisfied are you?", "Any additional comments?"), + properties["\$survey_questions"], + ) + + val setProperties = properties["\$set"] as? Map<*, *> + assertEquals(true, setProperties?.get("\$survey_dismissed/test-survey-id")) + } + + @Test + fun `survey dismissed marks partial completion false when there are no answers`() { + val delegate = RecordingDelegate() + val (integration, postHog) = createIntegration(delegate) + val survey = createSurvey(id = "empty-dismissed-survey", name = "Empty Dismissed Survey") + + integration.showSurvey(survey) + + val shownSurvey = assertNotNull(delegate.shownSurvey) + assertNotNull(delegate.onSurveyShown).invoke(shownSurvey) + assertNotNull(delegate.onSurveyClosed).invoke(shownSurvey) + + assertEquals("survey dismissed", postHog.event) + + val properties = assertNotNull(postHog.properties) + assertEquals(false, properties["\$survey_partially_completed"]) + assertNull(properties["\$survey_response"]) + assertEquals( + listOf("How satisfied are you?", "Any additional comments?"), + properties["\$survey_questions"], + ) + + val setProperties = properties["\$set"] as? Map<*, *> + assertEquals(true, setProperties?.get("\$survey_dismissed/empty-dismissed-survey")) + assertNull(properties["\$survey_response_1"]) + assertNull(properties["\$survey_response_question-1"]) + } +} From c2009483e255ae07c2de401b25c7ea2bc12f380f Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Mon, 27 Apr 2026 12:59:40 -0300 Subject: [PATCH 2/6] test(surveys): avoid direct gson access --- .../PostHogSurveysDismissedEventTest.kt | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt index 1d36dcf8..d5093e06 100644 --- a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt @@ -64,20 +64,21 @@ internal class PostHogSurveysDismissedEventTest { id: String, question: String, ): SurveyQuestion { - return serializer.gson.fromJson( - """ - { - "id": "$id", - "type": "open", - "question": "$question", - "description": null, - "descriptionContentType": "text", - "optional": false, - "buttonText": null, - "branching": null - } - """.trimIndent(), - SurveyQuestion::class.java, + return checkNotNull( + serializer.deserializeList( + listOf( + mapOf( + "id" to id, + "type" to "open", + "question" to question, + "description" to null, + "descriptionContentType" to "text", + "optional" to false, + "buttonText" to null, + "branching" to null, + ), + ), + )?.firstOrNull(), ) } From a9592ff448a9403e745b80aab543f04007ead24c Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Mon, 27 Apr 2026 13:29:49 -0300 Subject: [PATCH 3/6] fix(surveys): align android survey question payload --- .../surveys/PostHogSurveysIntegration.kt | 20 +++++++++++----- .../PostHogSurveysDismissedEventTest.kt | 23 +++++++++++++++++-- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt b/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt index b0b433d2..f4d9fdcb 100644 --- a/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt @@ -700,11 +700,6 @@ public class PostHogSurveysIntegration( survey: Survey, responses: Map, ): Map { - val questionProperties = - mutableMapOf( - "\$survey_questions" to survey.questions.map { it.question }, - ) - val responsesProperties = responses.mapNotNull { (key, response) -> response.toResponseValue()?.let { value -> @@ -712,7 +707,20 @@ public class PostHogSurveysIntegration( } }.toMap() - return questionProperties + responsesProperties + val surveyQuestions = + survey.questions.mapIndexed { index, question -> + mutableMapOf().apply { + question.id?.let { put("id", it) } + question.question?.let { put("question", it) } + + val responseKey = + question.id?.takeIf { it.isNotEmpty() }?.let(::getQuestionIdResponseKey) + ?: getLegacyResponseKey(index) + responsesProperties[responseKey]?.let { put("response", it) } + } + } + + return mapOf("\$survey_questions" to surveyQuestions) + responsesProperties } private fun surveyHasResponses(responses: Map): Boolean { diff --git a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt index d5093e06..f3a89d79 100644 --- a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt @@ -133,7 +133,17 @@ internal class PostHogSurveysDismissedEventTest { assertEquals("Great product!", properties["\$survey_response"]) assertEquals("Great product!", properties["\$survey_response_question-1"]) assertEquals( - listOf("How satisfied are you?", "Any additional comments?"), + listOf( + mapOf( + "id" to "question-1", + "question" to "How satisfied are you?", + "response" to "Great product!", + ), + mapOf( + "id" to "question-2", + "question" to "Any additional comments?", + ), + ), properties["\$survey_questions"], ) @@ -159,7 +169,16 @@ internal class PostHogSurveysDismissedEventTest { assertEquals(false, properties["\$survey_partially_completed"]) assertNull(properties["\$survey_response"]) assertEquals( - listOf("How satisfied are you?", "Any additional comments?"), + listOf( + mapOf( + "id" to "question-1", + "question" to "How satisfied are you?", + ), + mapOf( + "id" to "question-2", + "question" to "Any additional comments?", + ), + ), properties["\$survey_questions"], ) From f93f03ff65e74ab5c31472ccb2fa883a51e03fee Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Mon, 27 Apr 2026 13:31:32 -0300 Subject: [PATCH 4/6] test(surveys): cover android sent event payload --- .../PostHogSurveysDismissedEventTest.kt | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt index f3a89d79..5829eabc 100644 --- a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt @@ -111,6 +111,48 @@ internal class PostHogSurveysDismissedEventTest { ) } + @Test + fun `survey sent includes legacy and question id response keys`() { + val delegate = RecordingDelegate() + val (integration, postHog) = createIntegration(delegate) + val survey = createSurvey(id = "sent-survey", name = "Sent Survey") + + integration.showSurvey(survey) + + val shownSurvey = assertNotNull(delegate.shownSurvey) + assertNotNull(delegate.onSurveyShown).invoke(shownSurvey) + assertNotNull(delegate.onSurveyResponse).invoke(shownSurvey, 0, PostHogSurveyResponse.Text("Great product!")) + assertNotNull(delegate.onSurveyResponse).invoke(shownSurvey, 1, PostHogSurveyResponse.Text("Keep it up!")) + + assertEquals("survey sent", postHog.event) + + val properties = assertNotNull(postHog.properties) + assertEquals("Sent Survey", properties["\$survey_name"]) + assertEquals("sent-survey", properties["\$survey_id"]) + assertEquals("Great product!", properties["\$survey_response"]) + assertEquals("Keep it up!", properties["\$survey_response_1"]) + assertEquals("Great product!", properties["\$survey_response_question-1"]) + assertEquals("Keep it up!", properties["\$survey_response_question-2"]) + assertEquals( + listOf( + mapOf( + "id" to "question-1", + "question" to "How satisfied are you?", + "response" to "Great product!", + ), + mapOf( + "id" to "question-2", + "question" to "Any additional comments?", + "response" to "Keep it up!", + ), + ), + properties["\$survey_questions"], + ) + + val setProperties = properties["\$set"] as? Map<*, *> + assertEquals(true, setProperties?.get("\$survey_responded/sent-survey")) + } + @Test fun `survey dismissed includes responses and marks partial completion when there are answers`() { val delegate = RecordingDelegate() From bba4196d5c219ced4cf92881d272d6c5026aa983 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Mon, 27 Apr 2026 13:35:36 -0300 Subject: [PATCH 5/6] test(surveys): rename android payload test --- ...sDismissedEventTest.kt => PostHogSurveysEventPayloadTest.kt} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename posthog-android/src/test/java/com/posthog/android/surveys/{PostHogSurveysDismissedEventTest.kt => PostHogSurveysEventPayloadTest.kt} (99%) diff --git a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt similarity index 99% rename from posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt rename to posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt index 5829eabc..a0b7cbee 100644 --- a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDismissedEventTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt @@ -21,7 +21,7 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull @RunWith(AndroidJUnit4::class) -internal class PostHogSurveysDismissedEventTest { +internal class PostHogSurveysEventPayloadTest { private val context = ApplicationProvider.getApplicationContext() private val serializer = PostHogSerializer(PostHogConfig("test-api-key")) From 3582e65e18ff4a8f8ee5641c9d22008ebe22dedb Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Mon, 27 Apr 2026 19:00:50 -0300 Subject: [PATCH 6/6] fix survey response edge cases --- .changeset/wise-dolphins-sing.md | 3 +- .../surveys/PostHogSurveysIntegration.kt | 88 ++++++++++--------- .../surveys/PostHogSurveysEventPayloadTest.kt | 34 +++++++ .../posthog/surveys/PostHogSurveyResponse.kt | 2 +- .../surveys/PostHogSurveyResponseTest.kt | 6 ++ 5 files changed, 88 insertions(+), 45 deletions(-) diff --git a/.changeset/wise-dolphins-sing.md b/.changeset/wise-dolphins-sing.md index 3cb8fc13..db88d77b 100644 --- a/.changeset/wise-dolphins-sing.md +++ b/.changeset/wise-dolphins-sing.md @@ -1,5 +1,6 @@ --- +"posthog": patch "posthog-android": patch --- -Include survey responses on Android dismissal events, including question id based response keys and partial completion state. +Include survey responses on Android dismissal events, including question id based response keys and partial completion state. Null rating responses are ignored instead of being serialized as "null". diff --git a/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt b/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt index f4d9fdcb..5a535dfd 100644 --- a/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt @@ -248,11 +248,13 @@ public class PostHogSurveysIntegration( val onSurveyShown: OnPostHogSurveyShown = { shownSurvey -> // Check if shownSurvey is originalSurvey if (shownSurvey.id == originalSurvey.id) { - val currentActiveSurvey = activeSurvey - - // If currentActiveSurvey is null, set this originalSurvey as active - if (currentActiveSurvey == null) { - setActiveSurvey(originalSurvey) + // If no survey is active, set this originalSurvey as active + synchronized(activeSurveyLock) { + if (activeSurvey == null) { + activeSurvey = originalSurvey + activeSurveyCompleted = false + currentSurveyResponses.clear() + } } // Send survey shown event @@ -260,24 +262,27 @@ public class PostHogSurveysIntegration( // Clear up event-activated surveys if this survey has events if (hasEvents(originalSurvey)) { - eventActivatedSurveys.remove(originalSurvey.id) + synchronized(eventActivationLock) { + eventActivatedSurveys.remove(originalSurvey.id) + } } } else { config.logger.log("Received a show event for a non-matching survey: ${shownSurvey.id} vs ${originalSurvey.id}") } } - val onSurveyResponse: OnPostHogSurveyResponse = { responseSurvey, questionIndex, response -> - // Get current active survey - val currentActiveSurvey = activeSurvey + val onSurveyResponse: OnPostHogSurveyResponse = onSurveyResponse@{ responseSurvey, questionIndex, response -> + // Calculate next question based on current response + val nextQuestion = getNextQuestion(originalSurvey, questionIndex, response) + var responsesToSend: Map? = null - // Validate that this survey matches the currently active survey - if (currentActiveSurvey == null || responseSurvey.id != currentActiveSurvey.id) { - config.logger.log("Received a response event for a non-active survey") - null - } else { - // Calculate next question based on current response - val nextQuestion = getNextQuestion(originalSurvey, questionIndex, response) + synchronized(activeSurveyLock) { + // Validate that this survey matches the currently active survey + val currentActiveSurvey = activeSurvey + if (currentActiveSurvey == null || responseSurvey.id != currentActiveSurvey.id) { + config.logger.log("Received a response event for a non-active survey") + return@onSurveyResponse null + } // Store the response for survey completion tracking currentSurveyResponses[getLegacyResponseKey(questionIndex)] = response @@ -290,35 +295,44 @@ public class PostHogSurveysIntegration( // Send completion event if survey is finished if (activeSurveyCompleted) { - sendSurveySentEvent(originalSurvey, currentSurveyResponses) + responsesToSend = currentSurveyResponses.toMap() } - - nextQuestion } + + responsesToSend?.let { sendSurveySentEvent(originalSurvey, it) } + + nextQuestion } val onSurveyClosed: OnPostHogSurveyClosed = onSurveyClosed@{ _ -> - // Get current active survey and completion state - val currentActiveSurvey = activeSurvey - val surveyResponses = currentSurveyResponses.toMap() - - // Validate that this survey matches the currently active survey - if (currentActiveSurvey == null || originalSurvey.id != currentActiveSurvey.id) { - config.logger.log("[Surveys] Received a close event for a non-active survey") - return@onSurveyClosed + var surveyResponses: Map = emptyMap() + var wasSurveyCompleted = false + + synchronized(activeSurveyLock) { + // Validate that this survey matches the currently active survey + val currentActiveSurvey = activeSurvey + if (currentActiveSurvey == null || originalSurvey.id != currentActiveSurvey.id) { + config.logger.log("[Surveys] Received a close event for a non-active survey") + return@onSurveyClosed + } + + // Get current active survey and completion state + surveyResponses = currentSurveyResponses.toMap() + wasSurveyCompleted = activeSurveyCompleted + + activeSurvey = null + activeSurveyCompleted = false + currentSurveyResponses.clear() } // Send survey dismissed event if survey was not completed - if (!activeSurveyCompleted) { + if (!wasSurveyCompleted) { sendSurveyDismissedEvent(originalSurvey, surveyResponses) } // Mark survey as seen setSurveySeen(originalSurvey) - // Clear active survey - clearActiveSurvey() - // Show next survey in queue after a short delay Thread { Thread.sleep(NEXT_SURVEY_TRANSITION_DELAY_MS) @@ -610,18 +624,6 @@ public class PostHogSurveysIntegration( return survey.type == SurveyType.POPOVER || survey.type == SurveyType.WIDGET } - /** - * Sets the currently active survey. - * This prevents multiple surveys from being shown simultaneously. - */ - private fun setActiveSurvey(survey: Survey?) { - synchronized(activeSurveyLock) { - activeSurvey = survey - activeSurveyCompleted = false - currentSurveyResponses.clear() - } - } - /** * Clears the active survey when it's completed or dismissed. * This allows the next survey to be shown. diff --git a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt index a0b7cbee..ac190da1 100644 --- a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt @@ -229,4 +229,38 @@ internal class PostHogSurveysEventPayloadTest { assertNull(properties["\$survey_response_1"]) assertNull(properties["\$survey_response_question-1"]) } + + @Test + fun `survey dismissed ignores null rating response`() { + val delegate = RecordingDelegate() + val (integration, postHog) = createIntegration(delegate) + val survey = createSurvey(id = "null-rating-survey", name = "Null Rating Survey") + + integration.showSurvey(survey) + + val shownSurvey = assertNotNull(delegate.shownSurvey) + assertNotNull(delegate.onSurveyShown).invoke(shownSurvey) + assertNotNull(delegate.onSurveyResponse).invoke(shownSurvey, 0, PostHogSurveyResponse.Rating(null)) + assertNotNull(delegate.onSurveyClosed).invoke(shownSurvey) + + assertEquals("survey dismissed", postHog.event) + + val properties = assertNotNull(postHog.properties) + assertEquals(false, properties["\$survey_partially_completed"]) + assertNull(properties["\$survey_response"]) + assertNull(properties["\$survey_response_question-1"]) + assertEquals( + listOf( + mapOf( + "id" to "question-1", + "question" to "How satisfied are you?", + ), + mapOf( + "id" to "question-2", + "question" to "Any additional comments?", + ), + ), + properties["\$survey_questions"], + ) + } } diff --git a/posthog/src/main/java/com/posthog/surveys/PostHogSurveyResponse.kt b/posthog/src/main/java/com/posthog/surveys/PostHogSurveyResponse.kt index 768283c8..bd2cf34c 100644 --- a/posthog/src/main/java/com/posthog/surveys/PostHogSurveyResponse.kt +++ b/posthog/src/main/java/com/posthog/surveys/PostHogSurveyResponse.kt @@ -49,7 +49,7 @@ public sealed class PostHogSurveyResponse { is Text -> text is SingleChoice -> selectedChoice is MultipleChoice -> selectedChoices - is Rating -> rating.toString() + is Rating -> rating?.toString() is Link -> if (clicked) "link clicked" else null } } diff --git a/posthog/src/test/java/com/posthog/surveys/PostHogSurveyResponseTest.kt b/posthog/src/test/java/com/posthog/surveys/PostHogSurveyResponseTest.kt index 1e0c76ec..8b066ac0 100644 --- a/posthog/src/test/java/com/posthog/surveys/PostHogSurveyResponseTest.kt +++ b/posthog/src/test/java/com/posthog/surveys/PostHogSurveyResponseTest.kt @@ -42,6 +42,12 @@ internal class PostHogSurveyResponseTest { assertEquals("-1", response.toResponseValue()) } + @Test + fun `Rating response with null rating returns null`() { + val response = PostHogSurveyResponse.Rating(null) + assertNull(response.toResponseValue()) + } + @Test fun `Link response with clicked true returns link clicked`() { val response = PostHogSurveyResponse.Link(true)