From f1170497505ec804e6d1963007159f81d2966202 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 30 Apr 2022 20:54:29 -0500 Subject: [PATCH] users: remove 'hide deleted posts' account setting. This setting automatically added the `-status:deleted` metatag to all searches. This meant deleted posts were filtered out at the database level, rather than at the html level. This way searches wouldn't have less-than-full pages. The cost was that searches were slower, mainly because post counts weren't cached. Normally when you search for a tag, we can get the post count from the tags table. If the search is actually like `touhou -status:deleted`, then we don't know the count and we have to calculate it on demand. This option is being removed because it did the opposite of what people thought it did. People thought it made deleted posts visible, when actually it made them more hidden. --- app/helpers/application_helper.rb | 2 +- app/logical/post_query.rb | 16 +++++------ app/logical/post_query_builder.rb | 7 ++--- app/logical/post_sets/post.rb | 4 +-- app/logical/user_deletion.rb | 1 - app/models/post.rb | 11 ++++---- app/policies/user_policy.rb | 2 +- app/views/users/edit.html.erb | 1 - test/functional/posts_controller_test.rb | 12 --------- test/unit/post_query_builder_test.rb | 34 ------------------------ 10 files changed, 17 insertions(+), 73 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index a0195440d..42068c043 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -303,7 +303,7 @@ module ApplicationHelper id name level level_string theme always_resize_images can_upload_free can_approve_posts disable_categorized_saved_searches disable_mobile_gestures disable_post_tooltips enable_safe_mode - hide_deleted_posts show_deleted_children style_usernames + show_deleted_children style_usernames default_image_size ] + User::Roles.map { |role| :"is_#{role}?" } diff --git a/app/logical/post_query.rb b/app/logical/post_query.rb index fe7972aec..ad821a336 100644 --- a/app/logical/post_query.rb +++ b/app/logical/post_query.rb @@ -16,11 +16,10 @@ class PostQuery SINGLETON_METATAGS = ORDER_METATAGS + %w[limit random] attr_reader :current_user - private attr_reader :tag_limit, :safe_mode, :hide_deleted_posts, :builder + private attr_reader :tag_limit, :safe_mode, :builder delegate :tag?, :metatag?, :wildcard?, :metatags, :wildcards, :tag_names, :to_infix, :to_pretty_string, to: :ast alias_method :safe_mode?, :safe_mode - alias_method :hide_deleted_posts?, :hide_deleted_posts alias_method :to_s, :to_infix # Return a new PostQuery with aliases replaced. @@ -42,7 +41,7 @@ class PostQuery PostQuery.normalize(search, ...).with_implicit_metatags.posts end - def initialize(search_or_ast, current_user: User.anonymous, tag_limit: nil, safe_mode: false, hide_deleted_posts: false) + def initialize(search_or_ast, current_user: User.anonymous, tag_limit: nil, safe_mode: false) if search_or_ast.is_a?(AST) @ast = search_or_ast else @@ -52,16 +51,15 @@ class PostQuery @current_user = current_user @tag_limit = tag_limit @safe_mode = safe_mode - @hide_deleted_posts = hide_deleted_posts end # Build a new PostQuery from the given AST and the current settings. def build(ast) - PostQuery.new(ast, current_user: current_user, tag_limit: tag_limit, safe_mode: safe_mode, hide_deleted_posts: hide_deleted_posts) + PostQuery.new(ast, current_user: current_user, tag_limit: tag_limit, safe_mode: safe_mode) end def builder - @builder ||= PostQueryBuilder.new(search, current_user, tag_limit: tag_limit, safe_mode: safe_mode, hide_deleted_posts: hide_deleted_posts) + @builder ||= PostQueryBuilder.new(search, current_user, tag_limit: tag_limit, safe_mode: safe_mode) end def search @@ -189,19 +187,17 @@ class PostQuery TagAlias.aliases_for(tag_names) end - # Implicit metatags are metatags added by the user's account settings. rating:s is implicit - # under safe mode. -status:deleted is implicit when the "hide deleted posts" setting is on. + # Implicit metatags are metatags added by the user's account settings. rating:s is implicit under safe mode. def implicit_metatags metatags = [] metatags << AST.metatag("rating", "s") if safe_mode? - metatags << -AST.metatag("status", "deleted") if hide_deleted? metatags end # XXX unify with PostSets::Post#show_deleted? def hide_deleted? has_status_metatag = select_metatags(:status).any? { |metatag| metatag.value.downcase.in?(%w[deleted active any all unmoderated modqueue appealed]) } - hide_deleted_posts? && !has_status_metatag + !has_status_metatag end concerning :CountMethods do diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index ac739a262..2ff19fe9b 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -70,22 +70,19 @@ class PostQueryBuilder COUNT_METATAG_SYNONYMS.flat_map { |str| [str, "#{str}_asc"] } + CATEGORY_COUNT_METATAGS.flat_map { |str| [str, "#{str}_asc"] } - attr_reader :query_string, :current_user, :tag_limit, :safe_mode, :hide_deleted_posts + attr_reader :query_string, :current_user, :tag_limit, :safe_mode alias_method :safe_mode?, :safe_mode - alias_method :hide_deleted_posts?, :hide_deleted_posts # Initialize a post query. # @param query_string [String] the tag search # @param current_user [User] the user performing the search # @param tag_limit [Integer] the user's tag limit # @param safe_mode [Boolean] whether safe mode is enabled. if true, return only rating:s posts. - # @param hide_deleted_posts [Boolean] if true, filter out status:deleted posts. - def initialize(query_string, current_user = User.anonymous, tag_limit: nil, safe_mode: false, hide_deleted_posts: false) + def initialize(query_string, current_user = User.anonymous, tag_limit: nil, safe_mode: false) @query_string = query_string @current_user = current_user @tag_limit = tag_limit @safe_mode = safe_mode - @hide_deleted_posts = hide_deleted_posts end def metatag_matches(name, value, relation = Post.all, quoted: false) diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index 8fcdc2d0d..0a6efd87c 100644 --- a/app/logical/post_sets/post.rb +++ b/app/logical/post_sets/post.rb @@ -17,8 +17,8 @@ module PostSets alias_method :show_votes?, :show_votes def initialize(tags, page = 1, per_page = nil, user: CurrentUser.user, format: "html", show_votes: false) - @query = PostQueryBuilder.new(tags, user, tag_limit: user.tag_query_limit, safe_mode: CurrentUser.safe_mode?, hide_deleted_posts: user.hide_deleted_posts?) - @post_query = PostQuery.normalize(tags, current_user: user, tag_limit: user.tag_query_limit, safe_mode: CurrentUser.safe_mode?, hide_deleted_posts: user.hide_deleted_posts?) + @query = PostQueryBuilder.new(tags, user, tag_limit: user.tag_query_limit, safe_mode: CurrentUser.safe_mode?) + @post_query = PostQuery.normalize(tags, current_user: user, tag_limit: user.tag_query_limit, safe_mode: CurrentUser.safe_mode?) @normalized_query = post_query.with_implicit_metatags @tag_string = tags @page = page diff --git a/app/logical/user_deletion.rb b/app/logical/user_deletion.rb index 244fd3a06..3eab7cfb6 100644 --- a/app/logical/user_deletion.rb +++ b/app/logical/user_deletion.rb @@ -57,7 +57,6 @@ class UserDeletion user.last_forum_read_at = nil user.favorite_tags = "" user.blacklisted_tags = "" - user.hide_deleted_posts = false user.show_deleted_children = false user.time_zone = "Eastern Time (US & Canada)" user.save! diff --git a/app/models/post.rb b/app/models/post.rb index 0b87f62c3..7cf9ce042 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -920,7 +920,7 @@ class Post < ApplicationRecord end def sample(query, sample_size) - user_tag_match(query, safe_mode: false, hide_deleted_posts: false).reorder(:md5).limit(sample_size) + user_tag_match(query, safe_mode: false).reorder(:md5).limit(sample_size) end # unflattens the tag_string into one tag per row. @@ -1282,13 +1282,13 @@ class Post < ApplicationRecord # Perform a tag search as an anonymous user. No tag limit is enforced. def anon_tag_match(query) - user_tag_match(query, User.anonymous, tag_limit: nil, safe_mode: false, hide_deleted_posts: false) + user_tag_match(query, User.anonymous, tag_limit: nil, safe_mode: false) end # Perform a tag search as the system user, DanbooruBot. The search will # have moderator-level permissions. No tag limit is enforced. def system_tag_match(query) - user_tag_match(query, User.system, tag_limit: nil, safe_mode: false, hide_deleted_posts: false) + user_tag_match(query, User.system, tag_limit: nil, safe_mode: false) end # Perform a tag search as the current user, or as another user. @@ -1298,10 +1298,9 @@ class Post < ApplicationRecord # @param tag_limit [Integer] the maximum number of tags allowed per search. # An exception will be raised if the search has too many tags. # @param safe_mode [Boolean] if true, automatically add rating:s to the search - # @param hide_deleted_posts [Boolean] if true, automatically add -status:deleted to the search # @return [ActiveRecord::Relation] the set of resulting posts - def user_tag_match(query, user = CurrentUser.user, tag_limit: user.tag_query_limit, safe_mode: CurrentUser.safe_mode?, hide_deleted_posts: user.hide_deleted_posts?) - post_query = PostQuery.normalize(query, current_user: user, tag_limit: tag_limit, safe_mode: safe_mode, hide_deleted_posts: hide_deleted_posts) + def user_tag_match(query, user = CurrentUser.user, tag_limit: user.tag_query_limit, safe_mode: CurrentUser.safe_mode?) + post_query = PostQuery.normalize(query, current_user: user, tag_limit: tag_limit, safe_mode: safe_mode) post_query.with_implicit_metatags.posts end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 60b4b7ed8..55a13c540 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -47,7 +47,7 @@ class UserPolicy < ApplicationPolicy blacklisted_tags time_zone per_page custom_style theme receive_email_notifications always_resize_images new_post_navigation_layout enable_private_favorites - hide_deleted_posts style_usernames show_deleted_children + style_usernames show_deleted_children disable_categorized_saved_searches disable_tagged_filenames disable_mobile_gestures enable_safe_mode enable_desktop_mode disable_post_tooltips diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index bddd2b007..00b3aeaaf 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -60,7 +60,6 @@ <%= f.input :style_usernames, :as => :select, :label => "Colored usernames", :hint => "Color users according to their user level", :include_blank => false, :collection => [["Yes", "true"], ["No", "false"]] %> <%= f.input :new_post_navigation_layout, :as => :select, :label => "Navigation bar position", :include_blank => false, :collection => [["Below", "true"], ["Above", "false"]], :hint => "When browsing pools or posts, place navigation links above or below the image" %> - <%= f.input :hide_deleted_posts, :as => :select, :label => "Deleted post filter", :hint => "Remove deleted posts from search results", :include_blank => false, :collection => [["Yes", "true"], ["No", "false"]] %> <%= f.input :show_deleted_children, :as => :select, :label => "Show deleted children", :hint => "Show thumbnail borders on parent posts even if the children are deleted", :include_blank => false, :collection => [["Yes", "true"], ["No", "false"]] %> <%= f.input :disable_categorized_saved_searches, :hint => "Don't show dialog box when creating a new saved search", :as => :select, :collection => [["No", "false"], ["Yes", "true"]], :include_blank => false %> <% if policy(@user).can_enable_private_favorites? %> diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index b1e393409..0efe421cd 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -413,18 +413,6 @@ class PostsControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_select "#post_#{@post.id}", 1 end - - context "with the hide_deleted_posts option enabled" do - should "show deleted posts when searching for status:appealed" do - @user.update!(hide_deleted_posts: true) - create(:post_appeal, post: @post) - - get_auth posts_path(tags: "status:appealed"), @user - - assert_response :success - assert_select "#post_#{@post.id}", 1 - end - end end context "with restricted posts" do diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb index e04d4eb51..44daea01f 100644 --- a/test/unit/post_query_builder_test.rb +++ b/test/unit/post_query_builder_test.rb @@ -789,34 +789,6 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_tag_match([], "-status:all") end - should "respect the 'Deleted post filter' option when using the status: metatag" do - deleted = create(:post, is_deleted: true, is_banned: true) - undeleted = create(:post, is_banned: true) - - CurrentUser.hide_deleted_posts = true - assert_tag_match([undeleted], "status:banned") - assert_tag_match([undeleted], "status:active") - assert_tag_match([deleted], "status:deleted") - assert_tag_match([undeleted, deleted], "status:any") - assert_tag_match([undeleted, deleted], "status:all") - assert_tag_match([deleted], "status:banned status:deleted") - - assert_tag_match([], "-status:banned") - assert_tag_match([deleted], "-status:active") - assert_tag_match([undeleted], "-status:deleted") - #assert_tag_match([deleted], "-status:any") # XXX Broken - #assert_tag_match([deleted], "-status:all") - - CurrentUser.hide_deleted_posts = false - assert_tag_match([undeleted, deleted], "status:banned") - assert_tag_match([undeleted], "status:active") - assert_tag_match([deleted], "status:deleted") - assert_tag_match([undeleted, deleted], "status:any") - assert_tag_match([undeleted, deleted], "status:all") - - assert_fast_count(2, "status:banned") - end - should "return posts for the filetype: metatag" do png = create(:post, file_ext: "png") jpg = create(:post, file_ext: "jpg") @@ -1445,12 +1417,6 @@ class PostQueryBuilderTest < ActiveSupport::TestCase should "return the true count, if not cached" do assert_fast_count(1, "aaa score:42") end - - should "work with the hide_deleted_posts option turned on" do - create(:post, tag_string: "aaa", score: 42, is_deleted: true) - assert_fast_count(1, "aaa score:42", { hide_deleted_posts: true }) - assert_fast_count(2, "aaa score:42", { hide_deleted_posts: false }) - end end context "a blank search" do