Skip to content
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

Disable GPP stub when initialize=false is set until Fides.init() is called #5010

Merged
merged 9 commits into from
Jun 20, 2024

Conversation

gilluminate
Copy link
Contributor

@gilluminate gilluminate commented Jun 19, 2024

Closes PROD-2214

Description Of Changes

Initializing GPP without initializing Fides has been known to cause issues, especially in cases where Fides never gets initialized.

  • When FidesJS is downloaded with initialize=false, ensure that GPP is not initialized at all
  • When Fides.init() is called, ensure that the GPP stub is initialized promptly and is not delayed by any other blocking logic (e.g. API calls, etc.)

Code Changes

  • eliminated waste on resulting fides.js response by not printing config object twice.
  • include gppEnabled as part of the config options object (similar to existing tcfEnabled)
  • add new FidesInitializing event for benefit of GPP initialization and other similar use cases
  • wait for FidesInitializing to be dispatched before initializing GPP.
  • Add and clean up related Cypress tests

Steps to Confirm

  1. Open browser console
  2. visit demo page with GPP enabled but initialize set to false (eg. /fides-js-demo.html?gpp=true&initialize=false)
  3. observe browser console message that Fides was not initialized
  4. check if GPP is initialized by attempting to ping it __gpp('ping', (data) => {console.log(data)})
  5. observe browser console error that __gpp does not exist yet.
  6. initialize Fides using the browser console window.Fides.init()
  7. observe browser console debug messages indicating that Fides started normally
  8. check if GPP is initialized by attempting to ping it __gpp('ping', (data) => {console.log(data)})
  9. observe that the ping now displays a response indicating that GPP has been initialized.

image
image

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation:
    • documentation complete
  • Issue Requirements are Met
  • Update CHANGELOG.md

Copy link

vercel bot commented Jun 19, 2024

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 Jun 19, 2024 10:39pm

@gilluminate gilluminate force-pushed the gill/PROD-2214_mediavine-disable-gpp-stub branch from 2ec6b24 to 6a69e36 Compare June 19, 2024 21:33
@gilluminate gilluminate marked this pull request as ready for review June 19, 2024 21:39
Copy link

cypress bot commented Jun 19, 2024

Passing run #8427 ↗︎

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

Details:

Merge 4486259 into 71daa53...
Project: fides Commit: e316bdd04c ℹ️
Status: Passed Duration: 00:40 💡
Started: Jun 19, 2024 10:51 PM Ended: Jun 19, 2024 10:51 PM

Review all test suite changes for PR #5010 ↗︎

Comment on lines -293 to -295
isTestMode // let end-to-end tests set the config and initialize as needed
? ""
: `
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need isTestMode anymore, now that we have intialize=false

isTestMode // let end-to-end tests set the config and initialize as needed
? ""
: `
var fidesConfig = ${fidesConfigJSON};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no longer prints the config twice

Comment on lines -23 to -26
(function () {
if (window.fidesConfig?.experience?.gpp_settings?.enabled)
document.write('<script src="./lib/fides-ext-gpp.js"><\/script>');
})();
Copy link
Contributor Author

@gilluminate gilluminate Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handled using query params now that gppEnabled is on the config object

Copy link
Contributor

@NevilleS NevilleS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Thanks!

@gilluminate gilluminate merged commit e58c65b into main Jun 20, 2024
13 checks passed
@gilluminate gilluminate deleted the gill/PROD-2214_mediavine-disable-gpp-stub branch June 20, 2024 00:24
gilluminate added a commit that referenced this pull request Jun 20, 2024
Copy link

cypress bot commented Jun 20, 2024

Passing run #8428 ↗︎

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

Details:

Disable GPP stub when `initialize=false` is set until `Fides.init()` is called (...
Project: fides Commit: e58c65b030
Status: Passed Duration: 00:32 💡
Started: Jun 20, 2024 12:35 AM Ended: Jun 20, 2024 12:36 AM

Review all test suite changes for PR #5010 ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants