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

3851 Fix ability to make server side API calls from privacy-center #3895

Merged
merged 7 commits into from
Aug 17, 2023

Conversation

eastandwestwind
Copy link
Contributor

@eastandwestwind eastandwestwind commented Aug 3, 2023

Closes #3851 and #3854

Description Of Changes

This change allows us to, most importantly, be able to fetch experience data on the server-side.

Code Changes

  • Add new env var FIDES_PRIVACY_CENTER__SERVER_SIDE_FIDES_API_URL to set server-side url host (i.e. from one docker container to another)
  • Add new env var FIDES_PRIVACY_CENTER__IS_PREFETCH_ENABLED to determine whether or not to make prefetch calls from server

Steps to Confirm

  • Regression test only
  • Ensure these env vars are set:
FIDES_PRIVACY_CENTER__DEBUG=true
FIDES_PRIVACY_CENTER__IS_OVERLAY_ENABLED=true
FIDES_PRIVACY_CENTER__IS_PREFETCH_ENABLED=false
FIDES_PRIVACY_CENTER__IS_GEOLOCATION_ENABLED=true
FIDES_PRIVACY_CENTER__GEOLOCATION_API_URL=https://cdn-api.ethyca.com/location
FIDES_PRIVACY_CENTER__PRIVACY_CENTER_URL=http://localhost:3000
  • nox -s "fides_env(test)"
  • Ensure you have notices / experiences / systems set up for your testing region
  • Nav to http://localhost:3001/fides-js-demo.html and see that appropriate experience pops up

Pre-Merge Checklist

@cypress
Copy link

cypress bot commented Aug 3, 2023

Passing run #3588 ↗︎

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 f041bc1 into 3f1ebeb...
Project: fides Commit: f08a31a5fa ℹ️
Status: Passed Duration: 01:03 💡
Started: Aug 16, 2023 7:14 PM Ended: Aug 16, 2023 7:15 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 Aug 3, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02% ⚠️

Comparison is base (3f1ebeb) 87.36% compared to head (f041bc1) 87.35%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3895      +/-   ##
==========================================
- Coverage   87.36%   87.35%   -0.02%     
==========================================
  Files         317      317              
  Lines       19431    19431              
  Branches     2497     2497              
==========================================
- Hits        16976    16974       -2     
- Misses       2023     2024       +1     
- Partials      432      433       +1     

see 1 file with indirect coverage changes

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

Copy link
Contributor

@NevilleS NevilleS left a comment

Choose a reason for hiding this comment

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

Quick comment- but haven't finished reviewing yet 👍

Copy link
Contributor

@NevilleS NevilleS left a comment

Choose a reason for hiding this comment

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

Some nits here, but the main two points to get this ready to merge are:

  1. We should remove the server-side geolocation logic and test this on some feature branches instead
  2. We should set prefetch to false by default, for safety

@eastandwestwind eastandwestwind changed the title 3851 Adds server side prefetching of geolocation and experience 3851 Fix ability to make server side API calls from privacy-center Aug 4, 2023
Copy link
Contributor

@NevilleS NevilleS left a comment

Choose a reason for hiding this comment

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

Sorry for forgetting this! Ship it, it's disabled by default, so let's test it out with some staging and production (test) sites

@eastandwestwind eastandwestwind merged commit 80958ee into main Aug 17, 2023
@eastandwestwind eastandwestwind deleted the 3851-server-side-prefetching branch August 17, 2023 13:26
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.

Server-side (Privacy Center) API calls are blocked when testing locally
2 participants