-
Notifications
You must be signed in to change notification settings - Fork 80
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
Add notice consent to Fides string #5895
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
2eb6843
to
e280bcf
Compare
fefe384
to
b714d57
Compare
3cb239f
to
69574de
Compare
78268ad
to
771e970
Compare
771e970
to
53af48c
Compare
}); | ||
}); | ||
|
||
describe("shouldResurfaceBanner", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this method already existed, but I enhanced it quite a bit and used Cursor to write these tests.
Note: The AC_STRING can only exist if TC_STRING exists, as it's derived from it. | ||
When GPP is enabled, the GPP_STRING portion is automatically initialized during | ||
FidesJS initialization, either preserving any existing GPP string or using a | ||
default value. The GPP_STRING is independent and can exist with or without the | ||
other strings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved a lot of the docs for setting the string to the FidesOptions docs since that's more appropriate. These docs here are mostly for reading the string.
clients/fides-js/rollup.config.mjs
Outdated
@@ -16,7 +16,7 @@ const GZIP_SIZE_ERROR_KB = 45; // fail build if bundle size exceeds this | |||
const GZIP_SIZE_WARN_KB = 35; // log a warning if bundle size exceeds this | |||
|
|||
// TCF | |||
const GZIP_SIZE_TCF_ERROR_KB = 87; | |||
const GZIP_SIZE_TCF_ERROR_KB = 90; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're not at this max size yet, but we're getting close to it. Seemed appropriate to keep this more general rather than bumping it with every edit.
[cookie, savedConsent, experience, options, isAutomatedConsent], | ||
); | ||
const showBanner = useMemo(() => { | ||
return shouldResurfaceBanner(experience, cookie, savedConsent, options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all of the boolean logic was moved in to the method for reuse and stability.
propertyId, | ||
}) => { | ||
const { i18n, currentLocale, setCurrentLocale } = useI18n(); | ||
const parsedCookie: FidesCookie | undefined = getFidesConsentCookie(); | ||
const savedConsent = window.Fides.saved_consent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eastandwestwind I made sure the OT migration tests still pass with this change, but with the passed in savedConsent
it was missing any GPC updates that happened between initialization and the overlay updating here. As you may recall, we run the GPC logic asynchronously after initialize()
which is where the params on this component get set.
if (!this.cookie) { | ||
throw new Error("Should have a cookie"); | ||
} | ||
return shouldResurfaceConsent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved this logic in to the method as well
export const shouldResurfaceConsent = ( | ||
experience: PrivacyExperience | PrivacyExperienceMinimal, | ||
cookie: FidesCookie, | ||
export const shouldResurfaceBanner = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed for clarity
// TODO (PROD-1792): we should *also* resurface in the special case where the | ||
// saved consent is only recorded with a consentMethod of "dismiss" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since I was adding logic for GPC, I knocked this TODO out at the same time :)
consent: NoticeConsent | undefined, | ||
): boolean => { | ||
if (notice.consent_mechanism === ConsentMechanism.NOTICE_ONLY) { | ||
return true; | ||
} | ||
// Note about GPC - consent has already applied to the cookie at this point, so we can trust preference there | ||
// DEFER (PROD-1780): delete context arg for safety |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confirmed this was no longer used
@@ -209,7 +209,8 @@ export const fidesStringToConsent = ({ | |||
fidesString, | |||
cmpApi, | |||
}: FidesStringToConsentArgs) => { | |||
if (!fidesString || !cmpApi) { | |||
const { gpp: gppString }: DecodedFidesString = decodeFidesString(fidesString); | |||
if (!fidesString || !gppString || !cmpApi) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor optimization here, no reason to continue decoding the GPP string if one wasn't passed in.
@@ -258,8 +258,17 @@ const NoticeOverlay: FunctionComponent<OverlayProps> = ({ | |||
}, [cookie, options.debug]); | |||
|
|||
const handleDismiss = useCallback(() => { | |||
handleUpdatePreferences(ConsentMethod.DISMISS, initialEnabledNoticeKeys()); | |||
}, [handleUpdatePreferences, initialEnabledNoticeKeys]); | |||
if (!consentCookieObjHasSomeConsentSet(parsedCookie?.consent)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This addressed a bug where if someone returns to the page, loads the modal from the link, and then closes the modal, their previous consent method was changed to dismissed even though no changes were made.
@@ -460,8 +466,10 @@ export const TcfOverlay = ({ | |||
}, [cookie, options.debug]); | |||
|
|||
const handleDismiss = useCallback(() => { | |||
handleUpdateAllPreferences(ConsentMethod.DISMISS, draftIds); | |||
}, [handleUpdateAllPreferences, draftIds]); | |||
if (!consentCookieObjHasSomeConsentSet(parsedCookie?.consent)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same bug as above, but on the TCF side
3800dd2
to
3808c74
Compare
#### Example | ||
|
||
```ts | ||
const encoded = Fides.encodeNoticeConsentString({data_sales_and_sharing:0,analytics:1}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohhh this is so handy, thanks for adding this!
* @param base64String The base64 encoded Notice Consent string | ||
* @returns Decoded consent data object or null if decoding fails | ||
*/ | ||
export const decodeNoticeConsentString = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we also expose this on the Fides obj? I can see it might be a handy util for testing and onboarding customers
const [tc = "", ac = "", gpp = "", nc = ""] = | ||
fidesString.split(FIDES_SEPARATOR); | ||
// If there's no TC, remove AC | ||
return tc ? { tc, ac, gpp, nc } : { tc: "", ac: "", gpp, nc }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's the formatFidesStringWithGpp
method in this file- I think we still need to support nc
within this too
Closes LJ-305
Description Of Changes
Introduces Notice Consent String functionality to the FidesJS client, allowing programmatic control of notice consent preferences through a base64-encoded string. The changes include:
Code Changes
encodeNoticeConsentString
anddecodeNoticeConsentString
utility functions and exposedencodeNoticeConsentString
to the Fides window object.DecodedFidesString
interface to includenoticeConsent
fieldinitialize
to handle Notice Consent string applicationcontext
parameter fromresolveConsentValue
Steps to Confirm
window.Fides.encodeNoticeConsentString()
to get a Notice Consent String. Use the keys for the consent notices configured in step 1. Get the encoded string for various combinations of true/false values for those.fides_string=...
query string, and use each encoded Notice Consent string noted above as the value in the fourth slot of the comma separated Fides String (eg.fides_string=,,,eyJkYXRhX3NhbGVzX2FuZF9zaGFyaW5nIjowfQ==
)consent
value is correct and theconsentMethod
should be "script"api/v1/privacy-preferences
endpoint is correct.Pre-Merge Checklist
CHANGELOG.md
updated