Skip to content

TRAC-1395: fix - treat consent as granted when the store has cookie consent disabled - #3163

Open
parthshahp wants to merge 1 commit into
canaryfrom
trac-1395
Open

TRAC-1395: fix - treat consent as granted when the store has cookie consent disabled#3163
parthshahp wants to merge 1 commit into
canaryfrom
trac-1395

Conversation

@parthshahp

@parthshahp parthshahp commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

What/Why?

When cookieConsentEnabled (setting in the control panel) is off, c15t grants every consent category client-side but only in its in-memory store, so no c15t-consent cookie is ever written. hasConsentFor read the cookie as the sole source of truth and so returned false for every category on those stores.

hasConsentFor now falls back to the store's cookie consent setting when no consent cookie is present. An explicit consent cookie still takes precedence.

Also seeds an explicit functionality-consent cookie in the two alternate currency e2e tests so they assert the consented path rather than depending on the test store's cookieConsentEnabled setting, and moves the consent cookie builder duplicated in analytics-session.spec.ts into a shared test helper.

Testing

Tested using local tests as well as CI.

Before, you can see that I must refresh for the currency to take effect:

Screen.Recording.2026-08-05.at.5.06.47.PM.mov

After:

Screen.Recording.2026-08-05.at.3.47.55.PM.mov

Migration

No migration needed.

@vercel

vercel Bot commented Aug 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
catalyst Ready Ready Preview Aug 19, 2026 6:39pm

Request Review

@changeset-bot

changeset-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8c58638

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@bigcommerce/catalyst-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Bundle Size Report

Comparing against baseline from 06775b2 (2026-08-19).

No bundle size changes detected.

Comment thread core/lib/consent-manager/has-consent-for.ts Outdated
currency,
}) => {
const format = getFormatter();

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.

This is the actual e2e test fix: we can just set the cookie explicitly for the test that we need

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Unlighthouse Performance Comparison — Vercel

Comparing PR preview deployment Unlighthouse scores vs production Unlighthouse scores.

Summary Score

Aggregate score across all categories as reported by Unlighthouse.

Prod Desktop Prod Mobile Preview Desktop Preview Mobile
Score 90 93 91 95

Category Scores

Category Prod Desktop Prod Mobile Preview Desktop Preview Mobile
Performance 77 89 69 76
Accessibility 95 95 95 98
Best Practices 100 100 100 100
SEO 88 88 88 100

Core Web Vitals

Metric Prod Desktop Prod Mobile Preview Desktop Preview Mobile
LCP 3.6 s 3.7 s 4.2 s 6.6 s
CLS 0 0 0.037 0.011
FCP 1.2 s 1.2 s 1.2 s 1.2 s
TBT 0 ms 10 ms 10 ms 0 ms
Max Potential FID 40 ms 80 ms 60 ms 40 ms
Time to Interactive 3.7 s 3.7 s 6.5 s 6.6 s

Full Unlighthouse report →

return;
}

saveConsents('all');

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.

If cookie consent is disabled in the control panel, act like the user "accepts all".

@parthshahp parthshahp Aug 4, 2026

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.

On second thought; this might not be such a good idea: https://c15t.com/docs/shared/react/hooks/use-consent-manager/setting-consent

From the documentation, it looks like saveConsents stores the value in localstorage. If a merchant decides to later enable cookie consent, this "fake consent" may still be stored in localstorage for the user.

(Edit: this is a rare case we needn't account for)

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.

Are we planning to do something about this?

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.

Nope, we probably don't really need to account for this. The consent cookie has expiration anyway

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.

This is what I came up with to resolve the issue of:

  1. Consent cookies are disabled.
  2. Shopper goes to site and consent is granted for all scope.
  3. Consent cookies are enabled.
  4. Shopper goes to site, but does not see banner because consent is granted for all scope still.

I'm not sure if this is a real scenario that we should consider or not.

Another solution here would be to just call isCookieConsentEnabled on the set currency code server action. We can also cache this request, and we need to remember to call it for any server action that may also depend on consent cookies (like analytics).

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.

Thinking more about it I don't really like this localstorage solution, and we should just bite the bullet and make sure to check the consent cookie setting on relevant server actions, but I'll leave this up unchanged to get feedback first.

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 don't think that is a scenario we really need to worry about. Definitely an edge-case, because once a merchant enables it, the consent cookies will expire and will require consent after some time.

@chanceaclark chanceaclark Aug 4, 2026

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.

We store cookie consent both in localstorage and in cookies. You'll need to dive into what c15t does, but we made sure we designed it that way to integrate better with Next.js.

Edit: technically, this method is not GDPR or CCPA compliant but we document this here: https://docs.bigcommerce.com/developer/docs/storefront/catalyst/features/cookie-consent

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.

Ok, so then should I just go with

  1. the saveConsents('all'); approach (if cookie consent is enabled)
  2. make a request on server action specifically for set currency code?

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.

make a request on server action specifically for set currency code?

What is this for?

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.

The specific issue regarding currency code is this:

On the set currency code server action, we check the consent cookie.

Before this PR, we weren't saving a consent cookie even when the consent cookies were disabled, meaning that the currency code wasn't able to be changed.

if (!(await hasConsentFor('functionality'))) {

The current PR's solution to this is to just set the cookie with 'all' scopes when we see that consent cookies are disabled in the control panel.

The alternative I was suggesting to my above solution is: we make an extra request inside of the set currency code server action to check the consent cookies control panel setting on our own (create a new graphql request). If it is set to false, we don't care about the consent cookie.

The issue with the alternative is that it doesn't fix the analytics case (doesn't start visit + deletes visits).

if (!(await hasConsentFor('measurement'))) {

return;
}

saveConsents('all');

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.

Heads up that StartVisitOnConsent won't call startVisit for this path. c15t grants synchronously when enabled: false, so wasMeasurementGranted over there initializes to true — there's no false → true transition to observe. The analytics recovery for first-load is deferred to the second request once this cookie is readable. I think that's fine, mind adding a note to the comment above so it's not mysterious later?

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.

Sure, updated the comment!

@chanceaclark chanceaclark Aug 5, 2026

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.

Well... no it actually shouldn't be a comment. We need to actually make sure it fires whenever cookie consent is turned off.

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.

Answered with this change: #3163 (comment)

Comment thread core/components/consent-manager/persist-auto-granted-consent.tsx
const hasStartedVisit = useRef<boolean | null>(null);

if (hasStartedVisit.current === null) {
hasStartedVisit.current = getConsentCookie()?.['c.measurement'] ?? false;

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.

hasStartedVisit depends on if the cookie exists. We've removed the wasMeasurementGranted ref since that only starts the visit on a false -> true change.

@mfaris9

mfaris9 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

LGTM, but should we run this by legal for this scenario? It might be overkill, but I would hate to cause a GDPR issue:

Day 1: consent disabled -> shopper visits, cookie says "accepted everything"
Day 2: merchant enables consent in the control panel
Day 3: same shopper returns -> cookie already says "accepted", so
no banner shows and they're tracked as if they opted in

Copy link
Copy Markdown
Contributor Author

There's already a comment for this in the PR. TLDR this is an edge case and we have an expiration on the cookie anyway

…sabled

c15t grants every consent category when `cookieConsentEnabled` is off, but only
inside its in-memory store — it never persists them. `hasConsentFor` reads the
consent cookie, so it saw nothing and treated the shopper as having declined,
silently dropping the selected currency and preventing the analytics
visitor/visit cookies from ever being set.

The consent manager now persists that automatic grant, so the consent cookie
stays the single source of truth on both sides and `hasConsentFor` needs no
store lookup — no added work in the proxy, and no added fetches anywhere, since
c15t runs in offline mode and persisting is storage-only.

Known tradeoff: this records a grant the shopper was never asked for. If a
store later enables cookie consent, returning shoppers carrying that cookie
get `showPopup: false` and won't see the banner. Marking the grant and
resetting it on that transition was considered and rejected as more machinery
than the case warrants.

Also seeds an explicit functionality-consent cookie in the two alternate
currency e2e tests so they don't race the client-side grant or depend on the
test store's `cookieConsentEnabled` setting, and moves the consent cookie
builder duplicated in analytics-session.spec.ts into a shared test helper.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants