Skip to content

Conversation

@selanthiraiyan
Copy link
Contributor

@selanthiraiyan selanthiraiyan commented Feb 1, 2023

Closes: #8782

Description

Problem

User who was originally assigned treatment variation for a logged-out experimnent sees a different variation for the same experiment after logging out.

Reason for the change in assignment

  1. User launches app -> ABTest variations for logged-out context experiments are fetched from the server and saved.
  2. After login, due to Explat being reset, the locally saved experiments are deleted.
  3. We load the experiments for logged-in context. Now, only logged-in experiments have a variation assigned.
  4. User logs out from the Settings screen.
    1. Due to change in state, AppCoordinator navigates to the login screen here.
    2. This navigation to the login screen happens even before this line from Settings is executed.
    3. The navigation also happens before analytics (i.e. ABTest) is reloaded here.
  5. When the login screen is displayed, no variation value is assigned for the logged-out experiment, and by default, the control variant is shown to the user.

My fix in this PR

We fetched the variations for only specific contexts (logged in/out) when analytics is reset. I changed that behaviour and instead started fetching the variations for all available experiments.

  • The idea is to load the variations for all experiments irrespective of whether the user is logged in or not.
  • As an anonymous ID is assigned to requests sent in both logged in/out state here, we are receiving the correct variation for logged out experiments even if the user is logged in/
  • I believe the backend can use the provided anonID and auth token values to identify the device and user and provide correct variation values.

To verify this change works as expected

  • @selanthiraiyan to add logged-in and logged-out A/A tests to verify the behaviour and check for crossovers.

A better solution (Not in this PR)

Wait for the ABTest variations to reload and proceed.

Testing instructions

  • Install and launch the app. Logout if needed.
  • Launch and skip onboarding.
  • If you don't see the "Continue With WordPress.com" button to login you have been assigned treatment variation for the woocommerceios_login_rest_api_project_202301_v2 experiment and you can test this PR (If not you need to use a sandbox to for a variation. You can follow the instructions from this PR to assign a varition to your device.)
  • Login into the app using site creds.
  • Go to settings and logout.
  • You will be navigated to the login screen and ensure that you still don't see "Continue With WordPress.com" button. i.e. You are still assigned the treatment variation.

Screenshots

NA


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@selanthiraiyan selanthiraiyan changed the base branch from trunk to release/12.1 February 1, 2023 04:20
@selanthiraiyan selanthiraiyan changed the title Bugfix/8782 reload all ab test experiments Reload variations for all ABTest experiments Feb 1, 2023
@selanthiraiyan selanthiraiyan added type: bug A confirmed bug. feature: login Related to any part of the log in or sign in flow, or authentication. labels Feb 1, 2023
@selanthiraiyan selanthiraiyan marked this pull request as ready for review February 1, 2023 04:28
@selanthiraiyan selanthiraiyan changed the title Reload variations for all ABTest experiments [ABTest] Reload all experiments irrespective of logged in/out context Feb 1, 2023
@peril-woocommerce
Copy link

Warnings
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

@wpmobilebot
Copy link
Collaborator

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8788-c6320ab on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@itsmeichigo itsmeichigo self-assigned this Feb 1, 2023
Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works as expected to me 🎉 I like that this solution is simpler and more effective than making refreshUserData async and making changes everywhere.

What I'm concerned about is why we needed to register AB tests by context before. Will this affect the results of logged-in experiments if many users fail to reach the logged-in state?

@itsmeichigo
Copy link
Contributor

itsmeichigo commented Feb 1, 2023

In case starting the logged-in experiments too early can affect their results, how about we take a slightly different route:

  • After starting the logged-out experiments, we cache the assignments somewhere locally.
  • After logging out, if no variation value is found, we use the cached values instead.

WDYT? @selanthiraiyan

@selanthiraiyan
Copy link
Contributor Author

Will this affect the results of logged-in experiments if many users fail to reach the logged-in state?

This is a great point. I think you are right that this change will affect the results of experiments without an exposure event. We should explore other options.

After logging out, if no variation value is found, we use the cached values instead.

By doing this, we might end up using an old expired value as the variation instead of fetching and showing the latest variation value. We might be able to overcome this by using the Time-to-live value. I don't know whether I am missing any other edge cases.

@itsmeichigo
Copy link
Contributor

itsmeichigo commented Feb 1, 2023

By doing this, we might end up using an old expired value as the variation instead of fetching and showing the latest variation value.

Considering that anon_id stays the same for each device, I think that each device should be assigned to only one variant for each experiment. That means we don't need to worry about expired values - is that correct?

Plus, we will always fetch the assignments again after the user relaunches the app, so using cached values right after logging out should work IMO.

@selanthiraiyan
Copy link
Contributor Author

Considering that anon_id stays the same for each device, I think that each device should be assigned to only one variant for each experiment. That means we don't need to worry about expired values - is that correct?

I think you are right. 👍

Plus, we will always fetch the assignments again after the user relaunches the app, so using cached values right after logging out should work IMO.

I agree. I will explore this solution and raise a different PR.

Thank you, @itsmeichigo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: login Related to any part of the log in or sign in flow, or authentication. type: bug A confirmed bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants