From b18e4649c80f4bcad9fb82ffdfb947de4f0674f0 Mon Sep 17 00:00:00 2001 From: Dmitri Garbuzov Date: Tue, 3 Aug 2010 17:54:50 -0400 Subject: [PATCH] Properly scope finders in Question and Choice --- app/controllers/choices_controller.rb | 13 ++++++++++--- app/controllers/questions_controller.rb | 8 +++++++- app/models/choice.rb | 1 + app/models/prompt.rb | 1 + app/models/question.rb | 2 ++ spec/integration/choices_spec.rb | 27 +++++++++++---------------- spec/integration/questions_spec.rb | 40 +++++++++++++++++++--------------------- 7 files changed, 51 insertions(+), 41 deletions(-) diff --git a/app/controllers/choices_controller.rb b/app/controllers/choices_controller.rb index 9d2386a..f8de18d 100644 --- a/app/controllers/choices_controller.rb +++ b/app/controllers/choices_controller.rb @@ -8,7 +8,7 @@ class ChoicesController < InheritedResources::Base def index if params[:limit] - @question = Question.find(params[:question_id]) + @question = current_user.questions.find(params[:question_id]) find_options = {:conditions => {:question_id => @question.id}, :limit => params[:limit].to_i, @@ -21,7 +21,7 @@ class ChoicesController < InheritedResources::Base @choices = Choice.find(:all, find_options) else - @question = Question.find(params[:question_id], :include => :choices) #eagerloads ALL choices + @question = current_user.questions.find(params[:question_id], :include => :choices) #eagerloads ALL choices unless params[:include_inactive] @choices = @question.choices(true).active.find(:all) else @@ -88,9 +88,16 @@ class ChoicesController < InheritedResources::Base # prevent AttributeNotFound error and only update actual Choice columns, since we add extra information in 'show' method choice_attributes = Choice.new.attribute_names params[:choice] = params[:choice].delete_if {|key, value| !choice_attributes.include?(key)} + @question = current_user.questions.find(params[:question_id]) + @choice = @question.choices.find(params[:id]) update! end - + + def show + @question = current_user.questions.find(params[:question_id]) + @choice = @question.choices.find(params[:id]) + show! + end end diff --git a/app/controllers/questions_controller.rb b/app/controllers/questions_controller.rb index 529f147..e97e2b4 100644 --- a/app/controllers/questions_controller.rb +++ b/app/controllers/questions_controller.rb @@ -55,7 +55,7 @@ class QuestionsController < InheritedResources::Base def show - @question = Question.find(params[:id]) + @question = current_user.questions.find(params[:id]) begin @question_optional_information = @question.get_optional_information(params) @@ -302,9 +302,15 @@ class QuestionsController < InheritedResources::Base # prevent AttributeNotFound error and only update actual Question columns, since we add extra information in 'show' method question_attributes = Question.new.attribute_names params[:question] = params[:question].delete_if {|key, value| !question_attributes.include?(key)} + @question = current_user.questions.find(params[:id]) update! end + def index + @questions = current_user.questions.find(:all) + index! + end + protected end diff --git a/app/models/choice.rb b/app/models/choice.rb index 8f4455e..cbf4eb8 100644 --- a/app/models/choice.rb +++ b/app/models/choice.rb @@ -17,6 +17,7 @@ class Choice < ActiveRecord::Base after_save :update_questions_counter attr_protected :prompts_count, :wins, :losses, :score, :prompts_on_the_right_count, :prompts_on_the_left_count + attr_readonly :question_id def update_questions_counter self.question.update_attribute(:inactive_choices_count, self.question.choices.inactive.length) diff --git a/app/models/prompt.rb b/app/models/prompt.rb index 227a358..e74f964 100644 --- a/app/models/prompt.rb +++ b/app/models/prompt.rb @@ -24,6 +24,7 @@ class Prompt < ActiveRecord::Base named_scope :ids_only, :select => 'id' attr_protected :votes_count, :left_choice_id, :right_choice_id + attr_readonly :question_id def self.voted_on_by(u) select {|z| z.voted_on_by_user?(u)} diff --git a/app/models/question.rb b/app/models/question.rb index 0bc6634..ec1941e 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -27,6 +27,8 @@ class Question < ActiveRecord::Base attr_protected :votes_count, :inactive_choices_count, :choices_count, :active_items_count, :prompts_count + attr_readonly :site_id + def create_choices_from_ideas if ideas && ideas.any? ideas.each do |idea| diff --git a/spec/integration/choices_spec.rb b/spec/integration/choices_spec.rb index 7cccd6e..8cb9361 100644 --- a/spec/integration/choices_spec.rb +++ b/spec/integration/choices_spec.rb @@ -120,17 +120,16 @@ describe "Choices" do context "when trying to access another site's choices" do before do - @other_user = Factory(:email_confirmed_user) - @other_question = Factory.create(:aoi_question, :site => @other_user) - 5.times{ Factory.create(:choice, :question => @other_question) } + @orig_user = @api_user + @api_user = Factory(:email_confirmed_user) end it "should fail" do - pending("user scope") do - get_auth question_choices_path(@question, :format => 'xml'), :offset => 2, :limit => 4 - response.should_not be_success - end + get_auth question_choices_path(@question, :format => 'xml'), :offset => 2, :limit => 4 + response.should_not be_success end + + after { @api_user = @orig_user } end end @@ -156,10 +155,8 @@ describe "Choices" do end it "should fail" do - pending("user scope") do - get_auth question_choice_path(@other_question, @other_choice, :format => 'xml') - response.should_not be_success - end + get_auth question_choice_path(@other_question, @other_choice, :format => 'xml') + response.should_not be_success end end @@ -185,11 +182,9 @@ describe "Choices" do end it "should fail" do - pending("user scope") do - params = { :choice => { :data => "foo" } } - put_auth question_choice_path(@question, @choice, :format => 'xml'), params - response.should_not be_success - end + params = { :choice => { :data => "foo" } } + put_auth question_choice_path(@question, @choice, :format => 'xml'), params + response.should_not be_success end after { @api_user = @orig_user } diff --git a/spec/integration/questions_spec.rb b/spec/integration/questions_spec.rb index a3a9e69..899ce9a 100644 --- a/spec/integration/questions_spec.rb +++ b/spec/integration/questions_spec.rb @@ -13,14 +13,18 @@ describe "Questions" do response.should be_success end - it "should not return the questions of other api users" do - pending ("doesn't scope to the level of the user") do - other_user = Factory(:email_confirmed_user) - Factory.create(:aoi_question, :site => other_user) - get_auth questions_path + context "when calling index as another user" do + before do + @orig_user = @api_user + @api_user = Factory(:email_confirmed_user) + end + + it "should not return the questions of the original user" do + get_auth questions_path(:format => 'xml') response.should be_success response.body.should_not have_tag("question") end + after { @api_user = @orig_user } end end @@ -135,20 +139,18 @@ describe "Questions" do end end - context "GET 'show' trying to view others sites' questions" + context "GET 'show' trying to view others sites' questions" do before do @orig_user = @api_user @api_user = Factory(:email_confirmed_user) end - it "should fail" do - pending("user scope") do + it "should fail" do get_auth question_path(@question, :format => 'xml') response.should_not be_success end + after { @api_user = @orig_user } end - - after { @api_user = @orig_user } end describe "PUT 'update'" do @@ -166,12 +168,10 @@ describe "Questions" do end it "should not be able to change the site id" do - pending("needs attr_protected") do - original_site_id = @question.site_id - params = { :question => { :site_id => -1 } } - put_auth question_path(@question, :format => 'xml'), params - @question.reload.site_id.should == original_site_id - end + original_site_id = @question.site_id + params = { :question => { :site_id => -1 } } + put_auth question_path(@question, :format => 'xml'), params + @question.reload.site_id.should == original_site_id end it "should ignore protected attributes" do @@ -188,11 +188,9 @@ describe "Questions" do end it "should fail" do - pending("user scope") do - params = { :question => { :name => "foo" } } - put_auth question_path(@question, :format => 'xml'), params - response.should_not be_success - end + params = { :question => { :name => "foo" } } + put_auth question_path(@question, :format => 'xml'), params + response.should_not be_success end after { @api_user = @orig_user } -- libgit2 0.21.2