Merge pull request #2727 from evazion/fix-post-ratings

Validate post ratings and fix ratings locks

app/models/post.rb

@@ -45,7 +45,9 @@ class Post < ActiveRecord::Base
   has_many :disapprovals, :class_name => "PostDisapproval", :dependent => :destroy
   has_many :favorites, :dependent => :destroy
   validates_uniqueness_of :md5
+  validates_inclusion_of :rating, in: %w(s q e), message: "rating must be s, q, or e"
   validate :post_is_not_its_own_parent
+  validate :updater_can_change_rating
   attr_accessible :source, :rating, :tag_string, :old_tag_string, :old_parent_id, :old_source, :old_rating, :parent_id, :has_embedded_notes, :as => [:member, :builder, :gold, :platinum, :janitor, :moderator, :admin, :default]
   attr_accessible :is_rating_locked, :is_note_locked, :as => [:builder, :janitor, :moderator, :admin]
   attr_accessible :is_status_locked, :as => [:admin]
@@ -768,9 +770,7 @@ class Post < ActiveRecord::Base
           self.source = $1
 
         when /^rating:([qse])/i
-          unless is_rating_locked?
-            self.rating = $1.downcase
-          end
+          self.rating = $1.downcase
         end
       end
     end
@@ -1761,6 +1761,15 @@ class Post < ActiveRecord::Base
     save
   end
 
+  def updater_can_change_rating
+    if rating_changed? && is_rating_locked?
+      # Don't forbid changes if the rating lock was just now set in the same update.
+      if !is_rating_locked_changed?
+        errors.add(:rating, "is locked and cannot be changed. Unlock the post first.")
+      end
+    end
+  end
+
   def update_column(name, value)
     ret = super(name, value)
     notify_pubsub

test/unit/post_test.rb

@@ -1060,6 +1060,24 @@ class PostTest < ActiveSupport::TestCase
     end
   end
 
+  context "Updating:" do
+    context "A rating unlocked post" do
+      setup { @post = FactoryGirl.create(:post) }
+      subject { @post }
+
+      should_not allow_value("S", "safe", "derp").for(:rating)
+      should allow_value("s", "q", "e").for(:rating)
+    end
+
+    context "A rating locked post" do
+      setup { @post = FactoryGirl.create(:post, :is_rating_locked => true) }
+      subject { @post }
+
+      should_not allow_value("S", "safe", "derp").for(:rating)
+      should_not allow_value("s", "q", "e").for(:rating)
+    end
+  end
+
   context "Favorites:" do
     context "Removing a post from a user's favorites" do
       setup do
@@ -1704,6 +1722,34 @@ class PostTest < ActiveSupport::TestCase
   end
 
   context "Reverting: " do
+    context "a post that is rating locked" do
+      setup do
+        @post = FactoryGirl.create(:post, :rating => "s")
+        Timecop.travel(2.hours.from_now) do
+          @post.update({ :rating => "q", :is_rating_locked => true }, :as => :builder)
+        end
+      end
+
+      should "not revert the rating" do
+        assert_raises ActiveRecord::RecordInvalid do
+          @post.revert_to!(@post.versions.first)
+        end
+
+        assert_equal(["Rating is locked and cannot be changed. Unlock the post first."], @post.errors.full_messages)
+        assert_equal(@post.versions.last.rating, @post.reload.rating)
+      end
+
+      should "revert the rating after unlocking" do
+        @post.update({ :rating => "e", :is_rating_locked => false }, :as => :builder)
+        assert_nothing_raised do
+          @post.revert_to!(@post.versions.first)
+        end
+
+        assert(@post.valid?)
+        assert_equal(@post.versions.first.rating, @post.rating)
+      end
+    end
+
     context "a post that has been updated" do
       setup do
         @post = FactoryGirl.create(:post, :rating => "q", :tag_string => "aaa")