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.
This commit is contained in:
evazion
2022-04-30 20:54:29 -05:00
parent fdc1130aea
commit f117049750
10 changed files with 17 additions and 73 deletions

View File

@@ -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}?" }

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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