Skip to content

fix experience errors when window.Fides isn't present#6084

Merged
gilluminate merged 1 commit intomainfrom
gill/fix-experience-errors
Apr 24, 2025
Merged

fix experience errors when window.Fides isn't present#6084
gilluminate merged 1 commit intomainfrom
gill/fix-experience-errors

Conversation

@gilluminate
Copy link
Copy Markdown
Contributor

@gilluminate gilluminate commented Apr 23, 2025

Closes LJ-711

Description Of Changes

Part of the clean up function was setting window.Fides to null. This creates a race condition where sometimes the rest of the cleanup hasn't taken place yet, or the new iteration hasn't been created yet and the assumption of the code at that time is that window.Fides exists. At any rate, setting that to null was a bit overkill for the cleanup anyway, let's just remove it for now.

Code Changes

  • remove setting window.Fides to null in the fides-preview script

Steps to Confirm

  1. Save a TCF experience a few times in Admin UI

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 23, 2025

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

Name Status Preview Comments Updated (UTC)
fides-privacy-center ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 24, 2025 3:16pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Apr 24, 2025 3:16pm

@gilluminate gilluminate force-pushed the gill/fix-experience-errors branch from 9a6b0e9 to 980d132 Compare April 24, 2025 15:15
@gilluminate gilluminate merged commit a958bde into main Apr 24, 2025
17 checks passed
@gilluminate gilluminate deleted the gill/fix-experience-errors branch April 24, 2025 15:36
@cypress
Copy link
Copy Markdown

cypress Bot commented Apr 24, 2025

fides    Run #12867

Run Properties:  status check passed Passed #12867  •  git commit a958bde621: fix experience errors when window.Fides isn't present (#6084)
Project fides
Branch Review main
Run status status check passed Passed #12867
Run duration 00m 50s
Commit git commit a958bde621: fix experience errors when window.Fides isn't present (#6084)
Committer Jason Gill
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 5
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

@tvandort tvandort mentioned this pull request Apr 29, 2025
16 tasks
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