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] Surface Current Preference under Privacy Experience List endpoint #3302

Merged
merged 3 commits into from
May 18, 2023

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented May 15, 2023

❗ Depends on #3193
❗ Note, this endpoint is not yet "public" that will be a follow-up ticket

Closes #3237

Description Of Changes

Add a ?fides_user_device_id query param to the GET Privacy Experience List/Detail endpoint. This endpoint already embeds related privacy notices in the response. Add the current preferences of the user to each notice if they exist. Also add the default preferences for that endpoint.

Code Changes

  • Update the PrivacyExperience.get_related_privacy_notices method to temporarily cache the "current_preference" or an "outdated_preference" to each privacy notice for the given user if applicable. I only add one or the other, or neither, not both. This lasts for the current session.
  • Add a separate PrivacyNotice.default_preference property that always returns whatever the default preference is for that notice, regardless of whether a fides user device id is involved in the query
  • Add fides_user_device_id query param to both the privacy experience list and detail endpoints to additionally attach current preferences to embedded notices if they exist

Steps to Confirm

In postman collection:

  • Create privacy notices
  • Save a preference for one of the notices with a certain fides_user_device_id passed in in the request body
  • View the PrivacyExperienceList endpoint, note the default_preference field exists on the new notices
  • Add a fides_user_device_id query param and note that the user's current preference is returned
  • Update the notice itself
  • Query PrivacyExperienceList again with the fides user device id and note that this preference is now returned as an "outdated preference"

Pre-Merge Checklist

@pattisdr pattisdr changed the title Surface Current Preference under Privacy Experience List endpoint [Backend] Surface Current Preference under Privacy Experience List endpoint May 15, 2023
@cypress
Copy link

cypress bot commented May 15, 2023

Passing run #2043 ↗︎

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 088df1b into 1290bf1...
Project: fides Commit: 7d1be8ef7a ℹ️
Status: Passed Duration: 01:10 💡
Started: May 18, 2023 1:14 AM Ended: May 18, 2023 1:15 AM

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

@codecov
Copy link

codecov bot commented May 15, 2023

Codecov Report

Patch coverage: 96.66% and no project coverage change.

Comparison is base (1290bf1) 87.06% compared to head (088df1b) 87.06%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3302   +/-   ##
=======================================
  Coverage   87.06%   87.06%           
=======================================
  Files         309      309           
  Lines       18821    18847   +26     
  Branches     2461     2467    +6     
=======================================
+ Hits        16386    16410   +24     
- Misses       1995     1996    +1     
- Partials      440      441    +1     
Impacted Files Coverage Δ
src/fides/api/ops/models/privacy_notice.py 98.96% <86.66%> (-1.04%) ⬇️
...s/api/v1/endpoints/privacy_experience_endpoints.py 100.00% <100.00%> (ø)
...s/api/v1/endpoints/privacy_preference_endpoints.py 100.00% <100.00%> (+0.76%) ⬆️
src/fides/api/ops/models/privacy_experience.py 100.00% <100.00%> (ø)
src/fides/api/ops/models/privacy_preference.py 100.00% <100.00%> (ø)
src/fides/api/ops/schemas/privacy_experience.py 100.00% <100.00%> (ø)
src/fides/api/ops/schemas/privacy_notice.py 100.00% <100.00%> (ø)
src/fides/api/ops/schemas/privacy_preference.py 100.00% <100.00%> (ø)
.../ops/service/connectors/consent_email_connector.py 97.14% <100.00%> (ø)
src/fides/api/ops/util/consent_util.py 98.11% <100.00%> (-1.89%) ⬇️

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

Copy link
Contributor

@allisonking allisonking left a comment

Choose a reason for hiding this comment

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

nice, this makes sense to me!

@pattisdr
Copy link
Contributor Author

Thanks so much for looking this over @allisonking!

Base automatically changed from fides_3193_syncing_privacy_notices_and_experiences to main May 17, 2023 23:44
…List endpoint. If present, when embedding the privacy notices in the response it also queries for any saved preferences for that user and passes them along in the response.
…avor of the privacy experiences endpoint with embedded notices. Also remove verifying the address as we're looking at cors origins in middleware.
@pattisdr pattisdr force-pushed the fides_3237_surface_current_preferences_under_notices branch from d35486c to 088df1b Compare May 18, 2023 00:53
@pattisdr pattisdr merged commit 5303975 into main May 18, 2023
@pattisdr pattisdr deleted the fides_3237_surface_current_preferences_under_notices branch May 18, 2023 01:46
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] Surface Current Preference under Privacy Experience List endpoint
2 participants