Commit e333fbea5497673541543054194cf4933d94b508
1 parent
f25abd1e
Exists in
master
and in
1 other branch
Handling edge case when fewer than 2 active ideas
Showing
6 changed files
with
40 additions
and
45 deletions
Show diff stats
app/controllers/choices_controller.rb
... | ... | @@ -80,7 +80,7 @@ class ChoicesController < InheritedResources::Base |
80 | 80 | visitor_ideas = Proc.new { |options| options[:builder].tag!('visitor_ideas', visitor.items.count) } |
81 | 81 | |
82 | 82 | format.xml { render :xml => @choice.to_xml(:procs => [saved_choice_id, choice_status, visitor_votes, visitor_ideas]), :status => :ok } |
83 | - # format.xml { render :xml => @question.picked_prompt.to_xml(:methods => [:left_choice_text, :right_choice_text], :procs => [saved_choice_id, choice_status]), :status => :ok } | |
83 | + # TODO: Why are we rendering a question here? Is the prompt being used later on? | |
84 | 84 | format.json { render :json => @question.to_json(:procs => [saved_choice_id, choice_status]), :status => :ok } |
85 | 85 | else |
86 | 86 | format.xml { render :xml => @choice.errors, :status => :unprocessable_entity } |
... | ... | @@ -98,7 +98,6 @@ class ChoicesController < InheritedResources::Base |
98 | 98 | respond_to do |format| |
99 | 99 | if @choice.activate! |
100 | 100 | logger.info "successfully activated choice #{@choice.inspect}" |
101 | - Question.update_counters(@question.id, :inactive_choices_count => -1) | |
102 | 101 | format.xml { render :xml => true } |
103 | 102 | format.json { render :json => true } |
104 | 103 | else |
... | ... | @@ -121,7 +120,6 @@ class ChoicesController < InheritedResources::Base |
121 | 120 | format.json { render :json => false } |
122 | 121 | elsif @choice.deactivate! |
123 | 122 | logger.info "successfully deactivated choice #{@choice.inspect}" |
124 | - Question.update_counters(@question.id, :inactive_choices_count => 1 ) | |
125 | 123 | format.xml { render :xml => true } |
126 | 124 | format.json { render :json => true } |
127 | 125 | else |
... | ... | @@ -197,7 +195,6 @@ class ChoicesController < InheritedResources::Base |
197 | 195 | respond_to do |format| |
198 | 196 | if @choice.deactivate! |
199 | 197 | flag = Flag.create!(flag_params) |
200 | - puts "I AM CREATING A FLAG FOR #{@choice.inspect}" | |
201 | 198 | format.xml { render :xml => @choice.to_xml, :status => :created } |
202 | 199 | format.json { render :json => @choice.to_json, :status => :created } |
203 | 200 | else | ... | ... |
app/controllers/prompts_controller.rb
... | ... | @@ -67,9 +67,7 @@ class PromptsController < InheritedResources::Base |
67 | 67 | |
68 | 68 | #@prompt.choices.each(&:compute_score!) |
69 | 69 | respond_to do |format| |
70 | - if successful | |
71 | - | |
72 | - next_prompt = @question.choose_prompt | |
70 | + if successful && next_prompt = @question.choose_prompt | |
73 | 71 | |
74 | 72 | visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) |
75 | 73 | @a = current_user.record_appearance(visitor, next_prompt) |
... | ... | @@ -111,6 +109,7 @@ class PromptsController < InheritedResources::Base |
111 | 109 | authenticate |
112 | 110 | logger.info "#{current_user.inspect} is skipping." |
113 | 111 | @question = Question.find(params[:question_id]) |
112 | + | |
114 | 113 | @prompt = @question.prompts.find(params[:id]) #, :include => [{ :left_choice => :item }, { :right_choice => :item }]) |
115 | 114 | |
116 | 115 | time_viewed = params['params']['time_viewed'] |
... | ... | @@ -125,23 +124,7 @@ class PromptsController < InheritedResources::Base |
125 | 124 | |
126 | 125 | |
127 | 126 | respond_to do |format| |
128 | - if @skip = current_user.record_skip(visitor_identifier, appearance_lookup, @prompt, time_viewed, :skip_reason => skip_reason) | |
129 | - | |
130 | - if @question.uses_catchup? | |
131 | - logger.info("Question #{@question.id} is using catchup algorithm!") | |
132 | - @next_prompt = @question.pop_prompt_queue | |
133 | - if @next_prompt.nil? | |
134 | - @question.record_prompt_cache_miss | |
135 | - logger.info("Catchup prompt cache miss! Nothing in prompt_queue") | |
136 | - @next_prompt = @question.catchup_choose_prompt | |
137 | - else | |
138 | - @question.record_prompt_cache_hit | |
139 | - end | |
140 | - @question.send_later :add_prompt_to_queue | |
141 | - else | |
142 | - @next_prompt = @question.picked_prompt | |
143 | - end | |
144 | - | |
127 | + if @skip = current_user.record_skip(visitor_identifier, appearance_lookup, @prompt, time_viewed, :skip_reason => skip_reason) && @next_prompt = @question.choose_prompt | |
145 | 128 | |
146 | 129 | visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) |
147 | 130 | @a = current_user.record_appearance(visitor, @next_prompt) |
... | ... | @@ -152,11 +135,11 @@ class PromptsController < InheritedResources::Base |
152 | 135 | visitor_ideas = Proc.new { |options| options[:builder].tag!('visitor_ideas', visitor.items.count) } |
153 | 136 | |
154 | 137 | |
155 | - format.xml { render :xml => @question.picked_prompt.to_xml(:procs => [appearance_id, visitor_votes, visitor_ideas],:methods => [:left_choice_text, :right_choice_text]), :status => :ok } | |
156 | - format.json { render :json => @question.picked_prompt.to_json, :status => :ok } | |
138 | + format.xml { render :xml => @next_prompt.to_xml(:procs => [appearance_id, visitor_votes, visitor_ideas],:methods => [:left_choice_text, :right_choice_text]), :status => :ok } | |
139 | + format.json { render :json => @next_prompt.to_json, :status => :ok } | |
157 | 140 | else |
158 | - format.xml { render :xml => @skip, :status => :unprocessable_entity } | |
159 | - format.json { render :json => @skip, :status => :unprocessable_entity } | |
141 | + format.xml { render :xml => @prompt.to_xml, :status => :conflict} | |
142 | + format.json { render :json => @prompt.to_xml, :status => :conflict} | |
160 | 143 | end |
161 | 144 | end |
162 | 145 | end | ... | ... |
app/controllers/questions_controller.rb
... | ... | @@ -60,6 +60,17 @@ class QuestionsController < InheritedResources::Base |
60 | 60 | |
61 | 61 | @p = @question.choose_prompt(:algorithm => params[:algorithm]) |
62 | 62 | |
63 | + if @p.nil? | |
64 | + # could not find a prompt to choose | |
65 | + # I don't know the best http code for the api to respond, but 409 seems the closes | |
66 | + respond_to do |format| | |
67 | + format.xml { render :xml => @question, :status => :conflict and return} | |
68 | + format.json { render :json => @question, :status => :conflict and return} | |
69 | + end | |
70 | + | |
71 | + end | |
72 | + # @question.create_new_appearance - returns appearance, which we can then get the prompt from. | |
73 | + # At the very least add 'if create_appearance is true, | |
63 | 74 | # we sometimes request a question when no prompt is displayed |
64 | 75 | # TODO It would be a good idea to find these places and treat them like barebones |
65 | 76 | if !visitor_identifier.blank? | ... | ... |
app/models/choice.rb
1 | 1 | class Choice < ActiveRecord::Base |
2 | - include Activation | |
3 | 2 | |
4 | 3 | belongs_to :question, :counter_cache => true |
5 | 4 | belongs_to :item |
... | ... | @@ -103,6 +102,22 @@ class Choice < ActiveRecord::Base |
103 | 102 | |
104 | 103 | end |
105 | 104 | |
105 | + def activate! | |
106 | + (self.active = true) | |
107 | + self.save! | |
108 | + Question.update_counters(self.question_id, :inactive_choices_count => -1) | |
109 | + end | |
110 | + | |
111 | + def suspend! | |
112 | + (self.active = false) | |
113 | + self.save! | |
114 | + end | |
115 | + | |
116 | + def deactivate! | |
117 | + (self.active = false) | |
118 | + self.save! | |
119 | + Question.update_counters(self.question_id, :inactive_choices_count => 1) | |
120 | + end | |
106 | 121 | |
107 | 122 | protected |
108 | 123 | ... | ... |
app/models/question.rb
... | ... | @@ -31,6 +31,11 @@ class Question < ActiveRecord::Base |
31 | 31 | |
32 | 32 | def choose_prompt(options = {}) |
33 | 33 | |
34 | + # if there is one or fewer active choices, we won't be able to find a prompt | |
35 | + if self.choices_count - self.inactive_choices_count <= 1 | |
36 | + return nil | |
37 | + end | |
38 | + | |
34 | 39 | if self.uses_catchup? || options[:algorithm] == "catchup" |
35 | 40 | logger.info("Question #{self.id} is using catchup algorithm!") |
36 | 41 | next_prompt = self.pop_prompt_queue | ... | ... |
lib/activation.rb
... | ... | @@ -1,16 +0,0 @@ |
1 | -module Activation | |
2 | - def activate! | |
3 | - (self.active = true) | |
4 | - self.save! | |
5 | - end | |
6 | - | |
7 | - def suspend! | |
8 | - (self.active = false) | |
9 | - self.save! | |
10 | - end | |
11 | - | |
12 | - def deactivate! | |
13 | - (self.active = false) | |
14 | - self.save! | |
15 | - end | |
16 | -end | |
17 | 0 | \ No newline at end of file |