Switch TCF config to env var #4220

TheAndrewJackson · 2023-10-03T20:27:45Z

Closes #3788

Description Of Changes

This PR rips out the consent_settings table and endpoints in favor of using a simpler env var.

Code Changes

Remove consent settings model, schema, endpoints, and other consent settings related code
Refactor privacy preferences and experiences endpoints and tests to use the config var
Add new enable_tcf fixture

Steps to Confirm

privacy experience and privacy preferences tests pass

Pre-Merge Checklist

All CI Pipelines Succeeded
Documentation:
- documentation complete, PR opened in fidesdocs
- documentation issue created in fidesdocs
Issue Requirements are Met
Relevant Follow-Up Issues Created
Update CHANGELOG.md
For API changes, the Postman collection has been updated

cypress · 2023-10-03T20:42:39Z

Passing run #4466 ↗︎

0	4	0	0	0

Details:

Merge `d99ddb0` into `03d26a4`...
Project: fides	Commit: `57cc5c629d ℹ️`
Status: Passed	Duration: 00:57 💡
Started: Oct 4, 2023 3:25 PM	Ended: Oct 4, 2023 3:26 PM

Review all test suite changes for PR #4220 ↗︎

codecov · 2023-10-03T21:16:11Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (7932483) 87.67% compared to head (6c84b23) 87.65%.
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4220      +/-   ##
==========================================
- Coverage   87.67%   87.65%   -0.02%     
==========================================
  Files         333      331       -2     
  Lines       20743    20695      -48     
  Branches     2690     2689       -1     
==========================================
- Hits        18187    18141      -46     
+ Misses       2093     2091       -2     
  Partials      463      463

Files	Coverage Δ
src/fides/api/api/v1/api.py	`100.00% <ø> (ø)`
...i/api/v1/endpoints/privacy_experience_endpoints.py	`93.63% <100.00%> (-0.12%)`	⬇️
...i/api/v1/endpoints/privacy_preference_endpoints.py	`98.02% <100.00%> (-0.03%)`	⬇️
src/fides/api/app_setup.py	`68.11% <ø> (-0.81%)`	⬇️
src/fides/api/db/base.py	`100.00% <ø> (ø)`
src/fides/config/__init__.py	`96.22% <100.00%> (+0.07%)`	⬆️
src/fides/config/consent_settings.py	`100.00% <100.00%> (ø)`

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

TheAndrewJackson · 2023-10-04T13:18:09Z

@pattisdr This is currently merging into a larger TCF feature branch. It might be easier to merge this straight into main since there is a migration. If it's low risk to merge it into main I think it'll be worth it.

Also, it looks like this PR is failing the performance CI check. Do I need to worry about that for this change?

pattisdr · 2023-10-04T14:03:12Z

@TheAndrewJackson to answer your first question, yes I think this should definitely go into main due to the migration as you point out, can you repoint this branch?

TheAndrewJackson · 2023-10-04T14:20:47Z

@pattisdr repointed to main 👍

pattisdr · 2023-10-04T14:22:50Z

reviewing now thank you 💯

pattisdr

Nice changes here @TheAndrewJackson I just want to make sure this works for local development -

pattisdr · 2023-10-04T14:34:28Z

docker-compose.yml

@@ -28,6 +28,7 @@ services:
      FIDES__DEV_MODE: "True"
      FIDES__LOGGING__COLORIZE: "True"
      FIDES__USER__ANALYTICS_OPT_OUT: "True"
+      FIDES__CONSENT__TCF_ENABLED: "False"


It seems like hardcoding this to false is preventing setting tcf_enabled=true in the fides.toml file for local development from taking effect? Can you look at this and verify you can run fides locally with tcf enabled=true set from the fides.toml file? Maybe we can remove this line altogether

Good catch. I'll look into that now

That was causing the issue you described. I removed it from the docker compose file: d99ddb0

I tried updating it to be FIDES__CONSENT__TCF_ENABLED: ${FIDES__CONSENT__TCF_ENABLED-} but that caused validation issues on startup because it would pass in null instead of a boolean value.

pattisdr · 2023-10-04T15:08:30Z

Also, it looks like this PR is failing the performance CI check. Do I need to worry about that for this change?

I can't imagine that this change matters much for performance

pattisdr

Thank you looks good to me!

allisonking · 2023-10-04T16:42:56Z

oooh very excited for this! @TheAndrewJackson can you do me a favor and update this doc with the deetz of the new env var and remove the section about going through /docs to consent-settings, etc?

TheAndrewJackson · 2023-10-04T20:21:12Z

@allisonking yes! I'll do that now

TheAndrewJackson added 6 commits October 3, 2023 14:55

add new tcf env var

099b521

Remove ConsentSettings from API usage and tests

c0c73b8

lints

aabec7d

Refactor more tests

1b9f9b0

remove model and schema

f02dc58

drop consent settings

8bca358

TheAndrewJackson self-assigned this Oct 3, 2023

pattisdr mentioned this pull request Oct 3, 2023

Developer Friendly Privacy Preferences #4221

Merged

14 tasks

update changelog

8e7bb48

TheAndrewJackson requested a review from pattisdr October 4, 2023 13:16

TheAndrewJackson marked this pull request as ready for review October 4, 2023 13:16

TheAndrewJackson mentioned this pull request Oct 4, 2023

Add ability to turn TCF on/off #3788

Closed

Add new config var to toml an yml files

6c84b23

TheAndrewJackson changed the base branch from ajackson/enable-tcf/PROD-1069 to main October 4, 2023 14:20

pattisdr reviewed Oct 4, 2023

View reviewed changes

remove from docker compose file

d99ddb0

pattisdr approved these changes Oct 4, 2023

View reviewed changes

TheAndrewJackson merged commit 197355f into main Oct 4, 2023
38 checks passed

TheAndrewJackson deleted the ajackson/switch-tcf-config-to-env-var branch October 4, 2023 15:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch TCF config to env var #4220

Switch TCF config to env var #4220

TheAndrewJackson commented Oct 3, 2023 •

edited

Loading

cypress bot commented Oct 3, 2023 •

edited

Loading

codecov bot commented Oct 3, 2023 •

edited

Loading

TheAndrewJackson commented Oct 4, 2023

pattisdr commented Oct 4, 2023

TheAndrewJackson commented Oct 4, 2023

pattisdr commented Oct 4, 2023

pattisdr left a comment

pattisdr Oct 4, 2023 •

edited

Loading

TheAndrewJackson Oct 4, 2023

TheAndrewJackson Oct 4, 2023

pattisdr commented Oct 4, 2023

pattisdr left a comment

allisonking commented Oct 4, 2023

TheAndrewJackson commented Oct 4, 2023

Switch TCF config to env var #4220

Switch TCF config to env var #4220

Conversation

TheAndrewJackson commented Oct 3, 2023 • edited Loading

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

cypress bot commented Oct 3, 2023 • edited Loading

Passing run #4466 ↗︎

Review all test suite changes for PR #4220 ↗︎

codecov bot commented Oct 3, 2023 • edited Loading

Codecov Report

TheAndrewJackson commented Oct 4, 2023

pattisdr commented Oct 4, 2023

TheAndrewJackson commented Oct 4, 2023

pattisdr commented Oct 4, 2023

pattisdr left a comment

Choose a reason for hiding this comment

pattisdr Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

TheAndrewJackson Oct 4, 2023

Choose a reason for hiding this comment

TheAndrewJackson Oct 4, 2023

Choose a reason for hiding this comment

pattisdr commented Oct 4, 2023

pattisdr left a comment

Choose a reason for hiding this comment

allisonking commented Oct 4, 2023

TheAndrewJackson commented Oct 4, 2023

TheAndrewJackson commented Oct 3, 2023 •

edited

Loading

cypress bot commented Oct 3, 2023 •

edited

Loading

codecov bot commented Oct 3, 2023 •

edited

Loading

pattisdr Oct 4, 2023 •

edited

Loading