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

Conditionally Enabling Migrated Experience Configs #4674

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented Mar 8, 2024

Closes #PROD-1797

Description Of Changes

In the multitranslation data migration, we originally were disabling the new Experience Configs in the data migration as the safer default and added a manual step to enable them afterwards. However, we've learned it's preferable to ship with a preference towards enabling them if they can be enabled.

Code Changes

  • Strategy is to enable any Experience Configs if any (not all) of the attached notices are enabled
  • Since TCF doesn't have notices, copying over the enabled flag of the original Experience Config. This is likely to be enabled by default. However, other TCF flags are also required in addition for TCF to work. These TCF Experience Configs are also suppressed entirely without these extra TCF enabled flags too, so for many customers, this won't change anything.

Steps to Confirm

  • Pre-migration on main, enable the "essential" notice, leaving the data sales and sharing notice disabled
  • Post migration, any Experience Configs w/ the Essential notice (many of them) will be enabled, but the US Modal, for example, will still be disabled because its only notice is disabled
  • The TCF Config will be enabled if it was enabled prior to migration

Pre-Merge Checklist

…are enabled, let's likewise enable the Experience Config as the safer default. (Previously we thought the safer default was disabled).

Pull the new disabled TCF status off of the existing TCF config since we don't have notices.
Copy link

vercel bot commented Mar 8, 2024

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

1 Ignored Deployment
Name Status Preview Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Mar 8, 2024 10:48pm

@pattisdr pattisdr changed the title If any of the notices we're adding onto a migrated experience config … Conditionally Enabling Migrated Experience Configs Mar 8, 2024
@pattisdr pattisdr requested a review from adamsachs March 8, 2024 22:51
Copy link

cypress bot commented Mar 8, 2024

Passing run #6586 ↗︎

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 1fa8a7b into f4a75ae...
Project: fides Commit: a2228a8dde ℹ️
Status: Passed Duration: 00:33 💡
Started: Mar 8, 2024 10:59 PM Ended: Mar 8, 2024 10:59 PM

Review all test suite changes for PR #4674 ↗︎

Copy link

codecov bot commented Mar 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.58%. Comparing base (f4a75ae) to head (1fa8a7b).

Additional details and impacted files
@@                        Coverage Diff                        @@
##           PROD-1458_multi_language_support    #4674   +/-   ##
=================================================================
  Coverage                             86.58%   86.58%           
=================================================================
  Files                                   335      335           
  Lines                                 19867    19867           
  Branches                               2545     2545           
=================================================================
  Hits                                  17202    17202           
  Misses                                 2198     2198           
  Partials                                467      467           

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

Copy link
Contributor

@adamsachs adamsachs 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 @pattisdr , thanks for tackling this before heading out!

i'll go ahead and merge it into our integration branch.

@adamsachs adamsachs merged commit ecbcfbf into PROD-1458_multi_language_support Mar 10, 2024
39 checks passed
@adamsachs adamsachs deleted the PROD-1797_conditionally_enable_created_experience_configs branch March 10, 2024 17:47
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