From dc4d2e54b23c56b296e280bd978b7657e33d8742 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 6 Sep 2019 22:03:40 -0500 Subject: [PATCH] pools: stop using the pool_string field (#4160). Stop using the pool_string field internally, but keep maintaining it until we can drop it later. * Stop using the pool_string for `pool:` metatag searches. * Stop using the pool_string in the `Post#pools` method. This is used to get the list of pools on post show pages. --- app/logical/post_query_builder.rb | 36 +++++++++++++++++++++--- app/models/pool.rb | 10 +++++-- app/models/post.rb | 6 +--- app/models/tag.rb | 30 +++----------------- test/functional/posts_controller_test.rb | 10 +++++++ test/unit/post_test.rb | 19 ++++++++++--- 6 files changed, 69 insertions(+), 42 deletions(-) diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 3afc174be..d3082ffa8 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -211,10 +211,38 @@ class PostQueryBuilder end end - if q[:pool] == "none" - relation = relation.where("posts.pool_string = ''") - elsif q[:pool] == "any" - relation = relation.where("posts.pool_string != ''") + q[:pool].to_a.each do |pool_name| + case pool_name + when "none" + relation = relation.where.not(id: Pool.select("unnest(post_ids)")) + when "any" + relation = relation.where(id: Pool.select("unnest(post_ids)")) + when "series" + relation = relation.where(id: Pool.series.select("unnest(post_ids)")) + when "collection" + relation = relation.where(id: Pool.collection.select("unnest(post_ids)")) + when /\*/ + relation = relation.where(id: Pool.name_matches(pool_name).select("unnest(post_ids)")) + else + relation = relation.where(id: Pool.named(pool_name).select("unnest(post_ids)")) + end + end + + q[:pool_neg].to_a.each do |pool_name| + case pool_name + when "none" + relation = relation.where(id: Pool.select("unnest(post_ids)")) + when "any" + relation = relation.where.not(id: Pool.select("unnest(post_ids)")) + when "series" + relation = relation.where.not(id: Pool.series.select("unnest(post_ids)")) + when "collection" + relation = relation.where.not(id: Pool.collection.select("unnest(post_ids)")) + when /\*/ + relation = relation.where.not(id: Pool.name_matches(pool_name).select("unnest(post_ids)")) + else + relation = relation.where.not(id: Pool.named(pool_name).select("unnest(post_ids)")) + end end if q[:saved_searches] diff --git a/app/models/pool.rb b/app/models/pool.rb index 41b8c63d2..a98688408 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -112,16 +112,20 @@ class Pool < ApplicationRecord normalize_name(name).mb_chars.downcase end - def self.find_by_name(name) + def self.named(name) if name =~ /^\d+$/ - where("pools.id = ?", name.to_i).first + where("pools.id = ?", name.to_i) elsif name - where_ilike(:name, normalize_name_for_search(name)).first + where_ilike(:name, normalize_name_for_search(name)) else nil end end + def self.find_by_name(name) + named(name).try(:first) + end + def versions if PoolArchive.enabled? PoolArchive.where("pool_id = ?", id).order("id asc") diff --git a/app/models/post.rb b/app/models/post.rb index bac56349b..3bac9f01c 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1020,11 +1020,7 @@ class Post < ApplicationRecord module PoolMethods def pools - @pools ||= begin - return Pool.none if pool_string.blank? - pool_ids = pool_string.scan(/\d+/) - Pool.where(id: pool_ids).series_first - end + Pool.where("pools.post_ids && array[?]", id).series_first end def has_active_pools? diff --git a/app/models/tag.rb b/app/models/tag.rb index fa5986c20..0d8f7e616 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -619,37 +619,15 @@ class Tag < ApplicationRecord q[:disapproval_neg] << g2 when "-pool" - if g2.downcase == "none" - q[:pool] = "any" - elsif g2.downcase == "any" - q[:pool] = "none" - elsif g2.downcase == "series" - q[:tags][:exclude] << "pool:series" - elsif g2.downcase == "collection" - q[:tags][:exclude] << "pool:collection" - else - q[:tags][:exclude] << "pool:#{Pool.name_to_id(g2)}" - end + q[:pool_neg] ||= [] + q[:pool_neg] << g2 when "pool" - if g2.downcase == "none" - q[:pool] = "none" - elsif g2.downcase == "any" - q[:pool] = "any" - elsif g2.downcase == "series" - q[:tags][:related] << "pool:series" - elsif g2.downcase == "collection" - q[:tags][:related] << "pool:collection" - elsif g2.include?("*") - pool_ids = Pool.search(name_matches: g2, order: "post_count").limit(Danbooru.config.tag_query_limit).pluck(:id) - q[:tags][:include] += pool_ids.map { |id| "pool:#{id}" } - else - q[:tags][:related] << "pool:#{Pool.name_to_id(g2)}" - end + q[:pool] ||= [] + q[:pool] << g2 when "ordpool" pool_id = Pool.name_to_id(g2) - q[:tags][:related] << "pool:#{pool_id}" q[:ordpool] = pool_id when "-favgroup" diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index cbed45d93..e7785fdf5 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -143,6 +143,16 @@ class PostsControllerTest < ActionDispatch::IntegrationTest assert_response :success end + context "with pools" do + should "render the pool list" do + as(@user) { @post.update(tag_string: "newpool:comic") } + get post_path(@post) + + assert_response :success + assert_select "#pool-nav .pool-name", /Pool: comic/ + end + end + context "with only deleted comments" do setup do as(@user) { create(:comment, post: @post, is_deleted: true) } diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index b118e7de2..7c29e57fd 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -1948,17 +1948,28 @@ class PostTest < ActiveSupport::TestCase should "return posts for the pool: metatag" do SqsService.any_instance.stubs(:send_message) - FactoryBot.create(:pool, name: "test_a", category: "series") - FactoryBot.create(:pool, name: "test_b", category: "collection") - post1 = FactoryBot.create(:post, tag_string: "pool:test_a") - post2 = FactoryBot.create(:post, tag_string: "pool:test_b") + pool1 = create(:pool, name: "test_a", category: "series") + pool2 = create(:pool, name: "test_b", category: "collection") + post1 = create(:post, tag_string: "pool:test_a") + post2 = create(:post, tag_string: "pool:test_b") + + assert_tag_match([post1], "pool:#{pool1.id}") + assert_tag_match([post2], "pool:#{pool2.id}") + + assert_tag_match([post1], "pool:TEST_A") + assert_tag_match([post2], "pool:Test_B") assert_tag_match([post1], "pool:test_a") assert_tag_match([post2], "-pool:test_a") + + assert_tag_match([], "pool:test_a pool:test_b") assert_tag_match([], "-pool:test_a -pool:test_b") + assert_tag_match([post2, post1], "pool:test*") assert_tag_match([post2, post1], "pool:any") + assert_tag_match([post2, post1], "-pool:none") + assert_tag_match([], "-pool:any") assert_tag_match([], "pool:none") assert_tag_match([post1], "pool:series")