react-sdk icon indicating copy to clipboard operation
react-sdk copied to clipboard

[BUG] setForcedDecision does not work after update to version 3.0.1

Open haivle opened this issue 1 year ago • 6 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

SDK Version

@optimizely/react-sdk: 3.0.1

Current Behavior

After calling setForcedDecision, useDecision(name, { autoUpdate: true }) is updated.

Expected Behavior

useDecision returns the latest data

Steps To Reproduce

  1. Initialise Optimizely instance
optimizelyInstance = createInstance({
        sdkKey: env.OPTIMIZELY_SDK_KEY,
        odpOptions: {
               disabled: true,
        },
        logLevel: env.IS_VERBOSE ? 1 : 4,
});

optimizelyInstance.onUserUpdate((a) => {
      optimizely.setForcedDecision({ flagKey: "flag1" }, { variationKey: "variation1" });
});
  1. In React
const [decision] = useDecision("flag1", { 
    autoUpdate: true 
}); 

React Framework

No response

Browsers impacted

No response

Link

No response

Logs

No response

Severity

Blocking development

Workaround/Solution

No response

Recent Change

No response

Conflicts

No response

haivle avatar Apr 05 '24 08:04 haivle

Let me loop back to this. I'm working to finalise #255

mikechu-optimizely avatar Apr 05 '24 20:04 mikechu-optimizely

We've released v3.1.0 of the React SDK.

When you have a moment, please let me know if setForcedDecision works as you/we expect.

I'll create a test scenario as you've described to test.

mikechu-optimizely avatar Apr 09 '24 18:04 mikechu-optimizely

Hey there! Was going to write up an issue about this same topic. Currently on v3.1.0 and was running into issues forcing a user into a variation and wasn't sure if this is related to the same bug or not, but thought I would post it here.

We are currently writing a light wrapper around the useDecision react hook so that we do things like pass overrides and also have logic to force decisions before calling the useDecision hook.

Here is a simplified version of what our wrapper looks like:

function useDecisionWrapper(featureKey, options, overrides){ 
  const { optimizely } = useContext(OptimizelyContext);

  const [decisionContext, forcedDecision] = getForcedDecisionOverrides(featureKey); // grabs data from cookies to force a decision
  if(decisionContext && forcedDecision){
      const user = optimizely.getUserContext();
      user?.setForcedDecision(decisionContext, forcedDecision);
  }

  return useDecision(featureKey, options, overrides);
}

I'd assume that since we called setForcedDecision for the current user, calling useDecision would respect the forced decision but it seems to be not. Would love some feedback or suggestions if I am doing something wrong! I will also test v3.1.0 tomorrow! Thank you :)

byron-ino avatar Apr 10 '24 04:04 byron-ino

I did a quick code dive and see that in the JS SDK, decide() finds and should use a matched forced decision.

I've created an internal ticket (FSSDK-10120) to trace.

mikechu-optimizely avatar Apr 10 '24 14:04 mikechu-optimizely

Hey there @mikechu-optimizely! Just wanted to follow up on some findings I did today.

While playing around with the APIs, one interesting thing I found was that calling .decide() from the userContext object seems to force users into a decision as expected, but calling .decide() from the optimizely instance does not.

Example below shows us calling both user.decide() and optimizely.decide() and seeing different results

 optimizely = createOptimizelyInstance({
    sdkKey,
    datafile,
    datafileOptions: {
      autoUpdate: isServerSide ? false : true,
      ...datafileOptions,
    },
    logLevel: 'DEBUG',
    eventDispatcher: IS_PROD || enableTracking ? eventDispatcher : logOnlyEventDispatcher, // always enable tracking in prod
    odpOptions: { disabled: true }, // only needed if advanced audience targeting is on
    ...config,
  });

    const user = optimizely.getUserContext();
    user.setForcedDecision({ flagKey: "migration_initial_flag", ruleKey: 'initial_experiment' }, { variationKey: "off" });
    optimizely.setForcedDecision({ flagKey: "migration_initial_flag", ruleKey: 'initial_experiment' }, { variationKey: "off" });

    const optimizelyDecision = optimizely.decide('migration_initial_flag');
    const userDecision = user.decide('migration_initial_flag');

    console.log('optimizelyDecision', optimizelyDecision);
    console.log('userDecision:', userDecision);

The results of the console log are as followed:

optimizelyDecision:
{
    "variationKey": "on",
    "enabled": true,
    "variables": {},
    "ruleKey": "initial_experiment",
    "flagKey": "migration_initial_flag",
    "userContext": {
        "id": "e9b20618-732d-478e-9a9e-237496274489",
        "attributes": {
            "grx_internal_user": false,
            "location": "",
            "$opt_user_agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/123.0.0.0 Safari/537.36",
            "environment": "dev",
            "url_path": "/feature"
        }
    },
    "reasons": []
}
userDecision

{
  "variationKey": "off",
  "enabled": false,
  "variables": {},
  "ruleKey": "initial_experiment",
  "flagKey": "migration_initial_flag",
  "userContext": {
      "_qualifiedSegments": null,
      "optimizely": {}, //removed because its too big
      "userId": "e9b20618-732d-478e-9a9e-237496274489",
      "attributes": {
          "grx_internal_user": false,
          "location": "",
          "$opt_user_agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/123.0.0.0 Safari/537.36",
          "environment": "dev",
          "url_path": "/feature"
      },
      "forcedDecisionsMap": {
          "migration_initial_flag": {
              "initial_experiment": {
                  "variationKey": "off"
              }
          }
      }
  },
  "reasons": [
      "Variation (off) is mapped to flag (migration_initial_flag), rule (initial_experiment) and user (e9b20618-732d-478e-9a9e-237496274489) in the forced decision map."
  ]
}

As we can see, calling user.decide() respects the call to setForcedDecision but optimizely.decide() does not respect the setForcedDecision call. This is important because the underlying implementation of the useDecision react hook uses optimizely.decide() (Reference in Hooks.ts)

Any guidance or insight to successfully getting the useDecision hook to respect forced decisions would definitely be appreciated 🙏🏽

byron-ino avatar Apr 11 '24 05:04 byron-ino

Thanks for code diving with me.

I see in the underlying JS SDK that the OptimizelyUserContext.decide snags a copy of the current userContext with the forced decisions (if present) on its way into the client's decide() method.

So yeah, that's our difference.

🤔: React SDK's use of the client decide() instead of the user context's method means that forced decisions are not returned by the useDecision hook. I think this may have been by design at some point. I need to talk with our architect on this point since it's a public facing design.

mikechu-optimizely avatar Apr 11 '24 13:04 mikechu-optimizely

forced decisions is very important for automated tests and other quality assurance processes to make sure feature is working in production manually before starting gradual rollout. its been more than 5 months since issue was filed and last communication on the matter.

Its frustrating and disappointing as a paying corporate customer to see important feature being broken for months without end in sight

@mikechu-optimizely do you mind to prioritise the fix for this once working feature?

iamstarkov avatar Aug 14 '24 11:08 iamstarkov

I've pulled this Issue into today's stand-up. I'll update here by COB.

cc @junaed-optimizely

mikechu-optimizely avatar Aug 14 '24 13:08 mikechu-optimizely

We've pulled this issue & work ticket into this sprint.

mikechu-optimizely avatar Aug 14 '24 17:08 mikechu-optimizely

A new release v3.2.1 is out with the fix.

@iamstarkov, @byron-ino let us know if an upgrade fix the issue for you.

Thanks for your patience.

junaed-optimizely avatar Aug 15 '24 17:08 junaed-optimizely

I was also following this bug ^.^, I just tested my local project with the new version and works as expected 👍🏼 I hope others have the same outcome. Thanks for the quick fix.

juanpicado avatar Aug 16 '24 08:08 juanpicado

@junaed-optimizely @mikechu-optimizely thank you for a quick fix, we can confirm it works now

iamstarkov avatar Aug 16 '24 12:08 iamstarkov

Thanks for the confirmation updates. 🙏

mikechu-optimizely avatar Aug 16 '24 12:08 mikechu-optimizely

The issue has been resolved and confirmed by the clients. Closing this issue now. Thanks everyone!

junaed-optimizely avatar Aug 16 '24 12:08 junaed-optimizely