From 763ac1a7e04303d176fd18db187ced133f6dfd5e Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 8 Sep 2019 23:28:02 -0500 Subject: [PATCH] pools: stop maintaining pool category pseudotags in pool strings (#4160) Stop maintaining pool category pseudo tags (pool:series, pool:collection) in pool strings. They're no longer used and the changes to the `Post#pools` method in dc4d2e54b caused issues with this. Also allow Members to change the category of large pools again. This was only restricted because maintaining these pseudotags forced us to update every post in the pool whenever a pool's category was changed. --- app/jobs/update_pool_pseudo_tags_job.rb | 8 ----- app/models/pool.rb | 26 --------------- app/models/post.rb | 15 --------- config/danbooru_default_config.rb | 5 --- test/unit/pool_test.rb | 43 ++++--------------------- test/unit/post_archive_test.rb | 2 +- test/unit/post_test.rb | 12 +++---- 7 files changed, 13 insertions(+), 98 deletions(-) delete mode 100644 app/jobs/update_pool_pseudo_tags_job.rb diff --git a/app/jobs/update_pool_pseudo_tags_job.rb b/app/jobs/update_pool_pseudo_tags_job.rb deleted file mode 100644 index 3c827e504..000000000 --- a/app/jobs/update_pool_pseudo_tags_job.rb +++ /dev/null @@ -1,8 +0,0 @@ -class UpdatePoolPseudoTagsJob < ApplicationJob - queue_as :default - queue_with_priority 20 - - def perform(pool) - pool.update_category_pseudo_tags_for_posts - end -end diff --git a/app/models/pool.rb b/app/models/pool.rb index 6699fad32..c9fc5fe98 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -8,12 +8,10 @@ class Pool < ApplicationRecord 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 validate :updater_can_edit_deleted before_validation :normalize_post_ids before_validation :normalize_name - after_save :update_category_pseudo_tags_for_posts_async after_save :create_version after_create :synchronize! @@ -294,30 +292,6 @@ class Pool < ApplicationRecord creator.name end - def update_category_pseudo_tags_for_posts_async - if saved_change_to_category? - UpdatePoolPseudoTagsJob.perform_later(self) - end - end - - def update_category_pseudo_tags_for_posts - Post.where(id: post_ids).find_each do |post| - post.reload - post.set_pool_category_pseudo_tags - Post.where(:id => post.id).update_all(:pool_string => post.pool_string) - end - end - - def category_changeable_by?(user) - user.is_builder? || (user.is_member? && post_count <= Danbooru.config.pool_category_change_limit) - end - - def updater_can_change_category - 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" - end - end - def validate_name case name when /\A(any|none|series|collection)\z/i diff --git a/app/models/post.rb b/app/models/post.rb index d07b15d60..ac37b0aa0 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -30,7 +30,6 @@ class Post < ApplicationRecord validate :updater_can_change_rating before_save :update_tag_post_counts before_save :set_tag_counts - after_save :set_pool_category_pseudo_tags before_create :autoban after_save :create_version after_save :update_parent_on_save @@ -1041,7 +1040,6 @@ class Post < ApplicationRecord with_lock do self.pool_string = "#{pool_string} pool:#{pool.id}".strip - set_pool_category_pseudo_tags update_column(:pool_string, pool_string) unless new_record? pool.add!(self) end @@ -1053,7 +1051,6 @@ class Post < ApplicationRecord with_lock do self.pool_string = pool_string.gsub(/(?:\A| )pool:#{pool.id}(?:\Z| )/, " ").strip - set_pool_category_pseudo_tags update_column(:pool_string, pool_string) unless new_record? pool.remove!(self) end @@ -1064,18 +1061,6 @@ class Post < ApplicationRecord pool.remove!(self) end end - - def set_pool_category_pseudo_tags - self.pool_string = (pool_string.split - ["pool:series", "pool:collection"]).join(" ") - - pool_categories = pools.undeleted.pluck(:category) - if pool_categories.include?("series") - self.pool_string = "#{pool_string} pool:series".strip - end - if pool_categories.include?("collection") - self.pool_string = "#{pool_string} pool:collection".strip - end - end end module VoteMethods diff --git a/config/danbooru_default_config.rb b/config/danbooru_default_config.rb index d3164f4f4..38c4506f6 100644 --- a/config/danbooru_default_config.rb +++ b/config/danbooru_default_config.rb @@ -103,11 +103,6 @@ module Danbooru 2 end - # Members cannot change the category of pools with more than this many posts. - def pool_category_change_limit - 100 - end - # Users cannot search for more than X regular tags at a time. def base_tag_query_limit 6 diff --git a/test/unit/pool_test.rb b/test/unit/pool_test.rb index 43e0099ba..75d24199d 100644 --- a/test/unit/pool_test.rb +++ b/test/unit/pool_test.rb @@ -106,7 +106,7 @@ class PoolTest < ActiveSupport::TestCase assert_equal(@posts.map(&:id), @pool.post_ids) @posts.each(&:reload) - assert_equal(["pool:#{@pool.id} pool:series"] * @posts.size, @posts.map(&:pool_string)) + assert_equal(["pool:#{@pool.id}"] * @posts.size, @posts.map(&:pool_string)) end end @@ -161,7 +161,7 @@ class PoolTest < ActiveSupport::TestCase should "update any new posts that were added" do @p1.reload - assert_equal("pool:#{@pool.id} pool:series", @p1.pool_string) + assert_equal("pool:#{@pool.id}", @p1.pool_string) end end @@ -194,7 +194,7 @@ class PoolTest < ActiveSupport::TestCase end should "add the pool to the post" do - assert_equal("pool:#{@pool.id} pool:series", @p1.pool_string) + assert_equal("pool:#{@pool.id}", @p1.pool_string) end should "increment the post count" do @@ -211,7 +211,7 @@ class PoolTest < ActiveSupport::TestCase end should "not double add the pool to the post" do - assert_equal("pool:#{@pool.id} pool:series", @p1.pool_string) + assert_equal("pool:#{@pool.id}", @p1.pool_string) end should "not double increment the post count" do @@ -279,7 +279,7 @@ class PoolTest < ActiveSupport::TestCase end should "not affect the post" do - assert_equal("pool:#{@pool.id} pool:series", @p1.pool_string) + assert_equal("pool:#{@pool.id}", @p1.pool_string) end should "not affect the post count" do @@ -288,37 +288,6 @@ 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 - perform_enqueued_jobs do - @builder = create(:builder_user) - as(@builder) { @pool.update(category: "collection") } - end - - 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 = travel_to(1.month.ago) {FactoryBot.create(:user)} @@ -406,7 +375,7 @@ class PoolTest < ActiveSupport::TestCase @p2.reload @p3.reload assert_equal("", @p1.pool_string) - assert_equal("pool:#{@pool.id} pool:series", @p2.pool_string) + assert_equal("pool:#{@pool.id}", @p2.pool_string) assert_equal("", @p3.pool_string) end end diff --git a/test/unit/post_archive_test.rb b/test/unit/post_archive_test.rb index 25dc5c2b8..883fc6304 100644 --- a/test/unit/post_archive_test.rb +++ b/test/unit/post_archive_test.rb @@ -79,7 +79,7 @@ class PostArchiveTest < ActiveSupport::TestCase should "create a version" do assert_equal("tagme", @post.tag_string) - assert_equal("pool:#{@pool.id} pool:series", @post.pool_string) + assert_equal("pool:#{@pool.id}", @post.pool_string) assert_equal(1, @post.versions.size) assert_equal("tagme", @post.versions.last.tags) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 7c29e57fd..94ce84970 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -863,7 +863,7 @@ class PostTest < ActiveSupport::TestCase @post.reload @pool.reload assert_equal([@post.id], @pool.post_ids) - assert_equal("pool:#{@pool.id} pool:series", @post.pool_string) + assert_equal("pool:#{@pool.id}", @post.pool_string) end end @@ -894,7 +894,7 @@ class PostTest < ActiveSupport::TestCase @post.reload @pool.reload assert_equal([@post.id], @pool.post_ids) - assert_equal("pool:#{@pool.id} pool:series", @post.pool_string) + assert_equal("pool:#{@pool.id}", @post.pool_string) end end @@ -909,7 +909,7 @@ class PostTest < ActiveSupport::TestCase @post.reload @pool.reload assert_equal([@post.id], @pool.post_ids) - assert_equal("pool:#{@pool.id} pool:series", @post.pool_string) + assert_equal("pool:#{@pool.id}", @post.pool_string) end end @@ -920,7 +920,7 @@ class PostTest < ActiveSupport::TestCase @post.reload assert_not_nil(@pool) assert_equal([@post.id], @pool.post_ids) - assert_equal("pool:#{@pool.id} pool:series", @post.pool_string) + assert_equal("pool:#{@pool.id}", @post.pool_string) end end @@ -1768,10 +1768,10 @@ class PostTest < ActiveSupport::TestCase pool = FactoryBot.create(:pool) post.add_pool!(pool) post.reload - assert_equal("pool:#{pool.id} pool:series", post.pool_string) + assert_equal("pool:#{pool.id}", post.pool_string) post.add_pool!(pool) post.reload - assert_equal("pool:#{pool.id} pool:series", post.pool_string) + assert_equal("pool:#{pool.id}", post.pool_string) post.remove_pool!(pool) post.reload assert_equal("", post.pool_string)