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

endpoint to retrieve privacy notices by data use #2956

Merged
merged 6 commits into from
Apr 7, 2023

Conversation

adamsachs
Copy link
Contributor

@adamsachs adamsachs commented Mar 30, 2023

Partially closes #2834 (1 of 2 required endpoints)

Code Changes

  • new GET /api/v1/privacy-notice-by-data-use endpoint that returns a map of DataUses with their corresponding PrivacyNotices
    • Only DataUses that are associated with a System are included in the map.
    • DataUses that do not have any PrivacyNotice associated with them are included
      in the map, with empty lists.

Steps to Confirm

  • add some Systems with privacy declarations (easiest to do thru the UI)
  • add some PrivacyNotices that have DataUses that overlap with the privacy declarations of your Systems (must be manual POST /api/v1/privacy-notice API calls for now)
  • invoke the GET /api/v1/privacy-notice-by-data-use endpoint, and ensure the result looks as expected

Pre-Merge Checklist

@cypress
Copy link

cypress bot commented Mar 30, 2023

Passing run #1231 ↗︎

0 3 0 0 Flakiness 0

Details:

Merge 99cc638 into b62d238...
Project: fides Commit: d7aff7c5c1 ℹ️
Status: Passed Duration: 00:48 💡
Started: Apr 7, 2023 9:31 PM Ended: Apr 7, 2023 9:32 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 Mar 30, 2023

Codecov Report

Patch coverage: 97.67% and project coverage change: +0.01 🎉

Comparison is base (b62d238) 86.90% compared to head (4d42e5b) 86.92%.

❗ Current head 4d42e5b differs from pull request most recent head 99cc638. Consider uploading reports for the commit 99cc638 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2956      +/-   ##
==========================================
+ Coverage   86.90%   86.92%   +0.01%     
==========================================
  Files         303      303              
  Lines       17298    17322      +24     
  Branches     2223     2230       +7     
==========================================
+ Hits        15033    15057      +24     
  Misses       1849     1849              
  Partials      416      416              
Impacted Files Coverage Δ
src/fides/api/ctl/sql_models.py 98.15% <90.90%> (+0.06%) ⬆️
...i/ops/api/v1/endpoints/privacy_notice_endpoints.py 100.00% <100.00%> (ø)
src/fides/api/ops/api/v1/urn_registry.py 100.00% <100.00%> (ø)
src/fides/api/ops/models/privacy_notice.py 100.00% <100.00%> (ø)
src/fides/api/ops/schemas/privacy_notice.py 100.00% <100.00%> (ø)

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

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.

This is great, Adam. Endpoint is very clear and well documented. My main question is around data use hierarchies -

@adamsachs adamsachs force-pushed the 2834-privacy-notices-data-uses branch from a6d833e to d752110 Compare March 31, 2023 18:27
@adamsachs adamsachs force-pushed the 2834-privacy-notices-data-uses branch 2 times, most recently from 6f37192 to a309696 Compare April 6, 2023 19:08
@adamsachs adamsachs marked this pull request as ready for review April 6, 2023 19:08
@adamsachs
Copy link
Contributor Author

OK @pattisdr i think this one is also just about good to go, but no big rush on it!

there's a codecov miss i'm not too concerned about, although i'll try to circle back to it to provide some targeted tests on the System.get_system_data_uses() classmethod if i've got time!

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.

I had a slightly different interpretation here, wanted to see what you thought -

src/fides/api/ctl/sql_models.py Outdated Show resolved Hide resolved
@adamsachs adamsachs force-pushed the 2834-privacy-notices-data-uses branch from 0997d27 to 9b63264 Compare April 7, 2023 16:52
@adamsachs
Copy link
Contributor Author

@pattisdr i think this is ready for another look when you get a chance. i've taken your suggestion on the behavior for hierarchical overlaps - let me know if that looks correct to you!

(codecov still "fails" but doesn't even give me a concrete miss! 😒 )

@pattisdr
Copy link
Contributor

pattisdr commented Apr 7, 2023

looking 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.

looks solid @adamsachs great code comments as usual.

@adamsachs adamsachs force-pushed the 2834-privacy-notices-data-uses branch from 9b63264 to 4d42e5b Compare April 7, 2023 21:14
…int.

Also include an additional privacy notice in sample creationt call to have one that
is actually applicable to a system
this is needed to determine that a notice whose data use is the parent of
a data use associated with a system is applicable to that system.
@adamsachs adamsachs force-pushed the 2834-privacy-notices-data-uses branch from 4d42e5b to 99cc638 Compare April 7, 2023 21:15
@adamsachs adamsachs merged commit f54f62c into main Apr 7, 2023
@adamsachs adamsachs deleted the 2834-privacy-notices-data-uses branch April 7, 2023 21:57
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 Mapping Privacy Notices to Systems
2 participants