Prevent incorrect gppSection being set #4823

gilluminate · 2024-04-22T19:56:45Z

Closes #PROD-1913

Description Of Changes

US National should only ever be a parsed section if the US National approach is selected in Management > Consent

Code Changes

return [] early if approach is set to state and the region is "us"

Steps to Confirm

Enable GPP in Fides by setting the appropriate env vars
Run Fides Plus
Go to Management > Consent
Select the GPP state by state approach
Edit a privacy experience to add a GPP notice
Add “United States” as a location for the notice
Go to the consent test page or cookie house and append a geolocation param for a state that is not part of GPP (US-NY, for example)
Open the browser console and run __gpp('ping', ({parsedSections}) => {console.log(parsedSections)})

You should see an empty array [] instead of usnatv1 in the parsed sections.

Pre-Merge Checklist

All CI Pipelines Succeeded
Issue Requirements are Met
Update CHANGELOG.md

vercel · 2024-04-22T19:56:51Z

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

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	⬜️ Ignored (Inspect)	Visit Preview		Apr 23, 2024 5:37pm

cypress · 2024-04-22T20:11:15Z

Passing run #7417 ↗︎

0	4	0	0	0
⚠️ You've recorded test results over your free plan limit. Upgrade your plan to view test results.

Details:

Merge `f8850c1` into `3cd817f`...
Project: fides	Commit: `9ce99841b6 ℹ️`
Status: Passed	Duration: 00:36 💡
Started: Apr 23, 2024 5:51 PM	Ended: Apr 23, 2024 5:51 PM

Review all test suite changes for PR #4823 ↗︎

gilluminate · 2024-04-22T23:57:57Z

clients/fides-js/__tests__/lib/gpp/us-notices.ts

@@ -76,7 +76,7 @@ const mockPrivacyExperience = (override?: Partial<PrivacyExperience>) => {
    updated_at: "2023-12-07T22:03:26.052630+00:00",
    gpp_settings: {
      enabled: true,
-      us_approach: GPPUSApproach.STATE,
+      us_approach: GPPUSApproach.NATIONAL,


based on this bug description, all of the tests were running incorrectly by using the state approach. none of these should pass with that enabled!

gilluminate · 2024-04-22T23:58:55Z

clients/fides-js/__tests__/lib/gpp/us-notices.ts

-      gpp_settings: {
-        enabled: true,
-        us_approach: GPPUSApproach.NATIONAL, // But set setting to national
-        mspa_covered_transactions: true,
-        mspa_opt_out_option_mode: true,
-        mspa_service_provider_mode: false,
-        enable_tcfeu_string: true,
-      },


this is the default now

gilluminate · 2024-04-23T00:00:19Z

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

-
-  // Whether the developer forced the inclusion of the GPP extension via query param on the script tag
-  forceGpp: boolean;


with this bug fix, GPP doesn't really need to care if forceGpp is on or not, it will now be correct no matter what.

love that we have less overall code!

gilluminate · 2024-04-23T00:01:35Z

clients/fides-js/src/lib/gpp/us-notices.ts

+  if (
+    !gppSection ||
+    (experienceRegion === "us" && usApproach === GPPUSApproach.STATE)
+  ) {


here's where the bug was fixed

thanks for adding these helpful comments, very easy to follow :)

gilluminate · 2024-04-23T00:02:21Z

clients/fides-js/src/lib/gpp/us-notices.ts

-      (forceGpp && !experience?.gpp_settings) ||
-      experience?.gpp_settings?.us_approach === GPPUSApproach.STATE
-    ) {
-      cmpApi.setApplicableSections([-1]);


the logic for this was moved up where other applicable sections were being set, to avoid confusion

gilluminate · 2024-04-23T00:03:23Z

clients/fides-js/src/lib/gpp/us-notices.ts

+  if (
+    !gppSection ||
+    (experienceRegion === "us" && usApproach === GPPUSApproach.STATE)
+  ) {


same fix needed in both places. we should consider consolidating these 2 methods!

same as above ^

we should at last consolidate a check for experienceRegion being in the US

gilluminate · 2024-04-23T00:23:29Z

clients/fides-js/docs/interfaces/FidesOptions.md

-
-## Forcing GPP Inclusion
-
-In some cases it may be necessary to always support the IAB's Global Privacy Platform (GPP), even if a visiting user is from a location without a privacy regulation. Forcing the inclusion of the [GPP API](/docs/tutorials/consent-management/consent-management-configuration/cmp-javascript-apis#gpp-api) can be accomplished by including query parameter `gpp=true` on the FidesJS script:
-
-```
-<script src="path/to/fides.js?gpp=true"></script>
-```
-
-When the GPP API is included this way, the `applicableSections` property is set to `[-1]` whenever a user visits a page from a non-supported location.


had to move this. can't do arbitrary documentation here, since it gets auto-generated from the fides-js jsdocs. Moved this to a direct add in the fidesdocs repo.

ah right. This is always a point of confusion actually. Where to add docs / what will get autogenerated etc 😢

gilluminate · 2024-04-23T00:35:40Z

clients/fides-js/src/fides-ext-gpp.ts

+      if (!tcSet && !sectionsSet.length && !sectionsChanged.length) {
+        cmpApi.setApplicableSections([-1]);
+      }


logic moved here

gilluminate · 2024-04-23T00:36:45Z

clients/fides-js/src/lib/gpp/us-notices.ts

+  if (
+    experienceRegion?.toLowerCase().startsWith("us") &&
+    usApproach === GPPUSApproach.NATIONAL
+  ) {


non-US regions were slipping through if force gpp was true. making sure it's a U.S. region before forcing this setting.

is there a better / more robust way to check for "US" regions? If not can we at least add a shared method / util for this so it's more documented?

eastandwestwind

Thanks for adding unit tests! Would like to just consolidate / clarify some logic, see notes below.

eastandwestwind · 2024-04-23T13:26:24Z

clients/fides-js/docs/interfaces/FidesOptions.md

-
-## Forcing GPP Inclusion
-
-In some cases it may be necessary to always support the IAB's Global Privacy Platform (GPP), even if a visiting user is from a location without a privacy regulation. Forcing the inclusion of the [GPP API](/docs/tutorials/consent-management/consent-management-configuration/cmp-javascript-apis#gpp-api) can be accomplished by including query parameter `gpp=true` on the FidesJS script:
-
-```
-<script src="path/to/fides.js?gpp=true"></script>
-```
-
-When the GPP API is included this way, the `applicableSections` property is set to `[-1]` whenever a user visits a page from a non-supported location.


ah right. This is always a point of confusion actually. Where to add docs / what will get autogenerated etc 😢

eastandwestwind · 2024-04-23T13:29:11Z

clients/fides-js/src/lib/gpp/us-notices.ts

+  if (
+    !gppSection ||
+    (experienceRegion === "us" && usApproach === GPPUSApproach.STATE)
+  ) {


thanks for adding these helpful comments, very easy to follow :)

eastandwestwind · 2024-04-23T16:14:46Z

clients/fides-js/src/lib/gpp/us-notices.ts

+  if (
+    experienceRegion?.toLowerCase().startsWith("us") &&
+    usApproach === GPPUSApproach.NATIONAL
+  ) {


is there a better / more robust way to check for "US" regions? If not can we at least add a shared method / util for this so it's more documented?

eastandwestwind · 2024-04-23T16:19:06Z

clients/fides-js/src/lib/gpp/us-notices.ts

-    }
+  if (
+    !gppSection ||
+    (experienceRegion === "us" && usApproach === GPPUSApproach.STATE)


is this going to work if experienceRegion is something like "us_ny"? Can we follow the same approach we use on L 56?

experienceRegion here is the result of what's happening in L 56 so "us_ny" gets converted to "us" up there.

oh crap! I just realized that's not the case. This needs to be gppRegion!

eastandwestwind · 2024-04-23T16:19:30Z

clients/fides-js/src/lib/gpp/us-notices.ts

+  if (
+    !gppSection ||
+    (experienceRegion === "us" && usApproach === GPPUSApproach.STATE)
+  ) {


same as above ^

eastandwestwind · 2024-04-23T16:19:54Z

clients/fides-js/src/lib/gpp/us-notices.ts

+  if (
+    !gppSection ||
+    (experienceRegion === "us" && usApproach === GPPUSApproach.STATE)
+  ) {


we should at last consolidate a check for experienceRegion being in the US

eastandwestwind · 2024-04-23T16:20:33Z

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

-
-  // Whether the developer forced the inclusion of the GPP extension via query param on the script tag
-  forceGpp: boolean;


love that we have less overall code!

eastandwestwind · 2024-04-23T16:21:07Z

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

@@ -356,7 +356,7 @@ describe("Fides-js GPP extension", () => {
              supportedAPIs,
            } = data.pingData;
            expect(signalStatus).to.eql("ready");
-            expect(applicableSections).to.eql([]);
+            expect(applicableSections).to.eql([-1]);


gilluminate force-pushed the PROD-1913-gpp-us-national-section-should branch from 5daac54 to c59c876 Compare April 22, 2024 19:59

gilluminate force-pushed the PROD-1913-gpp-us-national-section-should branch 2 times, most recently from e9c1d5f to 56ad9b2 Compare April 22, 2024 20:37

Update CHANGELOG.md

c4b4214

gilluminate force-pushed the PROD-1913-gpp-us-national-section-should branch from 56ad9b2 to c4b4214 Compare April 22, 2024 20:50

gilluminate marked this pull request as draft April 22, 2024 21:05

GPPSection type definition

293b28e

gilluminate force-pushed the PROD-1913-gpp-us-national-section-should branch 2 times, most recently from ff5215b to 5c4bc47 Compare April 22, 2024 22:39

consistency updates

d7785ad

gilluminate force-pushed the PROD-1913-gpp-us-national-section-should branch 2 times, most recently from 1e9daa3 to d7785ad Compare April 22, 2024 22:54

gilluminate added 2 commits April 22, 2024 16:59

no sections means applicableSections = [-1]

bca3045

fix for non-us and for state approaches. update tests

c3c0d0a

gilluminate commented Apr 23, 2024

View reviewed changes

gilluminate marked this pull request as ready for review April 23, 2024 00:03

fixes

d3db38b

gilluminate commented Apr 23, 2024

View reviewed changes

gilluminate requested a review from eastandwestwind April 23, 2024 00:37

eastandwestwind requested changes Apr 23, 2024

View reviewed changes

updates and fixes based on CR

a6fd287

eastandwestwind approved these changes Apr 23, 2024

View reviewed changes

gilluminate added 2 commits April 23, 2024 11:20

extract region check as documented util funciton

3a2e9af

Merge branch 'main' into PROD-1913-gpp-us-national-section-should

1d909e3

vercel bot deployed to Preview April 23, 2024 17:27 View deployment

Update us-notices.ts

f8850c1

gilluminate merged commit d108ebe into main Apr 23, 2024
13 checks passed

gilluminate deleted the PROD-1913-gpp-us-national-section-should branch April 23, 2024 17:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent incorrect gppSection being set #4823

Prevent incorrect gppSection being set #4823

gilluminate commented Apr 22, 2024 •

edited

Loading

vercel bot commented Apr 22, 2024 •

edited

Loading

cypress bot commented Apr 22, 2024 •

edited

Loading

gilluminate Apr 22, 2024

gilluminate Apr 22, 2024

gilluminate Apr 23, 2024

eastandwestwind Apr 23, 2024

gilluminate Apr 23, 2024

eastandwestwind Apr 23, 2024

gilluminate Apr 23, 2024

gilluminate Apr 23, 2024

eastandwestwind Apr 23, 2024

eastandwestwind Apr 23, 2024

gilluminate Apr 23, 2024

eastandwestwind Apr 23, 2024

gilluminate Apr 23, 2024

gilluminate Apr 23, 2024 •

edited

Loading

eastandwestwind Apr 23, 2024 •

edited

Loading

eastandwestwind left a comment •

edited

Loading

eastandwestwind Apr 23, 2024

eastandwestwind Apr 23, 2024

eastandwestwind Apr 23, 2024 •

edited

Loading

eastandwestwind Apr 23, 2024

gilluminate Apr 23, 2024

gilluminate Apr 23, 2024

eastandwestwind Apr 23, 2024

eastandwestwind Apr 23, 2024

eastandwestwind Apr 23, 2024

eastandwestwind Apr 23, 2024


		// Whether the developer forced the inclusion of the GPP extension via query param on the script tag
		forceGpp: boolean;

Prevent incorrect gppSection being set #4823

Prevent incorrect gppSection being set #4823

Conversation

gilluminate commented Apr 22, 2024 • edited Loading

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

vercel bot commented Apr 22, 2024 • edited Loading

cypress bot commented Apr 22, 2024 • edited Loading

Passing run #7417 ↗︎

Review all test suite changes for PR #4823 ↗︎

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

gilluminate Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

eastandwestwind Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

eastandwestwind left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eastandwestwind Apr 23, 2024 • edited Loading

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

gilluminate commented Apr 22, 2024 •

edited

Loading

vercel bot commented Apr 22, 2024 •

edited

Loading

cypress bot commented Apr 22, 2024 •

edited

Loading

gilluminate Apr 23, 2024 •

edited

Loading

eastandwestwind Apr 23, 2024 •

edited

Loading

eastandwestwind left a comment •

edited

Loading

eastandwestwind Apr 23, 2024 •

edited

Loading