Merge pull request #625 from robin-phung/trial-goal-callback

Update Trial so that goals are accessible in callback

Author: andrehjr

lib/split/helper.rb

@@ -51,9 +51,14 @@ def finish_experiment(experiment, options = {:reset => true})
         return true
       else
         alternative_name = ab_user[experiment.key]
-        trial = Trial.new(:user => ab_user, :experiment => experiment,
-              :alternative => alternative_name)
-        trial.complete!(options[:goals], self)
+        trial = Trial.new(
+          :user => ab_user, 
+          :experiment => experiment,
+          :alternative => alternative_name,
+          :goals => options[:goals],
+        )      
+        
+        trial.complete!(self)
 
         if should_reset
           reset!(experiment)

lib/split/trial.rb

@@ -1,13 +1,15 @@
 # frozen_string_literal: true
 module Split
   class Trial
+    attr_accessor :goals
     attr_accessor :experiment
     attr_writer :metadata
 
     def initialize(attrs = {})
       self.experiment   = attrs.delete(:experiment)
       self.alternative  = attrs.delete(:alternative)
       self.metadata  = attrs.delete(:metadata)
+      self.goals = attrs.delete(:goals) || []
 
       @user             = attrs.delete(:user)
       @options          = attrs
@@ -33,7 +35,7 @@ def alternative=(alternative)
       end
     end
 
-    def complete!(goals=[], context = nil)
+    def complete!(context = nil)
       if alternative
         if Array(goals).empty?
           alternative.increment_completion

spec/helper_spec.rb

@@ -1138,15 +1138,9 @@ def should_finish_experiment(experiment_name, should_finish=true)
       end
 
       it "should increment the counter for the specified-goal completed alternative" do
-        expect(lambda {
-          expect(lambda {
-            ab_finished({"link_color" => ["purchase"]})
-          }).not_to change {
-            Split::Alternative.new(@alternative_name, @experiment_name).completed_count(@goal2)
-          }
-        }).to change {
-          Split::Alternative.new(@alternative_name, @experiment_name).completed_count(@goal1)
-        }.by(1)
+        expect{ ab_finished({"link_color" => ["purchase"]}) }
+          .to change{ Split::Alternative.new(@alternative_name, @experiment_name).completed_count(@goal2) }.by(0)
+          .and change{ Split::Alternative.new(@alternative_name, @experiment_name).completed_count(@goal1) }.by(1)
       end
     end
   end

spec/trial_spec.rb

@@ -229,38 +229,37 @@ def expect_alternative(trial, alternative_name)
   end
 
   describe "#complete!" do
-    let(:trial) { Split::Trial.new(:user => user, :experiment => experiment) }
     context 'when there are no goals' do
+      let(:trial) { Split::Trial.new(:user => user, :experiment => experiment) }
       it 'should complete the trial' do
         trial.choose!
         old_completed_count = trial.alternative.completed_count
         trial.complete!
-        expect(trial.alternative.completed_count).to be(old_completed_count+1)
+        expect(trial.alternative.completed_count).to eq(old_completed_count + 1)
       end
     end
 
-    context 'when there are many goals' do
-      let(:goals) { ['first', 'second'] }
+    context "when there are many goals" do
+      let(:goals) { [ "goal1", "goal2" ] }
       let(:trial) { Split::Trial.new(:user => user, :experiment => experiment, :goals => goals) }
-      shared_examples_for "goal completion" do
-        it 'should not complete the trial' do
-          trial.choose!
-          old_completed_count = trial.alternative.completed_count
-          trial.complete!(goal)
-          expect(trial.alternative.completed_count).to_not be(old_completed_count+1)
-        end
-      end
 
-      describe 'Array of Goals' do
-        let(:goal) { [goals.first] }
-        it_behaves_like 'goal completion'
+      it "increments the completed count corresponding to the goals" do
+        trial.choose!
+        old_completed_counts = goals.map{ |goal| [goal, trial.alternative.completed_count(goal)] }.to_h 
+        trial.complete!
+        goals.each { | goal | expect(trial.alternative.completed_count(goal)).to eq(old_completed_counts[goal] + 1) }
       end
+    end
 
-      describe 'String of Goal' do
-        let(:goal) { goals.first }
-        it_behaves_like 'goal completion'
+    context "when there is 1 goal of type string" do
+      let(:goal) { "goal" }
+      let(:trial) { Split::Trial.new(:user => user, :experiment => experiment, :goals => goal) }
+      it "increments the completed count corresponding to the goal" do
+        trial.choose!
+        old_completed_count = trial.alternative.completed_count(goal)
+        trial.complete!
+        expect(trial.alternative.completed_count(goal)).to eq(old_completed_count + 1)
       end
-
     end
   end