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

PROD-2599 Update PC to support hierarchical consent notices + Update all types #5291

Conversation

lucanovera
Copy link
Contributor

@lucanovera lucanovera commented Sep 17, 2024

Description Of Changes

Implement hierarchical relationships for Privacy Center consent. Update all openapi generated types, fix type issues.

BE branch needed: https://github.com/ethyca/fidesplus/pull/1602
FE admin-ui branch: #5264

Note for reviewer: You'll probably want to ignore the autogenerated types changes that are in the types/models folder. Also you'll see some type fixes, casting, adding ? !. Basically PC was using a set of hardcoded minimal types that were very outdated AND it still supports a "config-file" driven consent that doesn't use the api. The goal was to get everything updated and typescript working, but it will definitely look cleaner once we deprecate the config consent mode.

Code Changes

  • Modified NoticeDrivenConsent.tsx to handle notice children
  • Added ConsentItemAccordion
  • Updated UI to new designs, improved responsive experience
  • Updated all types generated with openapi:generate, fixed any conflicts between types

Screenshots

Screen Shot 2024-09-18 at 09 53 35

Screen Shot 2024-09-18 at 09 53 43

Steps to Confirm

Admin UI pre-requesites

  • Follow the steps from the admin-ui pr to create a notice with some notice children PROD-2476: Support hierarchical notices in Admin-UI #5264
  • Make sure both parent notices and children notices are enabled
  • Go to Experiences and edit the Privacy Center Experience, make sure the parent notice is added to the experience

Privacy Center

  • Go to the consent screen for privacy center
  • Check that when you open the accordion for the parent notice, the children notices are displayed too
  • Check that the toggle behavior is correct
    • When any children is toggle, parent should be toggled
    • When parent is toggle off, all children should toggle off
    • When parent is toggle on, all children should toggle on
  • Open browser dev tools, the network tab and then save your consent preferences. Check that the privacy-preferences request includes parent and child notices
  • Open the consent page again with dev tools still open. Check that the notices-served request includes the parent and child notices.
  • Check that the consent page is showing all toggles in the same state as it was when you saved it. This means the cookies were saved and the loaded correctly.

Pre-Merge Checklist

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

Copy link

vercel bot commented Sep 17, 2024

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 Sep 24, 2024 4:06pm

Lucano Vera added 2 commits September 17, 2024 10:23
…-Center-Update-consent-management-screen-to-render-hierarchical-notices
@lucanovera lucanovera changed the title PROD-2599 privacy center update consent management screen to render hierarchical notices PROD-2599 Update PC to support hierarchical consent notices + Update all types Sep 17, 2024
Copy link

cypress bot commented Sep 17, 2024

fides    Run #10090

Run Properties:  status check passed Passed #10090  •  git commit 7f6d45cbc2 ℹ️: Merge 9d8ebc1fbed092ad1d9813031449eff5d0eee977 into 0a0ea4f400ca6d1e10ad3b154f90...
Project fides
Branch Review refs/pull/5291/merge
Run status status check passed Passed #10090
Run duration 00m 40s
Commit git commit 7f6d45cbc2 ℹ️: Merge 9d8ebc1fbed092ad1d9813031449eff5d0eee977 into 0a0ea4f400ca6d1e10ad3b154f90...
Committer Lucano Vera
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

Copy link
Contributor

@eastandwestwind eastandwestwind 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 looking really great @lucanovera !

Let's just confirm with Jack about the toggle functionality, but otherwise we're good to go here

Lucano Vera added 2 commits September 24, 2024 13:00
@lucanovera
Copy link
Contributor Author

This is looking really great @lucanovera !

Let's just confirm with Jack about the toggle functionality, but otherwise we're good to go here

Thanks for the review! Jack confirmed the toggle behavior after watching the Loom demo, so we should be good to go.

Copy link
Contributor

@eastandwestwind eastandwestwind left a comment

Choose a reason for hiding this comment

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

lgtm!

@lucanovera lucanovera merged commit ff63603 into main Sep 24, 2024
13 checks passed
@lucanovera lucanovera deleted the PROD-2599-Privacy-Center-Update-consent-management-screen-to-render-hierarchical-notices branch September 24, 2024 20:43
Copy link

cypress bot commented Sep 24, 2024

fides    Run #10101

Run Properties:  status check passed Passed #10101  •  git commit ff63603425: PROD-2599 Update PC to support hierarchical consent notices + Update all types (...
Project fides
Branch Review main
Run status status check passed Passed #10101
Run duration 00m 39s
Commit git commit ff63603425: PROD-2599 Update PC to support hierarchical consent notices + Update all types (...
Committer Lucano Vera
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

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