From 9c36c839854314d4d3510389fe72edec70fec82a Mon Sep 17 00:00:00 2001
From: Dhruv Kapadia <git@dkapadia.com>
Date: Fri, 26 Mar 2010 11:56:28 -0400
Subject: [PATCH] Bug in cached score computation, test case

---
 app/models/visitor.rb                                |  4 ++++
 config/environments/test.rb                          |  4 ++++
 db/migrate/20100326153823_update_incorrect_scores.rb | 20 ++++++++++++++++++++
 spec/models/visitor_spec.rb                          | 37 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20100326153823_update_incorrect_scores.rb

diff --git a/app/models/visitor.rb b/app/models/visitor.rb
index a9853e6..12e4db5 100644
--- a/app/models/visitor.rb
+++ b/app/models/visitor.rb
@@ -25,6 +25,10 @@ class Visitor < ActiveRecord::Base
     
     loser_choice = other_choices.first
     votes.create!(:question_id => prompt.question_id, :prompt_id => prompt.id, :voter_id=> self.id, :choice_id => choice.id, :loser_choice_id => loser_choice.id)
+    # Votes count is a cached value, creating the vote above will increment it in the db, but to get the proper score, we need to increment it in the current object
+    # The updated votes_count object is not saved to the db, so we don't need to worry about double counting
+    # Alternatively, we could just do choice.reload, but that results in another db read
+    choice.votes_count +=1
     choice.compute_score! #update score after win
   end
   
diff --git a/config/environments/test.rb b/config/environments/test.rb
index 7763b1b..409fb44 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -49,3 +49,7 @@ PAYPAL_API_LOGIN              = ''
 PAYPAL_API_PASSWORD           = ''
 PAYPAL_API_SIGNATURE          = ''
 
+begin
+  require 'factory_girl'
+rescue LoadError
+end
diff --git a/db/migrate/20100326153823_update_incorrect_scores.rb b/db/migrate/20100326153823_update_incorrect_scores.rb
new file mode 100644
index 0000000..e2e67fa
--- /dev/null
+++ b/db/migrate/20100326153823_update_incorrect_scores.rb
@@ -0,0 +1,20 @@
+class UpdateIncorrectScores < ActiveRecord::Migration
+  def self.up
+
+	  # some scores were computed incorrectly as a result of a bug
+	  # This only affected the cached value of the score, so we only need to recalculate the correct score
+	  # for those choices voted on in the last few days
+	  startDate = 2.days.ago
+	  votes = Vote.find(:all, :conditions => ['created_at > ?', startDate])
+	  toupdate = votes.inject(Set.new){|updatethese, v| updatethese << v.choice_id}
+	  choices = Choice.find(:all, :conditions => {'id' => toupdate.to_a})
+
+	  choices.each do |c|
+		  c.compute_score!
+	  end
+
+  end
+
+  def self.down
+  end
+end
diff --git a/spec/models/visitor_spec.rb b/spec/models/visitor_spec.rb
index 6cb8ad4..c14b0ed 100644
--- a/spec/models/visitor_spec.rb
+++ b/spec/models/visitor_spec.rb
@@ -6,6 +6,7 @@ describe Visitor do
   it {should have_many :questions}
   it {should have_many :votes}
   it {should have_many :skips}
+  it {should have_many :clicks}
   
   before(:each) do
     @aoi_clone = Factory.create(:user, :email => "pius@alum.mit.edu", :password => "password", :password_confirmation => "password", :id => 8)
@@ -29,6 +30,9 @@ describe Visitor do
   
   it "should be able to determine ownership of a question" do
     @v.owns?(Question.new).should be_false
+
+    ownedquestion = Factory.create(:question, :site => @aoi_clone, :creator=> @johndoe)
+    @johndoe.owns?(ownedquestion).should be_true
   end
   
   it "should be able to vote for a prompt" do
@@ -37,10 +41,41 @@ describe Visitor do
     v = @v.vote_for! @prompt, 0
   end
   
-  it "should be able to vote for a prompt" do
+  it "should be able to skip a prompt" do
     #@prompt = @question.prompts.first
     @prompt.should_not be_nil
     v = @v.skip! @prompt
   end
+
+  it "should accurately update score counts after vote" do
+    prev_winner_score = @lc.score
+    prev_loser_score = @rc.score
+    
+    vote = @v.vote_for! @prompt, 0
+    
+    @lc.reload
+    @rc.reload
+
+    @lc.score.should > prev_winner_score
+    @rc.score.should < prev_loser_score
+  end
+  
+  it "should accurately update win and loss totals after vote" do
+    prev_winner_wins = @lc.wins
+    prev_winner_losses = @lc.losses
+    prev_loser_losses = @rc.losses
+    prev_loser_wins = @rc.wins
+    
+    vote = @v.vote_for! @prompt, 0
+    
+    @lc.reload
+    @rc.reload
+
+    @lc.wins.should == prev_winner_wins + 1
+    @lc.losses.should == prev_winner_losses
+    @rc.losses.should ==  prev_loser_losses + 1
+    @rc.wins.should ==  prev_winner_wins
+  end
+
     
 end
--
libgit2 0.21.2