Commit 1a4f43bd0212213bb58d2daf91f1f9b2f6455531
1 parent
1927f764
Exists in
master
and in
1 other branch
Prevent voters from voting twice on the same appearance
Showing
5 changed files
with
83 additions
and
4 deletions
Show diff stats
app/models/skip.rb
| @@ -4,5 +4,5 @@ class Skip < ActiveRecord::Base | @@ -4,5 +4,5 @@ class Skip < ActiveRecord::Base | ||
| 4 | belongs_to :prompt | 4 | belongs_to :prompt |
| 5 | has_one :appearance, :as => :answerable | 5 | has_one :appearance, :as => :answerable |
| 6 | 6 | ||
| 7 | - default_scope :conditions => {:valid_record => true} | 7 | + default_scope :conditions => "#{table_name}.valid_record = 1" |
| 8 | end | 8 | end |
app/models/visitor.rb
| @@ -25,7 +25,13 @@ class Visitor < ActiveRecord::Base | @@ -25,7 +25,13 @@ class Visitor < ActiveRecord::Base | ||
| 25 | if options[:appearance_lookup] | 25 | if options[:appearance_lookup] |
| 26 | @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup)) | 26 | @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup)) |
| 27 | return nil unless @appearance # don't allow people to fake appearance lookups | 27 | return nil unless @appearance # don't allow people to fake appearance lookups |
| 28 | - options.merge!(:appearance => @appearance) | 28 | + |
| 29 | + if @appearance.answered? | ||
| 30 | + options.merge!(:valid_record => false) | ||
| 31 | + options.merge!(:validity_information => "Appearance #{@appearance.id} already answered") | ||
| 32 | + else | ||
| 33 | + options.merge!(:appearance => @appearance) | ||
| 34 | + end | ||
| 29 | end | 35 | end |
| 30 | 36 | ||
| 31 | choice = prompt.choices[ordinality] #we need to guarantee that the choices are in the right order (by position) | 37 | choice = prompt.choices[ordinality] #we need to guarantee that the choices are in the right order (by position) |
| @@ -45,7 +51,12 @@ class Visitor < ActiveRecord::Base | @@ -45,7 +51,12 @@ class Visitor < ActiveRecord::Base | ||
| 45 | if options[:appearance_lookup] | 51 | if options[:appearance_lookup] |
| 46 | @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup)) | 52 | @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup)) |
| 47 | return nil unless @appearance | 53 | return nil unless @appearance |
| 48 | - options.merge!(:appearance => @appearance) | 54 | + if @appearance.answered? |
| 55 | + options.merge!(:valid_record => false) | ||
| 56 | + options.merge!(:validity_information => "Appearance #{@appearance.id} already answered") | ||
| 57 | + else | ||
| 58 | + options.merge!(:appearance => @appearance) | ||
| 59 | + end | ||
| 49 | end | 60 | end |
| 50 | 61 | ||
| 51 | options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :skipper_id => self.id) | 62 | options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :skipper_id => self.id) |
app/models/vote.rb
| @@ -18,7 +18,7 @@ class Vote < ActiveRecord::Base | @@ -18,7 +18,7 @@ class Vote < ActiveRecord::Base | ||
| 18 | named_scope :with_voter_ids, lambda { |*args| {:conditions => {:voter_id=> args.first }} } | 18 | named_scope :with_voter_ids, lambda { |*args| {:conditions => {:voter_id=> args.first }} } |
| 19 | named_scope :active, :include => :choice, :conditions => { 'choices.active' => true } | 19 | named_scope :active, :include => :choice, :conditions => { 'choices.active' => true } |
| 20 | 20 | ||
| 21 | - default_scope :conditions => {:valid_record => true} | 21 | + default_scope :conditions => "#{table_name}.valid_record = 1" |
| 22 | 22 | ||
| 23 | serialize :tracking | 23 | serialize :tracking |
| 24 | 24 |
spec/models/visitor_spec.rb
| @@ -118,6 +118,46 @@ describe Visitor do | @@ -118,6 +118,46 @@ describe Visitor do | ||
| 118 | s.should be_nil | 118 | s.should be_nil |
| 119 | Skip.count.should == skip_count | 119 | Skip.count.should == skip_count |
| 120 | end | 120 | end |
| 121 | + | ||
| 122 | + it "should mark a skip as invalid if submitted with an already answered appearance" do | ||
| 123 | + @appearance = @aoi_clone.record_appearance(@visitor, @prompt) | ||
| 124 | + @optional_skip_params = {:appearance_lookup => @appearance.lookup} | ||
| 125 | + allparams = @required_skip_params.merge(@optional_skip_params) | ||
| 126 | + | ||
| 127 | + valid_skip = @visitor.skip!(allparams) | ||
| 128 | + @visitor.skips.count.should == 1 | ||
| 129 | + @appearance.reload.answerable.should == valid_skip | ||
| 130 | + | ||
| 131 | + # we need to reset because vote_for deletes keys from the params | ||
| 132 | + allparams = @required_skip_params.merge(@optional_skip_params) | ||
| 133 | + invalid_skip = @visitor.skip!(allparams) | ||
| 134 | + invalid_skip.should_not be_nil | ||
| 135 | + | ||
| 136 | + invalid_skip.valid_record.should be_false | ||
| 137 | + invalid_skip.validity_information.should == "Appearance #{@appearance.id} already answered" | ||
| 138 | + @appearance.reload.answerable.should == valid_skip | ||
| 139 | + @visitor.reload.skips.count.should == 1 | ||
| 140 | + end | ||
| 141 | + | ||
| 142 | + it "should mark a vote as invalid if submitted with an already answered appearance" do | ||
| 143 | + @appearance = @aoi_clone.record_appearance(@visitor, @prompt) | ||
| 144 | + @optional_vote_params = {:appearance_lookup => @appearance.lookup} | ||
| 145 | + allparams = @required_vote_params.merge(@optional_vote_params) | ||
| 146 | + | ||
| 147 | + valid_vote = @visitor.vote_for!(allparams) | ||
| 148 | + @visitor.votes.count.should == 1 | ||
| 149 | + @appearance.reload.answerable.should == valid_vote | ||
| 150 | + | ||
| 151 | + # we need to reset because vote_for deletes keys from the params | ||
| 152 | + allparams = @required_vote_params.merge(@optional_vote_params) | ||
| 153 | + invalid_vote = @visitor.vote_for!(allparams) | ||
| 154 | + invalid_vote.should_not be_nil | ||
| 155 | + | ||
| 156 | + invalid_vote.valid_record.should be_false | ||
| 157 | + invalid_vote.validity_information.should == "Appearance #{@appearance.id} already answered" | ||
| 158 | + @appearance.reload.answerable.should == valid_vote | ||
| 159 | + @visitor.reload.votes.count.should == 1 | ||
| 160 | + end | ||
| 121 | 161 | ||
| 122 | it "should accurately update score counts after vote" do | 162 | it "should accurately update score counts after vote" do |
| 123 | 163 |
spec/models/vote_spec.rb
| @@ -79,4 +79,32 @@ describe Vote do | @@ -79,4 +79,32 @@ describe Vote do | ||
| 79 | @prompt.right_choice.reload | 79 | @prompt.right_choice.reload |
| 80 | @prompt.right_choice.score.should be_close 33, 1 | 80 | @prompt.right_choice.score.should be_close 33, 1 |
| 81 | end | 81 | end |
| 82 | + | ||
| 83 | + it "should not display invalid votes by default" do | ||
| 84 | + 5.times do | ||
| 85 | + Factory.create(:vote) | ||
| 86 | + end | ||
| 87 | + Vote.count.should == 5 | ||
| 88 | + | ||
| 89 | + v = Vote.last | ||
| 90 | + v.valid_record.should be_true | ||
| 91 | + | ||
| 92 | + v.valid_record = false | ||
| 93 | + v.save | ||
| 94 | + Vote.count.should == 4 | ||
| 95 | + end | ||
| 96 | + | ||
| 97 | + it "should allow default valid_record behavior to be overriden by default" do | ||
| 98 | + required_params = {:question => @question, :prompt => @prompt, | ||
| 99 | + :choice => @prompt.left_choice, | ||
| 100 | + :voter=> @question.site.default_visitor, | ||
| 101 | + :loser_choice => @prompt.right_choice} | ||
| 102 | + | ||
| 103 | + vote = Vote.create!(required_params) | ||
| 104 | + vote.valid_record.should be_true | ||
| 105 | + | ||
| 106 | + | ||
| 107 | + vote = Vote.create!(required_params.merge!(:valid_record => false)) | ||
| 108 | + vote.valid_record.should be_false | ||
| 109 | + end | ||
| 82 | end | 110 | end |