diff --git a/app/controllers/prompts_controller.rb b/app/controllers/prompts_controller.rb index f527219..ff54384 100644 --- a/app/controllers/prompts_controller.rb +++ b/app/controllers/prompts_controller.rb @@ -5,7 +5,7 @@ class PromptsController < 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 < 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 < InheritedResources::Base def skip - authenticate logger.info "#{current_user.inspect} is skipping." @question = Question.find(params[:question_id]) @@ -124,7 +103,17 @@ class PromptsController < 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) diff --git a/app/models/choice.rb b/app/models/choice.rb index dee5488..bc426e5 100644 --- a/app/models/choice.rb +++ b/app/models/choice.rb @@ -36,6 +36,12 @@ class Choice < 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 < 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 diff --git a/app/models/user.rb b/app/models/user.rb index b1d7233..1d95b41 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -31,12 +31,14 @@ class User < 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) diff --git a/app/models/visitor.rb b/app/models/visitor.rb index 5275344..a4699f2 100644 --- a/app/models/visitor.rb +++ b/app/models/visitor.rb @@ -16,26 +16,25 @@ class Visitor < 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 = {}) diff --git a/app/models/vote.rb b/app/models/vote.rb index bc4f890..ad42945 100644 --- a/app/models/vote.rb +++ b/app/models/vote.rb @@ -1,5 +1,11 @@ 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 < 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 diff --git a/config/routes.rb b/config/routes.rb index 7f75698..8b5706a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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 diff --git a/spec/controllers/prompts_controller_spec.rb b/spec/controllers/prompts_controller_spec.rb index 532352b..61d2f8e 100644 --- a/spec/controllers/prompts_controller_spec.rb +++ b/spec/controllers/prompts_controller_spec.rb @@ -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 diff --git a/spec/controllers/questions_controller_spec.rb b/spec/controllers/questions_controller_spec.rb index 0c2b4be..e35537b 100644 --- a/spec/controllers/questions_controller_spec.rb +++ b/spec/controllers/questions_controller_spec.rb @@ -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" diff --git a/spec/factories.rb b/spec/factories.rb index e8e76bb..2bdd82a 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ec89bbe..0adf7e0 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -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 diff --git a/spec/models/visitor_spec.rb b/spec/models/visitor_spec.rb index 15e74a2..31335c0 100644 --- a/spec/models/visitor_spec.rb +++ b/spec/models/visitor_spec.rb @@ -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 diff --git a/spec/models/vote_spec.rb b/spec/models/vote_spec.rb index 37cd881..da88bfc 100644 --- a/spec/models/vote_spec.rb +++ b/spec/models/vote_spec.rb @@ -1,19 +1,83 @@ 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 -- libgit2 0.21.2