-
Notifications
You must be signed in to change notification settings - Fork 78
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
TCF Legal Basis Dimension for Saving Preferences #4201
Conversation
…eferences for purposes, systems, and vendors. Exploring storing the combination of the TCF attribute and the legal basis as a single dimension, so everything can still be saved the same way, without special handling for the latter three. - Add new "purpose_consent", "purpose_legitimate_interests", "vendor_consent", "vendor_legitimate_interests", "system_consent", and "system_legitimate_interests" columns to PrivacyPreferenceHistory, CurrentPrivacyPreference, ServedNoticeHistory, and LastServedNotice tables. - Remove purpose, vendor, and system columns from these tables - Request body for saving privacy preferences now has new purpose_consent_preferences, purpose_legitimate_interests_preferences, vendor_consent_preferences, vendor_legitimate_interests_preferences, system_consent_preferences, and system_legitimate_interests_preferences, to save preferences against these fields and a legal basis simultaneously - Request body that tcf was served has added new list fields: tcf_consent_purposes, tcf_legitimate_interests_purposes, tcf_consent_vendors, tcf_legitimate_interests_vendors, tcf_consent_systems, tcf_legitimate_interests_systems - TCF Experience has new tcf_consent_purposes, tcf_legitimate_interests_purposes sections. tcf_vendors and tcf_systems sections so we can group its consent purposes, consent legitimate interests, features, special features, and special features under the same record. There is also a new "consent_preference" and "legitimate_interests_preference" to surface the default preferences and saved preferences for that vendor/system as a whole. - Added new schemas to more granularly represent the different sections of a schema (for example, new TCFSpecialFeatureRecord, which has specific default preferences for the type of preference) - Update logic for building new TCF Experience to have separate consent purposes and legitimate interests purposes sections. Remove the legal bases arrays. Vendors/systems have new subsections for consent purposes and legitimate interest purposes sections. - Update tc model code to eliminate all of the legal basis filtering except for removing disallowed types, as the Experience already separates them into the appropriate sections - Add new columns to consent reporting endpoint - Handle main tricky part around getting previously saved preferences for vendor consent and vendor legitimate interests and transform into the new format to embed these separately under the vendor and system sections in the TCF experience.
…nding rows and unique constraints.
Passing run #4510 ↗︎Details:
Review all test suite changes for PR #4201 ↗︎ |
…ections - tcf_consent_vendors, tcf_legitimate_interests_vendors, and tcf_vendor_relationships to make all TCF sections behave basically the same way without the special handling for the vendors section.
…t reporting, now that a legal basis attribute has been included.
…rnencehistory, lastservednotice, and servednoticehistory.
… or the privacy center, so the preferences are nested under a "preferences" key to make room for more info in the response body.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #4201 +/- ##
==========================================
+ Coverage 87.66% 87.72% +0.06%
==========================================
Files 331 331
Lines 20700 20869 +169
Branches 2690 2708 +18
==========================================
+ Hits 18146 18308 +162
- Misses 2091 2094 +3
- Partials 463 467 +4
☔ View full report in Codecov by Sentry. |
@allisonking and @galvana tagging you two for review as this adds breaking API changes to accommodate some pieces we missed and we'll need a set of corresponding FE changes! |
FYI I'm going to make an alpha tag of this branch so I can use it with fidesplus! done—it's 2.20.3a6 |
@allisonking I'm about to push one more commit! |
ahaha okay!! |
- Add missed unique constraints to currentprivacypreference and lastservednotice table - Get rid of unnecessary TCF_SERVED_FIELD_MAPPING - Clean up some types and docstrings in experience contents - Remove unnecessary TCFSavedandServed details
OK sorry about that @allisonking new commit pushed. I don't think this would affect the frontend so your 2.20.3a6 is probably fine, I was just going back through and cleaning up some messiness leftover from the POC state where I was exploring earlier ideas than what we settled on. But just in case, because I did some minor type clean up - |
# Conflicts: # CHANGELOG.md # tests/ops/api/v1/endpoints/test_privacy_experience_endpoints.py
… and split out by the type of preference saved.
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.
Big picture, we add several new sections to the TCF Experience and corresponding columns in four tables to store saved preferences and that consent was served.
Instead of a purpose section, for example, we've split this into a "purpose consent" and a "purpose legitimate interests" section, since users are afforded the opportunity to opt in our out of each section separately. We're doing the same thing for vendor and systems sections here, and we're also adding a new vendor relationships section.
src/fides/api/alembic/migrations/versions/81226042d7d4_remove_preferences_for_system_vendor_.py
Show resolved
Hide resolved
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.
amazing work @pattisdr !!! this was a huge refactor and it works really well, nice job!!
Thank you so much for your review @allisonking! I'll make these changes 💪 |
…ience in [tcf attribute][legal basis] format. Same for the last served notice schema for consistency.
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.
@pattisdr great work! i wasn't able to get as much into the weeds as i'd like - but i focused on migrations and unique constraints as you requested and those look good to me 👍
the broad strokes of the API updates (schema and backing logic/persistence) make total sense, and all the pieces i did dig into looked good. if @allisonking has confirmed this is working well for the FE's purposes (which it seems like she has!) then i think we should move forward with getting this merged ASAP so that we can start to test it on some real usage 👍
i'm sure i'll be digging deeper into the weeds soon anyways to make updates for additional System fields and the updated ID schema 😅
# Fetch previously saved records for the current user | ||
for tcf_component, field_name in TCF_COMPONENT_MAPPING.items(): | ||
for record in getattr(tcf_contents, tcf_component): | ||
has_tcf_contents = False | ||
for ( | ||
tcf_section_name, | ||
corresponding_db_field_name, | ||
) in TCF_SECTION_MAPPING.items(): | ||
# Loop through each top-level section in the TCF Contents | ||
tcf_section: List = getattr(tcf_contents, tcf_section_name) | ||
for record in tcf_section: | ||
# Loop through each record within a TCF section, and cache | ||
# any previously saved preferences if applicable |
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.
the changes here make sense -- if i'm following right, we just loop thru the sections now rather than twice, which is a nice optimization.
unrelated to this specific change, but something i thought about when looking closer at this block of code - we're going to be executing a db query (hopefully a pretty inexpensive one) for each section to retrieve the latest saved preferences for the user, right? would it make sense for us to inject the await asyncio.sleep(delay=0.001)
that @ThomasLaPiana had sprinkled in, to ensure we give up control of the main event loop when we execute these repeated queries? i didn't totally follow that trick, maybe it doesn't make sense here since we expect these queries to be heavily indexed/optimized...
src/fides/api/alembic/migrations/versions/9b98aba5bba8_remove_consent_settings.py
Outdated
Show resolved
Hide resolved
src/fides/api/alembic/migrations/versions/81886da90395_add_legal_basis_dimension.py
Show resolved
Hide resolved
- Adds the ability to take in a TC string and save privacy preferences. - Returns the decoded string in the response in mobile format
Closes https://github.com/ethyca/fidesplus/issues/1128
❗ Contains data migration: check downrev before merge
❗ Contains BREAKING API CHANGES. Do not merge without corresponding FE work: this requires corresponding changes to work with new privacy preferences request and response bodies, served notice request bodies, TCF Experience response, and building TC strings from that experience.
Description Of Changes
TCF Beta saves preferences against purposes, services, and vendors directly. However, we have to save preferences against a legal dimension simultaneously. For example, we need to save that a use opted into Purpose 8 with a consent legal basis.
Makes some changes around TCF to add a legal basis dimension when saving preferences against purposes, services, and vendors.
Code Changes
PATCH Privacy Preferences
Adds new sections for saving privacy preferences against purpose/vendors/systems with a legal basis dimension:
Request
PATCH http://localhost:8080/api/v1/privacy-preferences
Response
NOTE saving preferences now nests the list of saved preferences under a preferences key
PATCH Notices Served
Adds new sections for saving that notices were served for purpose/vendors/systems with a legal basis dimension:
PATCH http://localhost:8080/api/v1/notices-served
GET Privacy Experience
http://localhost:8080/api/v1/privacy-experience?show_disabled=true®ion=fr
Steps to Confirm
Pre-Merge Checklist
CHANGELOG.md