Repository navigation
fix(surveys): hide surveys while capture is disabled - #4833
lucasheriques merged 6 commits into
Conversation
A person who declines consent could fill in a survey, see the confirmation, and lose the answer. `capture()` returns early when capturing is opted out, and `sendSurveyEvent` ignores the result, so the `survey sent` event was dropped with no return value, no log, and no callback. Survey eligibility now checks `is_capturing()`. `canRenderSurvey`, `canRenderSurveyAsync`, and `displaySurvey` give a "capturing is opted out" reason, and the display loop keeps such a survey off screen. A response that still reaches `sendSurveyEvent` (an inline `renderSurvey` call, or an opt-out during the survey) is reported with `logger.critical`, the only level a production console shows. The gate is `is_capturing()`, not `has_opted_out_capturing()`, so cookieless `on_reject` (which does capture while consent is rejected) stays eligible. Generated-By: PostHog Desktop Task-Id: 807efdaa-badf-4f9c-adbf-aa93eb842fbf
🦔 PostHog Review reviewed this pull requestFound 2 must fix, 2 should fix, 0 consider. Published 4 findings (view the review). Resolved comments: 3 fixed, 1 left for you |
|
Size Change: +10.1 kB (+0.05%) Total Size: 20.7 MB 📦 View Changed
ℹ️ View Unchanged
|
posthog-js Compliance ReportDate: 2026-09-09 15:37:54 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
`checkSurveyEligibility` also backs the public `getActiveMatchingSurveys`, so gating it on `is_capturing()` silently emptied the discovery result for custom integrations. Those render API surveys themselves and record responses through their own backend (see `markSurveyAsSeen`), where PostHog's capture state says nothing about whether a response can be recorded. The gate moves to the paths where the SDK renders the survey and captures the response itself: a new `checkSurveyDisplayEligibility` backing `canRenderSurvey`, `canRenderSurveyAsync` and `displaySurvey`, plus the display loop and the delayed popover re-check. `getActiveMatchingSurveys` is capture-independent again, which also matches the React Native equivalent. Generated-By: PostHog Desktop Task-Id: 28bf59db-dab4-49ea-8023-b7330007e73f
`displaySurvey` only ran the eligibility check when `ignoreConditions === false`, so a forced display showed a survey the SDK could not record a response for. That is a first-party path: `@posthog/react`'s `useThumbSurvey` hard-codes `ignoreConditions: true`, and a consumer of the hook cannot change it. Capturing is a prerequisite, not one of the survey's display conditions, so the check moves out of that branch. `checkSurveyCaptureEligibility` on the survey manager exposes it on its own, and `checkSurveyDisplayEligibility` composes it with the rest of eligibility as before. Generated-By: PostHog Desktop Task-Id: 28bf59db-dab4-49ea-8023-b7330007e73f
The surveys bundle is loaded unversioned from the CDN, so a newly deployed bundle can run against an older cached core. `is_capturing` was only added in 1.260.0 — verified absent from 1.259.0 and present in 1.260.0 — so the new calls threw a TypeError once per display poll on those cores, stopping automatic surveys and breaking submission. `isCapturingEnabled` feature-detects the method and falls back to the core's own `has_opted_out_capturing()` gate, matching the guards already used for `getExtension` here and for `requestRouter.isIngestionEndpoint` in the recorder. Those cores have no cookieless mode, so the fallback is what `is_capturing()` would have returned there anyway. Generated-By: PostHog Desktop Task-Id: 28bf59db-dab4-49ea-8023-b7330007e73f
Replay incident risk checkThis diff touches code involved in past incidents. This is a heads-up, not a verdict: read the matched sections of INCIDENTS.md and answer their review questions before merging. For a judgment on whether this diff has the same failure mode, run the |
Three follow-ups to the capture gate. A tab widget draws its own trigger, so filtering it out of the display loop left the button on screen. A person who opted out after the tab appeared could still open the survey and lose the answer. The display poll now removes it, with the same deferral the selector widget uses while a survey is open. `displaySurvey` reported the block with `logger.warn`, which a production console does not show. `useThumbSurvey` in `@posthog/react` passes `ignoreConditions: true` and marks the response as given before it calls `displaySurvey`, so an opted-out visitor got no popover and no signal. The report now uses `logger.critical`, as `sendSurveyEvent` does. The reason text said "capturing is opted out". With `opt_out_capturing_by_default` the gate also covers a visitor who has not answered the consent banner yet, and that person made no choice. The text and the changeset now describe the capture state instead. Adds a regression test for the tab widget, and one for the public `canRenderSurvey` entry point, which had no opted-out case. Generated-By: PostHog Desktop Task-Id: 6a19c2b3-a75e-497e-86e1-30935de90016
marandaneto
left a comment
There was a problem hiding this comment.
Automated advisory code review. One reproduced compatibility regression.
|
@PostHog/team-surveys to confirm this PR since its a behaviour change, which makes sense IMO |
Gate direct and delayed renders, keep blocked submissions on the form with answers intact, and remove capture-disabled console warnings. Read capture state in the core without requiring a new method from an older surveys bundle. Regenerate the mangled property list and document the behavior. Validation: SDK build, source typecheck, lint, ES5/ES6 checks, 6288 unit tests plus the widget race regression, 267 survey browser tests across Chromium/Firefox/WebKit, four published-core compatibility tests, and ten functional tests passed. Optional test:typecheck is blocked by ambient stub packages; explicit type resolution also exposes broad test typing failures outside this change.
|
@marandaneto I agree that we should hide the built-in survey when capture is off. I've removed the If someone opts out while they're already answering, I want to keep their answers in the form and show a retry message when they try to submit. I've fixed that so we don't show a false thank-you or clear their saved progress. I've opened #4872 for surveys to work independently of analytics consent. I'd keep that separate from this fix. The browser tests cover opt-out and retry without enabling capture automatically, and all 57 CI checks pass. |
Problem
People can submit a survey while analytics capture is disabled and see a thank-you screen even though PostHog discards their answers.
Changes
canRenderSurvey()and its async equivalent without console warnings.getActiveMatchingSurveys().is_capturing().The changeset, documentation, and regenerated mangled-property list accompany these behavior changes. Independent survey submission is tracked separately in #4872.
Regression coverage exercises direct and delayed rendering, opt-out during submission, retry after opt-in, feedback tab clicks, and both compatibility directions.
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Codex updated the original PostHog agent implementation under human direction, using GitHub CLI, Vitest, and Playwright. Applied the replay incident-risk and PR-description skills.
Consent review: this adds checks before survey display and submission without enabling capture or changing replay masking. Compatibility checks cover published-core/new-bundle behavior and a core receiving a manager without the new method.
Optional
test:typecheckremains unsuccessful: ambient stub packages fail resolution, and explicit type resolution exposes wider test typing errors. Source compilation passes. Before/after screenshots were inspected locally; the screenshot uploader could not initialize.