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

Executable consent types for the privacy center #2193

Merged
merged 17 commits into from
Jan 12, 2023

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Jan 10, 2023

Closes #2122

Code Changes

  • Set default config.json executables to false, both in the privacy center directory, and the test_env privacy center directory
  • Remove load_executable_consent_options and references
  • Pass policy_key and a list of executable_options from the frontend to /consent-request/{{consent_request_id}}/preferences
  • Update queue_privacy_request_to_propagate_consent to make PrivacyRequest objs based on the list of passed in executable_options
  • Add an error message when there are multiple executable: true in the config.json

Steps to Confirm

  • Update test_env/privacy_center_config/config.json to have one consent option be executable: true
  • nox -s test_env
  • At localhost:3001, change your consent preferences. You should see the a new payload with executable_options sent to the endpoint if you look in the Network tab
  • The privacy request should be saved with all of the consent preferences listed in config.json
  • Update test_env/privacy_center_config/config.json to have more than one consent option be executable: true
  • nox -s test_env
  • Upon loading the privacy center, you should see an error message that you can't have more than one consent option set to executable

Pre-Merge Checklist

Description Of Changes

Endpoint now takes executable_options and privacy center passes those in
image

And there's also this error message, though I'm not quite sure it belongs in a toast?
image

I think the changes I made are mostly covered by existing test cases, but lemme know if there are other tests I should add!

@allisonking allisonking changed the base branch from main to consent-propagation January 10, 2023 23:04
@allisonking
Copy link
Contributor Author

@pattisdr Still working on this (especially the backend tests) but would appreciate if you think this is the right approach so far!

@allisonking allisonking marked this pull request as ready for review January 11, 2023 21:13
Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

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

Nice changes Allison. A few bugs and questions here.

Responding to:

And there's also this error message, though I'm not quite sure it belongs in a toast?

Hm, this is tricky. I was hoping we could get an invalid config.json to throw an error on next build so the customer is made aware. As-is, this doesn't prevent all executables=true from sending a request.

If we can't prevent it from building, or similar, perhaps they get a message that there's something wrong with the system, and we don't allow the consent request to be saved from the UI in the first place. Just brainstorming here.

@allisonking
Copy link
Contributor Author

Okay the backend fixes are in. Still mulling over how best to show an error when there are multiple executable=trues

@allisonking
Copy link
Contributor Author

Okay, I ended up putting some logic that gets called during the build stage and I think it works! To test:

npm run dev and npm run build should still work

But if you change the config.json such that there is more than one consent option with executable=True, when you npm run dev or npm run build you should see:

npm run dev:
image

npm run build:
image

The build one isn't as clean unfortunately, but it's nice that it runs through the same logic as npm run dev so we don't need separate cases for both.

The only drawback is you won't get immediate feedback when you change your config.json (if you're hot reloading in dev) that it's wrong, but maybe that's okay. If we really want that, we can restore the toast I had earlier

Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

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

@allisonking great work, nice solution with getting the build to fail so there's an early warning. This works great to enable this first iteration to limiting a single data use case to being executable.

@allisonking allisonking merged commit 87a5d46 into consent-propagation Jan 12, 2023
@allisonking allisonking deleted the aking/2122/executable-consent-types branch January 12, 2023 22:01
@pattisdr pattisdr mentioned this pull request Jan 17, 2023
14 tasks
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.

Configure the Privacy Center with which (if any) consent type should be “executable” using connectors
2 participants