Commit fbaef6f393dde97b71638f69a43fc4855f07e7b0
1 parent
96626af3
Exists in
master
and in
1 other branch
Refactoring prompt picking logic into question model
Showing
4 changed files
with
35 additions
and
32 deletions
Show diff stats
app/controllers/prompts_controller.rb
@@ -68,28 +68,15 @@ class PromptsController < InheritedResources::Base | @@ -68,28 +68,15 @@ class PromptsController < InheritedResources::Base | ||
68 | #@prompt.choices.each(&:compute_score!) | 68 | #@prompt.choices.each(&:compute_score!) |
69 | respond_to do |format| | 69 | respond_to do |format| |
70 | if successful | 70 | if successful |
71 | - #catchup_marketplace_ids = [120, 117, 1, 116] | ||
72 | - #TODO refactor into question model | ||
73 | - if @question.uses_catchup? | ||
74 | - logger.info("Question #{@question.id} is using catchup algorithm!") | ||
75 | - next_prompt = @question.pop_prompt_queue | ||
76 | - if next_prompt.nil? | ||
77 | - logger.info("Catchup prompt cache miss! Nothing in prompt_queue") | ||
78 | - next_prompt = @question.catchup_choose_prompt | ||
79 | - end | ||
80 | - @question.send_later :add_prompt_to_queue | ||
81 | - else | ||
82 | - next_prompt = @question.picked_prompt | ||
83 | - end | ||
84 | - | 71 | + |
72 | + next_prompt = @question.choose_prompt | ||
85 | 73 | ||
86 | visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) | 74 | visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) |
87 | @a = current_user.record_appearance(visitor, next_prompt) | 75 | @a = current_user.record_appearance(visitor, next_prompt) |
88 | 76 | ||
89 | appearance_id = Proc.new { |options| options[:builder].tag!('appearance_id', @a.lookup) } | 77 | appearance_id = Proc.new { |options| options[:builder].tag!('appearance_id', @a.lookup) } |
90 | - | ||
91 | visitor_votes = Proc.new { |options| options[:builder].tag!('visitor_votes', visitor.votes.count(:conditions => {:question_id => @question.id})) } | 78 | visitor_votes = Proc.new { |options| options[:builder].tag!('visitor_votes', visitor.votes.count(:conditions => {:question_id => @question.id})) } |
92 | - visitor_ideas = Proc.new { |options| options[:builder].tag!('visitor_ideas', visitor.items.count) } | 79 | + visitor_ideas = Proc.new { |options| options[:builder].tag!('visitor_ideas', visitor.items.count) } |
93 | 80 | ||
94 | 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 } | 81 | 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 } |
95 | 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 } | 82 | 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 } |
app/controllers/questions_controller.rb
@@ -57,22 +57,8 @@ class QuestionsController < InheritedResources::Base | @@ -57,22 +57,8 @@ class QuestionsController < InheritedResources::Base | ||
57 | @question = Question.find(params[:id]) | 57 | @question = Question.find(params[:id]) |
58 | visitor_identifier = params[:visitor_identifier] | 58 | visitor_identifier = params[:visitor_identifier] |
59 | unless params[:barebones] | 59 | unless params[:barebones] |
60 | - if params[:algorithm] && params[:algorithm] == "catchup" | ||
61 | - logger.info("Question #{@question.id} requested catchup algorithm!") | ||
62 | - | ||
63 | - @p = @question.pop_prompt_queue | ||
64 | - if @p.nil? | ||
65 | - logger.info("Catchup prompt cache miss! Nothing in prompt_queue") | ||
66 | - @p = @question.catchup_choose_prompt | ||
67 | - @question.record_prompt_cache_miss | ||
68 | - else | ||
69 | - @question.record_prompt_cache_hit | ||
70 | - end | ||
71 | - @question.send_later :add_prompt_to_queue | ||
72 | - | ||
73 | - else | ||
74 | - @p = @question.picked_prompt | ||
75 | - end | 60 | + |
61 | + @p = @question.choose_prompt(:algorithm => params[:algorithm]) | ||
76 | 62 | ||
77 | # we sometimes request a question when no prompt is displayed | 63 | # we sometimes request a question when no prompt is displayed |
78 | # TODO It would be a good idea to find these places and treat them like barebones | 64 | # TODO It would be a good idea to find these places and treat them like barebones |
app/models/question.rb
@@ -29,6 +29,28 @@ class Question < ActiveRecord::Base | @@ -29,6 +29,28 @@ class Question < ActiveRecord::Base | ||
29 | choices_count | 29 | choices_count |
30 | end | 30 | end |
31 | 31 | ||
32 | + def choose_prompt(options = {}) | ||
33 | + | ||
34 | + if self.uses_catchup? || options[:algorithm] == "catchup" | ||
35 | + logger.info("Question #{self.id} is using catchup algorithm!") | ||
36 | + next_prompt = self.pop_prompt_queue | ||
37 | + if next_prompt.nil? | ||
38 | + logger.info("DEBUG Catchup prompt cache miss! Nothing in prompt_queue") | ||
39 | + next_prompt = self.catchup_choose_prompt | ||
40 | + record_prompt_cache_miss | ||
41 | + else | ||
42 | + record_prompt_cache_hit | ||
43 | + end | ||
44 | + self.send_later :add_prompt_to_queue | ||
45 | + return next_prompt | ||
46 | + else | ||
47 | + #Standard choose prompt at random | ||
48 | + next_prompt = self.picked_prompt | ||
49 | + return next_prompt | ||
50 | + end | ||
51 | + | ||
52 | + end | ||
53 | + | ||
32 | #TODO: generalize for prompts of rank > 2 | 54 | #TODO: generalize for prompts of rank > 2 |
33 | #TODO: add index for rapid finding | 55 | #TODO: add index for rapid finding |
34 | def picked_prompt(rank = 2) | 56 | def picked_prompt(rank = 2) |
spec/models/question_spec.rb
@@ -59,12 +59,20 @@ describe Question do | @@ -59,12 +59,20 @@ describe Question do | ||
59 | @catchup_q = Factory.create(:aoi_question, :site => user, :creator => user.default_visitor) | 59 | @catchup_q = Factory.create(:aoi_question, :site => user, :creator => user.default_visitor) |
60 | 60 | ||
61 | @catchup_q.it_should_autoactivate_ideas = true | 61 | @catchup_q.it_should_autoactivate_ideas = true |
62 | + @catchup_q.uses_catchup = true | ||
62 | @catchup_q.save! | 63 | @catchup_q.save! |
63 | 64 | ||
64 | 100.times.each do |num| | 65 | 100.times.each do |num| |
65 | user.create_choice("visitor identifier", @catchup_q, {:data => num.to_s, :local_identifier => "exmaple"}) | 66 | user.create_choice("visitor identifier", @catchup_q, {:data => num.to_s, :local_identifier => "exmaple"}) |
66 | end | 67 | end |
67 | end | 68 | end |
69 | + | ||
70 | + | ||
71 | + it "should create a delayed job after requesting a prompt" do | ||
72 | + proc { @catchup_q.choose_prompt}.should change(Delayed::Job, :count).by(1) | ||
73 | + end | ||
74 | + | ||
75 | + | ||
68 | it "should choose an active prompt using catchup algorithm on a large number of choices" do | 76 | it "should choose an active prompt using catchup algorithm on a large number of choices" do |
69 | @catchup_q.reload | 77 | @catchup_q.reload |
70 | # Sanity check, 2 extra choices are autocreated when empty question created | 78 | # Sanity check, 2 extra choices are autocreated when empty question created |