Commit b18e4649c80f4bcad9fb82ffdfb947de4f0674f0
1 parent
c8450e04
Exists in
master
and in
1 other branch
Properly scope finders in Question and Choice
Showing
7 changed files
with
51 additions
and
41 deletions
Show diff stats
app/controllers/choices_controller.rb
| ... | ... | @@ -8,7 +8,7 @@ class ChoicesController < InheritedResources::Base |
| 8 | 8 | |
| 9 | 9 | def index |
| 10 | 10 | if params[:limit] |
| 11 | - @question = Question.find(params[:question_id]) | |
| 11 | + @question = current_user.questions.find(params[:question_id]) | |
| 12 | 12 | |
| 13 | 13 | find_options = {:conditions => {:question_id => @question.id}, |
| 14 | 14 | :limit => params[:limit].to_i, |
| ... | ... | @@ -21,7 +21,7 @@ class ChoicesController < InheritedResources::Base |
| 21 | 21 | @choices = Choice.find(:all, find_options) |
| 22 | 22 | |
| 23 | 23 | else |
| 24 | - @question = Question.find(params[:question_id], :include => :choices) #eagerloads ALL choices | |
| 24 | + @question = current_user.questions.find(params[:question_id], :include => :choices) #eagerloads ALL choices | |
| 25 | 25 | unless params[:include_inactive] |
| 26 | 26 | @choices = @question.choices(true).active.find(:all) |
| 27 | 27 | else |
| ... | ... | @@ -88,9 +88,16 @@ class ChoicesController < InheritedResources::Base |
| 88 | 88 | # prevent AttributeNotFound error and only update actual Choice columns, since we add extra information in 'show' method |
| 89 | 89 | choice_attributes = Choice.new.attribute_names |
| 90 | 90 | params[:choice] = params[:choice].delete_if {|key, value| !choice_attributes.include?(key)} |
| 91 | + @question = current_user.questions.find(params[:question_id]) | |
| 92 | + @choice = @question.choices.find(params[:id]) | |
| 91 | 93 | update! |
| 92 | 94 | end |
| 93 | - | |
| 95 | + | |
| 96 | + def show | |
| 97 | + @question = current_user.questions.find(params[:question_id]) | |
| 98 | + @choice = @question.choices.find(params[:id]) | |
| 99 | + show! | |
| 100 | + end | |
| 94 | 101 | |
| 95 | 102 | |
| 96 | 103 | end | ... | ... |
app/controllers/questions_controller.rb
| ... | ... | @@ -55,7 +55,7 @@ class QuestionsController < InheritedResources::Base |
| 55 | 55 | |
| 56 | 56 | |
| 57 | 57 | def show |
| 58 | - @question = Question.find(params[:id]) | |
| 58 | + @question = current_user.questions.find(params[:id]) | |
| 59 | 59 | |
| 60 | 60 | begin |
| 61 | 61 | @question_optional_information = @question.get_optional_information(params) |
| ... | ... | @@ -302,9 +302,15 @@ class QuestionsController < InheritedResources::Base |
| 302 | 302 | # prevent AttributeNotFound error and only update actual Question columns, since we add extra information in 'show' method |
| 303 | 303 | question_attributes = Question.new.attribute_names |
| 304 | 304 | params[:question] = params[:question].delete_if {|key, value| !question_attributes.include?(key)} |
| 305 | + @question = current_user.questions.find(params[:id]) | |
| 305 | 306 | update! |
| 306 | 307 | end |
| 307 | 308 | |
| 309 | + def index | |
| 310 | + @questions = current_user.questions.find(:all) | |
| 311 | + index! | |
| 312 | + end | |
| 313 | + | |
| 308 | 314 | protected |
| 309 | 315 | end |
| 310 | 316 | ... | ... |
app/models/choice.rb
| ... | ... | @@ -17,6 +17,7 @@ class Choice < ActiveRecord::Base |
| 17 | 17 | after_save :update_questions_counter |
| 18 | 18 | |
| 19 | 19 | attr_protected :prompts_count, :wins, :losses, :score, :prompts_on_the_right_count, :prompts_on_the_left_count |
| 20 | + attr_readonly :question_id | |
| 20 | 21 | |
| 21 | 22 | def update_questions_counter |
| 22 | 23 | self.question.update_attribute(:inactive_choices_count, self.question.choices.inactive.length) | ... | ... |
app/models/prompt.rb
| ... | ... | @@ -24,6 +24,7 @@ class Prompt < ActiveRecord::Base |
| 24 | 24 | named_scope :ids_only, :select => 'id' |
| 25 | 25 | |
| 26 | 26 | attr_protected :votes_count, :left_choice_id, :right_choice_id |
| 27 | + attr_readonly :question_id | |
| 27 | 28 | |
| 28 | 29 | def self.voted_on_by(u) |
| 29 | 30 | select {|z| z.voted_on_by_user?(u)} | ... | ... |
app/models/question.rb
| ... | ... | @@ -27,6 +27,8 @@ class Question < ActiveRecord::Base |
| 27 | 27 | attr_protected :votes_count, :inactive_choices_count, :choices_count, |
| 28 | 28 | :active_items_count, :prompts_count |
| 29 | 29 | |
| 30 | + attr_readonly :site_id | |
| 31 | + | |
| 30 | 32 | def create_choices_from_ideas |
| 31 | 33 | if ideas && ideas.any? |
| 32 | 34 | ideas.each do |idea| | ... | ... |
spec/integration/choices_spec.rb
| ... | ... | @@ -120,17 +120,16 @@ describe "Choices" do |
| 120 | 120 | |
| 121 | 121 | context "when trying to access another site's choices" do |
| 122 | 122 | before do |
| 123 | - @other_user = Factory(:email_confirmed_user) | |
| 124 | - @other_question = Factory.create(:aoi_question, :site => @other_user) | |
| 125 | - 5.times{ Factory.create(:choice, :question => @other_question) } | |
| 123 | + @orig_user = @api_user | |
| 124 | + @api_user = Factory(:email_confirmed_user) | |
| 126 | 125 | end |
| 127 | 126 | |
| 128 | 127 | it "should fail" do |
| 129 | - pending("user scope") do | |
| 130 | - get_auth question_choices_path(@question, :format => 'xml'), :offset => 2, :limit => 4 | |
| 131 | - response.should_not be_success | |
| 132 | - end | |
| 128 | + get_auth question_choices_path(@question, :format => 'xml'), :offset => 2, :limit => 4 | |
| 129 | + response.should_not be_success | |
| 133 | 130 | end |
| 131 | + | |
| 132 | + after { @api_user = @orig_user } | |
| 134 | 133 | end |
| 135 | 134 | |
| 136 | 135 | end |
| ... | ... | @@ -156,10 +155,8 @@ describe "Choices" do |
| 156 | 155 | end |
| 157 | 156 | |
| 158 | 157 | it "should fail" do |
| 159 | - pending("user scope") do | |
| 160 | - get_auth question_choice_path(@other_question, @other_choice, :format => 'xml') | |
| 161 | - response.should_not be_success | |
| 162 | - end | |
| 158 | + get_auth question_choice_path(@other_question, @other_choice, :format => 'xml') | |
| 159 | + response.should_not be_success | |
| 163 | 160 | end |
| 164 | 161 | end |
| 165 | 162 | |
| ... | ... | @@ -185,11 +182,9 @@ describe "Choices" do |
| 185 | 182 | end |
| 186 | 183 | |
| 187 | 184 | it "should fail" do |
| 188 | - pending("user scope") do | |
| 189 | - params = { :choice => { :data => "foo" } } | |
| 190 | - put_auth question_choice_path(@question, @choice, :format => 'xml'), params | |
| 191 | - response.should_not be_success | |
| 192 | - end | |
| 185 | + params = { :choice => { :data => "foo" } } | |
| 186 | + put_auth question_choice_path(@question, @choice, :format => 'xml'), params | |
| 187 | + response.should_not be_success | |
| 193 | 188 | end |
| 194 | 189 | |
| 195 | 190 | after { @api_user = @orig_user } | ... | ... |
spec/integration/questions_spec.rb
| ... | ... | @@ -13,14 +13,18 @@ describe "Questions" do |
| 13 | 13 | response.should be_success |
| 14 | 14 | end |
| 15 | 15 | |
| 16 | - it "should not return the questions of other api users" do | |
| 17 | - pending ("doesn't scope to the level of the user") do | |
| 18 | - other_user = Factory(:email_confirmed_user) | |
| 19 | - Factory.create(:aoi_question, :site => other_user) | |
| 20 | - get_auth questions_path | |
| 16 | + context "when calling index as another user" do | |
| 17 | + before do | |
| 18 | + @orig_user = @api_user | |
| 19 | + @api_user = Factory(:email_confirmed_user) | |
| 20 | + end | |
| 21 | + | |
| 22 | + it "should not return the questions of the original user" do | |
| 23 | + get_auth questions_path(:format => 'xml') | |
| 21 | 24 | response.should be_success |
| 22 | 25 | response.body.should_not have_tag("question") |
| 23 | 26 | end |
| 27 | + after { @api_user = @orig_user } | |
| 24 | 28 | end |
| 25 | 29 | end |
| 26 | 30 | |
| ... | ... | @@ -135,20 +139,18 @@ describe "Questions" do |
| 135 | 139 | end |
| 136 | 140 | end |
| 137 | 141 | |
| 138 | - context "GET 'show' trying to view others sites' questions" | |
| 142 | + context "GET 'show' trying to view others sites' questions" do | |
| 139 | 143 | before do |
| 140 | 144 | @orig_user = @api_user |
| 141 | 145 | @api_user = Factory(:email_confirmed_user) |
| 142 | 146 | end |
| 143 | 147 | |
| 144 | - it "should fail" do | |
| 145 | - pending("user scope") do | |
| 148 | + it "should fail" do | |
| 146 | 149 | get_auth question_path(@question, :format => 'xml') |
| 147 | 150 | response.should_not be_success |
| 148 | 151 | end |
| 152 | + after { @api_user = @orig_user } | |
| 149 | 153 | end |
| 150 | - | |
| 151 | - after { @api_user = @orig_user } | |
| 152 | 154 | end |
| 153 | 155 | |
| 154 | 156 | describe "PUT 'update'" do |
| ... | ... | @@ -166,12 +168,10 @@ describe "Questions" do |
| 166 | 168 | end |
| 167 | 169 | |
| 168 | 170 | it "should not be able to change the site id" do |
| 169 | - pending("needs attr_protected") do | |
| 170 | - original_site_id = @question.site_id | |
| 171 | - params = { :question => { :site_id => -1 } } | |
| 172 | - put_auth question_path(@question, :format => 'xml'), params | |
| 173 | - @question.reload.site_id.should == original_site_id | |
| 174 | - end | |
| 171 | + original_site_id = @question.site_id | |
| 172 | + params = { :question => { :site_id => -1 } } | |
| 173 | + put_auth question_path(@question, :format => 'xml'), params | |
| 174 | + @question.reload.site_id.should == original_site_id | |
| 175 | 175 | end |
| 176 | 176 | |
| 177 | 177 | it "should ignore protected attributes" do |
| ... | ... | @@ -188,11 +188,9 @@ describe "Questions" do |
| 188 | 188 | end |
| 189 | 189 | |
| 190 | 190 | it "should fail" do |
| 191 | - pending("user scope") do | |
| 192 | - params = { :question => { :name => "foo" } } | |
| 193 | - put_auth question_path(@question, :format => 'xml'), params | |
| 194 | - response.should_not be_success | |
| 195 | - end | |
| 191 | + params = { :question => { :name => "foo" } } | |
| 192 | + put_auth question_path(@question, :format => 'xml'), params | |
| 193 | + response.should_not be_success | |
| 196 | 194 | end |
| 197 | 195 | |
| 198 | 196 | after { @api_user = @orig_user } | ... | ... |