Commit bec07833a2fce4f99bb26d8906981a13d89504bb
1 parent
fdb1c90a
Exists in
master
and in
1 other branch
Disallowed mass assignment of some attributes (counts)
Showing
4 changed files
with
41 additions
and
4 deletions
Show diff stats
app/models/choice.rb
| @@ -16,6 +16,9 @@ class Choice < ActiveRecord::Base | @@ -16,6 +16,9 @@ class Choice < ActiveRecord::Base | ||
| 16 | 16 | ||
| 17 | after_save :update_questions_counter | 17 | after_save :update_questions_counter |
| 18 | 18 | ||
| 19 | + attr_protected :prompts_count, :votes_count, :loss_count, :wins, :losses, :score, | ||
| 20 | + :prompts_on_the_right_count, :prompts_on_the_left_count | ||
| 21 | + | ||
| 19 | def update_questions_counter | 22 | def update_questions_counter |
| 20 | self.question.update_attribute(:inactive_choices_count, self.question.choices.inactive.length) | 23 | self.question.update_attribute(:inactive_choices_count, self.question.choices.inactive.length) |
| 21 | end | 24 | end |
| @@ -24,7 +27,8 @@ class Choice < ActiveRecord::Base | @@ -24,7 +27,8 @@ class Choice < ActiveRecord::Base | ||
| 24 | def lose! | 27 | def lose! |
| 25 | Choice.increment_counter(:loss_count, self.id) | 28 | Choice.increment_counter(:loss_count, self.id) |
| 26 | self.loss_count +=1 # reflect the update just done above, so score is correct | 29 | self.loss_count +=1 # reflect the update just done above, so score is correct |
| 27 | - Choice.update(self.id, :score => compute_score) | 30 | + self.score = compute_score |
| 31 | + self.save | ||
| 28 | end | 32 | end |
| 29 | 33 | ||
| 30 | def win! | 34 | def win! |
app/models/prompt.rb
| @@ -23,7 +23,8 @@ class Prompt < ActiveRecord::Base | @@ -23,7 +23,8 @@ class Prompt < ActiveRecord::Base | ||
| 23 | named_scope :active, :include => [:left_choice, :right_choice], :conditions => { 'left_choice.active' => true, 'right_choice.active' => true } | 23 | named_scope :active, :include => [:left_choice, :right_choice], :conditions => { 'left_choice.active' => true, 'right_choice.active' => true } |
| 24 | named_scope :ids_only, :select => 'id' | 24 | named_scope :ids_only, :select => 'id' |
| 25 | 25 | ||
| 26 | - | 26 | + attr_protected :votes_count, :left_choice_id, :right_choice_id |
| 27 | + | ||
| 27 | def self.voted_on_by(u) | 28 | def self.voted_on_by(u) |
| 28 | select {|z| z.voted_on_by_user?(u)} | 29 | select {|z| z.voted_on_by_user?(u)} |
| 29 | end | 30 | end |
app/models/question.rb
| @@ -20,9 +20,13 @@ class Question < ActiveRecord::Base | @@ -20,9 +20,13 @@ class Question < ActiveRecord::Base | ||
| 20 | has_many :skips | 20 | has_many :skips |
| 21 | has_many :densities | 21 | has_many :densities |
| 22 | has_many :appearances | 22 | has_many :appearances |
| 23 | - | 23 | + |
| 24 | attr_accessor :ideas | 24 | attr_accessor :ideas |
| 25 | after_create :create_choices_from_ideas | 25 | after_create :create_choices_from_ideas |
| 26 | + | ||
| 27 | + attr_protected :votes_count, :inactive_choices_count, :choices_count, | ||
| 28 | + :active_items_count, :prompts_count | ||
| 29 | + | ||
| 26 | def create_choices_from_ideas | 30 | def create_choices_from_ideas |
| 27 | if ideas && ideas.any? | 31 | if ideas && ideas.any? |
| 28 | ideas.each do |idea| | 32 | ideas.each do |idea| |
spec/models/choice_spec.rb
| @@ -23,12 +23,40 @@ describe Choice do | @@ -23,12 +23,40 @@ describe Choice do | ||
| 23 | :question => @question, | 23 | :question => @question, |
| 24 | :data => 'hi there' | 24 | :data => 'hi there' |
| 25 | } | 25 | } |
| 26 | + | ||
| 27 | + @unreasonable_value = 9999 | ||
| 28 | + @protected_attributes = {} | ||
| 29 | + [ :prompts_count, | ||
| 30 | + :votes_count, | ||
| 31 | + :loss_count, | ||
| 32 | + :wins, | ||
| 33 | + :losses, | ||
| 34 | + :score, | ||
| 35 | + :prompts_on_the_right_count, | ||
| 36 | + :prompts_on_the_left_count | ||
| 37 | + ].each{|key| @protected_attributes[key] = @unreasonable_value} | ||
| 38 | + | ||
| 26 | end | 39 | end |
| 27 | 40 | ||
| 28 | it "should create a new instance given valid attributes" do | 41 | it "should create a new instance given valid attributes" do |
| 29 | Choice.create!(@valid_attributes) | 42 | Choice.create!(@valid_attributes) |
| 30 | end | 43 | end |
| 31 | - | 44 | + |
| 45 | + it "should not manually set protected attributes when created" do | ||
| 46 | + choice1 = Choice.create!(@valid_attributes.merge(@protected_attributes)) | ||
| 47 | + @protected_attributes.each_key do |key| | ||
| 48 | + choice1[key].should_not == @unreasonable_value | ||
| 49 | + end | ||
| 50 | + end | ||
| 51 | + | ||
| 52 | + it "should not allow mass assignment of protected attributes" do | ||
| 53 | + choice1 = Choice.create!(@valid_attributes) | ||
| 54 | + choice1.update_attributes(@protected_attributes) | ||
| 55 | + @protected_attributes.each_key do |key| | ||
| 56 | + choice1[key].should_not == @unreasonable_value | ||
| 57 | + end | ||
| 58 | + end | ||
| 59 | + | ||
| 32 | it "should deactivate a choice" do | 60 | it "should deactivate a choice" do |
| 33 | choice1 = Choice.create!(@valid_attributes.merge(:data => '1234')) | 61 | choice1 = Choice.create!(@valid_attributes.merge(:data => '1234')) |
| 34 | choice1.deactivate! | 62 | choice1.deactivate! |