fix(surveys): include dismissal responses on Android #497
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -248,73 +248,91 @@ public class PostHogSurveysIntegration( | |
| val onSurveyShown: OnPostHogSurveyShown = { shownSurvey -> | ||
| // Check if shownSurvey is originalSurvey | ||
| if (shownSurvey.id == originalSurvey.id) { | ||
| // 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 | ||
| sendSurveyShownEvent(originalSurvey) | ||
|
|
||
| // Clear up event-activated surveys if this survey has events | ||
| if (hasEvents(originalSurvey)) { | ||
| 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 = onSurveyResponse@{ responseSurvey, questionIndex, response -> | ||
| // Calculate next question based on current response | ||
| val nextQuestion = getNextQuestion(originalSurvey, questionIndex, response) | ||
| var responsesToSend: Map<String, PostHogSurveyResponse>? = null | ||
|
|
||
| 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 | ||
| 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) { | ||
| responsesToSend = currentSurveyResponses.toMap() | ||
| } | ||
| } | ||
|
|
||
| responsesToSend?.let { sendSurveySentEvent(originalSurvey, it) } | ||
|
|
||
| nextQuestion | ||
| } | ||
|
|
||
| val onSurveyClosed: OnPostHogSurveyClosed = onSurveyClosed@{ _ -> | ||
| var surveyResponses: Map<String, PostHogSurveyResponse> = 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 (!wasSurveyCompleted) { | ||
| sendSurveyDismissedEvent(originalSurvey, surveyResponses) | ||
| } | ||
|
|
||
| // Mark survey as seen | ||
| setSurveySeen(originalSurvey) | ||
|
|
||
| // 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 | ||
| } | ||
|
|
||
| /** | ||
| * 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<String, PostHogSurveyResponse>, | ||
| ) { | ||
| 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, | ||
| responses: Map<String, PostHogSurveyResponse>, | ||
| ) { | ||
| val additionalProperties = | ||
| 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<String, PostHogSurveyResponse>, | ||
| ): Map<String, Any> { | ||
| val responsesProperties = | ||
| responses.mapNotNull { (key, response) -> | ||
| response.toResponseValue()?.let { value -> | ||
| key to value | ||
| } | ||
| }.toMap() | ||
|
|
||
| val surveyQuestions = | ||
| survey.questions.mapIndexed { index, question -> | ||
| mutableMapOf<String, Any>().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<String, PostHogSurveyResponse>): Boolean { | ||
| return responses.values.any { it.toResponseValue() != null } | ||
|
Contributor
There was a problem hiding this comment. Given how Rating accepts a nullable integer, and how it's converted to a response value, it looks like it could return the string Seems like a weird edge case, though, and given the way we're tracking responses, it looks unlikely? I'll leave it up to you.
Contributor
There was a problem hiding this comment. I'm not 100% sure if this will generate "null" string either, but doesn't hurt to be covered in a test?
Contributor
There was a problem hiding this comment. Yeah looks like it will generate "null" We should probably change this from
Contributor
Author
There was a problem hiding this comment. Yep, fixed this. Rating(null) now returns null instead of the string "null", and I added both a core response test and a dismissed payload test for that case. |
||
| } | ||
|
|
||
| /** | ||
| * Helper method to send survey events with consistent properties | ||
| */ | ||
|
|
@@ -759,14 +788,22 @@ 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_<index>". | ||
| */ | ||
| private fun getLegacyResponseKey(index: Int): String { | ||
| return if (index == 0) { | ||
| "\$survey_response" | ||
| } else { | ||
| "\$survey_response_$index" | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Generate the property key used to store a response for a given question id. | ||
| * Returns "$survey_response_<questionId>". | ||
| */ | ||
| private fun getQuestionIdResponseKey(questionId: String): String { | ||
| return "\$survey_response_$questionId" | ||
| } | ||
|
|
||
| // Seen Survey Tracking Methods | ||
|
|
||
| /** | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was pre-existing but I think
currentSurveyResponsesshould be synchronized with activeSurveyLockThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, fixed. I wrapped the active survey response/completion reads and writes with activeSurveyLock and snapshot the responses before sending the event.