-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: Support page level full redirection #43
Conversation
@ramboz Could you help review this? Please let me know where I could improve. Thank you :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor fine-tuning comments, but otherwise all good 👍
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #43 +/- ##
=====================================
Coverage ? 92.89%
=====================================
Files ? 1
Lines ? 704
Branches ? 12
=====================================
Hits ? 654
Misses ? 38
Partials ? 12 ☔ View full report in Codecov by Sentry. |
@ramboz Could you help review this again? Thank you :) I realized the test failures were caused by the fireRUM function. Initially, I defined all configurations upfront—experiment, campaign, and audience—regardless of whether runExperiment, runCampaign, or serveAudience was calling it. To fix it, I refactored it to create configurations only when needed. Also, I added this part into documentation 😀 And I figured out why my local npm test always fail all tests:
|
Description
The aim is to run full-page experimentation and personalization use cases that fully redirect to the target URL instead of just replacing the content for a marketer.
The new properties are introduced:
When simulate the variant, rather than 'replace' the page, url is redirected by calling
window.location.replace(url)
APIHow Has This Been Tested?
Test link:
Types of changes
Checklist: