New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STRF-4612 only show cookie privacy notice for EU IP addresses #1381

Merged
merged 1 commit into from Dec 10, 2018

Conversation

Projects
None yet
6 participants
@bookernath
Copy link
Contributor

bookernath commented Nov 9, 2018

What?

Depends on bigcommerce/bigcommerce#27154 in which we surface a new key in the context indicating if the shopper has an EU IP address.

This will only show the cookie notification for those shoppers, so that you don't bug shoppers when you aren't legally required to.

Leaving this in the theme code allows the behavior to be further customized by a theme developer if desired.

@bigbot

This comment has been minimized.

Copy link

bigbot commented Nov 9, 2018

@bookernath

This comment has been minimized.

Copy link
Contributor Author

bookernath commented Nov 19, 2018

@carsonreinke did you have feedback on this change?

@junedkazi
Copy link
Contributor

junedkazi left a comment

Please add a changelog before we merge this PR.

@bookernath bookernath force-pushed the bookernath:STRF-4612-eu branch from 82c0d1c to afa8e65 Nov 19, 2018

@bookernath bookernath changed the title (WIP/DO NOT MERGE) STRF-4612 only show cookie privacy notice for EU IP addresses STRF-4612 only show cookie privacy notice for EU IP addresses Nov 19, 2018

@bookernath bookernath force-pushed the bookernath:STRF-4612-eu branch 2 times, most recently from 0ed286c to c4c8476 Nov 19, 2018

@carsonreinke

This comment has been minimized.

Copy link

carsonreinke commented Nov 20, 2018

@bookernath Can't you be a someone from the EU and not be in the EU? I feel like it is less confusing, either you work with the a country requiring this or you don't. If you do want to add this, maybe make it a bit more flexible for other filtering in the future asking for consent.

@bookernath

This comment has been minimized.

Copy link
Contributor Author

bookernath commented Nov 20, 2018

@carsonreinke thanks for your feedback.

The EU cookie law (to the best of my understanding) primarily applies to 1. EU-owned websites and 2. websites targeted at EU citizens. So when we think of the best default behavior of our platform (of which Cornerstone is a representation) it makes sense to only bug shoppers when absolutely necessary - when they are browsing from the EU.

An EU-owned or EU-targeted business would be best advised to modify this behavior to target a broader range of shoppers, to your point.

I had considered a future enhancement in which this would be a toggle-able setting in theme editor - what do you think about that?

@carsonreinke

This comment has been minimized.

Copy link

carsonreinke commented Nov 27, 2018

@bookernath Just because you are a citizen, does not mean you are in the physical location. A EU targeted website should not try and figure who is a citizen or not, it just needs to be on.

I could not find anything on people actually doing this.

@xenph

This comment has been minimized.

Copy link

xenph commented Nov 27, 2018

The law doesn't target EU citizens in non EU countries. Local law applies in that case. It's correct to target EU located IP addresses.

@carsonreinke

This comment has been minimized.

Copy link

carsonreinke commented Nov 27, 2018

@xenph You might be right. I guess if there some precedence for this somewhere, please send that over. It just sounds like the wrong path.

@xenph

This comment has been minimized.

Copy link

xenph commented Nov 27, 2018

Hi, I'm Christopher Beckett, BigCommerce's Data Privacy Leader 😄 If you want a further view of EDPB's recent guidance regarding the territorial scope of the GDPR, please read this: https://edpb.europa.eu/sites/edpb/files/files/file1/edpb_guidelines_3_2018_territorial_scope_en.pdf. However, typically I read these so you don't have to 😄

@carsonreinke

This comment has been minimized.

Copy link

carsonreinke commented Nov 28, 2018

Thanks @xenph, appreciate the feedback and sorry for the noise.

@flair-duncan

This comment has been minimized.

Copy link
Contributor

flair-duncan commented Nov 28, 2018

@bookernath As you're already doing a GeoIP lookup to generate settings.is_eu_ip_address could you expose additional data in a new location object, i.e. continent, country and city?

Could then do something like {{#if settings.location.continent '===' 'eu'}} and store owners/developers would benefit from the additional location data available without having to rely on third party services...

🤔 Just an idea 🙄

@bookernath

This comment has been minimized.

Copy link
Contributor Author

bookernath commented Nov 28, 2018

@flair-duncan I think that's a great idea; due to some stuff happening on the backend it would be a little bit more of a project than this change (we have an existing method to calculate "is in the EU"), but I'm willing to consider it for the future. We're kicking it around internally now.

@bookernath bookernath force-pushed the bookernath:STRF-4612-eu branch 2 times, most recently from 78dda06 to a10fe2d Nov 28, 2018

@junedkazi

This comment has been minimized.

Copy link
Contributor

junedkazi commented Dec 1, 2018

@bookernath are we waiting on anything to get this merged ? Also the PR will need a rebase.

@bookernath

This comment has been minimized.

Copy link
Contributor Author

bookernath commented Dec 10, 2018

@junedkazi we're waiting for bigcommerce/bigcommerce#27154 to hit production.

@bookernath bookernath force-pushed the bookernath:STRF-4612-eu branch from a10fe2d to 3fe50cb Dec 10, 2018

@junedkazi

This comment has been minimized.

Copy link
Contributor

junedkazi commented Dec 10, 2018

I think it is fine to merge it bcoz we don't plan on releasing cornerstone until we have all the backend changes rolled out completely.

@bookernath

This comment has been minimized.

Copy link
Contributor Author

bookernath commented Dec 10, 2018

Sure, let's do it!

@junedkazi junedkazi merged commit 8cad23c into bigcommerce:master Dec 10, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@bookernath bookernath deleted the bookernath:STRF-4612-eu branch Dec 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment