Fix #3978: Pool name/category validations not being enforced.

app/models/pool.rb

@@ -3,8 +3,8 @@ require 'ostruct'
 class Pool < ApplicationRecord
   class RevertError < Exception ; end
 
-  validates_uniqueness_of :name, :case_sensitive => false, :if => :saved_change_to_name?
-  validate :validate_name, :if => :saved_change_to_name?
+  validates_uniqueness_of :name, case_sensitive: false, if: :name_changed?
+  validate :validate_name, if: :name_changed?
   validates_inclusion_of :category, :in => %w(series collection)
   validate :updater_can_change_category
   validate :updater_can_remove_posts
@@ -369,12 +369,12 @@ class Pool < ApplicationRecord
   end
 
   def category_changeable_by?(user)
-    user.is_builder? || (user.is_member? && post_count <= 100)
+    user.is_builder? || (user.is_member? && post_count <= Danbooru.config.pool_category_change_limit)
   end
 
   def updater_can_change_category
-    if saved_change_to_category? && !category_changeable_by?(CurrentUser.user)
-      errors[:base] << "You cannot change the category of pools with greater than 100 posts"
+    if category_changed? && !category_changeable_by?(CurrentUser.user)
+      errors[:base] << "You cannot change the category of pools with greater than #{Danbooru.config.pool_category_change_limit} posts"
       false
     else
       true

config/danbooru_default_config.rb

@@ -144,6 +144,11 @@ module Danbooru
       2
     end
 
+    # Members cannot change the category of pools with more than this many posts.
+    def pool_category_change_limit
+      100
+    end
+
     # Whether safe mode should be enabled. Safe mode hides all non-rating:safe posts from view.
     def enable_safe_mode?(request, user)
       !!(request.host =~ /safe/ || request.params[:safe_mode] || user.enable_safe_mode?)

test/unit/pool_test.rb

@@ -120,7 +120,7 @@ class PoolTest < ActiveSupport::TestCase
 
   context "Updating a pool" do
     setup do
-      @pool = FactoryBot.create(:pool)
+      @pool = FactoryBot.create(:pool, category: "series")
       @p1 = FactoryBot.create(:post)
       @p2 = FactoryBot.create(:post)
     end
@@ -241,6 +241,35 @@ class PoolTest < ActiveSupport::TestCase
       end
     end
 
+    context "by changing the category" do
+      setup do
+        Danbooru.config.stubs(:pool_category_change_limit).returns(1)
+        @pool.add!(@p1)
+        @pool.add!(@p2)
+      end
+
+      teardown do
+        Danbooru.config.unstub(:pool_category_change_limit)
+      end
+
+      should "not allow Members to change the category of large pools" do
+        @member = FactoryBot.create(:member_user)
+        as(@member) { @pool.update(category: "collection") }
+
+        assert_equal(["You cannot change the category of pools with greater than 1 posts"], @pool.errors[:base])
+      end
+
+      should "allow Builders to change the category of large pools" do
+        @builder = FactoryBot.create(:builder_user)
+        as(@builder) { @pool.update(category: "collection") }
+
+        assert_equal(true, @pool.valid?)
+        assert_equal("collection", @pool.category)
+        assert_equal("pool:#{@pool.id} pool:collection", @p1.reload.pool_string)
+        assert_equal("pool:#{@pool.id} pool:collection", @p2.reload.pool_string)
+      end
+    end
+
     should "create new versions for each distinct user" do
       assert_equal(1, @pool.versions.size)
       user2 = Timecop.travel(1.month.ago) {FactoryBot.create(:user)}
@@ -294,11 +323,8 @@ class PoolTest < ActiveSupport::TestCase
     end
 
     context "when validating names" do
-      should "not be valid for bad names" do
-        ["foo,bar", "foo*bar", "123", "___", "   ", "any", "none", "series", "collection"].each do |bad_name|
-          pool = Pool.create(name: bad_name)
-          assert pool.invalid?
-        end
+      ["foo,bar", "foo*bar", "123", "___", "   ", "any", "none", "series", "collection"].each do |bad_name|
+        should_not allow_value(bad_name).for(:name)
       end
     end
   end