Updates GTM datalayer to include all documented details, includes consentMethod in all fides events where applicable #4847

eastandwestwind · 2024-04-30T16:06:32Z

Closes https://ethyca.atlassian.net/browse/PROD-2006

Description Of Changes

Updates GTM datalayer to include all details documented in our docs
Includes consentMethod in all fides events where applicable

Code Changes

Pushes additional properties to the datalayer in our GTM helper
Update core Fides event helper to build consentMethod details off the cookie
Add tests

Steps to Confirm

Nav to cookie house or another test page (http://localhost:3000/fides-js-components-demo.html)
Make a consent selection
window.dataLayer entries should include not only your consent vals, but also details like consentMethod where applicable

Pre-Merge Checklist

All CI Pipelines Succeeded
Documentation:
- documentation complete, PR opened in fidesdocs
- documentation issue created in fidesdocs
Issue Requirements are Met
Relevant Follow-Up Issues Created
Update CHANGELOG.md
For API changes, the Postman collection has been updated

vercel · 2024-04-30T16:06:38Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 3, 2024 4:27pm

cypress · 2024-04-30T16:18:47Z

Passing run #7606 ↗︎

0	4	0	0	0

Details:

Merge `3097615` into `221cb05`...
Project: fides	Commit: `fce085dd0f ℹ️`
Status: Passed	Duration: 00:35 💡
Started: May 3, 2024 4:37 PM	Ended: May 3, 2024 4:38 PM

Review all test suite changes for PR #4847 ↗︎

…sentMethod in all fides events where applicable

eastandwestwind · 2024-05-01T16:25:06Z

clients/privacy-center/cypress/e2e/consent.cy.ts

              },
            });

+          // TODO- right now we have to use a wait before our cy.waitUntilFidesInitialized() call which is flakey in CI


we saw a cy.wait working locally, but in CI this sometimes works and sometimes doesn't

I don't really like lingering TODOs like this... I'd prefer you delete this commented out block entirely TBH, the GTM events are pretty well exercised elsewhere.

eastandwestwind · 2024-05-01T16:26:02Z

clients/privacy-center/cypress/e2e/consent-banner-tcf.cy.ts

+        cy.get("@dataLayerPush")
+          .its("secondCall.args.0")
+          .its("Fides.fides_string")
+          .should("contain", ",1~");


partial match and not as part of the assertion on line 681 above, because the fides_string generated is inconsistent

NevilleS

This looks pretty solid, a few tweaks here and I think there might be a surprise bug in the FidesInitialized event fired by the getInitialFides part of the code...

NevilleS · 2024-05-01T19:20:48Z

clients/fides-js/src/integrations/gtm.ts

-interface FidesVariable {
-  consent: NoticeConsent;
-}
+type FidesVariable = FidesEvent["detail"];


✨ I'm glad this worked

NevilleS · 2024-05-01T19:21:21Z

clients/fides-js/src/lib/consent-types.ts

@@ -174,6 +174,7 @@ export type FidesJSIdentity = Record<string, string>;
 * {
 *   "version": "0.9.0",
 *   "createdAt": "2023-01-01T12:00:00.000Z",
+ *   "consentMethod": "accept",


NevilleS · 2024-05-01T19:26:42Z

clients/privacy-center/components/consent/ConfigDrivenConsent.tsx

@@ -79,6 +79,7 @@ const ConfigDrivenConsent = ({
      };
    });
    const cookie: FidesCookie = getOrMakeFidesCookie();
+    cookie.fides_meta.consentMethod = ConsentMethod.SAVE; // include the consentMethod as extra metadata


Oh, interesting, yes this makes sense too... good eye.

NevilleS · 2024-05-01T19:31:20Z

clients/privacy-center/cypress/e2e/consent.cy.ts

@@ -325,9 +331,32 @@ describe("Consent settings", () => {
                  analytics: true,
                  gpc_test: true,
                },
+                extraDetails: undefined,


Hmm- why is this? Shouldn't the cookie include consentMethod: "save" (your test above asserts that)

hm weird. The 2nd init is being populated correctly with "save" but the 1st init is not, and currently trying to debug

Logging it right before we 1st call FidesInitalized is showing up fine with save:

console.log(cookie.fides_meta.consentMethod) dispatchFidesEvent("FidesInitialized", cookie, config.options.debug);

the event log is even looking good, it's just not making it into the dataLayer:

Dispatching event type FidesInitialized with cookie {"consent":{"data_sales":false,"tracking":false,"analytics":true,"gpc_test":true},"identity":{"fides_user_device_id":"9ccb17a8-4104-4968-81a7-db5004350c97"},"fides_meta":{"version":"0.9.0","createdAt":"2024-05-02T10:52:43.981Z","updatedAt":"2024-05-02T10:52:44.367Z","consentMethod":"save"},"tcf_consent":{}} using consent method save

Solved- turns out we have to keep window.Fides in sync more often because Fides.gtm() sometimes runs after the init events. In this case we fire a synthetic event that depends on props off of window.Fides. I've additionally updated our test sites to run Fides.gtm() earlier.

NevilleS · 2024-05-01T19:31:41Z

clients/privacy-center/cypress/e2e/consent.cy.ts

              },
            });

+          // TODO- right now we have to use a wait before our cy.waitUntilFidesInitialized() call which is flakey in CI


I don't really like lingering TODOs like this... I'd prefer you delete this commented out block entirely TBH, the GTM events are pretty well exercised elsewhere.

clients/fides-js/src/lib/events.ts

…consistently, fix tests and CR cleanup

eastandwestwind · 2024-05-02T21:23:34Z

clients/fides-js/src/fides.ts

@@ -117,6 +123,7 @@ async function init(this: FidesGlobal, config: FidesConfig) {
  });
  if (initialFides) {
    Object.assign(this, initialFides);
+    updateWindowFides(this);


Our synthetic GTM event (https://github.com/ethyca/fides/pull/4847/files#diff-a04bb0647e72a17425850d5589eee0a2f72b4f239a0d721899cd15029555be6aR62) requires window.Fides to contain up-to-date properties

This is such a subtle footgun! Gah, the state mgmt in FidesJS has a lot of footguns

agreed. This is the flakiest part of FidesJS

NevilleS

Awesome, thanks!

NevilleS · 2024-05-03T15:44:10Z

clients/fides-js/src/fides.ts

@@ -117,6 +123,7 @@ async function init(this: FidesGlobal, config: FidesConfig) {
  });
  if (initialFides) {
    Object.assign(this, initialFides);
+    updateWindowFides(this);


This is such a subtle footgun! Gah, the state mgmt in FidesJS has a lot of footguns

…sentMethod in all fides events where applicable (#4847)

eastandwestwind added 4 commits May 1, 2024 12:12

Updates GTM datalayer to include all documented details, includes con…

7cac0d5

…sentMethod in all fides events where applicable

adds changelog

e2863c6

bump wait time for GH CI

41b37bc

bump wait time

2c77d44

eastandwestwind force-pushed the PROD-2006 branch from 40e0017 to 2c77d44 Compare May 1, 2024 10:12

eastandwestwind added 3 commits May 1, 2024 12:28

comment out check for init called twice in tests

344549a

use partial str match for fides string test

481853d

format

56c9599

eastandwestwind commented May 1, 2024

View reviewed changes

NevilleS requested changes May 1, 2024

View reviewed changes

eastandwestwind added 2 commits May 2, 2024 16:49

update gtm lib to run sooner on test sites, update window.Fides more …

c862c2d

…consistently, fix tests and CR cleanup

Merge branch 'main' of github.com:ethyca/fides into PROD-2006

1fb87cd

vercel bot deployed to Preview May 2, 2024 14:52 View deployment

eastandwestwind added 2 commits May 2, 2024 16:53

fix additional test

0eb14a3

resolve conflicts

c0bbbc2

vercel bot deployed to Preview May 2, 2024 20:57 View deployment

eastandwestwind requested a review from NevilleS May 2, 2024 21:21

eastandwestwind commented May 2, 2024

View reviewed changes

NevilleS approved these changes May 3, 2024

View reviewed changes

pull latest, fix conflicts

3097615

vercel bot deployed to Preview May 3, 2024 16:27 View deployment

eastandwestwind merged commit bd9f752 into main May 3, 2024
13 checks passed

eastandwestwind deleted the PROD-2006 branch May 3, 2024 16:40

Kelsey-Ethyca mentioned this pull request May 3, 2024

Update changelog for release 2.35.1 #4861

Merged

9 tasks

Kelsey-Ethyca pushed a commit that referenced this pull request May 3, 2024

Updates GTM datalayer to include all documented details, includes con…

eefcbe6

…sentMethod in all fides events where applicable (#4847)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates GTM datalayer to include all documented details, includes consentMethod in all fides events where applicable #4847

Updates GTM datalayer to include all documented details, includes consentMethod in all fides events where applicable #4847

eastandwestwind commented Apr 30, 2024 •

edited

Loading

vercel bot commented Apr 30, 2024 •

edited

Loading

cypress bot commented Apr 30, 2024 •

edited

Loading

eastandwestwind May 1, 2024

NevilleS May 1, 2024

eastandwestwind May 1, 2024

NevilleS left a comment

NevilleS May 1, 2024

NevilleS May 1, 2024

NevilleS May 1, 2024

NevilleS May 1, 2024

eastandwestwind May 2, 2024

eastandwestwind May 2, 2024

eastandwestwind May 2, 2024

eastandwestwind May 2, 2024

NevilleS May 1, 2024

eastandwestwind May 2, 2024

NevilleS May 3, 2024

eastandwestwind May 3, 2024

NevilleS left a comment

NevilleS May 3, 2024

Updates GTM datalayer to include all documented details, includes consentMethod in all fides events where applicable #4847

Updates GTM datalayer to include all documented details, includes consentMethod in all fides events where applicable #4847

Conversation

eastandwestwind commented Apr 30, 2024 • edited Loading

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

vercel bot commented Apr 30, 2024 • edited Loading

cypress bot commented Apr 30, 2024 • edited Loading

Passing run #7606 ↗︎

Review all test suite changes for PR #4847 ↗︎

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NevilleS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NevilleS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eastandwestwind commented Apr 30, 2024 •

edited

Loading

vercel bot commented Apr 30, 2024 •

edited

Loading

cypress bot commented Apr 30, 2024 •

edited

Loading