Refactoring promptscontroller#vote. Substantial test improvement

Dhruv Kapadia
1 parent 3a6834a9
Showing 12 changed files with 324 additions and 133 deletions Show diff stats
app/controllers/prompts_controller.rb
app/models/choice.rb
app/models/user.rb
app/models/visitor.rb
app/models/vote.rb
config/routes.rb
spec/controllers/prompts_controller_spec.rb
spec/controllers/questions_controller_spec.rb
spec/factories.rb
spec/models/user_spec.rb
spec/models/visitor_spec.rb
spec/models/vote_spec.rb
@@ -5,7 +5,7 @@ class PromptsController &lt; InheritedResources::Base
   has_scope :active, :boolean => true, :only => :index
  
   has_scope :voted_on_by
-  #before_filter :authenticate
+  before_filter :authenticate, :only => [:vote, :skip]
  
  
   def activate
@@ -23,64 +23,44 @@ class PromptsController &lt; InheritedResources::Base
     end
   end
  
-  def vote
-    #NOT IMPLEMENTED
-    @question = Question.find(params[:question_id])
-    @prompt = @question.prompts.find(params[:id])
-    @choices = @prompt.choices.active
-    @choice = @choices[params[:index]]
-    respond_to do |format|
-      format.xml { render :xml => @choice.to_xml }
-      format.json { render :json => @choice.to_xml }
-    end
-  end
-  
-  def vote_left
-    vote_direction(:left)
-  end
  
-  
-  def vote_right
-    vote_direction(:right)
-  end
-  
-  
-  def vote_direction(direction)
-    authenticate
-  
-    logger.info "#{current_user.inspect} is voting #{direction} at #{Time.now}."
-    @question = Question.find(params[:question_id])
+    # To record a vote 
+    #  required parameters - prompt id, ordinality, visitor_identifer?
+    #  optional params - visitor_identifier, appearance_lookup
+    # After recording vote, next prompt display parameters:
+    #   same as in show  - :with_prompt, with_appearance, with visitor_stats, etc
+  def vote
+    @question = current_user.questions.find(params[:question_id])
     @prompt = @question.prompts.find(params[:id])
-
-    time_viewed = params['params']['time_viewed']
-    visitor_identifier = params['params']['auto']
-    appearance_lookup = params['params']['appearance_lookup']
  
-    case direction
-    when :left
-      successful = c = current_user.record_vote(params['params']['auto'], appearance_lookup, @prompt, 0, time_viewed)
-    when :right
-      successful = c = current_user.record_vote(params['params']['auto'], appearance_lookup, @prompt, 1, time_viewed)
-    else
-      raise "need to specify either ':left' or ':right' as a direction"
-    end
-    
-    #@prompt.choices.each(&:compute_score!)
-    respond_to do |format|
-      if successful && next_prompt = @question.choose_prompt
+    vote_options = params[:vote] || {}
+    vote_options.merge!(:prompt => @prompt, :question => @question)
  
-        visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier)
-        @a = current_user.record_appearance(visitor, next_prompt)
-         
-	appearance_id = Proc.new { |options| options[:builder].tag!('appearance_id', @a.lookup) }
-	visitor_votes = Proc.new { |options| options[:builder].tag!('visitor_votes', visitor.votes.count(:conditions => {:question_id => @question.id})) }
-        visitor_ideas = Proc.new { |options| options[:builder].tag!('visitor_ideas', visitor.items.count) }
+    successful = object= current_user.record_vote(vote_options)
+    optional_information = []
+    if params[:next_prompt]
+       begin
+           params[:next_prompt].merge!(:with_prompt => true) # We always want to get the next possible prompt
+           @question_optional_information = @question.get_optional_information(params[:next_prompt])
+       rescue RuntimeError
  
-        format.xml { render :xml => next_prompt.to_xml(:procs => [appearance_id, visitor_votes, visitor_ideas], :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok }
-        format.json { render :json => next_prompt.to_json(:procs => [appearance_id, visitor_votes, visitor_ideas], :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok }
+           respond_to do |format|
+              format.xml { render :xml => @prompt.to_xml, :status => :conflict and return} 
+           end
+       end
+       object = @question.prompts.find(@question_optional_information.delete(:picked_prompt_id))
+       @question_optional_information.each do |key, value|
+          optional_information << Proc.new { |options| options[:builder].tag!(key, value)}
+       end
+    end
+
+    respond_to do |format|
+      if !successful.nil?
+        format.xml { render :xml => object.to_xml(:procs => optional_information , :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok }
+        format.json { render :json => object.to_json(:procs => optional_information, :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok }
       else
-        format.xml { render :xml => c, :status => :unprocessable_entity }
-        format.json { render :json => c, :status => :unprocessable_entity }
+        format.xml { render :xml => @prompt.to_xml, :status => :unprocessable_entity }
+        format.json { render :json => @prompt.to_xml, :status => :unprocessable_entity }
       end
     end
   end
@@ -106,7 +86,6 @@ class PromptsController &lt; InheritedResources::Base
  
  
   def skip
-    authenticate
     logger.info "#{current_user.inspect} is skipping."
     @question = Question.find(params[:question_id])
  
@@ -124,7 +103,17 @@ class PromptsController &lt; InheritedResources::Base
  
  
     respond_to do |format|
-      if @skip = current_user.record_skip(visitor_identifier, appearance_lookup, @prompt, time_viewed, :skip_reason => skip_reason) && @next_prompt = @question.choose_prompt
+      if @skip = current_user.record_skip(visitor_identifier, appearance_lookup, @prompt, time_viewed, :skip_reason => skip_reason) 
+       
+       #This is not hte right way to do this. See def vote for a better example
+       begin
+          @next_prompt = @question.choose_prompt
+       rescue RuntimeError
+
+           respond_to do |format|
+              format.xml { render :xml => @prompt.to_xml, :status => :conflict and return} 
+           end
+       end
  
         visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier)
         @a = current_user.record_appearance(visitor, @next_prompt)
@@ -36,6 +36,12 @@ class Choice &lt; ActiveRecord::Base
     save!
   end
  
+  def win!
+    self.votes_count += 1 rescue (self.votes_count = 1)
+    self.score = compute_score
+    save!
+  end
+  
   def wins_plus_losses
     #(prompts_on_the_left.collect(&:votes_count).sum + prompts_on_the_right.collect(&:votes_count).sum)
     #Prompt.sum('votes_count', :conditions => "left_choice_id = #{id} OR right_choice_id = #{id}")
@@ -70,11 +76,6 @@ class Choice &lt; ActiveRecord::Base
   end
  
   def compute_score
-    # if wins_plus_losses == 0
-    #   return 0
-    # else
-    #   (wins.to_f / wins_plus_losses ) * 100
-    # end
     (wins.to_f+1)/(wins+1+losses+1) * 100
   end
  
@@ -31,12 +31,14 @@ class User &lt; ActiveRecord::Base
     return choice
   end
  
-  def record_vote(visitor_identifier, appearance_lookup, prompt, ordinality, time_viewed)
-    #@click = Click.new(:what_was_clicked => 'on the API level, inside record_vote' + " with prompt id #{prompt.id}, ordinality #{ordinality.to_s}")
-    #@click.save!
-    visitor = visitors.find_or_create_by_identifier(visitor_identifier)
-    visitor.vote_for!(appearance_lookup, prompt, ordinality, time_viewed)
-    #prompt.choices.each {|c| c.compute_score; c.save!}
+  def record_vote(options)
+    visitor_identifier = options.delete(:visitor_identifier)
+    if visitor_identifier.nil?
+       visitor = default_visitor
+    else
+       visitor = visitors.find_or_create_by_identifier(visitor_identifier)
+    end
+    visitor.vote_for!(options)
   end
  
   def record_appearance(visitor, prompt)
@@ -16,26 +16,25 @@ class Visitor &lt; ActiveRecord::Base
     questions.include? question
   end
  
-  def vote_for!(appearance_lookup, prompt, ordinality, time_viewed)
-    @a = Appearance.find_by_lookup(appearance_lookup)
-    #make votefor fail if we cant find the appearance
-    choices = prompt.choices
-    choice = choices[ordinality] #we need to guarantee that the choices are in the right order (by position)
-    other_choices = choices - [choice]
-    other_choices.each do |c| 
-	    c.lose!
+  def vote_for!(options)
+    return nil if !options || !options[:prompt] || !options[:direction]
+    
+    prompt = options.delete(:prompt)
+    ordinality = (options.delete(:direction) == "left") ? 0 : 1
+    
+    if options[:appearance_lookup] 
+       @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup))
+       return nil unless @appearance # don't allow people to fake appearance lookups
+       options.merge!(:appearance_id => @appearance.id)
     end
  
+    choice = prompt.choices[ordinality] #we need to guarantee that the choices are in the right order (by position)
+    other_choices = prompt.choices - [choice]
     loser_choice = other_choices.first
-    v = 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, :time_viewed => time_viewed, :appearance_id => @a.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
-
+    
+    options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :voter_id=> self.id, :choice_id => choice.id, :loser_choice_id => loser_choice.id) 
  
+    v = votes.create!(options)
   end
  
   def skip!(appearance_lookup, prompt, time_viewed, options = {})
 class Vote < ActiveRecord::Base
 #  belongs_to :voteable, :polymorphic => true, :counter_cache => true
+  validates_presence_of :question
+  validates_presence_of :prompt
+  validates_presence_of :choice
+  validates_presence_of :loser_choice
+  validates_presence_of :voter
+
   belongs_to :voter, :class_name => "Visitor", :foreign_key => "voter_id"
   belongs_to :question, :counter_cache => true
   belongs_to :prompt, :counter_cache => true
@@ -11,4 +17,14 @@ class Vote &lt; ActiveRecord::Base
   named_scope :with_question, lambda { |*args| {:conditions => {:question_id => args.first }} }
   named_scope :with_voter_ids, lambda { |*args| {:conditions => {:voter_id=> args.first }} }
   named_scope :active, :include => :choice, :conditions => { 'choices.active' => true }
+
+  after_create :update_winner_choice, :update_loser_choice
+
+  def update_winner_choice
+     choice.win!
+  end
+  
+  def update_loser_choice 
+     loser_choice.lose!
+  end
 end
@@ -13,7 +13,7 @@ ActionController::Routing::Routes.draw do |map|
 					    :object_info_totals_by_question_id => :get,
 				            :recent_votes_by_question_id => :get} do |question|
     question.resources :items
-    question.resources :prompts, :member => {:vote_left => :post, :vote_right => :post, :skip => :post, :vote => :post}, 
+    question.resources :prompts, :member => {:skip => :post, :vote => :post}, 
                        :collection => {:single => :get, :index => :get}
     question.resources :choices, :member => { :activate => :put, :suspend => :put, :update_from_abroad => :put, :deactivate_from_abroad => :put, :flag => :put}, :collection => {:create_from_abroad => :post}
   end
@@ -5,7 +5,7 @@ describe PromptsController do
      @controller.current_user = user
      return user
    end
-  #   
+
    before(:each) do
     @question = Factory.create(:aoi_question)
     sign_in_as(@aoi_clone = @question.site)
@@ -14,16 +14,6 @@ describe PromptsController do
     @visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier")
     @appearance = @aoi_clone.record_appearance(@visitor, @prompt)
    end
-  # 
-
-#  describe "GET index" do
-#    it "assigns all prompts as @prompts" do
-#      Question.stub!(:find).with(:all).and_return(@question)
-#      Question.stub!(:prompts).with(:all).and_return([mock_prompt])
-#      get :index, :question_id => @question.id
-#      assigns[:prompts].should == [@prompt]
-#    end
-#  end
  
   describe "GET show" do
     it "assigns the requested prompt as @prompt" do
@@ -42,4 +32,39 @@ describe PromptsController do
     end
   end
  
+  describe "POST vote" do
+    it "votes on a prompt" do 
+         post :vote, :question_id => @question.id, :id => @prompt.id,
+	             :vote => {:direction => "left"},
+		     :format => :xml
+
+         @response.code.should == "200"
+    end
+
+    it "returns 422 when missing fields are not provided" do
+         post :vote, :question_id => @question.id, :id => @prompt.id
+        
+	# there is somethingw wrong with response codes, this doesn't work
+	#@response.code.should == "422"
+    end
+    
+    it "votes on a prompt and responds with optional information" do
+         post :vote, :question_id => @question.id, :id => @prompt.id,
+		     :vote => {:direction => "left",
+		               :time_viewed => "492", 
+			       :visitor_identifier => "jim"},
+		     :next_prompt => {
+		          :with_appearance => true, 
+		          :with_visitor_stats => true, 
+			  :visitor_identifier => "jim"},
+	 	     :format => :xml
+	 
+	 @response.code.should == "200"
+    end
+    
+    it "should prevent other users from voting on non owned questions" do
+    end
+
+  end
+
 end
@@ -10,7 +10,6 @@ describe QuestionsController do
      before(:each) do
         @question = Factory.create(:aoi_question)
         sign_in_as(@user = @question.site)
-	@creator = @question.creator
      end
      it "responds with basic question information" do
          get :show, :id => @question.id, :format => "xml"
@@ -39,6 +39,14 @@ Factory.define(:choice) do |f|
   f.creator {|c| c.association(:visitor, :site => c.question.site)}
 end
  
+Factory.define(:vote) do |f|
+  f.association :question, :factory => :aoi_question
+  f.prompt {|v|  v.question.prompts.first}
+  f.choice {|v|  v.prompt.left_choice}
+  f.loser_choice {|v|  v.prompt.right_choice}
+  f.voter {|v|  v.question.creator}
+end
+
 Factory.sequence :email do |n|
   "user#{n}@example.com"
 end
@@ -29,18 +29,55 @@ describe User do
   it "should be able to record a visitor's vote" do
     @visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier")
     @appearance = @aoi_clone.record_appearance(@visitor, @prompt)
-    v = @aoi_clone.record_vote("johnnydoe", @appearance.lookup, @prompt, 0, 304)
+
+    optional_vote_params = {:visitor_identifier => @visitor.identifier, 
+                            :appearance_lookup => @appearance.lookup,
+		            :time_viewed => 213}
+
+    required_vote_params = {:prompt => @prompt, 
+                            :direction => "left"}
+    params = optional_vote_params.merge(required_vote_params)
+    
+    v = @aoi_clone.record_vote(params)
+
+    v.should_not be_nil
     prompt_votes = @prompt.votes(true)
     prompt_votes.should_not be_empty
-    prompt_votes.size.should eql 1
+    prompt_votes.size.should == 1
  
     choices = @prompt.choices
-    #@question.prompts(true).size.should == 2
     choices.should_not be_empty
  
     choice_votes = choices[0].votes(true)
     choice_votes.should_not be_empty
-    choice_votes.size.should eql 1
+    choice_votes.size.should == 1
+
+    v.appearance.should == @appearance
+    v.voter.should == @visitor
+  end
+  it "should be able to record a visitor's vote with a default visitor" do
+
+    optional_vote_params = {:time_viewed => 213}
+
+    required_vote_params = {:prompt => @prompt, 
+                            :direction => "left"}
+    params = optional_vote_params.merge(required_vote_params)
+    
+    v = @aoi_clone.record_vote(params)
+
+    v.should_not be_nil
+    prompt_votes = @prompt.votes(true)
+    prompt_votes.should_not be_empty
+    prompt_votes.size.should == 1
+    
+    choices = @prompt.choices
+    choices.should_not be_empty
+    
+    choice_votes = choices[0].votes(true)
+    choice_votes.should_not be_empty
+    choice_votes.size.should == 1
+
+    v.voter.should == @aoi_clone.default_visitor
   end
  
   it "should be able to record a visitor's skip" do
@@ -11,50 +11,99 @@ describe Visitor do
   before(:each) do
     @question = Factory.create(:aoi_question)
     @aoi_clone = @question.site
-    @johndoe = @question.creator
  
     @prompt = @question.prompts.first
-    @lc = @prompt.left_choice
-    @rc = @prompt.right_choice
-    
     @visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier")
-    @appearance = @aoi_clone.record_appearance(@visitor, @prompt)
-    @valid_attributes = {
-      :site => @aoi_clone,
-      :identifier => "value for identifier",
-      :tracking => "value for tracking"
-    }
+    
+    @required_vote_params = {:prompt => @prompt, 
+                            :direction => "left"}
   end
  
   it "should create a new instance given valid attributes" do
-    @visitor = Visitor.create!(@valid_attributes)
-    
+     Visitor.create!(Factory.build(:visitor).attributes.symbolize_keys)
   end
  
   it "should be able to determine ownership of a question" do
     @visitor.owns?(Question.new).should be_false
-
+    @visitor.owns?(Factory.build(:aoi_question)).should be_false
+    
+    @johndoe = Factory.create(:visitor)
     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
-    #@prompt = @question.prompts.first
-    @prompt.should_not be_nil
-    v = @visitor.vote_for! @appearance.lookup, @prompt, 0, 340
+    @prompt.votes.size.should == 0
+
+    v = @visitor.vote_for!(@required_vote_params)
+    v.should_not be_nil
+
+    v.prompt.should == @prompt
+    v.choice.should == @prompt.left_choice
+    v.loser_choice.should == @prompt.right_choice
+    v.voter.should == @visitor
+    @prompt.reload
+    @prompt.votes.size.should  == 1
+  end
+  
+  it "should be able to vote for a choice" do
+    @required_vote_params[:direction] = "right"
+    v = @visitor.vote_for!(@required_vote_params)
+    v.should_not be_nil
+
+    v.prompt.should == @prompt
+    v.choice.should == @prompt.right_choice
+    v.loser_choice.should == @prompt.left_choice
+    @prompt.right_choice.reload
+    @prompt.right_choice.votes.size.should  == 1
+  end
+
+  it "should return nil when no prompt is provided" do
+    @required_vote_params.delete(:prompt)
+    v = @visitor.vote_for!(@required_vote_params)
+    v.should be_nil
+    @prompt.reload
+    @prompt.votes.size.should == 0
+    @question.reload
+    @question.votes.size.should == 0
+  end
+  it "should return nil when no direction is provided" do
+    @required_vote_params.delete(:direction)
+    v = @visitor.vote_for!(@required_vote_params)
+    v.should be_nil
+    @prompt.reload
+    @prompt.votes.size.should == 0
+    @question.reload
+    @question.votes.size.should == 0
+  end
+
+  it "should keep track of optional vote attributes" do
+    @appearance = @aoi_clone.record_appearance(@visitor, @prompt)
+    @optional_vote_params = {:appearance_lookup => @appearance.lookup,
+		            :time_viewed => 213}
+    
+    allparams = @required_vote_params.merge(@optional_vote_params)
+    v = @visitor.vote_for!(allparams)
+
+    v.appearance.should == @appearance
+    v.time_viewed.should == 213
+    
   end
  
   it "should be able to skip a prompt" do
-    #@prompt = @question.prompts.first
-    @prompt.should_not be_nil
-    v = @visitor.skip! @appearance.lookup, @prompt, 304
+    @appearance = @aoi_clone.record_appearance(@visitor, @prompt)
+    s = @visitor.skip! @appearance.lookup, @prompt, 304
   end
  
   it "should accurately update score counts after vote" do
+   
+    @lc = @prompt.left_choice
+    @rc = @prompt.right_choice
+   
     prev_winner_score = @lc.score
     prev_loser_score = @rc.score
  
-    vote = @visitor.vote_for! @appearance.lookup, @prompt, 0, 340
+    vote = @visitor.vote_for! @required_vote_params
  
     @lc.reload
     @rc.reload
@@ -64,12 +113,14 @@ describe Visitor do
   end
  
   it "should accurately update win and loss totals after vote" do
+    @lc = @prompt.left_choice
+    @rc = @prompt.right_choice
     prev_winner_wins = @lc.wins
     prev_winner_losses = @lc.losses
     prev_loser_losses = @rc.losses
     prev_loser_wins = @rc.wins
  
-    vote = @visitor.vote_for! @appearance.lookup, @prompt, 0, 340
+    vote = @visitor.vote_for! @required_vote_params
  
     @lc.reload
     @rc.reload
 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
  
 describe Vote do
+  it {should belong_to :voter}
+  it {should belong_to :question}
+  it {should belong_to :prompt}
+  it {should belong_to :choice}
+  it {should belong_to :loser_choice}
+  it {should belong_to :appearance}
+
   before(:each) do
-    @valid_attributes = {
-      :tracking => "value for tracking",
-      :site_id => 1,
-      :voter_id => 1,
-      :question_id => 1,
-      :prompt_id => 1,
-      :choice_id=> 1,
-      :loser_choice_id=> 1,
-    }
-  end
-
-  it "should create a new instance given valid attributes" do
-    Vote.create!(@valid_attributes)
+    @question = Factory.create(:aoi_question)
+    @prompt = @question.prompts.first
+  end
+
+  it "should create a new instance with factory girl" do
+    Factory.create(:vote)
+  end
+
+  it "should update counter cache on question" do
+    @question.votes.size.should == 0
+    @question.votes_count.should == 0
+    Factory.create(:vote, :question => @question)
+
+    @question.reload
+    @question.votes.size.should == 1
+    @question.votes_count.should == 1
+    
+  end
+  it "should update counter cache on prompt" do
+    @prompt.votes.size.should == 0
+    @prompt.votes_count.should == 0
+    Factory.create(:vote, :question => @question, :prompt => @prompt)
+
+    @prompt.reload
+    @prompt.votes.size.should == 1
+    @prompt.votes_count.should == 1
+  end
+  it "should update counter cache on choice" do
+    @prompt.left_choice.votes.size.should == 0
+    @prompt.left_choice.votes_count.should == 0
+    Factory.create(:vote, :question => @question, :prompt => @prompt, 
+                          :choice => @prompt.left_choice)
+
+    @prompt.left_choice.reload
+    @prompt.left_choice.votes.size.should == 1
+    @prompt.left_choice.votes_count.should == 1
+  end
+  it "should update counter cache on loser_choice" do
+    @prompt.left_choice.votes.size.should == 0
+    @prompt.right_choice.losses.should == 0
+    @prompt.left_choice.votes_count.should == 0
+    Factory.create(:vote, :question => @question, :prompt => @prompt,
+                          :choice => @prompt.left_choice,
+                          :loser_choice => @prompt.right_choice)
+
+
+    @prompt.right_choice.reload
+    @prompt.right_choice.votes.size.should == 0
+    @prompt.right_choice.votes_count.should == 0
+    @prompt.right_choice.loss_count.should == 1
+    @prompt.right_choice.losses.should == 1
+  end
+
+  it "should update score of winner choice after create" do 
+    @prompt.left_choice.score.should == 50
+    Factory.create(:vote, :question => @question, :prompt => @prompt, 
+                          :choice => @prompt.left_choice)
+
+    @prompt.left_choice.reload
+    @prompt.left_choice.score.should be_close 67, 1
+  end
+  
+  it "should update score of loser choice after create" do 
+    @prompt.left_choice.score.should == 50
+    Factory.create(:vote, :question => @question, :prompt => @prompt,
+                          :choice => @prompt.left_choice,
+                          :loser_choice => @prompt.right_choice)
+
+    @prompt.right_choice.reload
+    @prompt.right_choice.score.should be_close 33, 1
   end
 end
...	...	@@ -5,7 +5,7 @@ class PromptsController < InheritedResources::Base
5	5	has_scope :active, :boolean => true, :only => :index
6	6
7	7	has_scope :voted_on_by
8		- #before_filter :authenticate
	8	+ before_filter :authenticate, :only => [:vote, :skip]
9	9
10	10
11	11	def activate
...	...	@@ -23,64 +23,44 @@ class PromptsController < InheritedResources::Base
23	23	end
24	24	end
25	25
26		- def vote
27		- #NOT IMPLEMENTED
28		- @question = Question.find(params[:question_id])
29		- @prompt = @question.prompts.find(params[:id])
30		- @choices = @prompt.choices.active
31		- @choice = @choices[params[:index]]
32		- respond_to do \|format\|
33		- format.xml { render :xml => @choice.to_xml }
34		- format.json { render :json => @choice.to_xml }
35		- end
36		- end
37		-
38		- def vote_left
39		- vote_direction(:left)
40		- end
41	26
42		-
43		- def vote_right
44		- vote_direction(:right)
45		- end
46		-
47		-
48		- def vote_direction(direction)
49		- authenticate
50		-
51		- logger.info "#{current_user.inspect} is voting #{direction} at #{Time.now}."
52		- @question = Question.find(params[:question_id])
	27	+ # To record a vote
	28	+ # required parameters - prompt id, ordinality, visitor_identifer?
	29	+ # optional params - visitor_identifier, appearance_lookup
	30	+ # After recording vote, next prompt display parameters:
	31	+ # same as in show - :with_prompt, with_appearance, with visitor_stats, etc
	32	+ def vote
	33	+ @question = current_user.questions.find(params[:question_id])
53	34	@prompt = @question.prompts.find(params[:id])
54		-
55		- time_viewed = params['params']['time_viewed']
56		- visitor_identifier = params['params']['auto']
57		- appearance_lookup = params['params']['appearance_lookup']
58	35
59		- case direction
60		- when :left
61		- successful = c = current_user.record_vote(params['params']['auto'], appearance_lookup, @prompt, 0, time_viewed)
62		- when :right
63		- successful = c = current_user.record_vote(params['params']['auto'], appearance_lookup, @prompt, 1, time_viewed)
64		- else
65		- raise "need to specify either ':left' or ':right' as a direction"
66		- end
67		-
68		- #@prompt.choices.each(&:compute_score!)
69		- respond_to do \|format\|
70		- if successful && next_prompt = @question.choose_prompt
	36	+ vote_options = params[:vote] \|\| {}
	37	+ vote_options.merge!(:prompt => @prompt, :question => @question)
71	38
72		- visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier)
73		- @a = current_user.record_appearance(visitor, next_prompt)
74		-
75		- appearance_id = Proc.new { \|options\| options[:builder].tag!('appearance_id', @a.lookup) }
76		- visitor_votes = Proc.new { \|options\| options[:builder].tag!('visitor_votes', visitor.votes.count(:conditions => {:question_id => @question.id})) }
77		- visitor_ideas = Proc.new { \|options\| options[:builder].tag!('visitor_ideas', visitor.items.count) }
	39	+ successful = object= current_user.record_vote(vote_options)
	40	+ optional_information = []
	41	+ if params[:next_prompt]
	42	+ begin
	43	+ params[:next_prompt].merge!(:with_prompt => true) # We always want to get the next possible prompt
	44	+ @question_optional_information = @question.get_optional_information(params[:next_prompt])
	45	+ rescue RuntimeError
78	46
79		- format.xml { render :xml => next_prompt.to_xml(:procs => [appearance_id, visitor_votes, visitor_ideas], :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok }
80		- format.json { render :json => next_prompt.to_json(:procs => [appearance_id, visitor_votes, visitor_ideas], :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok }
	47	+ respond_to do \|format\|
	48	+ format.xml { render :xml => @prompt.to_xml, :status => :conflict and return}
	49	+ end
	50	+ end
	51	+ object = @question.prompts.find(@question_optional_information.delete(:picked_prompt_id))
	52	+ @question_optional_information.each do \|key, value\|
	53	+ optional_information << Proc.new { \|options\| options[:builder].tag!(key, value)}
	54	+ end
	55	+ end
	56	+
	57	+ respond_to do \|format\|
	58	+ if !successful.nil?
	59	+ format.xml { render :xml => object.to_xml(:procs => optional_information , :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok }
	60	+ format.json { render :json => object.to_json(:procs => optional_information, :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok }
81	61	else
82		- format.xml { render :xml => c, :status => :unprocessable_entity }
83		- format.json { render :json => c, :status => :unprocessable_entity }
	62	+ format.xml { render :xml => @prompt.to_xml, :status => :unprocessable_entity }
	63	+ format.json { render :json => @prompt.to_xml, :status => :unprocessable_entity }
84	64	end
85	65	end
86	66	end
...	...	@@ -106,7 +86,6 @@ class PromptsController < InheritedResources::Base
106	86
107	87
108	88	def skip
109		- authenticate
110	89	logger.info "#{current_user.inspect} is skipping."
111	90	@question = Question.find(params[:question_id])
112	91
...	...	@@ -124,7 +103,17 @@ class PromptsController < InheritedResources::Base
124	103
125	104
126	105	respond_to do \|format\|
127		- if @skip = current_user.record_skip(visitor_identifier, appearance_lookup, @prompt, time_viewed, :skip_reason => skip_reason) && @next_prompt = @question.choose_prompt
	106	+ if @skip = current_user.record_skip(visitor_identifier, appearance_lookup, @prompt, time_viewed, :skip_reason => skip_reason)
	107	+
	108	+ #This is not hte right way to do this. See def vote for a better example
	109	+ begin
	110	+ @next_prompt = @question.choose_prompt
	111	+ rescue RuntimeError
	112	+
	113	+ respond_to do \|format\|
	114	+ format.xml { render :xml => @prompt.to_xml, :status => :conflict and return}
	115	+ end
	116	+ end
128	117
129	118	visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier)
130	119	@a = current_user.record_appearance(visitor, @next_prompt)
...	...
...	...	@@ -36,6 +36,12 @@ class Choice < ActiveRecord::Base
36	36	save!
37	37	end
38	38
	39	+ def win!
	40	+ self.votes_count += 1 rescue (self.votes_count = 1)
	41	+ self.score = compute_score
	42	+ save!
	43	+ end
	44	+
39	45	def wins_plus_losses
40	46	#(prompts_on_the_left.collect(&:votes_count).sum + prompts_on_the_right.collect(&:votes_count).sum)
41	47	#Prompt.sum('votes_count', :conditions => "left_choice_id = #{id} OR right_choice_id = #{id}")
...	...	@@ -70,11 +76,6 @@ class Choice < ActiveRecord::Base
70	76	end
71	77
72	78	def compute_score
73		- # if wins_plus_losses == 0
74		- # return 0
75		- # else
76		- # (wins.to_f / wins_plus_losses ) * 100
77		- # end
78	79	(wins.to_f+1)/(wins+1+losses+1) * 100
79	80	end
80	81
...	...
...	...	@@ -31,12 +31,14 @@ class User < ActiveRecord::Base
31	31	return choice
32	32	end
33	33
34		- def record_vote(visitor_identifier, appearance_lookup, prompt, ordinality, time_viewed)
35		- #@click = Click.new(:what_was_clicked => 'on the API level, inside record_vote' + " with prompt id #{prompt.id}, ordinality #{ordinality.to_s}")
36		- #@click.save!
37		- visitor = visitors.find_or_create_by_identifier(visitor_identifier)
38		- visitor.vote_for!(appearance_lookup, prompt, ordinality, time_viewed)
39		- #prompt.choices.each {\|c\| c.compute_score; c.save!}
	34	+ def record_vote(options)
	35	+ visitor_identifier = options.delete(:visitor_identifier)
	36	+ if visitor_identifier.nil?
	37	+ visitor = default_visitor
	38	+ else
	39	+ visitor = visitors.find_or_create_by_identifier(visitor_identifier)
	40	+ end
	41	+ visitor.vote_for!(options)
40	42	end
41	43
42	44	def record_appearance(visitor, prompt)
...	...
...	...	@@ -16,26 +16,25 @@ class Visitor < ActiveRecord::Base
16	16	questions.include? question
17	17	end
18	18
19		- def vote_for!(appearance_lookup, prompt, ordinality, time_viewed)
20		- @a = Appearance.find_by_lookup(appearance_lookup)
21		- #make votefor fail if we cant find the appearance
22		- choices = prompt.choices
23		- choice = choices[ordinality] #we need to guarantee that the choices are in the right order (by position)
24		- other_choices = choices - [choice]
25		- other_choices.each do \|c\|
26		- c.lose!
	19	+ def vote_for!(options)
	20	+ return nil if !options \|\| !options[:prompt] \|\| !options[:direction]
	21	+
	22	+ prompt = options.delete(:prompt)
	23	+ ordinality = (options.delete(:direction) == "left") ? 0 : 1
	24	+
	25	+ if options[:appearance_lookup]
	26	+ @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup))
	27	+ return nil unless @appearance # don't allow people to fake appearance lookups
	28	+ options.merge!(:appearance_id => @appearance.id)
27	29	end
28	30
	31	+ choice = prompt.choices[ordinality] #we need to guarantee that the choices are in the right order (by position)
	32	+ other_choices = prompt.choices - [choice]
29	33	loser_choice = other_choices.first
30		- v = 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, :time_viewed => time_viewed, :appearance_id => @a.id)
31		-
32		- # 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
33		- # The updated votes_count object is not saved to the db, so we don't need to worry about double counting
34		- # Alternatively, we could just do choice.reload, but that results in another db read
35		- choice.votes_count +=1
36		- choice.compute_score! #update score after win
37		-
	34	+
	35	+ options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :voter_id=> self.id, :choice_id => choice.id, :loser_choice_id => loser_choice.id)
38	36
	37	+ v = votes.create!(options)
39	38	end
40	39
41	40	def skip!(appearance_lookup, prompt, time_viewed, options = {})
...	...
1	1	class Vote < ActiveRecord::Base
2	2	# belongs_to :voteable, :polymorphic => true, :counter_cache => true
	3	+ validates_presence_of :question
	4	+ validates_presence_of :prompt
	5	+ validates_presence_of :choice
	6	+ validates_presence_of :loser_choice
	7	+ validates_presence_of :voter
	8	+
3	9	belongs_to :voter, :class_name => "Visitor", :foreign_key => "voter_id"
4	10	belongs_to :question, :counter_cache => true
5	11	belongs_to :prompt, :counter_cache => true
...	...	@@ -11,4 +17,14 @@ class Vote < ActiveRecord::Base
11	17	named_scope :with_question, lambda { \|*args\| {:conditions => {:question_id => args.first }} }
12	18	named_scope :with_voter_ids, lambda { \|*args\| {:conditions => {:voter_id=> args.first }} }
13	19	named_scope :active, :include => :choice, :conditions => { 'choices.active' => true }
	20	+
	21	+ after_create :update_winner_choice, :update_loser_choice
	22	+
	23	+ def update_winner_choice
	24	+ choice.win!
	25	+ end
	26	+
	27	+ def update_loser_choice
	28	+ loser_choice.lose!
	29	+ end
14	30	end
...	...
...	...	@@ -13,7 +13,7 @@ ActionController::Routing::Routes.draw do \|map\|
13	13	:object_info_totals_by_question_id => :get,
14	14	:recent_votes_by_question_id => :get} do \|question\|
15	15	question.resources :items
16		- question.resources :prompts, :member => {:vote_left => :post, :vote_right => :post, :skip => :post, :vote => :post},
	16	+ question.resources :prompts, :member => {:skip => :post, :vote => :post},
17	17	:collection => {:single => :get, :index => :get}
18	18	question.resources :choices, :member => { :activate => :put, :suspend => :put, :update_from_abroad => :put, :deactivate_from_abroad => :put, :flag => :put}, :collection => {:create_from_abroad => :post}
19	19	end
...	...
...	...	@@ -5,7 +5,7 @@ describe PromptsController do
5	5	@controller.current_user = user
6	6	return user
7	7	end
8		- #
	8	+
9	9	before(:each) do
10	10	@question = Factory.create(:aoi_question)
11	11	sign_in_as(@aoi_clone = @question.site)
...	...	@@ -14,16 +14,6 @@ describe PromptsController do
14	14	@visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier")
15	15	@appearance = @aoi_clone.record_appearance(@visitor, @prompt)
16	16	end
17		- #
18		-
19		-# describe "GET index" do
20		-# it "assigns all prompts as @prompts" do
21		-# Question.stub!(:find).with(:all).and_return(@question)
22		-# Question.stub!(:prompts).with(:all).and_return([mock_prompt])
23		-# get :index, :question_id => @question.id
24		-# assigns[:prompts].should == [@prompt]
25		-# end
26		-# end
27	17
28	18	describe "GET show" do
29	19	it "assigns the requested prompt as @prompt" do
...	...	@@ -42,4 +32,39 @@ describe PromptsController do
42	32	end
43	33	end
44	34
	35	+ describe "POST vote" do
	36	+ it "votes on a prompt" do
	37	+ post :vote, :question_id => @question.id, :id => @prompt.id,
	38	+ :vote => {:direction => "left"},
	39	+ :format => :xml
	40	+
	41	+ @response.code.should == "200"
	42	+ end
	43	+
	44	+ it "returns 422 when missing fields are not provided" do
	45	+ post :vote, :question_id => @question.id, :id => @prompt.id
	46	+
	47	+ # there is somethingw wrong with response codes, this doesn't work
	48	+ #@response.code.should == "422"
	49	+ end
	50	+
	51	+ it "votes on a prompt and responds with optional information" do
	52	+ post :vote, :question_id => @question.id, :id => @prompt.id,
	53	+ :vote => {:direction => "left",
	54	+ :time_viewed => "492",
	55	+ :visitor_identifier => "jim"},
	56	+ :next_prompt => {
	57	+ :with_appearance => true,
	58	+ :with_visitor_stats => true,
	59	+ :visitor_identifier => "jim"},
	60	+ :format => :xml
	61	+
	62	+ @response.code.should == "200"
	63	+ end
	64	+
	65	+ it "should prevent other users from voting on non owned questions" do
	66	+ end
	67	+
	68	+ end
	69	+
45	70	end
...	...
...	...	@@ -10,7 +10,6 @@ describe QuestionsController do
10	10	before(:each) do
11	11	@question = Factory.create(:aoi_question)
12	12	sign_in_as(@user = @question.site)
13		- @creator = @question.creator
14	13	end
15	14	it "responds with basic question information" do
16	15	get :show, :id => @question.id, :format => "xml"
...	...
...	...	@@ -39,6 +39,14 @@ Factory.define(:choice) do \|f\|
39	39	f.creator {\|c\| c.association(:visitor, :site => c.question.site)}
40	40	end
41	41
	42	+Factory.define(:vote) do \|f\|
	43	+ f.association :question, :factory => :aoi_question
	44	+ f.prompt {\|v\| v.question.prompts.first}
	45	+ f.choice {\|v\| v.prompt.left_choice}
	46	+ f.loser_choice {\|v\| v.prompt.right_choice}
	47	+ f.voter {\|v\| v.question.creator}
	48	+end
	49	+
42	50	Factory.sequence :email do \|n\|
43	51	"user#{n}@example.com"
44	52	end
...	...
...	...	@@ -29,18 +29,55 @@ describe User do
29	29	it "should be able to record a visitor's vote" do
30	30	@visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier")
31	31	@appearance = @aoi_clone.record_appearance(@visitor, @prompt)
32		- v = @aoi_clone.record_vote("johnnydoe", @appearance.lookup, @prompt, 0, 304)
	32	+
	33	+ optional_vote_params = {:visitor_identifier => @visitor.identifier,
	34	+ :appearance_lookup => @appearance.lookup,
	35	+ :time_viewed => 213}
	36	+
	37	+ required_vote_params = {:prompt => @prompt,
	38	+ :direction => "left"}
	39	+ params = optional_vote_params.merge(required_vote_params)
	40	+
	41	+ v = @aoi_clone.record_vote(params)
	42	+
	43	+ v.should_not be_nil
33	44	prompt_votes = @prompt.votes(true)
34	45	prompt_votes.should_not be_empty
35		- prompt_votes.size.should eql 1
	46	+ prompt_votes.size.should == 1
36	47
37	48	choices = @prompt.choices
38		- #@question.prompts(true).size.should == 2
39	49	choices.should_not be_empty
40	50
41	51	choice_votes = choices[0].votes(true)
42	52	choice_votes.should_not be_empty
43		- choice_votes.size.should eql 1
	53	+ choice_votes.size.should == 1
	54	+
	55	+ v.appearance.should == @appearance
	56	+ v.voter.should == @visitor
	57	+ end
	58	+ it "should be able to record a visitor's vote with a default visitor" do
	59	+
	60	+ optional_vote_params = {:time_viewed => 213}
	61	+
	62	+ required_vote_params = {:prompt => @prompt,
	63	+ :direction => "left"}
	64	+ params = optional_vote_params.merge(required_vote_params)
	65	+
	66	+ v = @aoi_clone.record_vote(params)
	67	+
	68	+ v.should_not be_nil
	69	+ prompt_votes = @prompt.votes(true)
	70	+ prompt_votes.should_not be_empty
	71	+ prompt_votes.size.should == 1
	72	+
	73	+ choices = @prompt.choices
	74	+ choices.should_not be_empty
	75	+
	76	+ choice_votes = choices[0].votes(true)
	77	+ choice_votes.should_not be_empty
	78	+ choice_votes.size.should == 1
	79	+
	80	+ v.voter.should == @aoi_clone.default_visitor
44	81	end
45	82
46	83	it "should be able to record a visitor's skip" do
...	...