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

[Backend] More Privacy Notice Data Use Validation Checks #3156

Merged
merged 4 commits into from
Apr 26, 2023

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented Apr 25, 2023

Closes #3154

Code Changes

  • More tests to confirm this "startswith" data use check is sufficient for the in-region checks we want to run.

Steps to Confirm

  • This PR just adds more tests so nothing really to check. If you wanted you could confirm that you could create privacy notices in the API (existing Postman collection Privacy Notices > Create Privacy Notices to have a sibling data use (For example, create a CA advertising.first_party.personalized notice and a California advertising.third_party.personalized notice) and confirm it is allowed. Confirm a parent child data use in the same region is still not allowed.

Pre-Merge Checklist

Description Of Changes

Context: #3120 (comment)

Adding additional checks here to confirm sibling or nephew relationships are allowed in the hierarchy. I thought this didn't work but the starts with check does handle these use cases so adding further tests here.

…otice is allowed. Relax slightly to allow things like data use siblings, or nephews, but still prevent parents/grandparents.
@pattisdr pattisdr changed the title Update the logic for inspecting whether a new data use on a privacy n… [Backend] Update Data Use Validation for Creating Privacy Notices Apr 25, 2023
@pattisdr pattisdr marked this pull request as ready for review April 25, 2023 19:29
@pattisdr pattisdr requested a review from adamsachs April 25, 2023 19:32
@pattisdr
Copy link
Contributor Author

pattisdr commented Apr 25, 2023

@adamsachs when you have a minute, I have a small change to your logic as to whether a data use should be allowed on a privacy notice in the same region based on updated requirements. Let me know if this doesn't make sense to you.

EDIT: logic is fine, just adding more tests here

@cypress
Copy link

cypress bot commented Apr 25, 2023

Passing run #1600 ↗︎

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 7dc050f into ea5975e...
Project: fides Commit: 9dfbeb99af ℹ️
Status: Passed Duration: 00:54 💡
Started: Apr 26, 2023 4:30 PM Ended: Apr 26, 2023 4:31 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 Apr 25, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (c350a5e) 87.51% compared to head (7dc050f) 87.52%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3156   +/-   ##
=======================================
  Coverage   87.51%   87.52%           
=======================================
  Files         309      310    +1     
  Lines       18010    18024   +14     
  Branches     2341     2338    -3     
=======================================
+ Hits        15762    15775   +13     
- Misses       1822     1823    +1     
  Partials      426      426           
Impacted Files Coverage Δ
src/fides/api/ctl/sql_models.py 98.30% <100.00%> (+<0.01%) ⬆️
src/fides/api/ops/models/policy.py 87.27% <100.00%> (ø)
src/fides/api/ops/models/privacy_notice.py 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

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

@pattisdr pattisdr changed the title [Backend] Update Data Use Validation for Creating Privacy Notices [Backend] More Privacy Notice Data Use Validation Checks Apr 26, 2023
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.

this looks good! thanks for the extra iterations to get it nice and clean :)

@pattisdr pattisdr merged commit f3f7a2a into main Apr 26, 2023
@pattisdr pattisdr deleted the fides_3154_data_use_updates branch April 26, 2023 17:42
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.

[Backend] Verify Privacy Notice Data Use Validation
2 participants