From d2a0b089f4b65ee95ed55d5b73bd6110ff32aa15 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 19 Oct 2016 19:18:50 -0500 Subject: [PATCH 1/4] Add test for setting invalid ratings. --- test/unit/post_test.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 6173c2179..b754dfbc5 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -1060,6 +1060,16 @@ 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 + end + context "Favorites:" do context "Removing a post from a user's favorites" do setup do From c01e03b1930eb2f2241391a30cc6318e850dd50d Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 19 Oct 2016 19:16:42 -0500 Subject: [PATCH 2/4] Validate post ratings. Prevent ratings from being set to invalid values via the API: PUT /posts/1.json?post[rating]=Z --- app/models/post.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/post.rb b/app/models/post.rb index cc3734a79..8faaa91a7 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -45,6 +45,7 @@ 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 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] From 88248e7ec701652997650a6452218d4437374f19 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 19 Oct 2016 19:20:59 -0500 Subject: [PATCH 3/4] Add tests for reverting rating-locked posts. --- test/unit/post_test.rb | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index b754dfbc5..d78c34ebc 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -1068,6 +1068,14 @@ class PostTest < ActiveSupport::TestCase 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 @@ -1714,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") From 0006b76c4d9772e574f7af26aeec7b2692871696 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 19 Oct 2016 19:21:32 -0500 Subject: [PATCH 4/4] Always obey rating locks; make rerating locked posts an error. Currently rating locks are only obeyed when using the rating: metatag. They aren't obeyed when: * Changing the rating via the API. * Changing the rating via 'Rate Safe' in the mode menu (uses the API). * Reverting to previous versions. Also, the current behavior is to ignore the rating: metatag if the post is locked. This patch instead makes the update fail completely (note that this could affect trying to mass revert posts that may be rating locked). Note: the check for `!is_rating_locked_changed?` is so that PUT /posts/1.json?post[rating]=s&post[is_rating_locked]=true works (ie., locking and changing the rating at the same time is okay). --- app/models/post.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 8faaa91a7..c5c7099f0 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -47,6 +47,7 @@ class Post < ActiveRecord::Base 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] @@ -769,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 @@ -1762,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