Post#approve!: move validation to post_approval.rb

app/models/post.rb

@@ -46,6 +46,7 @@ class Post < ActiveRecord::Base
   has_many :notes, :dependent => :destroy
   has_many :comments, lambda {includes(:creator, :updater).order("comments.id")}, :dependent => :destroy
   has_many :children, lambda {order("posts.id")}, :class_name => "Post", :foreign_key => "parent_id"
+  has_many :approvals, :class_name => "PostApproval", :dependent => :destroy
   has_many :disapprovals, :class_name => "PostDisapproval", :dependent => :destroy
   has_many :favorites, :dependent => :destroy
 
@@ -290,8 +291,8 @@ class Post < ActiveRecord::Base
   end
 
   module ApprovalMethods
-    def is_approvable?
-      !is_status_locked? && (is_pending? || is_flagged? || is_deleted?) && !PostApproval.approved?(CurrentUser.id, id)
+    def is_approvable?(user = CurrentUser.user)
+      !is_status_locked? && (is_pending? || is_flagged? || is_deleted?) && !approved_by?(user)
     end
 
     def flag!(reason, options = {})
@@ -321,21 +322,6 @@ class Post < ActiveRecord::Base
     end
 
     def approve!
-      if is_status_locked?
-        errors.add(:is_status_locked, "; post cannot be approved")
-        raise ApprovalError.new("Post is locked and cannot be approved")
-      end
-
-      if uploader_id == CurrentUser.id
-        errors.add(:base, "You cannot approve a post you uploaded")
-        raise ApprovalError.new("You cannot approve a post you uploaded")
-      end
-
-      if approver_id == CurrentUser.id || PostApproval.approved?(CurrentUser.id, id)
-        errors.add(:approver, "have already approved this post")
-        raise ApprovalError.new("You have previously approved this post and cannot approve it again")
-      end
-
       flags.each {|x| x.resolve!}
       self.is_flagged = false
       self.is_pending = false
@@ -351,6 +337,10 @@ class Post < ActiveRecord::Base
       save!
     end
 
+    def approved_by?(user)
+      approver == user || approvals.where(user: user).exists?
+    end
+
     def disapproved_by?(user)
       PostDisapproval.where(:user_id => user.id, :post_id => id).exists?
     end
@@ -1396,7 +1386,7 @@ class Post < ActiveRecord::Base
       end
 
       if !CurrentUser.is_admin? 
-        if approver_id == CurrentUser.id || PostApproval.approved?(CurrentUser.id, id)
+        if approved_by?(CurrentUser.user)
           raise ApprovalError.new("You have previously approved this post and cannot undelete it")
         elsif uploader_id == CurrentUser.id
           raise ApprovalError.new("You cannot undelete a post you uploaded")

app/models/post_approval.rb

@@ -1,12 +1,24 @@
 class PostApproval < ActiveRecord::Base
   belongs_to :user
-  belongs_to :post
+  belongs_to :post, inverse_of: :approvals
+
+  validate :validate_approval
 
   def self.prune!
     where("created_at < ?", 1.month.ago).delete_all
   end
 
-  def self.approved?(user_id, post_id)
-    where(user_id: user_id, post_id: post_id).exists?
+  def validate_approval
+    if post.is_status_locked?
+      errors.add(:post, "is locked and cannot be approved")
+    end
+
+    if post.uploader == user
+      errors.add(:base, "You cannot approve a post you uploaded")
+    end
+
+    if post.approved_by?(user)
+      errors.add(:base, "You have previously approved this post and cannot approve it again")
+    end
   end
 end

app/models/user.rb

@@ -77,6 +77,7 @@ class User < ActiveRecord::Base
   #after_create :notify_sock_puppets
   has_many :feedback, :class_name => "UserFeedback", :dependent => :destroy
   has_many :posts, :foreign_key => "uploader_id"
+  has_many :post_approvals, :dependent => :destroy
   has_many :post_votes
   has_many :bans, lambda {order("bans.id desc")}
   has_one :recent_ban, lambda {order("bans.id desc")}, :class_name => "Ban"