Commit 87857f3f0db376bf07ad5bc4907581f44682e8b3
1 parent
c9546a47
Exists in
master
and in
1 other branch
Test caught bugfix. Modifying tests
Showing
4 changed files
with
44 additions
and
117 deletions
Show diff stats
app/models/question.rb
@@ -26,13 +26,13 @@ class Question < ActiveRecord::Base | @@ -26,13 +26,13 @@ class Question < ActiveRecord::Base | ||
26 | attr_accessor :ideas | 26 | attr_accessor :ideas |
27 | 27 | ||
28 | def item_count | 28 | def item_count |
29 | - choices_count | 29 | + choices.size |
30 | end | 30 | end |
31 | 31 | ||
32 | def choose_prompt(options = {}) | 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 | 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 | 35 | + if self.choices.size - self.inactive_choices_count <= 1 |
36 | return nil | 36 | return nil |
37 | end | 37 | end |
38 | 38 |
spec/controllers/questions_controller_spec.rb
@@ -40,12 +40,12 @@ describe QuestionsController do | @@ -40,12 +40,12 @@ describe QuestionsController do | ||
40 | describe "GET show normal" do | 40 | describe "GET show normal" do |
41 | before(:each) do | 41 | before(:each) do |
42 | Question.stub!(:find).with("37").and_return(mock_question) | 42 | Question.stub!(:find).with("37").and_return(mock_question) |
43 | - mock_question.stub!(:picked_prompt).and_return(mock_prompt) | ||
44 | end | 43 | end |
45 | 44 | ||
46 | 45 | ||
47 | it "assigns the requested question as @question" do | 46 | it "assigns the requested question as @question" do |
48 | Question.stub!(:find).with("37").and_return(mock_question) | 47 | Question.stub!(:find).with("37").and_return(mock_question) |
48 | + mock_question.should_receive(:choose_prompt).and_return(mock_prompt) | ||
49 | #TODO it shouldn't call this unless we are generating an appearance, right? | 49 | #TODO it shouldn't call this unless we are generating an appearance, right? |
50 | 50 | ||
51 | get :show, :id => "37" | 51 | get :show, :id => "37" |
@@ -74,6 +74,7 @@ describe QuestionsController do | @@ -74,6 +74,7 @@ describe QuestionsController do | ||
74 | 74 | ||
75 | #TODO this is not a particularly intutive param to pass in order to create an appearance | 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 | 76 | it "creates an appearance when a visitor identifier is a param" do |
77 | + mock_question.should_receive(:choose_prompt).and_return(mock_prompt) | ||
77 | get :show, :id => "37", :visitor_identifier => @visitor_identifier | 78 | get :show, :id => "37", :visitor_identifier => @visitor_identifier |
78 | assigns[:question].should equal(mock_question) | 79 | assigns[:question].should equal(mock_question) |
79 | assigns[:p].should equal(mock_prompt) | 80 | assigns[:p].should equal(mock_prompt) |
@@ -89,14 +90,10 @@ describe QuestionsController do | @@ -89,14 +90,10 @@ describe QuestionsController do | ||
89 | end | 90 | end |
90 | 91 | ||
91 | describe "calls catchup algorithm" do | 92 | 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 | 93 | ||
97 | #TODO Refactor out to use uses_catchup? | 94 | #TODO Refactor out to use uses_catchup? |
98 | it "should pop prompt from cached queue using the catchup algorithm if params dictate" do | 95 | 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) | 96 | + mock_question.should_receive(:choose_prompt).with(:algorithm => "catchup").and_return(mock_prompt) |
100 | 97 | ||
101 | get :show, :id => "37", :visitor_identifier => @visitor_identifier, :algorithm => "catchup" | 98 | get :show, :id => "37", :visitor_identifier => @visitor_identifier, :algorithm => "catchup" |
102 | assigns[:question].should equal(mock_question) | 99 | assigns[:question].should equal(mock_question) |
@@ -105,8 +102,7 @@ describe QuestionsController do | @@ -105,8 +102,7 @@ describe QuestionsController do | ||
105 | end | 102 | end |
106 | 103 | ||
107 | it "should handle cache misses gracefully" do | 104 | 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) | 105 | + mock_question.should_receive(:choose_prompt).with(:algorithm => "catchup").and_return(mock_prompt) |
110 | 106 | ||
111 | get :show, :id => "37", :visitor_identifier => @visitor_identifier, :algorithm => "catchup" | 107 | get :show, :id => "37", :visitor_identifier => @visitor_identifier, :algorithm => "catchup" |
112 | assigns[:question].should equal(mock_question) | 108 | assigns[:question].should equal(mock_question) |
@@ -117,111 +113,5 @@ describe QuestionsController do | @@ -117,111 +113,5 @@ describe QuestionsController do | ||
117 | end | 113 | end |
118 | end | 114 | end |
119 | 115 | ||
120 | - # | ||
121 | - # describe "GET new" do | ||
122 | - # it "assigns a new question as @question" do | ||
123 | - # Question.stub!(:new).and_return(mock_question) | ||
124 | - # get :new | ||
125 | - # assigns[:question].should equal(mock_question) | ||
126 | - # end | ||
127 | - # end | ||
128 | - # | ||
129 | - # describe "GET edit" do | ||
130 | - # it "assigns the requested question as @question" do | ||
131 | - # Question.stub!(:find).with("37").and_return(mock_question) | ||
132 | - # get :edit, :id => "37" | ||
133 | - # assigns[:question].should equal(mock_question) | ||
134 | - # end | ||
135 | - # end | ||
136 | - # | ||
137 | - # describe "POST create" do | ||
138 | - # | ||
139 | - # describe "with valid params" do | ||
140 | - # it "assigns a newly created question as @question" do | ||
141 | - # Question.stub!(:new).with({'these' => 'params'}).and_return(mock_question(:save => true)) | ||
142 | - # post :create, :question => {:these => 'params'} | ||
143 | - # assigns[:question].should equal(mock_question) | ||
144 | - # end | ||
145 | - # | ||
146 | - # it "redirects to the created question" do | ||
147 | - # Question.stub!(:new).and_return(mock_question(:save => true)) | ||
148 | - # post :create, :question => {} | ||
149 | - # response.should redirect_to(question_url(mock_question)) | ||
150 | - # end | ||
151 | - # end | ||
152 | - # | ||
153 | - # describe "with invalid params" do | ||
154 | - # it "assigns a newly created but unsaved question as @question" do | ||
155 | - # Question.stub!(:new).with({'these' => 'params'}).and_return(mock_question(:save => false)) | ||
156 | - # post :create, :question => {:these => 'params'} | ||
157 | - # assigns[:question].should equal(mock_question) | ||
158 | - # end | ||
159 | - # | ||
160 | - # it "re-renders the 'new' template" do | ||
161 | - # Question.stub!(:new).and_return(mock_question(:save => false)) | ||
162 | - # post :create, :question => {} | ||
163 | - # response.should render_template('new') | ||
164 | - # end | ||
165 | - # end | ||
166 | - # | ||
167 | - # end | ||
168 | - # | ||
169 | - # describe "PUT update" do | ||
170 | - # | ||
171 | - # describe "with valid params" do | ||
172 | - # it "updates the requested question" do | ||
173 | - # Question.should_receive(:find).with("37").and_return(mock_question) | ||
174 | - # mock_question.should_receive(:update_attributes).with({'these' => 'params'}) | ||
175 | - # put :update, :id => "37", :question => {:these => 'params'} | ||
176 | - # end | ||
177 | - # | ||
178 | - # it "assigns the requested question as @question" do | ||
179 | - # Question.stub!(:find).and_return(mock_question(:update_attributes => true)) | ||
180 | - # put :update, :id => "1" | ||
181 | - # assigns[:question].should equal(mock_question) | ||
182 | - # end | ||
183 | - # | ||
184 | - # it "redirects to the question" do | ||
185 | - # Question.stub!(:find).and_return(mock_question(:update_attributes => true)) | ||
186 | - # put :update, :id => "1" | ||
187 | - # response.should redirect_to(question_url(mock_question)) | ||
188 | - # end | ||
189 | - # end | ||
190 | - # | ||
191 | - # describe "with invalid params" do | ||
192 | - # it "updates the requested question" do | ||
193 | - # Question.should_receive(:find).with("37").and_return(mock_question) | ||
194 | - # mock_question.should_receive(:update_attributes).with({'these' => 'params'}) | ||
195 | - # put :update, :id => "37", :question => {:these => 'params'} | ||
196 | - # end | ||
197 | - # | ||
198 | - # it "assigns the question as @question" do | ||
199 | - # Question.stub!(:find).and_return(mock_question(:update_attributes => false)) | ||
200 | - # put :update, :id => "1" | ||
201 | - # assigns[:question].should equal(mock_question) | ||
202 | - # end | ||
203 | - # | ||
204 | - # it "re-renders the 'edit' template" do | ||
205 | - # Question.stub!(:find).and_return(mock_question(:update_attributes => false)) | ||
206 | - # put :update, :id => "1" | ||
207 | - # response.should render_template('edit') | ||
208 | - # end | ||
209 | - # end | ||
210 | - # | ||
211 | - # end | ||
212 | - # | ||
213 | - # describe "DELETE destroy" do | ||
214 | - # it "destroys the requested question" do | ||
215 | - # Question.should_receive(:find).with("37").and_return(mock_question) | ||
216 | - # mock_question.should_receive(:destroy) | ||
217 | - # delete :destroy, :id => "37" | ||
218 | - # end | ||
219 | - # | ||
220 | - # it "redirects to the questions list" do | ||
221 | - # Question.stub!(:find).and_return(mock_question(:destroy => true)) | ||
222 | - # delete :destroy, :id => "1" | ||
223 | - # response.should redirect_to(questions_url) | ||
224 | - # end | ||
225 | - # end | ||
226 | 116 | ||
227 | end | 117 | end |
spec/models/choice_spec.rb
@@ -24,8 +24,9 @@ describe Choice do | @@ -24,8 +24,9 @@ describe Choice do | ||
24 | end | 24 | end |
25 | 25 | ||
26 | it "should generate prompts after creation" do | 26 | it "should generate prompts after creation" do |
27 | + prev_choices = @question.choices.size | ||
27 | @question.prompts.should_not be_empty | 28 | @question.prompts.should_not be_empty |
28 | - choice1 = Choice.create!(@valid_attributes.merge(:data => '1234')) | 29 | + proc {choice1 = Choice.create!(@valid_attributes.merge(:data => '1234'))}.should change(@question.prompts, :count).by(prev_choices*2) |
29 | @question.prompts.should_not be_empty | 30 | @question.prompts.should_not be_empty |
30 | end | 31 | end |
31 | 32 | ||
@@ -34,4 +35,31 @@ describe Choice do | @@ -34,4 +35,31 @@ describe Choice do | ||
34 | choice1.deactivate! | 35 | choice1.deactivate! |
35 | choice1.should_not be_active | 36 | choice1.should_not be_active |
36 | end | 37 | end |
38 | + | ||
39 | + it "should update a question's counter cache on creation" do | ||
40 | + @question.choices.size.should == 2 | ||
41 | + choice1 = Choice.create!(@valid_attributes.merge(:data => '1234')) | ||
42 | + @question.reload | ||
43 | + @question.choices.size.should == 3 | ||
44 | + end | ||
45 | + | ||
46 | + it "should update a question's counter cache on activation" do | ||
47 | + prev_inactive = @question.inactive_choices_count | ||
48 | + choice1 = Choice.create!(@valid_attributes.merge(:data => '1234')) | ||
49 | + choice1.deactivate! | ||
50 | + @question.reload | ||
51 | + @question.inactive_choices_count.should == prev_inactive + 1 | ||
52 | + choice1.activate! | ||
53 | + @question.reload | ||
54 | + @question.inactive_choices_count.should == prev_inactive | ||
55 | + choice1.should be_active | ||
56 | + end | ||
57 | + | ||
58 | + it "should update a question's counter cache on deactivation" do | ||
59 | + prev_inactive = @question.inactive_choices_count | ||
60 | + choice1 = Choice.create!(@valid_attributes.merge(:data => '1234')) | ||
61 | + choice1.deactivate! | ||
62 | + @question.reload | ||
63 | + @question.inactive_choices_count.should == prev_inactive + 1 | ||
64 | + end | ||
37 | end | 65 | end |
spec/models/question_spec.rb
@@ -52,6 +52,15 @@ describe Question do | @@ -52,6 +52,15 @@ describe Question do | ||
52 | prompt.active?.should == true | 52 | prompt.active?.should == true |
53 | end | 53 | end |
54 | 54 | ||
55 | + it "should return nil if there is no possible prompt to choose" do | ||
56 | + q = @aoi_clone.create_question("foobarbaz", {:name => 'foo'}) | ||
57 | + q.choices.first.deactivate! | ||
58 | + q.reload | ||
59 | + q.choose_prompt.should be_nil | ||
60 | + | ||
61 | + end | ||
62 | + | ||
63 | + | ||
55 | 64 | ||
56 | context "catchup algorithm" do | 65 | context "catchup algorithm" do |
57 | before(:all) do | 66 | before(:all) do |