jaculabilis/danbooru
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

pools: stop maintaining pool category pseudotags in pool strings (#4160)

Browse Source 
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.
This commit is contained in:
evazion
2019-09-08 23:28:02 -05:00
parent d0f060d8eb
commit 763ac1a7e0
7 changed files with 13 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
app/jobs/update_pool_pseudo_tags_job.rb
View File

@@ -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

26
app/models/pool.rb
View File

@@ -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

15
app/models/post.rb
View File

@@ -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

5
config/danbooru_default_config.rb
View File

@@ -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

43
test/unit/pool_test.rb
View File

@@ -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

2
test/unit/post_archive_test.rb
View File

@@ -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)

12
test/unit/post_test.rb
View File

@@ -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)