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

posts: stop using pool_string attribute.

Browse Source 
Stop using the pool_string attribute on posts:

* Stop updating it when adding or removing posts from pools.
* Stop returning pool_string in the /posts.json API.
* Stop including the `data-pools` attribute on thumbnails.

The pool_string attribute was used in the past to facilitate pool:X
searches. Posts had a hidden pool_string attribute that contained a list
of every pool the post belonged to. These pools were treated like fake
hidden tags on the post and a search for `pool:X` was treated like a tag
search.

The pool_string has no longer been used for this purpose for a long time
now, and was only maintained for API compatibility purposes. Getting rid
of it eliminates a bunch of legacy cruft relating to adding and removing
posts from pools.

If you need to see which pools a post belongs to, do this:

* https://danbooru.donmai.us/pools.json?search[post_ids_include_any]=318550

The `data-pools` attribute on thumbnails was used by some people to add
custom borders to pooled posts with custom CSS. This will no longer
work. This was already broken because it included things like collection
pools and deleted pools, which you probably didn't want. Use a
userscript to add this attribute back to thumbnails if you need it.
This commit is contained in:
evazion
2021-10-07 05:55:43 -05:00
parent 595e02ab45
commit 7d503f088e
7 changed files with 13 additions and 161 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
app/components/post_preview_component.rb
View File

@@ -69,7 +69,6 @@ class PostPreviewComponent < ApplicationComponent
"data-id" => post.id,
"data-has-sound" => post.has_tag?("sound"),
"data-tags" => post.tag_string,
"data-pools" => post.pool_string,
"data-approver-id" => post.approver_id,
"data-rating" => post.rating,
"data-large-width" => post.large_image_width,

5
app/controllers/pools_controller.rb
View File

@@ -45,11 +45,8 @@ class PoolsController < ApplicationController
end
def update
# need to do this in order for synchronize! to work correctly
@pool = authorize Pool.find(params[:id])
@pool.attributes = permitted_attributes(@pool)
@pool.synchronize
@pool.save
@pool.update(permitted_attributes(@pool))
unless @pool.errors.any?
flash[:notice] = "Pool updated"
end

28
app/models/pool.rb
View File

@@ -10,7 +10,6 @@ class Pool < ApplicationRecord
validate :updater_can_edit_deleted
before_validation :normalize_post_ids
before_validation :normalize_name
after_create :synchronize!
after_save :create_version
deletable
@@ -130,7 +129,7 @@ class Pool < ApplicationRecord
self.post_ids = version.post_ids
self.name = version.name
self.description = version.description
synchronize!
save!
end
def contains?(post_id)
@@ -161,7 +160,6 @@ class Pool < ApplicationRecord
with_lock do
update(post_ids: post_ids + [post.id])
post.add_pool!(self, true)
end
end
@@ -171,7 +169,6 @@ class Pool < ApplicationRecord
with_lock do
reload
update(post_ids: post_ids - [post.id])
post.remove_pool!(self)
end
end
@@ -181,29 +178,6 @@ class Pool < ApplicationRecord
Post.joins("JOIN (#{pool_posts.to_sql}) pool_posts ON pool_posts.post_id = posts.id").order("pool_posts.pool_index ASC")
end
def synchronize
post_ids_before = post_ids_before_last_save || post_ids_was
added = post_ids - post_ids_before
removed = post_ids_before - post_ids
added.each do |post_id|
post = Post.find(post_id)
post.add_pool!(self, true)
end
removed.each do |post_id|
post = Post.find(post_id)
post.remove_pool!(self)
end
normalize_post_ids
end
def synchronize!
synchronize
save if will_save_change_to_post_ids?
end
def post_count
post_ids.size
end

41
app/models/post.rb
View File

@@ -10,6 +10,8 @@ class Post < ApplicationRecord
NOTE_COPY_TAGS = %w[translated partially_translated check_translation translation_request reverse_translation
annotated partially_annotated check_annotation annotation_request]
self.ignored_columns = [:pool_string]
deletable
before_validation :merge_old_changes
@@ -561,23 +563,23 @@ class Post < ApplicationRecord
case tag
when /^-pool:(\d+)$/i
pool = Pool.find_by_id($1.to_i)
remove_pool!(pool) if pool
pool&.remove!(self)
when /^-pool:(.+)$/i
pool = Pool.find_by_name($1)
remove_pool!(pool) if pool
pool&.remove!(self)
when /^pool:(\d+)$/i
pool = Pool.find_by_id($1.to_i)
add_pool!(pool) if pool
pool&.add!(self)
when /^pool:(.+)$/i
pool = Pool.find_by_name($1)
add_pool!(pool) if pool
pool&.add!(self)
when /^newpool:(.+)$/i
pool = Pool.find_by_name($1)
add_pool!(pool) if pool
pool&.add!(self)
when /^fav:(.+)$/i
add_favorite(CurrentUser.user)
@@ -777,35 +779,6 @@ class Post < ApplicationRecord
pools.undeleted.present?
end
def belongs_to_pool?(pool)
pool_string =~ /(?:\A| )pool:#{pool.id}(?:\Z| )/
end
def belongs_to_pool_with_id?(pool_id)
pool_string =~ /(?:\A| )pool:#{pool_id}(?:\Z| )/
end
def add_pool!(pool, force = false)
return if belongs_to_pool?(pool)
return if pool.is_deleted? && !force
with_lock do
self.pool_string = "#{pool_string} pool:#{pool.id}".strip
update_column(:pool_string, pool_string) unless new_record?
pool.add!(self)
end
end
def remove_pool!(pool)
return unless belongs_to_pool?(pool)
with_lock do
self.pool_string = pool_string.gsub(/(?:\A| )pool:#{pool.id}(?:\Z| )/, " ").strip
update_column(:pool_string, pool_string) unless new_record?
pool.remove!(self)
end
end
def remove_from_all_pools
pools.find_each do |pool|
pool.remove!(self)

60
test/unit/pool_test.rb
View File

@@ -68,13 +68,6 @@ class PoolTest < ActiveSupport::TestCase
should "initialize the post count" do
assert_equal(@posts.size, @pool.post_count)
end
should "synchronize the posts with the pool" do
assert_equal(@posts.map(&:id), @pool.post_ids)
@posts.each(&:reload)
assert_equal(["pool:#{@pool.id}"] * @posts.size, @posts.map(&:pool_string))
end
end
context "Reverting a pool" do
@@ -94,19 +87,15 @@ class PoolTest < ActiveSupport::TestCase
assert_equal([], @pool.versions[0].post_ids)
assert_equal([@p1.id], @pool.versions[1].post_ids)
assert_equal([@p1.id, @p2.id], @pool.versions[2].post_ids)
assert_equal([@p1.id, @p2.id], @pool.post_ids)
assert_equal([@p1.id, @p2.id], @pool.reload.post_ids)
end
should "update its post_ids" do
@pool.revert_to!(@pool.versions[1])
assert_equal([@p1.id], @pool.reload.post_ids)
assert_equal("pool:#{@pool.id}", @p1.reload.pool_string)
#assert_equal("", @p2.reload.pool_string)
assert_equal([@p1.id], @pool.post_ids)
@pool.revert_to!(@pool.versions[0])
assert_equal([], @pool.reload.post_ids)
assert_equal("", @p1.reload.pool_string)
#assert_equal("", @p2.reload.pool_string)
end
end
@@ -125,7 +114,6 @@ class PoolTest < ActiveSupport::TestCase
context "by #attributes=" do
setup do
@pool.attributes = {post_ids: [@p1.id, @p2.id]}
@pool.synchronize
@pool.save
end
@@ -138,10 +126,6 @@ class PoolTest < ActiveSupport::TestCase
assert_equal([@p1.id], @pool.post_ids)
end
should "add the pool to the post" do
assert_equal("pool:#{@pool.id}", @p1.pool_string)
end
should "increment the post count" do
assert_equal(1, @pool.post_count)
end
@@ -155,10 +139,6 @@ class PoolTest < ActiveSupport::TestCase
assert_equal([@p1.id], @pool.post_ids)
end
should "not double add the pool to the post" do
assert_equal("pool:#{@pool.id}", @p1.pool_string)
end
should "not double increment the post count" do
assert_equal(1, @pool.post_count)
end
@@ -171,7 +151,6 @@ class PoolTest < ActiveSupport::TestCase
@pool.update_attribute(:is_deleted, true)
@pool.post_ids += [@p2.id]
@pool.synchronize!
@pool.save
@pool.reload
@p2.reload
@@ -181,10 +160,6 @@ class PoolTest < ActiveSupport::TestCase
assert_equal([@p1.id, @p2.id], @pool.post_ids)
end
should "add the pool to the post" do
assert_equal("pool:#{@pool.id}", @p2.pool_string)
end
should "increment the post count" do
assert_equal(2, @pool.post_count)
end
@@ -205,10 +180,6 @@ class PoolTest < ActiveSupport::TestCase
assert_equal([], @pool.post_ids)
end
should "remove the pool from the post" do
assert_equal("", @p1.pool_string)
end
should "update the post count" do
assert_equal(0, @pool.post_count)
end
@@ -223,10 +194,6 @@ class PoolTest < ActiveSupport::TestCase
assert_equal([@p1.id], @pool.post_ids)
end
should "not affect the post" do
assert_equal("pool:#{@pool.id}", @p1.pool_string)
end
should "not affect the post count" do
assert_equal(1, @pool.post_count)
end
@@ -289,29 +256,6 @@ class PoolTest < ActiveSupport::TestCase
@pool.add!(@p3)
end
context "that is synchronized" do
setup do
@pool.reload
@pool.post_ids = [@p2.id]
@pool.synchronize!
end
should "update the pool" do
@pool.reload
assert_equal(1, @pool.post_count)
assert_equal([@p2.id], @pool.post_ids)
end
should "update the posts" do
@p1.reload
@p2.reload
@p3.reload
assert_equal("", @p1.pool_string)
assert_equal("pool:#{@pool.id}", @p2.pool_string)
assert_equal("", @p3.pool_string)
end
end
should "find the neighbors for the first post" do
assert_nil(@pool.previous_post_id(@p1.id))
assert_equal(@p2.id, @pool.next_post_id(@p1.id))

37
test/unit/post_test.rb
View File

@@ -751,7 +751,6 @@ class PostTest < ActiveSupport::TestCase
@post.reload
@pool.reload
assert_equal([@post.id], @pool.post_ids)
assert_equal("pool:#{@pool.id}", @post.pool_string)
end
end
@@ -759,7 +758,7 @@ class PostTest < ActiveSupport::TestCase
setup do
@pool = FactoryBot.create(:pool)
@post = FactoryBot.create(:post, :tag_string => "aaa")
@post.add_pool!(@pool)
@pool.add!(@post)
@post.tag_string = "aaa -pool:#{@pool.id}"
@post.save
end
@@ -768,7 +767,6 @@ class PostTest < ActiveSupport::TestCase
@post.reload
@pool.reload
assert_equal([], @pool.post_ids)
assert_equal("", @post.pool_string)
end
end
@@ -782,7 +780,6 @@ class PostTest < ActiveSupport::TestCase
@post.reload
@pool.reload
assert_equal([@post.id], @pool.post_ids)
assert_equal("pool:#{@pool.id}", @post.pool_string)
end
end
@@ -797,7 +794,6 @@ class PostTest < ActiveSupport::TestCase
@post.reload
@pool.reload
assert_equal([@post.id], @pool.post_ids)
assert_equal("pool:#{@pool.id}", @post.pool_string)
end
end
@@ -808,7 +804,6 @@ class PostTest < ActiveSupport::TestCase
@post.reload
assert_not_nil(@pool)
assert_equal([@post.id], @pool.post_ids)
assert_equal("pool:#{@pool.id}", @post.pool_string)
end
end
@@ -1661,36 +1656,6 @@ class PostTest < ActiveSupport::TestCase
setup do
SqsService.any_instance.stubs(:send_message)
end
context "Removing a post from a pool" do
should "update the post's pool string" do
post = FactoryBot.create(:post)
pool = FactoryBot.create(:pool)
post.add_pool!(pool)
post.remove_pool!(pool)
post.reload
assert_equal("", post.pool_string)
post.remove_pool!(pool)
post.reload
assert_equal("", post.pool_string)
end
end
context "Adding a post to a pool" do
should "update the post's pool string" do
post = FactoryBot.create(:post)
pool = FactoryBot.create(:pool)
post.add_pool!(pool)
post.reload
assert_equal("pool:#{pool.id}", post.pool_string)
post.add_pool!(pool)
post.reload
assert_equal("pool:#{pool.id}", post.pool_string)
post.remove_pool!(pool)
post.reload
assert_equal("", post.pool_string)
end
end
end
context "Uploading:" do

2
test/unit/post_version_test.rb
View File

@@ -73,7 +73,7 @@ class PostVersionTest < ActiveSupport::TestCase
should "create a version" do
assert_equal("tagme", @post.tag_string)
assert_equal("pool:#{@pool.id}", @post.pool_string)
assert_equal([@pool.id], @post.pools.pluck(:id))
assert_equal(1, @post.versions.size)
assert_equal("tagme", @post.versions.last.tags)