Skip to content

fix(surveys): include dismissal responses on Android#497

Merged
lucasheriques merged 6 commits intomainfrom
fix/survey-dismissal-responses
Apr 27, 2026
Merged

fix(surveys): include dismissal responses on Android#497
lucasheriques merged 6 commits intomainfrom
fix/survey-dismissal-responses

Conversation

@lucasheriques
Copy link
Copy Markdown
Contributor

@lucasheriques lucasheriques commented Apr 27, 2026

💡 Motivation and Context

  • include in-progress survey responses on survey dismissed events
  • add $survey_partially_completed for dismissed surveys
  • store both legacy index-based response keys and $survey_response_<questionId> keys on Android
  • align $survey_questions with RN/iOS so it contains { id, question, response } objects instead of only question strings
  • add a patch changeset for release tracking

Companion PRs

💚 How did you test it?

  • added focused dismissal event coverage in PostHogSurveysEventPayloadTest
  • added focused survey sent payload coverage for legacy keys, question-id keys, and richer $survey_questions
  • local Gradle test execution is blocked in this environment because no Android SDK is configured (ANDROID_HOME / local.properties missing)

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.
  • Ran pnpm changeset to generate a changeset file
  • Added the "release" label to the PR to indicate we're publishing new versions for the affected packages

@lucasheriques lucasheriques requested a review from a team as a code owner April 27, 2026 15:38
}

private fun surveyHasResponses(responses: Map<String, PostHogSurveyResponse>): Boolean {
return responses.values.any { it.toResponseValue() != null }
Copy link
Copy Markdown
Contributor

@dustinbyrne dustinbyrne Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given how Rating accepts a nullable integer, and how it's converted to a response value, it looks like it could return the string "null" and cause this function to return true.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure if this will generate "null" string either, but doesn't hurt to be covered in a test?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah looks like it will generate "null"

We should probably change this from is Rating -> rating.toString() to is Rating -> rating?.toString()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// Store the response for survey completion tracking
currentSurveyResponses[getResponseKey(questionIndex)] = response
currentSurveyResponses[getLegacyResponseKey(questionIndex)] = response
Copy link
Copy Markdown
Contributor

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 currentSurveyResponses should be synchronized with activeSurveyLock

Copy link
Copy Markdown
Contributor Author

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.

}

private fun surveyHasResponses(responses: Map<String, PostHogSurveyResponse>): Boolean {
return responses.values.any { it.toResponseValue() != null }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure if this will generate "null" string either, but doesn't hurt to be covered in a test?

@github-actions
Copy link
Copy Markdown
Contributor

posthog-android Compliance Report

Date: 2026-04-27 22:06:14 UTC
Duration: 512ms

⚠️ Some Tests Failed

0/1 tests passed, 1 failed


Feature_Flags Tests

⚠️ 0/1 tests passed, 1 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 300ms

Failures

request_payload.request_with_person_properties_device_id

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag'

@lucasheriques lucasheriques merged commit ed31712 into main Apr 27, 2026
13 checks passed
@lucasheriques lucasheriques deleted the fix/survey-dismissal-responses branch April 27, 2026 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants