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

Fides Cloud Configuration: DNS Record and Privacy Center URL #4034

Merged
merged 26 commits into from
Sep 12, 2023

Conversation

TheAndrewJackson
Copy link
Contributor

@TheAndrewJackson TheAndrewJackson commented Sep 6, 2023

Closes fidesplus#1064 Closes fidesplus#1077

Description Of Changes

This PR updates the admin ui to use the new fides cloud config api to aid in environment configuration.

Code Changes

  • Add fides cloud config migration and model
  • Add fides cloud config endpoint to plus slice and update feature slice
  • Update api types ( and fix SystemHistory issue that resulted from that)
  • Add DNS records page
  • Update javascript tag component to use new privacy center url config var and include Fides.gtm()

Steps to Confirm

Pre-Merge Checklist

@cypress
Copy link

cypress bot commented Sep 6, 2023

Passing run #4067 ↗︎

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 d3ceb56 into 267e35b...
Project: fides Commit: 7535a870aa ℹ️
Status: Passed Duration: 01:24 💡
Started: Sep 12, 2023 12:48 PM Ended: Sep 12, 2023 12:49 PM

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

@TheAndrewJackson TheAndrewJackson changed the title DNS records page Fides Cloud Configuration: DNS Record and Privacy Center URL Sep 7, 2023
@TheAndrewJackson TheAndrewJackson self-assigned this Sep 7, 2023
@TheAndrewJackson TheAndrewJackson marked this pull request as ready for review September 7, 2023 20:56
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.

awesome work! I was able to get it running and populating locally 💥

one nit is can this copy button for gtm be right aligned?
image

@@ -252,6 +262,12 @@ const configureNavRoute = ({
return undefined;
}

// If the target route would require fides cloud in a non-fides-cloud environment,
// exclude it from the group.
if (route.requiresFidesCloud && !hasFidesCloud) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you also add a test for this? there are pretty extensive tests in nav-config.test.ts so would be nice to keep its coverage up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes! I'll add one now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tests here: d3ceb56

clients/admin-ui/src/features/common/nav/v2/routes.ts Outdated Show resolved Hide resolved
clients/admin-ui/src/features/common/table/cells.tsx Outdated Show resolved Hide resolved
clients/admin-ui/src/pages/management/dns-records.tsx Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01% ⚠️

Comparison is base (c851530) 87.39% compared to head (d3ceb56) 87.39%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4034      +/-   ##
==========================================
- Coverage   87.39%   87.39%   -0.01%     
==========================================
  Files         319      320       +1     
  Lines       19595    19602       +7     
  Branches     2512     2512              
==========================================
+ Hits        17125    17131       +6     
- Misses       2033     2034       +1     
  Partials      437      437              
Files Changed Coverage Δ
src/fides/api/db/base.py 100.00% <100.00%> (ø)
src/fides/api/models/fides_cloud.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

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

@TheAndrewJackson TheAndrewJackson merged commit 2f90192 into main Sep 12, 2023
@TheAndrewJackson TheAndrewJackson deleted the ajackson/dns_records/PROD-1035 branch September 12, 2023 13:29
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.

2 participants