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

1117 Support and implement additional configs through Fides.init() #4262

Merged
merged 11 commits into from
Oct 17, 2023

Conversation

eastandwestwind
Copy link
Contributor

@eastandwestwind eastandwestwind commented Oct 12, 2023

Closes https://github.com/ethyca/fidesplus/issues/1117

Description Of Changes

Adds support for new config options, using Fides.init(), for exaxmple:

window.Fides.init({
    consent: {
      options: [{
         fidesEmbed: true,
         fidesDisableSaveApi: true,
         fidesString: "",
      }]
    },
});

Details:

  1. Supports a fides_embed boolean config option. If this option is true, we "embed" the fides.js overlay UI (ie. “Layer 2”) into a web page instead of as a pop-up overlay, and never render the banner (ie. “Layer 1”). That page will be loaded in either the website or via a native app webview for managing privacy preferences instead of in a pop-up overlay. Use <div id="fides-embed-container"></div> as the parent DOM node for the embedded overlay.
  2. Supports a fides_disable_save_api boolean config option. If this option is true, we should disable the save to API, but still trigger the FidesUpdated event and save preferences to the cookie.
  3. Supports a fides_string config option. If it's set, fides_tc_string should supercede the cookie and any experience preferences, whether pre-fetched or client-side fetched. Importantly, if the fides_tc_string does not define a preference for a given consent item, but the experience defines it, then we set the preference for that item to use the default_preference from experience

Code Changes

  • Adds all new config options to init()
  • Implements all new config options according to the above acceptance criteria
  • Write full-coverage e2e tests to test how user preferences take precedence from the cookie, experience (both client-side fetched and pre-fetched), and explicit fides_string

Steps to Confirm

  • Run e2e tests to confirm: consent-banner-tcf.cy.ts and consent-banner.cy.ts

Pre-Merge Checklist

@cypress
Copy link

cypress bot commented Oct 12, 2023

Passing run #4669 ↗︎

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 a2c8a48 into 1e90657...
Project: fides Commit: b04b7a5dc4 ℹ️
Status: Passed Duration: 00:50 💡
Started: Oct 17, 2023 1:33 PM Ended: Oct 17, 2023 1:34 PM

Review all test suite changes for PR #4262 ↗︎

Copy link
Contributor

@allisonking allisonking left a comment

Choose a reason for hiding this comment

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

whew, nice work @eastandwestwind , lots of new features here! left some comments, but I don't think any of them are that big!

clients/fides-js/src/components/Overlay.tsx Outdated Show resolved Hide resolved
clients/fides-js/src/components/Overlay.tsx Outdated Show resolved Hide resolved
clients/fides-js/src/components/Overlay.tsx Outdated Show resolved Hide resolved
clients/fides-js/src/lib/a11y-dialog.tsx Outdated Show resolved Hide resolved
clients/fides-js/src/components/ConsentModal.tsx Outdated Show resolved Hide resolved
clients/fides-js/src/fides-tcf.ts Outdated Show resolved Hide resolved
clients/fides-js/src/fides-tcf.ts Outdated Show resolved Hide resolved
clients/fides-js/src/lib/tcf/utils.ts Outdated Show resolved Hide resolved
clients/fides-js/src/lib/tcf/utils.ts Outdated Show resolved Hide resolved
clients/fides-js/src/fides-tcf.ts Outdated Show resolved Hide resolved
@eastandwestwind eastandwestwind force-pushed the 1117-additional-config-init branch 2 times, most recently from 9ac7397 to 4310e7e Compare October 13, 2023 15:50
Copy link
Contributor

@allisonking allisonking left a comment

Choose a reason for hiding this comment

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

nice job @eastandwestwind !! I think it's looking good—I added a discussion question which might be worth thinking about, but it doesn't necessarily block this PR.

also, I think this PR also closes #4265 but can you confirm? if it does, go ahead and assign it to yourself :)

clients/fides-js/src/fides-tcf.ts Outdated Show resolved Hide resolved
clients/fides-js/src/fides-tcf.ts Outdated Show resolved Hide resolved
clients/fides-js/src/fides-tcf.ts Show resolved Hide resolved
clients/fides-js/src/lib/consent.ts Show resolved Hide resolved
clients/fides-js/src/lib/cookie.ts Show resolved Hide resolved
clients/fides-js/src/lib/cookie.ts Show resolved Hide resolved
clients/fides-js/src/lib/initialize.ts Outdated Show resolved Hide resolved
@eastandwestwind eastandwestwind merged commit e8bcfcc into main Oct 17, 2023
11 checks passed
@eastandwestwind eastandwestwind deleted the 1117-additional-config-init branch October 17, 2023 15:44
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