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

PROD-1396 Fix typeerror when tcf vendors have no dataDeclaration #4465

Merged
merged 6 commits into from
Nov 29, 2023

Conversation

eastandwestwind
Copy link
Contributor

@eastandwestwind eastandwestwind commented Nov 28, 2023

Closes https://ethyca.atlassian.net/browse/PROD-1396

Description Of Changes

Fix type errors when tcf vendors have no dataDeclaration. This PR fixes 2 possible errors that could arise in this case. I've also added e2e tests to confirm this is fixed.

Code Changes

  • Fix typeerrors
  • Set up e2e test to confirm this is fixed

Steps to Confirm

  • Run e2e tests

Pre-Merge Checklist

Copy link

vercel bot commented Nov 28, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Nov 29, 2023 4:40pm

Copy link

cypress bot commented Nov 28, 2023

Passing run #5378 ↗︎

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 b9a3c03 into 1c500ab...
Project: fides Commit: 5d05647344 ℹ️
Status: Passed Duration: 00:38 💡
Started: Nov 29, 2023 4:49 PM Ended: Nov 29, 2023 4:50 PM

Review all test suite changes for PR #4465 ↗︎

@adamsachs
Copy link
Contributor

adamsachs commented Nov 29, 2023

UAT tested and looks good to me! ✅ (would like some extra confirmation that my testing steps below are valid 😄)

  • running a fidesplus BE with TCF enabled (slim/dev)
  • checked out this OSS branch
  • ran turbo run dev from ./clients/admin-ui
  • went to admin UI - localhost:3000 in my browser - and added all compass vendors via bulk vendor add UI
  • stopped admin UI client, then ran turbo run dev from ./clients/privacy-center
  • went to http://localhost:3000/fides-js-demo.html?geolocation=fr in my browser:
    • although things took a while to load (~20s - this is expected, due to the BE taking a while to load the TCF overlay, which is at least partially alleviated in another PR), everything loaded well, including the TCF overlay with 1000+ vendors
    • confirmed no errors in JS console

(note on performance - using the BE from this branch, the overlay does load a lot quicker after the "initial" fetch, i.e. once the TCF contents are cached 👍 )

image

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.

looks great! just a typing nit

@eastandwestwind eastandwestwind merged commit 4f990a3 into main Nov 29, 2023
13 checks passed
@eastandwestwind eastandwestwind deleted the PROD-1396 branch November 29, 2023 17:55
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.

3 participants