diff --git a/app/controllers/choices_controller.rb b/app/controllers/choices_controller.rb index 2a4aff8..79daaa6 100644 --- a/app/controllers/choices_controller.rb +++ b/app/controllers/choices_controller.rb @@ -60,8 +60,11 @@ class ChoicesController < InheritedResources::Base @question = Question.find params[:question_id] + visitor_identifier = params['params']['auto'] + visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) + respond_to do |format| - if @choice = current_user.create_choice(params['params']['auto'], @question, {:data => params['params']['data'], + if @choice = current_user.create_choice(visitor_identifier, @question, {:data => params['params']['data'], :local_identifier => params['params']['local_identifier']}) saved_choice_id = Proc.new { |options| options[:builder].tag!('saved_choice_id', @choice.id) } choice_status = Proc.new { |options| @@ -71,7 +74,10 @@ class ChoicesController < InheritedResources::Base Question.update_counters(@question.id, :inactive_choices_count => @choice.active? ? 0 : 1) - format.xml { render :xml => @choice.to_xml(:procs => [saved_choice_id, choice_status]), :status => :ok } + 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) } + + format.xml { render :xml => @choice.to_xml(:procs => [saved_choice_id, choice_status, visitor_votes, visitor_ideas]), :status => :ok } # format.xml { render :xml => @question.picked_prompt.to_xml(:methods => [:left_choice_text, :right_choice_text], :procs => [saved_choice_id, choice_status]), :status => :ok } format.json { render :json => @question.to_json(:procs => [saved_choice_id, choice_status]), :status => :ok } else diff --git a/app/controllers/prompts_controller.rb b/app/controllers/prompts_controller.rb index b72c3a1..ab5efd6 100644 --- a/app/controllers/prompts_controller.rb +++ b/app/controllers/prompts_controller.rb @@ -84,12 +84,16 @@ class PromptsController < InheritedResources::Base end - @a = current_user.record_appearance(visitor_identifier, next_prompt) + 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) } - format.xml { render :xml => next_prompt.to_xml(:procs => [appearance_id], :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], :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok } + 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 } else format.xml { render :xml => c, :status => :unprocessable_entity } format.json { render :json => c, :status => :unprocessable_entity } diff --git a/app/controllers/questions_controller.rb b/app/controllers/questions_controller.rb index dfc3365..ad53d7f 100644 --- a/app/controllers/questions_controller.rb +++ b/app/controllers/questions_controller.rb @@ -74,7 +74,8 @@ class QuestionsController < InheritedResources::Base # we sometimes request a question when no prompt is displayed # TODO It would be a good idea to find these places and treat them like barebones if !visitor_identifier.blank? - @a = current_user.record_appearance(visitor_identifier, @p) + visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) + @a = current_user.record_appearance(visitor, @p) logger.info("creating appearance!") else @a = nil @@ -85,10 +86,15 @@ class QuestionsController < InheritedResources::Base picked_prompt_id = Proc.new { |options| options[:builder].tag!('picked_prompt_id', @p.id) } 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) } + the_procs = [left_choice_text, right_choice_text, picked_prompt_id] if @a the_procs << appearance_id + the_procs << visitor_votes + the_procs << visitor_ideas end show! do |format| diff --git a/app/models/user.rb b/app/models/user.rb index 5ce0bce..71b9c37 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -39,9 +39,7 @@ class User < ActiveRecord::Base #prompt.choices.each {|c| c.compute_score; c.save!} end - def record_appearance(visitor_identifier, prompt) - visitor = visitors.find_or_create_by_identifier(visitor_identifier) - + def record_appearance(visitor, prompt) a = Appearance.create(:voter => visitor, :prompt => prompt, :question_id => prompt.question_id, :lookup => Digest::MD5.hexdigest(rand(10000000000).to_s + visitor.id.to_s + prompt.id.to_s) ) end diff --git a/db/migrate/20100423154040_add_vistor_index_to_votes_table.rb b/db/migrate/20100423154040_add_vistor_index_to_votes_table.rb new file mode 100644 index 0000000..1e0a09f --- /dev/null +++ b/db/migrate/20100423154040_add_vistor_index_to_votes_table.rb @@ -0,0 +1,11 @@ +class AddVistorIndexToVotesTable < ActiveRecord::Migration + def self.up + add_index :votes, :voter_id + add_index :items, :creator_id + end + + def self.down + remove_index :votes, :voter_id + remove_index :items, :creator_id + end +end diff --git a/spec/controllers/questions_controller_spec.rb b/spec/controllers/questions_controller_spec.rb index f5d8a95..59191c6 100644 --- a/spec/controllers/questions_controller_spec.rb +++ b/spec/controllers/questions_controller_spec.rb @@ -24,6 +24,10 @@ describe QuestionsController do def mock_appearance(stubs={}) @mock_appearance||= mock_model(Appearance, stubs) end + + def mock_visitor(stubs={}) + @mock_visitor||= mock_model(Visitor, stubs) + end # # describe "GET index" do # it "assigns all questions as @questions" do @@ -39,26 +43,80 @@ describe QuestionsController do mock_question.stub!(:picked_prompt).and_return(mock_prompt) end + it "assigns the requested question as @question" do Question.stub!(:find).with("37").and_return(mock_question) #TODO it shouldn't call this unless we are generating an appearance, right? get :show, :id => "37" assigns[:question].should equal(mock_question) - assigns[:prompt].should equal(mock_prompt) + assigns[:p].should equal(mock_prompt) assigns[:a].should be_nil end - #TODO this is not a particularly intutive param to pass in order to create an appearance - it "creates an appearance when a visitor identifier is a param" do - @user.stub!(:record_appearance).with("somelongunique32charstring", mock_prompt).and_return(mock_appearance) - get :show, :id => "37", :visitor_identifier => "somelongunique32charstring" + + it "does not create an appearance when the 'barebones' param is set" do + get :show, :id => "37", :barebones => true assigns[:question].should equal(mock_question) - assigns[:prompt].should equal(mock_prompt) - assigns[:a].should equal(mock_appearance) + assigns[:p].should be_nil + assigns[:a].should be_nil + end + + describe "creates an appearance" do + before(:each) do + @visitor_identifier = "somelongunique32charstring" + #stub broken + visitor_list = [mock_visitor] + @user.stub!(:visitors).and_return(visitor_list) + visitor_list.stub!(:find_or_create_by_identifier).and_return(mock_visitor) + @user.stub!(:record_appearance).with(mock_visitor, mock_prompt).and_return(mock_appearance) + end + + #TODO this is not a particularly intutive param to pass in order to create an appearance + it "creates an appearance when a visitor identifier is a param" do + get :show, :id => "37", :visitor_identifier => @visitor_identifier + assigns[:question].should equal(mock_question) + assigns[:p].should equal(mock_prompt) + assigns[:a].should equal(mock_appearance) + + end + it "does not create an appearance when the 'barebones' param is set, even when a visitor id is sent" do + get :show, :id => "37", :barebones => true, :visitor_identifier => @visitor_identifier + assigns[:question].should equal(mock_question) + assigns[:p].should be_nil + assigns[:a].should be_nil + end + + describe "calls catchup algorithm" do + before(:each) do + mock_question.should_receive(:send_later).with(:add_prompt_to_queue) + end + + + #TODO Refactor out to use uses_catchup? + it "should pop prompt from cached queue using the catchup algorithm if params dictate" do + mock_question.should_receive(:pop_prompt_queue).and_return(mock_prompt) + + get :show, :id => "37", :visitor_identifier => @visitor_identifier, :algorithm => "catchup" + assigns[:question].should equal(mock_question) + assigns[:p].should equal(mock_prompt) + assigns[:a].should equal(mock_appearance) + end + + it "should handle cache misses gracefully" do + mock_question.should_receive(:pop_prompt_queue).and_return(nil) + mock_question.should_receive(:catchup_choose_prompt).and_return(mock_prompt) + + get :show, :id => "37", :visitor_identifier => @visitor_identifier, :algorithm => "catchup" + assigns[:question].should equal(mock_question) + assigns[:p].should equal(mock_prompt) + assigns[:a].should equal(mock_appearance) + end + end end end + # # describe "GET new" do # it "assigns a new question as @question" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5096a20..fd69fa5 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -11,7 +11,8 @@ describe User do @rc = Factory.create(:choice, :question => @question, :creator => @johndoe, :data => 'goodbye gorgeous') @prompt = Factory.create(:prompt, :question => @question, :tracking => 'sample', :left_choice => @lc, :right_choice => @rc) - @appearance = @aoi_clone.record_appearance("test visitor identifier", @prompt) + @visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier") + @appearance = @aoi_clone.record_appearance(@visitor, @prompt) end diff --git a/spec/models/visitor_spec.rb b/spec/models/visitor_spec.rb index 6a0c873..232e666 100644 --- a/spec/models/visitor_spec.rb +++ b/spec/models/visitor_spec.rb @@ -15,7 +15,9 @@ describe Visitor do @lc = Factory.create(:choice, :question => @question, :creator => @johndoe, :data => 'hello gorgeous') @rc = Factory.create(:choice, :question => @question, :creator => @johndoe, :data => 'goodbye gorgeous') @prompt = Factory.create(:prompt, :question => @question, :tracking => 'sample', :left_choice => @lc, :right_choice => @rc) - @appearance = @aoi_clone.record_appearance("test visitor identifier", @prompt) + + @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", diff --git a/spec/routing/clicks_routing_spec.rb b/spec/routing/clicks_routing_spec.rb deleted file mode 100644 index 3a48ed7..0000000 --- a/spec/routing/clicks_routing_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') - -describe ClicksController do - #clicks is no longer accessible -end -- libgit2 0.21.2