fix(surveys): exclude web-only surveys on Android - #690
Conversation
🦔 ReviewHog reviewed this pull requestFound 1 must fix, 0 should fix, 0 consider. Published 1 finding (view the review). |
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
Surveys scoped to the web via a CSS `selector` or `url` display condition were leaking onto native Android because those conditions were not checked during survey matching. Exclude every survey with a non-empty `url` or `selector`, including surveys that also have device-type or event targeting. Android cannot evaluate the web condition, so native targeting alone must not make the survey eligible. This matches the behavior in posthog-ios and posthog-react-native. Generated-By: PostHog Code Task-Id: 0fc1a1b9-118e-48f1-8c40-d399f533d150
e5fc46d to
8ee7722
Compare
|
Reviews (1): Last reviewed commit: "fix(surveys): exclude web-only surveys o..." | Re-trigger Greptile |
| /** | ||
| * Returns whether a survey has CSS selector or URL conditions that cannot be | ||
| * evaluated outside a browser DOM. These surveys are excluded from native | ||
| * matching even when they also have device-type or event conditions. | ||
| */ | ||
| private fun hasWebConditions(survey: Survey): Boolean { | ||
| val conditions = survey.conditions ?: return false | ||
| return !conditions.url.isNullOrEmpty() || !conditions.selector.isNullOrEmpty() | ||
| } |
There was a problem hiding this comment.
i think this makes sense but it'd also mean that a survey that is permissive for mobile users (intentionally) would stop appearing, so it's better to confirm with @PostHog/team-surveys for its impact
@PostHog/team-surveys you can approve if thats ok
There was a problem hiding this comment.
oh looks like the pr is already merged for ios
we probably need this in react native and flutter then, but i'd still check with surveys team
There was a problem hiding this comment.
yeah this was originally initiated by SDK compliance monitor
via surveys/spec.md
- Filter active matching surveys. Eligibility checks include active/running status, device type, wait-period rules, linked/targeting/internal feature flags, optional event/action activation conditions, and platform-specific display constraints. Non-web/native SDKs MUST exclude surveys whose only display-targeting conditions are web-only (a CSS
selectorand/orurlmatch) — those conditions are unevaluable outside a browser DOM, so displaying such a survey natively renders an inert prompt with no working targeting.
arnohillen
left a comment
There was a problem hiding this comment.
[arno's agent] Reviewed at head 8ee7722: change verified against head code, CI classified, no blocking findings.
|
[arno's agent] The showNextSurvey test exercises the onSurveysLoaded auto-trigger rather than calling showNextSurvey directly, so a regression in showNextSurvey itself would not fail it. |
💡 Motivation and Context
Why: Surveys scoped to the web via a CSS
selectororurldisplay condition were leaking onto native Android and rendering as inert prompts with non-functional targeting.PostHogSurveysIntegration.getActiveMatchingSurveys()did not checkconditions.urlorconditions.selector. Android cannot evaluate either condition, so matching native device-type or event targeting is not enough to make such a survey eligible.This brings posthog-android in line with the canonical
surveyscontract in PostHog/sdk-specs and the behavior already shipped in posthog-ios and posthog-react-native.Analogous of PostHog/posthog-ios#733
What changed
urlorselectorcondition.💚 How did you test it?
./gradlew :posthog-android:testDebugUnitTest --tests 'com.posthog.android.surveys.PostHogSurveysWebConditionsTest'make testmake checkFormat📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Fully autonomous
Authored by PostHog Code. A Changeset entry was added since the repo uses Changesets rather than direct CHANGELOG edits.
Created with PostHog Code