From fbaef6f393dde97b71638f69a43fc4855f07e7b0 Mon Sep 17 00:00:00 2001 From: Dhruv Kapadia Date: Mon, 17 May 2010 12:21:24 -0400 Subject: [PATCH] Refactoring prompt picking logic into question model --- app/controllers/prompts_controller.rb | 19 +++---------------- app/controllers/questions_controller.rb | 18 ++---------------- app/models/question.rb | 22 ++++++++++++++++++++++ spec/models/question_spec.rb | 8 ++++++++ 4 files changed, 35 insertions(+), 32 deletions(-) diff --git a/app/controllers/prompts_controller.rb b/app/controllers/prompts_controller.rb index 09f9cd1..022298a 100644 --- a/app/controllers/prompts_controller.rb +++ b/app/controllers/prompts_controller.rb @@ -68,28 +68,15 @@ class PromptsController < InheritedResources::Base #@prompt.choices.each(&:compute_score!) respond_to do |format| if successful - #catchup_marketplace_ids = [120, 117, 1, 116] - #TODO refactor into question model - if @question.uses_catchup? - logger.info("Question #{@question.id} is using catchup algorithm!") - next_prompt = @question.pop_prompt_queue - if next_prompt.nil? - logger.info("Catchup prompt cache miss! Nothing in prompt_queue") - next_prompt = @question.catchup_choose_prompt - end - @question.send_later :add_prompt_to_queue - else - next_prompt = @question.picked_prompt - end - + + next_prompt = @question.choose_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) } + visitor_ideas = Proc.new { |options| options[:builder].tag!('visitor_ideas', visitor.items.count) } 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 } diff --git a/app/controllers/questions_controller.rb b/app/controllers/questions_controller.rb index 20f082e..f563835 100644 --- a/app/controllers/questions_controller.rb +++ b/app/controllers/questions_controller.rb @@ -57,22 +57,8 @@ class QuestionsController < InheritedResources::Base @question = Question.find(params[:id]) visitor_identifier = params[:visitor_identifier] unless params[:barebones] - if params[:algorithm] && params[:algorithm] == "catchup" - logger.info("Question #{@question.id} requested catchup algorithm!") - - @p = @question.pop_prompt_queue - if @p.nil? - logger.info("Catchup prompt cache miss! Nothing in prompt_queue") - @p = @question.catchup_choose_prompt - @question.record_prompt_cache_miss - else - @question.record_prompt_cache_hit - end - @question.send_later :add_prompt_to_queue - - else - @p = @question.picked_prompt - end + + @p = @question.choose_prompt(:algorithm => params[:algorithm]) # 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 diff --git a/app/models/question.rb b/app/models/question.rb index 6c476b8..674baa8 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -29,6 +29,28 @@ class Question < ActiveRecord::Base choices_count end + def choose_prompt(options = {}) + + if self.uses_catchup? || options[:algorithm] == "catchup" + logger.info("Question #{self.id} is using catchup algorithm!") + next_prompt = self.pop_prompt_queue + if next_prompt.nil? + logger.info("DEBUG Catchup prompt cache miss! Nothing in prompt_queue") + next_prompt = self.catchup_choose_prompt + record_prompt_cache_miss + else + record_prompt_cache_hit + end + self.send_later :add_prompt_to_queue + return next_prompt + else + #Standard choose prompt at random + next_prompt = self.picked_prompt + return next_prompt + end + + end + #TODO: generalize for prompts of rank > 2 #TODO: add index for rapid finding def picked_prompt(rank = 2) diff --git a/spec/models/question_spec.rb b/spec/models/question_spec.rb index cc71320..411b0e8 100644 --- a/spec/models/question_spec.rb +++ b/spec/models/question_spec.rb @@ -59,12 +59,20 @@ describe Question do @catchup_q = Factory.create(:aoi_question, :site => user, :creator => user.default_visitor) @catchup_q.it_should_autoactivate_ideas = true + @catchup_q.uses_catchup = true @catchup_q.save! 100.times.each do |num| user.create_choice("visitor identifier", @catchup_q, {:data => num.to_s, :local_identifier => "exmaple"}) end end + + + it "should create a delayed job after requesting a prompt" do + proc { @catchup_q.choose_prompt}.should change(Delayed::Job, :count).by(1) + end + + it "should choose an active prompt using catchup algorithm on a large number of choices" do @catchup_q.reload # Sanity check, 2 extra choices are autocreated when empty question created -- libgit2 0.21.2