assaf / vanity

Experiment Driven Development for Ruby
http://vanity.labnotes.org
MIT License
1.55k stars 269 forks source link

`Vanity.ab_test` when called from views doesn't work with `use_js = true` #305

Closed neudabei closed 7 years ago

neudabei commented 8 years ago

Hi there,

This PR is not for merge, but includes failing tests to describe an issue we've been having.

Steps to reproduce:

  1. Configure vanity with use_js = true
  2. Set up an A/B experiment :pie_or_cake
  3. Use Vanity.ab_test(:pie_or_cake) in a view

    Expected behaviour

Visitors to the page hosting the tested view should be added to the experiment via the JS callback

Actual behaviour

No callback is sent, and visitors are not added

Issue Description

We've been using Vanity and were confused by the methods ab_test(:pie_or_cake) and Vanity.ab_test(:pie_or_cake). Calling the latter from a view does not lead vanity_js to add participants to the experiment. However calling ab_test(:pie_or_cake) followed by vanity_js in the same view does work.

The tests included in this PR illustrate the difference in behaviour.

Vanity.ab_test(:pie_or_cake) only works when the experiment is initiated in the controller.

I think this can be attributed to the fact that the vanity method ab_test exists twice in the codebase of the Vanity gem [1][2]. When calling ab_test the method ab_test from [1] is invoked and when calling Vanity.ab_test the method ab_test from [2] is invoked.

The difference between the two seems to lie in how information about the initialization of the Vanity AB test for a participant is shared between the controller and the view. When Vanity.ab_test, meaning ab_test from [2] is called from the view the information is merely set on the instance of the controller at a time the view has already been rendered. Subsequently a call to vanity_js [3] from the same view does not insert the Javascript snippet into the view needed to add participants to the experiment.

Therefore Vanity.ab_test works when being called from the controller prior to rendering of a page that calls vanity_js. If the Vanity experiment should be initialized from a view or one of its helpers we should call ab_test instead.

Unfortunately we were not able to find a fix for this issue, but hopefully reproducing this behavior in the test is somewhat useful.

phillbaker commented 8 years ago

@neudabei thanks for the test case, I'll see if I can take a look at fixing this. This may be similar to the bug reported here: https://github.com/assaf/vanity/issues/199#issuecomment-237904592

phillbaker commented 7 years ago

@neudabei I've pushed a fix on the bugfix/js branch, which fixes the test case here. Want to give it a shot before I merge it?

neudabei commented 7 years ago

@phillbaker That fixed it. We are now able to call either Vanity.ab_test(:pie_or_cake) or ab_test(:pie_or_cake) in the view. The JS callback works in both cases and participants are added to the experiment. That's great. Thanks a lot for the fix!