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

[sc-118386] - Adds proper support to MVT experiments #83

Merged

Conversation

vlacerda
Copy link
Contributor

@vlacerda vlacerda commented Jun 17, 2024

Description

Disclaimer: This PR enables code found here.

(To test you need to run both branches locally and make Opticks a local dependency.)

This PR adds support to MVT tests and simplifies the usage of Optimizely JS SDK. This is achieved through:

Removing the distinction between feature and experiment.

Optimizely's SDK now makes no distinction on their decisionInfo contract, leading to a simplification of code and a need for conventions to be set in place.

Retro-compatible support for on and off

We added temporary retro-compatibility to transforming on and off variants into b and a respectively.
Why we needed this? We used to use the property decisionInfo.enabled to determine which side the customer would fall in a rollout, by using convertBooleanToggleToFeatureVariant to convert enabled:true|false into b or a.
enabled in its turn, on Optimizely's side responds to "On", "Off" automatically. This logic was fine if we had only rollouts. However, enabled is not used on MVT (enabled is boolean, MVT is multivariant).
We had to find a different way to return the variant. Given that now we can set the value of variationKey, we can as well use a, b, c, ... directly into Optimizely's dashboard. This led to a simplification on our code and more intuitive usage of the product.

Shortcut Story

https://app.shortcut.com/findhotel/story/XXXXX

[sc-XXXXX]

How to review/test this change

Checklist

Mandatory

  • I have made sure this PR is associated to a Shortcut story
  • I have filled this PR's metadata as needed (labels, assignee, reviewers)
  • All new and existing tests passed

Optional (as needed)

  • I have tested my changes in a CE
  • I have updated the documentation
  • I have added tests to cover my changes

@vlacerda vlacerda changed the title [sc-118386] - Adds proper support to MVT experiments and simplifies the usage of Op… [sc-118386] - Adds proper support to MVT experiments Jun 17, 2024
@vlacerda vlacerda force-pushed the sc-118386-use-the-new-optimizely-datafile-in-production branch from a43d02c to b7f0b0a Compare June 19, 2024 13:18
@vlacerda vlacerda merged commit 5e7f8ae into master Jun 19, 2024
1 check passed
@vlacerda vlacerda deleted the sc-118386-use-the-new-optimizely-datafile-in-production branch June 19, 2024 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