Adding visitor_votes and visitor_ideas to all vote reponses

Dhruv Kapadia
1 parent 05ae50f0
Showing 9 changed files with 104 additions and 23 deletions Show diff stats
app/controllers/choices_controller.rb
app/controllers/prompts_controller.rb
app/controllers/questions_controller.rb
app/models/user.rb
db/migrate/20100423154040_add_vistor_index_to_votes_table.rb
spec/controllers/questions_controller_spec.rb
spec/models/user_spec.rb
spec/models/visitor_spec.rb
spec/routing/clicks_routing_spec.rb
@@ -60,8 +60,11 @@ class ChoicesController &lt; 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 &lt; 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
@@ -84,12 +84,16 @@ class PromptsController &lt; 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 }
@@ -74,7 +74,8 @@ class QuestionsController &lt; 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 &lt; 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|
@@ -39,9 +39,7 @@ class User &lt; 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
@@ -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
@@ -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
@@ -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
  
  
@@ -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",
@@ -1,5 +0,0 @@
-require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
-
-describe ClicksController do
-	#clicks is no longer accessible 
-end
...	...	@@ -60,8 +60,11 @@ class ChoicesController < InheritedResources::Base
60	60
61	61	@question = Question.find params[:question_id]
62	62
	63	+ visitor_identifier = params['params']['auto']
	64	+ visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier)
	65	+
63	66	respond_to do \|format\|
64		- if @choice = current_user.create_choice(params['params']['auto'], @question, {:data => params['params']['data'],
	67	+ if @choice = current_user.create_choice(visitor_identifier, @question, {:data => params['params']['data'],
65	68	:local_identifier => params['params']['local_identifier']})
66	69	saved_choice_id = Proc.new { \|options\| options[:builder].tag!('saved_choice_id', @choice.id) }
67	70	choice_status = Proc.new { \|options\|
...	...	@@ -71,7 +74,10 @@ class ChoicesController < InheritedResources::Base
71	74
72	75	Question.update_counters(@question.id, :inactive_choices_count => @choice.active? ? 0 : 1)
73	76
74		- format.xml { render :xml => @choice.to_xml(:procs => [saved_choice_id, choice_status]), :status => :ok }
	77	+ visitor_votes = Proc.new { \|options\| options[:builder].tag!('visitor_votes', visitor.votes.count(:conditions => {:question_id => @question.id})) }
	78	+ visitor_ideas = Proc.new { \|options\| options[:builder].tag!('visitor_ideas', visitor.items.count) }
	79	+
	80	+ format.xml { render :xml => @choice.to_xml(:procs => [saved_choice_id, choice_status, visitor_votes, visitor_ideas]), :status => :ok }
75	81	# format.xml { render :xml => @question.picked_prompt.to_xml(:methods => [:left_choice_text, :right_choice_text], :procs => [saved_choice_id, choice_status]), :status => :ok }
76	82	format.json { render :json => @question.to_json(:procs => [saved_choice_id, choice_status]), :status => :ok }
77	83	else
...	...
...	...	@@ -84,12 +84,16 @@ class PromptsController < InheritedResources::Base
84	84	end
85	85
86	86
87		- @a = current_user.record_appearance(visitor_identifier, next_prompt)
	87	+ visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier)
	88	+ @a = current_user.record_appearance(visitor, next_prompt)
88	89
89	90	appearance_id = Proc.new { \|options\| options[:builder].tag!('appearance_id', @a.lookup) }
	91	+
	92	+ visitor_votes = Proc.new { \|options\| options[:builder].tag!('visitor_votes', visitor.votes.count(:conditions => {:question_id => @question.id})) }
	93	+ visitor_ideas = Proc.new { \|options\| options[:builder].tag!('visitor_ideas', visitor.items.count) }
90	94
91		- 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 }
92		- 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 }
	95	+ 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 }
	96	+ 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 }
93	97	else
94	98	format.xml { render :xml => c, :status => :unprocessable_entity }
95	99	format.json { render :json => c, :status => :unprocessable_entity }
...	...
...	...	@@ -74,7 +74,8 @@ class QuestionsController < InheritedResources::Base
74	74	# we sometimes request a question when no prompt is displayed
75	75	# TODO It would be a good idea to find these places and treat them like barebones
76	76	if !visitor_identifier.blank?
77		- @a = current_user.record_appearance(visitor_identifier, @p)
	77	+ visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier)
	78	+ @a = current_user.record_appearance(visitor, @p)
78	79	logger.info("creating appearance!")
79	80	else
80	81	@a = nil
...	...	@@ -85,10 +86,15 @@ class QuestionsController < InheritedResources::Base
85	86	picked_prompt_id = Proc.new { \|options\| options[:builder].tag!('picked_prompt_id', @p.id) }
86	87	appearance_id = Proc.new { \|options\| options[:builder].tag!('appearance_id', @a.lookup) }
87	88
	89	+ visitor_votes = Proc.new { \|options\| options[:builder].tag!('visitor_votes', visitor.votes.count(:conditions => {:question_id => @question.id})) }
	90	+ visitor_ideas = Proc.new { \|options\| options[:builder].tag!('visitor_ideas', visitor.items.count) }
	91	+
88	92	the_procs = [left_choice_text, right_choice_text, picked_prompt_id]
89	93
90	94	if @a
91	95	the_procs << appearance_id
	96	+ the_procs << visitor_votes
	97	+ the_procs << visitor_ideas
92	98	end
93	99
94	100	show! do \|format\|
...	...
...	...	@@ -39,9 +39,7 @@ class User < ActiveRecord::Base
39	39	#prompt.choices.each {\|c\| c.compute_score; c.save!}
40	40	end
41	41
42		- def record_appearance(visitor_identifier, prompt)
43		- visitor = visitors.find_or_create_by_identifier(visitor_identifier)
44		-
	42	+ def record_appearance(visitor, prompt)
45	43	a = Appearance.create(:voter => visitor, :prompt => prompt, :question_id => prompt.question_id,
46	44	:lookup => Digest::MD5.hexdigest(rand(10000000000).to_s + visitor.id.to_s + prompt.id.to_s) )
47	45	end
...	...
...	...	@@ -0,0 +1,11 @@
	1	+class AddVistorIndexToVotesTable < ActiveRecord::Migration
	2	+ def self.up
	3	+ add_index :votes, :voter_id
	4	+ add_index :items, :creator_id
	5	+ end
	6	+
	7	+ def self.down
	8	+ remove_index :votes, :voter_id
	9	+ remove_index :items, :creator_id
	10	+ end
	11	+end
...	...
...	...	@@ -24,6 +24,10 @@ describe QuestionsController do
24	24	def mock_appearance(stubs={})
25	25	@mock_appearance\|\|= mock_model(Appearance, stubs)
26	26	end
	27	+
	28	+ def mock_visitor(stubs={})
	29	+ @mock_visitor\|\|= mock_model(Visitor, stubs)
	30	+ end
27	31	#
28	32	# describe "GET index" do
29	33	# it "assigns all questions as @questions" do
...	...	@@ -39,26 +43,80 @@ describe QuestionsController do
39	43	mock_question.stub!(:picked_prompt).and_return(mock_prompt)
40	44	end
41	45
	46	+
42	47	it "assigns the requested question as @question" do
43	48	Question.stub!(:find).with("37").and_return(mock_question)
44	49	#TODO it shouldn't call this unless we are generating an appearance, right?
45	50
46	51	get :show, :id => "37"
47	52	assigns[:question].should equal(mock_question)
48		- assigns[:prompt].should equal(mock_prompt)
	53	+ assigns[:p].should equal(mock_prompt)
49	54	assigns[:a].should be_nil
50	55	end
51	56
52		- #TODO this is not a particularly intutive param to pass in order to create an appearance
53		- it "creates an appearance when a visitor identifier is a param" do
54		- @user.stub!(:record_appearance).with("somelongunique32charstring", mock_prompt).and_return(mock_appearance)
55		- get :show, :id => "37", :visitor_identifier => "somelongunique32charstring"
	57	+
	58	+ it "does not create an appearance when the 'barebones' param is set" do
	59	+ get :show, :id => "37", :barebones => true
56	60	assigns[:question].should equal(mock_question)
57		- assigns[:prompt].should equal(mock_prompt)
58		- assigns[:a].should equal(mock_appearance)
	61	+ assigns[:p].should be_nil
	62	+ assigns[:a].should be_nil
	63	+ end
	64	+
	65	+ describe "creates an appearance" do
	66	+ before(:each) do
	67	+ @visitor_identifier = "somelongunique32charstring"
	68	+ #stub broken
	69	+ visitor_list = [mock_visitor]
	70	+ @user.stub!(:visitors).and_return(visitor_list)
	71	+ visitor_list.stub!(:find_or_create_by_identifier).and_return(mock_visitor)
	72	+ @user.stub!(:record_appearance).with(mock_visitor, mock_prompt).and_return(mock_appearance)
	73	+ end
	74	+
	75	+ #TODO this is not a particularly intutive param to pass in order to create an appearance
	76	+ it "creates an appearance when a visitor identifier is a param" do
	77	+ get :show, :id => "37", :visitor_identifier => @visitor_identifier
	78	+ assigns[:question].should equal(mock_question)
	79	+ assigns[:p].should equal(mock_prompt)
	80	+ assigns[:a].should equal(mock_appearance)
	81	+
	82	+ end
59	83
	84	+ it "does not create an appearance when the 'barebones' param is set, even when a visitor id is sent" do
	85	+ get :show, :id => "37", :barebones => true, :visitor_identifier => @visitor_identifier
	86	+ assigns[:question].should equal(mock_question)
	87	+ assigns[:p].should be_nil
	88	+ assigns[:a].should be_nil
	89	+ end
	90	+
	91	+ describe "calls catchup algorithm" do
	92	+ before(:each) do
	93	+ mock_question.should_receive(:send_later).with(:add_prompt_to_queue)
	94	+ end
	95	+
	96	+
	97	+ #TODO Refactor out to use uses_catchup?
	98	+ it "should pop prompt from cached queue using the catchup algorithm if params dictate" do
	99	+ mock_question.should_receive(:pop_prompt_queue).and_return(mock_prompt)
	100	+
	101	+ get :show, :id => "37", :visitor_identifier => @visitor_identifier, :algorithm => "catchup"
	102	+ assigns[:question].should equal(mock_question)
	103	+ assigns[:p].should equal(mock_prompt)
	104	+ assigns[:a].should equal(mock_appearance)
	105	+ end
	106	+
	107	+ it "should handle cache misses gracefully" do
	108	+ mock_question.should_receive(:pop_prompt_queue).and_return(nil)
	109	+ mock_question.should_receive(:catchup_choose_prompt).and_return(mock_prompt)
	110	+
	111	+ get :show, :id => "37", :visitor_identifier => @visitor_identifier, :algorithm => "catchup"
	112	+ assigns[:question].should equal(mock_question)
	113	+ assigns[:p].should equal(mock_prompt)
	114	+ assigns[:a].should equal(mock_appearance)
	115	+ end
	116	+ end
60	117	end
61	118	end
	119	+
62	120	#
63	121	# describe "GET new" do
64	122	# it "assigns a new question as @question" do
...	...
...	...	@@ -11,7 +11,8 @@ describe User do
11	11	@rc = Factory.create(:choice, :question => @question, :creator => @johndoe, :data => 'goodbye gorgeous')
12	12	@prompt = Factory.create(:prompt, :question => @question, :tracking => 'sample', :left_choice => @lc, :right_choice => @rc)
13	13
14		- @appearance = @aoi_clone.record_appearance("test visitor identifier", @prompt)
	14	+ @visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier")
	15	+ @appearance = @aoi_clone.record_appearance(@visitor, @prompt)
15	16	end
16	17
17	18
...	...
...	...	@@ -15,7 +15,9 @@ describe Visitor do
15	15	@lc = Factory.create(:choice, :question => @question, :creator => @johndoe, :data => 'hello gorgeous')
16	16	@rc = Factory.create(:choice, :question => @question, :creator => @johndoe, :data => 'goodbye gorgeous')
17	17	@prompt = Factory.create(:prompt, :question => @question, :tracking => 'sample', :left_choice => @lc, :right_choice => @rc)
18		- @appearance = @aoi_clone.record_appearance("test visitor identifier", @prompt)
	18	+
	19	+ @visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier")
	20	+ @appearance = @aoi_clone.record_appearance(@visitor, @prompt)
19	21	@valid_attributes = {
20	22	:site => @aoi_clone,
21	23	:identifier => "value for identifier",
...	...
...	...	@@ -1,5 +0,0 @@
1		-require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
2		-
3		-describe ClicksController do
4		- #clicks is no longer accessible
5		-end