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

Render new TCF fields in the overlay using the data map #4286

Merged
merged 24 commits into from
Oct 19, 2023

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Oct 16, 2023

Closes #4255

Description Of Changes

Renders the following TCF fields from data from the data map as opposed to querying gvl.json directly:

  • cookie storage disclosure copy
  • retention periods for purposes in a vendor
  • privacy policy/legitimate interest disclosure urls

Not all of these fields were fully passed over from the backend, so there are some backend changes in this PR as well.

Code Changes

  • Update TS types
  • Simplify some logic now that vendor relationships contain all vendors
  • Derive storage disclosure info from TCFVendorRelationships
  • Add privacy_policy_url to TCFVendorRelationships in the backend
  • Derive urls from TCFVendorRelationships
  • Add retention period to EmbeddedPurposes in vendor data
  • Remove no longer needed GVL types
  • Cypress tests

Steps to Confirm

  • Set up TCF and on one of those systems, fill out the "Cookie properties" section in admin-ui for the system
  • Spin up the TCF overlay and you should see these cookie properties reflected in the vendor tab, when you click on a vendor
    image
  • Edit the system's privacy policy and legitimate interest disclosure. This should reflect in the TCF overlay
    image
  • In one of the system's data uses, change its "Retention period" to a different number. You should see this reflect in the "Purposes" section in the vendor
    image

Pre-Merge Checklist

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

@cypress
Copy link

cypress bot commented Oct 16, 2023

Passing run #4698 ↗︎

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 d2e3511 into 8471288...
Project: fides Commit: 9f8753977d ℹ️
Status: Passed Duration: 00:57 💡
Started: Oct 19, 2023 6:58 PM Ended: Oct 19, 2023 6:59 PM

Review all test suite changes for PR #4286 ↗︎

@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7b15ac1) 87.76% compared to head (d2e3511) 87.76%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4286   +/-   ##
=======================================
  Coverage   87.76%   87.76%           
=======================================
  Files         331      331           
  Lines       20922    20928    +6     
  Branches     2714     2715    +1     
=======================================
+ Hits        18362    18368    +6     
  Misses       2093     2093           
  Partials      467      467           
Files Coverage Δ
src/fides/api/schemas/tcf.py 100.00% <100.00%> (ø)
src/fides/api/util/tcf/tcf_experience_contents.py 94.23% <100.00%> (+0.07%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

@allisonking backend is looking good from my POV - nice work! i can see now it wasn't quite as straightforward as it seemed when talking it over. just had a small nit on the fixtures, though it's nuanced enough that i wouldn't blame you if you choose to ignore me 👍

and i'd like @pattisdr to also confirm things look good, since i'm still pretty fresh in the overlay generation codepath!

tests/fixtures/application_fixtures.py Outdated Show resolved Hide resolved
src/fides/api/schemas/tcf.py Outdated Show resolved Hide resolved
@pattisdr
Copy link
Contributor

Reviewing now!

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.

Noting concerns about differences in retention periods on the same Purpose across Systems or within the same System -

src/fides/api/schemas/tcf.py Outdated Show resolved Hide resolved
src/fides/api/util/tcf/tcf_experience_contents.py Outdated Show resolved Hide resolved
src/fides/api/util/tcf/tcf_experience_contents.py Outdated Show resolved Hide resolved
@allisonking allisonking marked this pull request as ready for review October 17, 2023 22:40
@allisonking
Copy link
Contributor Author

Thanks for all the assistance @pattisdr and @adamsachs ! I think this is ready for review now 👍

@pattisdr
Copy link
Contributor

Thanks Allison, starting review!

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 work Allison, especially moving some of this from the GVL into being pulled from the datamap in the backend. Mostly minor cleanup and another test requested!

src/fides/api/util/tcf/tcf_experience_contents.py Outdated Show resolved Hide resolved
src/fides/api/util/tcf/tcf_experience_contents.py Outdated Show resolved Hide resolved
src/fides/api/util/tcf/tcf_experience_contents.py Outdated Show resolved Hide resolved
clients/fides-js/src/lib/tcf/types.ts Outdated Show resolved Hide resolved
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.

wonderful job 🌞

@allisonking allisonking merged commit 6e7697a into main Oct 19, 2023
46 checks passed
@allisonking allisonking deleted the aking/4255/tcf-fields branch October 19, 2023 20:11
@allisonking
Copy link
Contributor Author

thanks for the thorough review @pattisdr !

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.

TCF overlay FE: support new TCF related fields on System model
3 participants