Civic uk: Accept/reject behaviour #10685
Conversation
|
Size Change: +30.6 kB (+3%) Total Size: 907 kB
ℹ️ View Unchanged
|
rcantin-w
changed the title
| <> | ||
| {consentState && ( | ||
| <Head> | ||
| <script dangerouslySetInnerHTML={{ __html: segmentSnippet }} /> |
There was a problem hiding this comment.
I originally also put the GTM script in this conditional, but I was experiencing what I think is this Next bug where on loading it/removing it/re-adding it, random meta tags would duplicate themselves, even if they had a key. Someone mentioned that for them it only happened when GTM was enabled, and it does seem to be the case here as well.
I think it's ok though as we set the consent to denied, so no hits are being sent.
There was a problem hiding this comment.
so no hits are being sent.
I believe the tag still "phones home" via what the docs describe as a "cookie-less ping".
There is the possibly contentious case for adding the GTM script before consent that we still record some analytics data about these users in a legally compliant manner (so long as we set the consent state in the data layer correctly). See https://support.google.com/tagmanager/answer/13802165?sjid=2210990408479536825-EU for more details about what gets communicated to Google in the situation consent is denied.
See "Behavioral modeling for consent mode" for one of the "features" available to us. Notably modelled data for un-consented visitors is not sent via any BigQuery integration.
We should be sure we're comfortable with this, and check with @LaurenFBaily.
There was a problem hiding this comment.
Had a look at this and the documentation, I'm happy with that.
rcantin-w
changed the title
| }; | ||
|
|
||
| // We send toggles as an event parameter to GA4 so we can determine the condition in which a particular event took place. | ||
| // GA4 now limits event parameter values to 100 characters: https://support.google.com/analytics/answer/9267744?hl=en | ||
| // So instead of sending the whole toggles JSON blob, we only look at the "test" typed toggles and send a concatenated string made of the toggles' name | ||
| // , preceeded with a! if its value is false. | ||
| function createToggleString(toggles: Toggles | undefined): string | null { | ||
| function createABToggleString(toggles: Toggles | undefined): string | null { |
There was a problem hiding this comment.
From what I gathered, this only returns AB test strings, so I renamed to clarify.
| @@ -70,11 +70,11 @@ const CivicUK = () => ( | |||
| '__utmv', | |||
| ], | |||
| onAccept: function () { | |||
| const event = new CustomEvent('analyticsConsentChange', {}); | |||
| const event = new CustomEvent('analyticsConsentChanged', { detail: { consent: 'granted' }}); | |||
| @@ -162,14 +162,22 @@ const PageLayoutComponent: FunctionComponent<Props> = ({ | |||
| <> | |||
| <Head> | |||
| <title>{fullTitle}</title> | |||
| <meta name="description" content={description} /> | |||
| <meta key="metadescription" name="description" content={description} /> | |||
There was a problem hiding this comment.
Would it be useful to add a comment linking to the next issue describing why we're adding key here?
There was a problem hiding this comment.
I think not here, as the Next issue is saying that this should work but doesn't. It's a recommendation from their official documentation for next/head, so "normal".
If we wanted to keep track of that issue I could add it to where we load the GTM script and explain that's why we're not removing it?
| } else { | ||
| // If the feature flag is ON but consent has yet to be defined | ||
| return false; | ||
| } | ||
| } else { | ||
| return true; |
There was a problem hiding this comment.
In the future we'll need the default no consent state to be false, worth adding a comment to be double sure we do that?
There was a problem hiding this comment.
When we remove the toggle and therefore the first condition, and only keep the second condition, with a change to the comments, that's what it'll do 👍
if (consentCookie !== undefined) {
const civicUKCookie: CivicUKCookie = JSON.parse(
decodeURIComponent(consentCookie)
);
return civicUKCookie.optionalCookies?.analytics === 'accepted';
} else {
// If consent has yet to be defined, return false by default
return false;
}
}
We'll for sure test that a lot, it's why we'll requisition e2e for a bit, with all the toggle stuff out of the way 🤞
| data.toggles?.cookiesWork?.value | ||
| ? `function gtag(){window.dataLayer.push(arguments);} | ||
|
|
||
| gtag('consent', 'default', { |
There was a problem hiding this comment.
I think that we need some more fields here to adhere to "consent mode v2": https://developers.google.com/tag-platform/security/guides/consent?consentmode=advanced#upgrade-consent-v2, specifically ad_user_data and ad_personalization.
I expect we'll just hard-code the values to denied though.
There was a problem hiding this comment.
Yess I wasn't sure we used those because adding them caused me TS errors, looks like the gtag helpers don't know about them?
No overload matches this call.
The last overload gave the following error.
Object literal may only specify known properties, and 'ad_personalization' does not exist in type 'ConsentParams'.
That being said, as I'm looking at this, it's only a problem in the TS code when we update the values, and not on the default/initialisation one. And we should only change the analytics_storage value if they allow analytics and the others should be denied from the get go, including ad_storage, which I currently mark as granted if analytics is. So I'll change that to add everything to the consent initialisation and only change analytics_storage on consent given.
If ever we want those to be changeable, we'll deal with TS errors then. Not a problem for now. I'll stop thinking out loud and apply changes 👍
There was a problem hiding this comment.
We can review this on Monday but changes applied:
(default setting)
(on rescinding consent)
Who is this for?
Cookie work, #10657
What is it doing for them?
common/services/app/civic-uk.tsdetailproperty that gives us the new value, which can then be used forgtag()consent.ConsentAndScriptscomponent that is loaded in_appand fires the Civic UK banner as well as Segment's script if it has consent to be there. See comment I left to explain about the GTM script which was there originally but I had to remove.dataGtmTriggeron our preference centre button so it could be our test on GTM tracking.keyson meta tags as they were getting doubled up (again, see comment I mentioned above). This actually changed nothing because of what I think is an identified bug, but I thought I'd leave them there as other meta tags already had them and it's recommended to avoid multiple tags.For whoever reviews
I reckon the best thing is (1) review code but (2) let's have a call and go over tag manager tracking tests/previews.