From 73c400bbb76b8d7e7f02783fc4aa6b3165575008 Mon Sep 17 00:00:00 2001 From: Dhruv Kapadia Date: Thu, 15 Jul 2010 12:22:53 -0400 Subject: [PATCH] Refactoring skip and vote to use polymorphism in appearance --- app/models/appearance.rb | 7 ++----- app/models/skip.rb | 2 +- app/models/user.rb | 3 +-- app/models/visitor.rb | 4 ++-- app/models/vote.rb | 2 +- db/migrate/20100714160406_add_polymorphic_answerable_to_appearance.rb | 11 +++++++++++ db/schema.rb | 4 +++- lib/tasks/prune_db.rake | 15 +++++++++++++++ spec/models/appearance_spec.rb | 3 +-- spec/models/skip_spec.rb | 1 + spec/models/vote_spec.rb | 2 +- 11 files changed, 39 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20100714160406_add_polymorphic_answerable_to_appearance.rb diff --git a/app/models/appearance.rb b/app/models/appearance.rb index 2fa38c4..aea34c6 100644 --- a/app/models/appearance.rb +++ b/app/models/appearance.rb @@ -3,12 +3,9 @@ class Appearance < ActiveRecord::Base belongs_to :prompt belongs_to :question - #technically, an appearance should either one vote or one skip, not one of both objects, but these declarations provide some useful helper methods - # we could refactor this to use rails polymorphism, but currently the foreign key is stored in the vote and skip object - has_one :vote - has_one :skip + belongs_to :answerable, :polymorphic => true def answered? - vote || skip + !self.answerable_id.nil? end end diff --git a/app/models/skip.rb b/app/models/skip.rb index f5f4202..596055b 100644 --- a/app/models/skip.rb +++ b/app/models/skip.rb @@ -2,5 +2,5 @@ class Skip < ActiveRecord::Base belongs_to :skipper, :class_name => "Visitor", :foreign_key => "skipper_id" belongs_to :question belongs_to :prompt - belongs_to :appearance + has_one :appearance, :as => :answerable end diff --git a/app/models/user.rb b/app/models/user.rb index 1791cbb..7b211a8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -41,8 +41,7 @@ class User < ActiveRecord::Base end def record_appearance(visitor, prompt) - a = Appearance.create(:voter => visitor, :prompt => prompt, :question_id => prompt.question_id, - :lookup => Digest::MD5.hexdigest(rand(10000000000).to_s + visitor.id.to_s + prompt.id.to_s) ) + a = Appearance.create(:voter => visitor, :prompt => prompt, :question_id => prompt.question_id, :site_id => self.id, :lookup => Digest::MD5.hexdigest(rand(10000000000).to_s + visitor.id.to_s + prompt.id.to_s) ) end diff --git a/app/models/visitor.rb b/app/models/visitor.rb index 2257437..9ce9424 100644 --- a/app/models/visitor.rb +++ b/app/models/visitor.rb @@ -25,7 +25,7 @@ class Visitor < ActiveRecord::Base 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 - options.merge!(:appearance_id => @appearance.id) + options.merge!(:appearance => @appearance) end choice = prompt.choices[ordinality] #we need to guarantee that the choices are in the right order (by position) @@ -45,7 +45,7 @@ class Visitor < ActiveRecord::Base if options[:appearance_lookup] @appearance = prompt.appearances.find_by_lookup(options.delete(:appearance_lookup)) return nil unless @appearance - options.merge!(:appearance_id => @appearance.id) + options.merge!(:appearance => @appearance) end options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :skipper_id => self.id) diff --git a/app/models/vote.rb b/app/models/vote.rb index 81eb208..57054fc 100644 --- a/app/models/vote.rb +++ b/app/models/vote.rb @@ -11,7 +11,7 @@ class Vote < ActiveRecord::Base belongs_to :prompt, :counter_cache => true belongs_to :choice, :counter_cache => true belongs_to :loser_choice, :class_name => "Choice", :foreign_key => "loser_choice_id" - belongs_to :appearance + has_one :appearance, :as => :answerable named_scope :recent, lambda { |*args| {:conditions => ["created_at > ?", (args.first || Date.today.beginning_of_day)]} } named_scope :with_question, lambda { |*args| {:conditions => {:question_id => args.first }} } diff --git a/db/migrate/20100714160406_add_polymorphic_answerable_to_appearance.rb b/db/migrate/20100714160406_add_polymorphic_answerable_to_appearance.rb new file mode 100644 index 0000000..de61de1 --- /dev/null +++ b/db/migrate/20100714160406_add_polymorphic_answerable_to_appearance.rb @@ -0,0 +1,11 @@ +class AddPolymorphicAnswerableToAppearance < ActiveRecord::Migration + def self.up + add_column :appearances, :answerable_id, :integer + add_column :appearances, :answerable_type, :string + end + + def self.down + remove_column :appearances, :answerable_id + remove_column :appearances, :answerable_type + end +end diff --git a/db/schema.rb b/db/schema.rb index a17d60d..8f6d532 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 => 20100621164536) do +ActiveRecord::Schema.define(:version => 20100714160406) do create_table "appearances", :force => true do |t| t.integer "voter_id" @@ -19,6 +19,8 @@ ActiveRecord::Schema.define(:version => 20100621164536) do t.string "lookup" t.datetime "created_at" t.datetime "updated_at" + t.integer "answerable_id" + t.string "answerable_type" end add_index "appearances", ["lookup"], :name => "index_appearances_on_lookup" diff --git a/lib/tasks/prune_db.rake b/lib/tasks/prune_db.rake index 2933ddc..01f8e18 100644 --- a/lib/tasks/prune_db.rake +++ b/lib/tasks/prune_db.rake @@ -62,5 +62,20 @@ namespace :prune_db do end end end + + task(:move_vote_and_skip_ids_to_appearance => :environment) do + Vote.find_each do |v| + @appearance = Appearance.find(v.appearance_id) + @appearance.answerable = v + @appearance.save + + end + Skip.find_each do |s| + @appearance = Appearance.find(s.appearance_id) + @appearance.answerable = s + @appearance.save + end + end + end diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 43f7cca..a248e51 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -7,8 +7,7 @@ describe Appearance do it {should belong_to :voter} it {should belong_to :prompt} it {should belong_to :question} - it {should have_one :vote} - it {should have_one :skip} + it {should belong_to :answerable} it "should create a new instance given valid attributes" do Appearance.create!(@valid_attributes) diff --git a/spec/models/skip_spec.rb b/spec/models/skip_spec.rb index 024885a..1fe6da0 100644 --- a/spec/models/skip_spec.rb +++ b/spec/models/skip_spec.rb @@ -3,6 +3,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe Skip do it {should belong_to :prompt} it {should belong_to :skipper} + it {should have_one :appearance} before(:each) do @valid_attributes = { diff --git a/spec/models/vote_spec.rb b/spec/models/vote_spec.rb index da88bfc..5dadec5 100644 --- a/spec/models/vote_spec.rb +++ b/spec/models/vote_spec.rb @@ -6,7 +6,7 @@ describe Vote do it {should belong_to :prompt} it {should belong_to :choice} it {should belong_to :loser_choice} - it {should belong_to :appearance} + it {should have_one :appearance} before(:each) do @question = Factory.create(:aoi_question) -- libgit2 0.21.2