Adds proper support to MVT experiments and simplifies the usage of Op…

…timizely's SDK after update to >v4.5
viodotcom · Jun 17, 2024 · ea62f11 · ea62f11
1 parent fc32e5b
commit ea62f11
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 74 deletions.
diff --git a/packages/lib/src/integrations/optimizely.ts b/packages/lib/src/integrations/optimizely.ts
@@ -36,7 +36,6 @@ type ExperimentCacheType = {[key in ToggleIdType]: ExperimentToggleValueType}
 type CacheType = FeatureEnabledCacheType | ExperimentCacheType
 type ForcedTogglesType = {[Tkey in ToggleIdType]: ToggleValueType}
 
-let featureEnabledCache: FeatureEnabledCacheType = {}
 let experimentCache: ExperimentCacheType = {}
 const forcedToggles: ForcedTogglesType = {}
 
@@ -54,7 +53,6 @@ export const registerLibrary = (lib) => {
   optimizely = lib
 }
 
-const clearFeatureEnabledCache = () => (featureEnabledCache = {})
 const clearExperimentCache = () => (experimentCache = {})
 
 /**
@@ -77,7 +75,6 @@ export const forceToggles = (toggleKeyValues: {
 }
 
 const invalidateCaches = () => {
-  clearFeatureEnabledCache()
   clearExperimentCache()
 }
 
@@ -134,24 +131,13 @@ export enum ExperimentType {
  *
  * It would be best if Opticks abstracts this difference from the client in future versions.
  */
-interface ActivateMVTNotificationPayload extends ListenerPayload {
+interface ActivateNotificationPayload extends ListenerPayload {
   type: ExperimentType.mvt
   decisionInfo: {
     experimentKey: ToggleIdType
     variationKey: VariantType
   }
 }
-interface ActivateFlagNotificationPayload extends ListenerPayload {
-  type: ExperimentType.flag
-  decisionInfo: {
-    flagKey: ToggleIdType
-    enabled: boolean
-  }
-}
-
-export type ActivateNotificationPayload =
-  | ActivateMVTNotificationPayload
-  | ActivateFlagNotificationPayload
 
 /**
  * Initializes Opticks with the supplied Optimizely datafile,
@@ -185,11 +171,32 @@ export const initialize = (
  */
 export const addActivateListener = (
   listener: NotificationListener<ActivateNotificationPayload>
-) =>
-  optimizelyClient.notificationCenter.addNotificationListener(
+) => {
+  const decisionListener = (payload: ActivateNotificationPayload) => {
+    const {variationKey} = payload.decisionInfo
+    const decision =
+      variationKey === 'on' ? 'b' : variationKey === 'off' ? 'a' : variationKey
+
+    const updatedPayload = {
+      ...payload,
+      decisionInfo: {...payload.decisionInfo, variationKey: decision}
+    }
+
+    return listener(updatedPayload)
+  }
+
+  /**
+   * This is a temporary support for the initial convention defined during the migration that "on" === "b" and "off" === "a"
+   * With the latest SDK version, A/B tests and target deliveries return a string key for the variation
+   * We will migrate the current experiments to the new convention and remove this temporary support.
+   * In the new convention we will always use the variation key as the decision.
+   */
+  // TODO (@vlacerda) [2024-06-30]: By this time we should ping @vlacerda to evaluate again if the fix is still needed and remove it if not.
+  return optimizelyClient.notificationCenter.addNotificationListener(
     NOTIFICATION_TYPES.DECISION,
-    listener
+    decisionListener
   )
+}
 
 const isForcedOrCached = (toggleId: ToggleIdType, cache: CacheType): boolean =>
   forcedToggles.hasOwnProperty(toggleId) || cache.hasOwnProperty(toggleId)
@@ -209,27 +216,6 @@ const validateUserId = (id) => {
   if (!id) throw new Error('Opticks: Fatal error: user id is not set')
 }
 
-const getToggleDecisionStatus = (
-  toggleId: ToggleIdType
-): ExperimentToggleValueType => {
-  validateUserId(userId)
-
-  const DEFAULT = false
-
-  if (isForcedOrCached(toggleId, featureEnabledCache)) {
-    const value = getForcedOrCached(toggleId, featureEnabledCache)
-    return typeof value === 'boolean' ? value : DEFAULT
-  }
-
-  userContext = optimizelyClient.createUserContext(
-    userId,
-    audienceSegmentationAttributes
-  )
-  const decision = userContext.decide(toggleId)
-
-  return (featureEnabledCache[toggleId] = decision.enabled)
-}
-
 /**
  * Determines whether a user satisfies the audience requirements for a toggle.
 
@@ -241,34 +227,43 @@ const getToggleDecisionStatus = (
 export const isUserInRolloutAudience = (toggleId: ToggleIdType) => {
   // @ts-expect-error we're being naughty here and using internals
   const config = optimizelyClient.projectConfigManager.getConfig()
+  // feature in the config object represents an a/b test
   const feature = config.featureKeyMap[toggleId]
+  // rollout is a targeted delivery
   const rollout = config.rolloutIdMap[feature.rolloutId]
+  // both a/b tests and targeted deliveries can have audiences
+  const allRules = [...rollout.experiments]
+
+  /**
+   * The feature object supplies ids through experimentIds.
+   * We find the rules for each experiment and add them to the allRules array.
+   */
+  if (feature.experimentIds.length > 0) {
+    const {experimentIds} = feature
+    const experimentRules = experimentIds.map(
+      (experimentId) => config.experimentIdMap[experimentId]
+    )
+    allRules.push(...experimentRules)
+  }
 
-  const endIndex = rollout.experiments.length - 1
-  let index: number
-  let isInAnyAudience = false
-
-  for (index = 0; index <= endIndex; index++) {
-    const rolloutRule = rollout.experiments[index]
-
+  const isInAnyAudience = allRules.reduce((acc, rule) => {
     // Reference: https://github.com/optimizely/javascript-sdk/blob/851b06622fa6a0239500b3b65e2d3937334960de/lib/core/decision_service/index.ts#L403
     const decisionIfUserIsInAudience =
       // @ts-expect-error we're being naughty here and using internals
       optimizelyClient.decisionService.checkIfUserIsInAudience(
         config,
-        rolloutRule,
+        rule,
         'rule',
         userContext,
         audienceSegmentationAttributes,
         ''
       )
 
-    if (
-      decisionIfUserIsInAudience.result &&
-      !isPausedBooleanToggle(rolloutRule)
-    )
-      isInAnyAudience = true
-  }
+    if (decisionIfUserIsInAudience.result && !isPausedBooleanToggle(rule))
+      return true
+
+    return acc
+  }, false)
 
   return isInAnyAudience
 }
@@ -302,19 +297,26 @@ const getToggle = (toggleId: ToggleIdType): ExperimentToggleValueType => {
     return typeof value === 'string' ? value : DEFAULT
   }
 
+  userContext = optimizelyClient.createUserContext(
+    userId,
+    audienceSegmentationAttributes
+  )
+
+  const variationKey = userContext.decide(toggleId).variationKey
+
+  /**
+   * This is a temporary support for the initial convention defined during the migration that "on" === "b" and "off" === "a"
+   * With the latest SDK version, A/B tests and target deliveries return a string key for the variation
+   * We will migrate the current experiments to the new convention and remove this temporary support.
+   * In the new convention we will always use the variation key as the decision.
+   */
+  // TODO (@vlacerda) [2024-06-30]: By this time we should ping @vlacerda to evaluate again if the fix is still needed and remove it if not.
+  const decision =
+    variationKey === 'on' ? 'b' : variationKey === 'off' ? 'a' : variationKey
+
   // Assuming the variation keys follow a, b, c, etc. convention
   // TODO: Enforce ^ ?
-  return (experimentCache[toggleId] =
-    optimizelyClient.activate(
-      toggleId,
-      userId,
-      audienceSegmentationAttributes
-    ) || DEFAULT)
-}
-
-const convertBooleanToggleToFeatureVariant = (toggleId: ToggleIdType) => {
-  const isFeatureEnabled = getToggleDecisionStatus(toggleId)
-  return isFeatureEnabled ? 'b' : 'a'
+  return (experimentCache[toggleId] = decision || DEFAULT)
 }
 
 /**
@@ -335,15 +337,7 @@ export function toggle<A extends any[]>(
   ...variants: A
 ): ToggleFuncReturnType<A>
 export function toggle(toggleId: ToggleIdType, ...variants) {
-  // An A/B/C... test
-  if (variants.length > 2) {
-    return baseToggle(getToggle)(toggleId, ...variants)
-  } else {
-    return baseToggle(convertBooleanToggleToFeatureVariant)(
-      toggleId,
-      ...variants
-    )
-  }
+  return baseToggle(getToggle)(toggleId, ...variants)
 }
 
 /**

diff --git a/packages/lib/src/types.ts b/packages/lib/src/types.ts
@@ -1,6 +1,8 @@
 export type ToggleIdType = string
 
-export type VariantType = 'a' | 'b' | 'c' | 'd' | 'e' | 'f'
+// TODO (@vlacerda) [2024-06-30]: By this time we should ping @vlacerda to evaluate again if the fix is still needed and remove it if not.
+// For more context, look at the other comment in this file in optimizely.ts marked with the same TODO date.
+export type VariantType = 'a' | 'b' | 'c' | 'd' | 'e' | 'f' | 'off' | 'on'
 
 // Value Toggle
 export type ToggleType = {