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

Adds intake and storage of Global Privacy Control Signal props for Consent #2599

Merged
merged 26 commits into from
Feb 22, 2023

Conversation

eastandwestwind
Copy link
Contributor

@eastandwestwind eastandwestwind commented Feb 15, 2023

❗ Contains a DB migration! Check for any conflicts before merging. ❗

Closes #2240

Code Changes

  • Updates Consent table to store 2 new columns pertaining to GPC: has_gpc_flag and conflicts_with_gpc
  • Updates Consent schema to intake these 2 fields as optional
  • Adds tests

Steps to Confirm

  • list any manual steps for reviewers to confirm the changes

Pre-Merge Checklist

Description Of Changes

Write some things here about the changes and any potential caveats

@eastandwestwind eastandwestwind changed the title Record all applications of a Global Privacy Control Signal BE: Record all applications of a Global Privacy Control Signal Feb 15, 2023
@eastandwestwind eastandwestwind changed the title BE: Record all applications of a Global Privacy Control Signal Adds intake and storage of Global Privacy Control Signal props for Consent Feb 15, 2023
@cypress
Copy link

cypress bot commented Feb 16, 2023

Passing run #290 ↗︎

0 3 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge bf9fcb7 into c35ce37...
Project: fides Commit: 1b4481954a ℹ️
Status: Passed Duration: 00:50 💡
Started: Feb 22, 2023 5:55 PM Ended: Feb 22, 2023 5:56 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@codecov
Copy link

codecov bot commented Feb 21, 2023

Codecov Report

Base: 86.22% // Head: 86.24% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (7ea9242) compared to base (ea5cf1d).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 7ea9242 differs from pull request most recent head 7456a4b. Consider uploading reports for the commit 7456a4b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2599      +/-   ##
==========================================
+ Coverage   86.22%   86.24%   +0.01%     
==========================================
  Files         289      289              
  Lines       15780    15800      +20     
  Branches     1985     1986       +1     
==========================================
+ Hits        13607    13626      +19     
- Misses       1785     1786       +1     
  Partials      388      388              
Impacted Files Coverage Δ
.../ops/api/v1/endpoints/consent_request_endpoints.py 87.33% <ø> (ø)
src/fides/api/ops/models/privacy_request.py 95.90% <100.00%> (+0.02%) ⬆️
src/fides/api/ops/schemas/privacy_request.py 100.00% <100.00%> (ø)
src/fides/cli/commands/util.py 57.89% <0.00%> (-0.78%) ⬇️
src/fides/cli/__init__.py 92.00% <0.00%> (-0.73%) ⬇️
src/fides/core/api.py 82.85% <0.00%> (ø)
src/fides/core/user.py 100.00% <0.00%> (ø)
src/fides/cli/commands/db.py 81.48% <0.00%> (ø)
src/fides/core/config/user_settings.py 100.00% <0.00%> (ø)
src/fides/core/utils.py 86.15% <0.00%> (+5.30%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sanders41
Copy link
Contributor

I just testing locally and thing started without error. Going to try a restart on the tests here. Hopefully we aren't back to a CI only issue 😕

@eastandwestwind
Copy link
Contributor Author

@sanders41 it's just the migration down rev that needed to be bumped. It's weird that this is the way it manifests in CI though.

@sanders41
Copy link
Contributor

@sanders41 it's just the migration down rev that needed to be bumped. It's weird that this is the way it manifests in CI though.

And that it doesn't happen locally. Very weird.

@eastandwestwind eastandwestwind merged commit 5b430ec into main Feb 22, 2023
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.

BE: Record all applications of a Global Privacy Control Signal
2 participants