diff --git a/app/models/visitor.rb b/app/models/visitor.rb index 2ff8d64..29f5949 100644 --- a/app/models/visitor.rb +++ b/app/models/visitor.rb @@ -21,18 +21,6 @@ class Visitor < ActiveRecord::Base prompt = options.delete(:prompt) ordinality = (options.delete(:direction) == "left") ? 0 : 1 - - if options[:appearance_lookup] - @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup)) - return nil unless @appearance # don't allow people to fake appearance lookups - - if @appearance.answered? - options.merge!(:valid_record => false) - options.merge!(:validity_information => "Appearance #{@appearance.id} already answered") - else - options.merge!(:appearance => @appearance) - end - end if options.delete(:skip_fraud_protection) last_answered_appearance = self.appearances.find(:first, @@ -43,6 +31,13 @@ class Visitor < ActiveRecord::Base options.merge!(:validity_information => "Fraud protection: last visitor action was a skip") end end + + associate_appearance = false + if options[:appearance_lookup] + @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup)) + return nil unless @appearance # don't allow people to fake appearance lookups + associate_appearance = true + end choice = prompt.choices[ordinality] #we need to guarantee that the choices are in the right order (by position) other_choices = prompt.choices - [choice] @@ -51,6 +46,8 @@ class Visitor < ActiveRecord::Base options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :voter_id=> self.id, :choice_id => choice.id, :loser_choice_id => loser_choice.id) v = votes.create!(options) + safely_associate_appearance(v, @appearance) if associate_appearance + v end def skip!(options) @@ -58,18 +55,35 @@ class Visitor < ActiveRecord::Base prompt = options.delete(:prompt) + associate_appearance = false if options[:appearance_lookup] @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup)) return nil unless @appearance - if @appearance.answered? - options.merge!(:valid_record => false) - options.merge!(:validity_information => "Appearance #{@appearance.id} already answered") - else - options.merge!(:appearance => @appearance) - end + associate_appearance = true end options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :skipper_id => self.id) prompt_skip = skips.create!(options) + if associate_appearance + safely_associate_appearance(prompt_skip, @appearance) + end + prompt_skip end + + # Safely associates appearance with object, but making sure no other object + # is already associated wit this appearance. object is either vote or skip. + def safely_associate_appearance(object, appearance) + # Manually update Appearance with id to ensure no double votes for a + # single appearance. Only update the answerable_id if it is NULL. + # If we can't find any rows to update, then this object should be invalid. + rows_updated = Appearance.update_all("answerable_id = #{object.id}, answerable_type = '#{object.class.to_s}'", "id = #{appearance.id} AND answerable_id IS NULL") + if rows_updated === 1 + # update relationship the ActiveRecord way, now + # that we know it is safe to do so + object.update_attributes!(:appearance => appearance) + else + object.update_attributes!(:valid_record => false, :validity_information => "Appearance #{appearance.id} already answered") + end + end + end diff --git a/db/migrate/20111017171903_add_index_on_answerable_to_appearances.rb b/db/migrate/20111017171903_add_index_on_answerable_to_appearances.rb new file mode 100644 index 0000000..07c7e16 --- /dev/null +++ b/db/migrate/20111017171903_add_index_on_answerable_to_appearances.rb @@ -0,0 +1,9 @@ +class AddIndexOnAnswerableToAppearances < ActiveRecord::Migration + def self.up + add_index :appearances, [:answerable_id, :answerable_type] + end + + def self.down + remove_index :appearances, [:answerable_id, :answerable_type] + end +end diff --git a/db/schema.rb b/db/schema.rb index 683d931..b603a7f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -9,7 +9,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20110628160608) do +ActiveRecord::Schema.define(:version => 20111017171903) do create_table "appearances", :force => true do |t| t.integer "voter_id" @@ -25,6 +25,7 @@ ActiveRecord::Schema.define(:version => 20110628160608) do t.string "validity_information" end + add_index "appearances", ["answerable_id", "answerable_type"], :name => "index_appearances_on_answerable_id_and_answerable_type" add_index "appearances", ["lookup"], :name => "index_appearances_on_lookup" add_index "appearances", ["prompt_id"], :name => "index_appearances_on_prompt_id" add_index "appearances", ["question_id", "voter_id"], :name => "index_appearances_on_question_id_voter_id" -- libgit2 0.21.2