From 1aa2f090e045e73e7dfdaf89c0cfc4e526d267a9 Mon Sep 17 00:00:00 2001 From: Jonas Huckestein Date: Wed, 2 Oct 2013 15:50:37 -0400 Subject: [PATCH 1/6] add include_rails_helper option --- README.mdown | 1 + lib/split/configuration.rb | 2 ++ lib/split/engine.rb | 8 +++++--- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/README.mdown b/README.mdown index 81bdfa1e..c33ffdb2 100644 --- a/README.mdown +++ b/README.mdown @@ -344,6 +344,7 @@ Split.configure do |config| config.enabled = true config.persistence = Split::Persistence::SessionAdapter #config.start_manually = false ## new test will have to be started manually from the admin panel. default false + config.include_rails_helper = true end ``` diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb index e0eaaaac..01f69c7e 100644 --- a/lib/split/configuration.rb +++ b/lib/split/configuration.rb @@ -17,6 +17,7 @@ class Configuration attr_accessor :on_trial_complete attr_accessor :on_experiment_reset attr_accessor :on_experiment_delete + attr_accessor :include_rails_helper attr_reader :experiments @@ -172,6 +173,7 @@ def initialize @experiments = {} @persistence = Split::Persistence::SessionAdapter @algorithm = Split::Algorithms::WeightedSample + @include_rails_helper = true end private diff --git a/lib/split/engine.rb b/lib/split/engine.rb index 692655f1..04392e76 100644 --- a/lib/split/engine.rb +++ b/lib/split/engine.rb @@ -1,8 +1,10 @@ module Split class Engine < ::Rails::Engine initializer "split" do |app| - ActionController::Base.send :include, Split::Helper - ActionController::Base.helper Split::Helper + if Split.configuration.include_rails_helper + ActionController::Base.send :include, Split::Helper + ActionController::Base.helper Split::Helper + end end end -end \ No newline at end of file +end From 86a497ddd59f825730e2105dab8db091c414bf02 Mon Sep 17 00:00:00 2001 From: Jonas Huckestein Date: Wed, 2 Oct 2013 17:13:53 -0400 Subject: [PATCH 2/6] fix experiment setting parsing for boolean values --- lib/split/configuration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb index 01f69c7e..625ce9e0 100644 --- a/lib/split/configuration.rb +++ b/lib/split/configuration.rb @@ -180,7 +180,7 @@ def initialize def value_for(hash, key) if hash.kind_of?(Hash) - hash[key.to_s] || hash[key.to_sym] + hash.has_key?(key.to_s) ? hash[key.to_s] : hash[key.to_sym] end end From 59942e24609e81ac1e77bffe3b2261392b46df2f Mon Sep 17 00:00:00 2001 From: Jonas Huckestein Date: Wed, 2 Oct 2013 17:14:36 -0400 Subject: [PATCH 3/6] include resettable experiment setting from Split.configuration.experiments --- lib/split/configuration.rb | 4 ++++ spec/configuration_spec.rb | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb index 625ce9e0..011fffd9 100644 --- a/lib/split/configuration.rb +++ b/lib/split/configuration.rb @@ -119,6 +119,10 @@ def normalized_experiments if goals = value_for(settings, :goals) experiment_config[experiment_name.to_sym][:goals] = goals end + + if (resettable = value_for(settings, :resettable)) != nil + experiment_config[experiment_name.to_sym][:resettable] = resettable + end end experiment_config diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 568c8453..44b429f5 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -86,7 +86,7 @@ end it 'should normalize experiments' do - @config.normalized_experiments.should == {:my_experiment=>{:alternatives=>["Control Opt", ["Alt One", "Alt Two"]]}} + @config.normalized_experiments.should == {:my_experiment=>{:resettable=>false,:alternatives=>["Control Opt", ["Alt One", "Alt Two"]]}} end end @@ -112,7 +112,7 @@ end it "should normalize experiments" do - @config.normalized_experiments.should == {:my_experiment=>{:alternatives=>[{"Control Opt"=>0.67}, + @config.normalized_experiments.should == {:my_experiment=>{:resettable=>false,:alternatives=>[{"Control Opt"=>0.67}, [{"Alt One"=>0.1}, {"Alt Two"=>0.23}]]}, :another_experiment=>{:alternatives=>["a", ["b"]]}} end @@ -139,7 +139,7 @@ end it "should normalize experiments" do - @config.normalized_experiments.should == {:my_experiment=>{:alternatives=>["Control Opt", ["Alt One", "Alt Two"]]}} + @config.normalized_experiments.should == {:my_experiment=>{:resettable=>false,:alternatives=>["Control Opt", ["Alt One", "Alt Two"]]}} end end From 217d41d20c654a8d02746e5b67fcee97de497c80 Mon Sep 17 00:00:00 2001 From: Jonas Huckestein Date: Wed, 2 Oct 2013 17:15:27 -0400 Subject: [PATCH 4/6] Add Split::EncapsulatedHelper that you can mix into Rails controllers and models that will only expose ab_test and ab_test_finished --- lib/split/encapsulated_helper.rb | 52 ++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 lib/split/encapsulated_helper.rb diff --git a/lib/split/encapsulated_helper.rb b/lib/split/encapsulated_helper.rb new file mode 100644 index 00000000..6542a94f --- /dev/null +++ b/lib/split/encapsulated_helper.rb @@ -0,0 +1,52 @@ +# Split's helper exposes all kinds of methods we don't want to +# mix into our model classes. +# +# This module exposes only two methods +# - ab_test and +# - ab_test_finished +# that can safely be mixed into any class. +# +# Passes the instance of the class that it's mixed into to the +# Split persistence adapter as context. +# +module Split + module EncapsulatedHelper + + class ContextShim + include Split::Helper + def initialize(context) + @context = context + end + def ab_user + @ab_user ||= Split::Persistence.adapter.new(@context) + end + end + + def ab_test(*arguments) + ret = split_context_shim.ab_test(*arguments) + # TODO there must be a better way to pass a block straight + # through to the original ab_test + if block_given? + if defined?(capture) # a block in a rails view + block = Proc.new { yield(ret) } + concat(capture(ret, &block)) + false + else + yield(ret) + end + else + ret + end + end + + def ab_test_finished(*arguments) + split_context_shim.finished *arguments + end + + private + + def split_context_shim + @split_context_shim ||= ContextShim.new self + end + end +end From 77d454e89b54170e620845a2f353b93d56b905c8 Mon Sep 17 00:00:00 2001 From: Jonas Huckestein Date: Wed, 2 Oct 2013 17:23:04 -0400 Subject: [PATCH 5/6] include encapsulated_helper --- lib/split.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/split.rb b/lib/split.rb index f876b119..25cbae61 100755 --- a/lib/split.rb +++ b/lib/split.rb @@ -7,6 +7,7 @@ helper metric persistence + encapsulated_helper trial version].each do |f| require "split/#{f}" From 67601fc3f9e5094d4ee1f35719a48871acd07a9d Mon Sep 17 00:00:00 2001 From: Jonas Huckestein Date: Thu, 3 Oct 2013 13:11:45 -0400 Subject: [PATCH 6/6] fix for params not found --- lib/split/encapsulated_helper.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/split/encapsulated_helper.rb b/lib/split/encapsulated_helper.rb index 6542a94f..4797c809 100644 --- a/lib/split/encapsulated_helper.rb +++ b/lib/split/encapsulated_helper.rb @@ -14,12 +14,16 @@ module EncapsulatedHelper class ContextShim include Split::Helper - def initialize(context) + def initialize(context, original_params) @context = context + @_params = original_params end def ab_user @ab_user ||= Split::Persistence.adapter.new(@context) end + def params + @_params + end end def ab_test(*arguments) @@ -45,8 +49,10 @@ def ab_test_finished(*arguments) private + # instantiate and memoize a context shim in case of multiple ab_test* calls def split_context_shim - @split_context_shim ||= ContextShim.new self + _params = defined?(params) ? params : {} + @split_context_shim ||= ContextShim.new(self, _params) end end end