From 2eb89a8354161bc47230495a539e163b45539053 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 29 Sep 2021 05:28:21 -0500 Subject: [PATCH] Fix #4601: Hide deleted pools by default in pool search. * On /pools, hide deleted pools by default in HTML responses. Don't filter out deleted pools in API responses. * API change: on /forum_topics, only hide deleted forum topics by default for HTML responses, not for API responses. Explicitly do https://danbooru.donmai.us/forum_topics.json?search[is_deleted]=false to filter out deleted topics. * API change: on /tags, only hide empty tags by default for HTML responses, not for API responses. Explicitly do https://danbooru.donmai.us/tags.json?search[is_empty]=false to filter out empty tags. * API change: on /pools, default to 20 posts per page for API responses, not 40. * API change: add `search[is_empty]` param to /tags.json endpoint. `search[hide_empty]=true` is deprecated in favor of `search[is_empty]=false`. * On /pools, add option to show/hide deleted pools in search form. * Fix the /forum_topics page putting `search[order]=sticky&limit=40` in the URL when browsing past page 1. --- app/controllers/forum_topics_controller.rb | 11 +++++----- app/controllers/pools_controller.rb | 6 +++++- app/controllers/tags_controller.rb | 7 ++++++- app/models/application_record.rb | 13 ++++++++++-- app/models/forum_topic.rb | 4 ---- app/models/tag.rb | 6 ++++++ app/views/pools/_search.html.erb | 1 + test/functional/tags_controller_test.rb | 24 +++++++++++++--------- 8 files changed, 49 insertions(+), 23 deletions(-) diff --git a/app/controllers/forum_topics_controller.rb b/app/controllers/forum_topics_controller.rb index e9ac76aa9..cf4082597 100644 --- a/app/controllers/forum_topics_controller.rb +++ b/app/controllers/forum_topics_controller.rb @@ -15,11 +15,12 @@ class ForumTopicsController < ApplicationController end def index - params[:search] ||= {} - params[:search][:order] ||= "sticky" if request.format.html? - params[:limit] ||= 40 - - @forum_topics = authorize ForumTopic.visible(CurrentUser.user).paginated_search(params) + if request.format.html? + limit = params.fetch(:limit, 40) + @forum_topics = authorize ForumTopic.visible(CurrentUser.user).paginated_search(params, limit: limit, defaults: { order: "sticky", is_deleted: false }) + else + @forum_topics = authorize ForumTopic.visible(CurrentUser.user).paginated_search(params) + end if request.format.atom? @forum_topics = @forum_topics.includes(:creator, :original_post) diff --git a/app/controllers/pools_controller.rb b/app/controllers/pools_controller.rb index f29a3e243..c6ed65d98 100644 --- a/app/controllers/pools_controller.rb +++ b/app/controllers/pools_controller.rb @@ -12,7 +12,11 @@ class PoolsController < ApplicationController end def index - @pools = authorize Pool.paginated_search(params, count_pages: true) + if request.format.html? + @pools = authorize Pool.paginated_search(params, count_pages: true, defaults: { is_deleted: false }) + else + @pools = authorize Pool.paginated_search(params, count_pages: true) + end respond_with(@pools) end diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 8deac803c..819adbd0d 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -7,7 +7,12 @@ class TagsController < ApplicationController end def index - @tags = authorize Tag.paginated_search(params, hide_empty: true) + if request.format.html? + @tags = authorize Tag.paginated_search(params, defaults: { hide_empty: true }) + else + @tags = authorize Tag.paginated_search(params) + end + @tags = @tags.includes(:consequent_aliases) if request.format.html? respond_with(@tags) end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 91c67e40e..78675e6de 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -14,12 +14,21 @@ class ApplicationRecord < ActiveRecord::Base extending(PaginationExtension).paginate(*args, **options) end - def paginated_search(params, count_pages: params[:search].present?, **defaults) + # Perform a search using the model's `search` method, then paginate the results. + # + # params [Hash] The URL request params from the user + # page [Integer] The page number + # limit [Integer] The number of posts per page + # count_pages [Boolean] If true, show the exact number of pages of + # results. If false (the default), don't count the exact number of pages + # of results; assume there are too many pages to count. + # defaults [Hash] The default params for the search + def paginated_search(params, page: params[:page], limit: params[:limit], count_pages: params[:search].present?, defaults: {}) search_params = params.fetch(:search, {}).permit! search_params = defaults.merge(search_params).with_indifferent_access max_limit = (params[:format] == "sitemap") ? 10_000 : 1_000 - search(search_params).paginate(params[:page], limit: params[:limit], max_limit: max_limit, search_count: count_pages) + search(search_params).paginate(page, limit: limit, max_limit: max_limit, search_count: count_pages) end end end diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index d649018e8..2b63bf081 100644 --- a/app/models/forum_topic.rb +++ b/app/models/forum_topic.rb @@ -118,10 +118,6 @@ class ForumTopic < ApplicationRecord q = q.apply_default_order(params) end - unless params[:is_deleted].present? - q = q.active - end - q end end diff --git a/app/models/tag.rb b/app/models/tag.rb index 804fe88d9..68981774b 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -287,6 +287,12 @@ class Tag < ApplicationRecord q = q.name_or_alias_matches(params[:name_or_alias_matches]) end + if params[:is_empty].to_s.truthy? + q = q.empty + elsif params[:is_empty].to_s.falsy? + q = q.nonempty + end + if params[:hide_empty].to_s.truthy? q = q.nonempty end diff --git a/app/views/pools/_search.html.erb b/app/views/pools/_search.html.erb index 66adb5064..211f681ec 100644 --- a/app/views/pools/_search.html.erb +++ b/app/views/pools/_search.html.erb @@ -2,6 +2,7 @@ <%= f.input :name_matches, label: "Name", input_html: { value: params.dig(:search, :name_matches), "data-autocomplete": "pool" } %> <%= f.input :description_matches, label: "Description", input_html: { value: params.dig(:search, :description_matches) } %> <%= f.input :post_tags_match, label: "Post tags", input_html: { value: params.dig(:search, :post_tags_match), "data-autocomplete": "tag-query" } %> + <%= f.input :is_deleted, label: "Deleted?", as: :select, include_blank: true, selected: params[:search][:is_deleted] %> <%= f.input :category, collection: %w[series collection], include_blank: true, selected: params[:search][:category] %> <%= f.input :order, collection: [%w[Last\ updated updated_at], %w[Name name], %w[Recently\ created created_at], %w[Post\ count post_count]], include_blank: true, selected: params.dig(:search, :order) %> <%= f.submit "Search" %> diff --git a/test/functional/tags_controller_test.rb b/test/functional/tags_controller_test.rb index f00bfb7ab..7da163454 100644 --- a/test/functional/tags_controller_test.rb +++ b/test/functional/tags_controller_test.rb @@ -36,10 +36,12 @@ class TagsControllerTest < ActionDispatch::IntegrationTest context "searching" do setup do as(@user) do - @miku = create(:character_tag, name: "hatsune_miku") + @miku = create(:character_tag, name: "miku") + @hatsune_miku = create(:character_tag, name: "hatsune_miku") @wokada = create(:artist_tag, name: "wokada") @vocaloid = create(:copyright_tag, name: "vocaloid") @weapon = create(:tag, name: "weapon") + @axe = create(:tag, name: "axe") @empty = create(:tag, name: "empty", post_count: 0) create(:tag_alias, antecedent_name: "miku", consequent_name: "hatsune_miku") @@ -54,22 +56,24 @@ class TagsControllerTest < ActionDispatch::IntegrationTest assert_response :success end - should respond_to_search({}).with { [@weapon, @vocaloid, @wokada, @miku, @tag] } - should respond_to_search(name_matches: "hatsune_miku").with { @miku } - should respond_to_search(name_normalize: "HATSUNE_MIKU ").with { @miku } - should respond_to_search(name_or_alias_matches: "miku").with { @miku } - should respond_to_search(fuzzy_name_matches: "hatsune_mika", order: "similarity").with { @miku } + should respond_to_search({}).with { [@empty, @axe, @weapon, @vocaloid, @wokada, @hatsune_miku, @miku, @tag] } + should respond_to_search(name_matches: "hatsune_miku").with { @hatsune_miku } + should respond_to_search(name_normalize: "HATSUNE_MIKU ").with { @hatsune_miku } + should respond_to_search(name_or_alias_matches: "miku").with { [@hatsune_miku, @miku] } + should respond_to_search(fuzzy_name_matches: "hatsune_mika", order: "similarity").with { @hatsune_miku } should respond_to_search(name: "empty", hide_empty: "true").with { [] } should respond_to_search(name: "empty", hide_empty: "false").with { [@empty] } + should respond_to_search(name: "empty", is_empty: "true").with { [@empty] } + should respond_to_search(name: "empty", is_empty: "false").with { [] } context "using includes" do should respond_to_search(name: "wokada", has_artist: "true").with { @wokada } - should respond_to_search(name: "hatsune_miku", has_artist: "false").with { @miku } - should respond_to_search(name: "hatsune_miku", has_wiki_page: "true").with { @miku } + should respond_to_search(name: "hatsune_miku", has_artist: "false").with { @hatsune_miku } + should respond_to_search(name: "hatsune_miku", has_wiki_page: "true").with { @hatsune_miku } should respond_to_search(name: "vocaloid", has_wiki_page: "false").with { @vocaloid } - should respond_to_search(consequent_aliases: {antecedent_name: "miku"}).with { @miku } + should respond_to_search(consequent_aliases: {antecedent_name: "miku"}).with { @hatsune_miku } should respond_to_search(consequent_implications: {antecedent_name: "axe"}).with { @weapon } - should respond_to_search(wiki_page: {body_matches: "*vocaloid*"}).with { @miku } + should respond_to_search(wiki_page: {body_matches: "*vocaloid*"}).with { @hatsune_miku } should respond_to_search(artist: {is_banned: "false"}).with { @wokada } should respond_to_search(has_dtext_links: "true").with { @vocaloid } end