diff --git a/.changeset/wise-dolphins-sing.md b/.changeset/wise-dolphins-sing.md new file mode 100644 index 00000000..db88d77b --- /dev/null +++ b/.changeset/wise-dolphins-sing.md @@ -0,0 +1,6 @@ +--- +"posthog": patch +"posthog-android": patch +--- + +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 437db2c7..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,61 +262,77 @@ 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[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 // 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 + 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 - // 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 + activeSurvey = null + activeSurveyCompleted = false + currentSurveyResponses.clear() } // Send survey dismissed event if survey was not completed - if (!activeSurveyCompleted) { - sendSurveyDismissedEvent(originalSurvey) + 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) @@ -606,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. @@ -652,26 +658,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 +677,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 +698,37 @@ public class PostHogSurveysIntegration( ) } + private fun buildSurveyResponseProperties( + survey: Survey, + responses: Map, + ): Map { + val responsesProperties = + responses.mapNotNull { (key, response) -> + response.toResponseValue()?.let { value -> + key to value + } + }.toMap() + + 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 { + return responses.values.any { it.toResponseValue() != null } + } + /** * Helper method to send survey events with consistent properties */ @@ -759,7 +788,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 +796,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/PostHogSurveysEventPayloadTest.kt b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt new file mode 100644 index 00000000..ac190da1 --- /dev/null +++ b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt @@ -0,0 +1,266 @@ +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 PostHogSurveysEventPayloadTest { + 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 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(), + ) + } + + 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 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() + 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( + 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"], + ) + + 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( + mapOf( + "id" to "question-1", + "question" to "How satisfied are you?", + ), + mapOf( + "id" to "question-2", + "question" to "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"]) + } + + @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)