{{ message }}
fix(growth): preserve non-accept consent decisions on banner re-init#45187
Merged
seanoliver merged 2 commits intomasterfrom Apr 23, 2026
Merged
Conversation
Users in GDPR regions who denied or partially opted out via Privacy Settings were getting re-prompted on every page load. The March FE-2648 fix only recognized uniformly-accepted ucData — any other shape (deny-all, mixed, partial opt-out) fell through and was treated as no prior decision. detectPriorConsent now returns per-service decisions, with uc_settings parsing for the fast cross-app nav case (previously treated as uniform accept, which silently upgraded deny users). Orchestration extracted from initUserCentrics into applyPriorDecisionToSDK with a coverage cross-check that re-prompts on newly-added ruleset services. All parse paths fail closed on corruption to avoid over-consent bias. GROWTH-790
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughdetectPriorConsent now returns a structured PriorConsentDecision (null | {kind:'uniform-accept'} | {kind:'decisions', decisions: UserDecision[]}) and localStorage parsing fails closed on malformed data. A new applyPriorDecisionToSDK applies prior decisions to the Usercentrics SDK after init with coverage checks and banner suppression. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ConsentState
participant UC as Usercentrics
Client->>ConsentState: detectPriorConsent()
activate ConsentState
ConsentState->>ConsentState: parse localStorage (ucData / uc_settings)
alt valid uniform accept
ConsentState-->>Client: {kind: "uniform-accept"}
else valid per-service decisions
ConsentState-->>Client: {kind: "decisions", decisions: [...]}
else malformed / missing
ConsentState-->>Client: null
end
deactivate ConsentState
Client->>UC: UC.init()
activate UC
UC-->>Client: initialized / throws
deactivate UC
Client->>ConsentState: applyPriorDecisionToSDK(UC, initialUIValues, priorDecision)
activate ConsentState
alt priorDecision = {kind:"uniform-accept"}
ConsentState->>UC: acceptAllServices()
ConsentState->>Client: suppress banner/toast
else priorDecision = {kind:"decisions", decisions}
ConsentState->>ConsentState: check coverage vs current non-essential services
alt coverage complete
ConsentState->>UC: updateServices(decisions)
ConsentState->>Client: suppress banner/toast
else coverage incomplete
ConsentState->>Client: show banner (do not restore)
end
else priorDecision = null
ConsentState->>Client: show banner
end
deactivate ConsentState
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ivasilov
approved these changes
Apr 23, 2026
seanoliver
deleted the
sean/growth-790-cookie-banner-re-prompts-users-with-prior-non-accepted
branch
Contributor
Braintrust eval report |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Problem
Cookie banner keeps re-prompting GDPR users who denied consent or made a partial opt-out via Privacy Settings — they can't get rid of it. Reported by Christian Gedde-Dahl (Front SU-362240, mygame.no) and a Supabase support engineer independently.
The March fix for FE-2648 handled the accept case — users got their banner dismissal stomped when GTM's Usercentrics integration migrated localStorage from
uc_settingstoucData/ucString. But that fix only recognized uniformly-accepteducData, so any other shape (deny-all, essentials-plus-some-tracking, partial opt-out via the Privacy Settings modal) fell through and was treated as "no prior decision." Banner re-prompts on every page load.Christian's and his colleague's
ucData.consent.servicesshowed 13 services accepted (essentials + functional) and 4 tracking services denied — the shape you get from toggling off the Marketing category in Privacy Settings. Our detection ignored it.Changes
detectPriorConsentnow returns a discriminated union —null,{ kind: 'uniform-accept' }, or{ kind: 'decisions'; decisions }— so we restore per-service state faithfully instead of flattening to deny-all.uc_settingsfor the fast cross-app nav case. The old fallback treateduc_user_interaction === "true"as uniform-accept, which silently upgraded deny users (GDPR violation waiting to happen). Now the flag is just a gate confirming the user actually interacted, and we read real decisions fromuc_settings.services[].initUserCentricsinto exportedapplyPriorDecisionToSDK(UC, initialUIValues, priorDecision)so it's unit-testable without mocking the dynamic SDK import.ucData/uc_settingscorruption. Cherry-picking the valid subset would bias toward over-consent, which is the worst-direction bias in this domain.Testing
27 unit tests covering
detectPriorConsentparsing (bothucDataanduc_settingspaths, fail-closed on malformed or partially-corrupt blobs, combined scenarios) andapplyPriorDecisionToSDKorchestration (uniform accept, decisions with full coverage, uncovered-non-essential compliance guard, essentials-only negative control, null fallthrough, SDK-already-consented passthrough).Can't fully repro on staging — CSP blocks GTM on preview, so the
ucDatamigration never fires. Same limitation as the original FE-2648 fix. Will verify in production post-merge by asking Christian to reload and watching support ticket volume.Reviewed twice by Codex with a full iteration between passes; final pass found no functional blockers.
GROWTH-790
Summary by CodeRabbit
Bug Fixes
Refactor
Tests