DEV: Centralize logic for applying order to filtered posts. (#14634)

Instead of leaking ordering of the posts all around the class, we
centralize it in a method making the code easier to understand. In a
future PR, we will also introduce a plugin API to allow custom ordering
and the change in this commit helps to faciliate that.
This commit is contained in:
Alan Guo Xiang Tan 2021-10-19 10:37:46 +08:00 committed by GitHub
parent 1d131fcaff
commit 903a9e1c0d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 17 additions and 18 deletions

View File

@ -108,6 +108,7 @@ class TopicView
@page = @page.to_i > 1 ? @page.to_i : calculate_page @page = @page.to_i > 1 ? @page.to_i : calculate_page
setup_filtered_posts setup_filtered_posts
@filtered_posts = apply_default_order(@filtered_posts)
filter_posts(options) filter_posts(options)
if @posts && !@skip_custom_fields if @posts && !@skip_custom_fields
@ -159,7 +160,7 @@ class TopicView
if is_mega_topic? if is_mega_topic?
nil nil
else else
Gaps.new(filtered_post_ids, unfiltered_posts.order(:sort_order).pluck(:id)) Gaps.new(filtered_post_ids, apply_default_order(unfiltered_posts).pluck(:id))
end end
end end
end end
@ -318,18 +319,18 @@ class TopicView
posts_before = 1 if posts_before.zero? posts_before = 1 if posts_before.zero?
sort_order = get_sort_order(post_number) sort_order = get_sort_order(post_number)
before_post_ids = @filtered_posts.order(sort_order: :desc) before_post_ids = @filtered_posts.reverse_order
.where("posts.sort_order < ?", sort_order) .where("posts.sort_order < ?", sort_order)
.limit(posts_before) .limit(posts_before)
.pluck(:id) .pluck(:id)
post_ids = before_post_ids + @filtered_posts.order(sort_order: :asc) post_ids = before_post_ids + @filtered_posts
.where("posts.sort_order >= ?", sort_order) .where("posts.sort_order >= ?", sort_order)
.limit(@limit - before_post_ids.length) .limit(@limit - before_post_ids.length)
.pluck(:id) .pluck(:id)
if post_ids.length < @limit if post_ids.length < @limit
post_ids = post_ids + @filtered_posts.order(sort_order: :desc) post_ids = post_ids + @filtered_posts.reverse_order
.where("posts.sort_order < ?", sort_order) .where("posts.sort_order < ?", sort_order)
.offset(before_post_ids.length) .offset(before_post_ids.length)
.limit(@limit - post_ids.length) .limit(@limit - post_ids.length)
@ -347,7 +348,7 @@ class TopicView
min = 1 if min == 0 && @exclude_first min = 1 if min == 0 && @exclude_first
filter_posts_by_ids( filter_posts_by_ids(
@filtered_posts.order(:sort_order) @filtered_posts
.offset(min) .offset(min)
.limit(@limit) .limit(@limit)
.pluck(:id) .pluck(:id)
@ -582,7 +583,7 @@ class TopicView
end end
def recent_posts def recent_posts
@filtered_posts.by_newest.with_user.first(25) @filtered_posts.unscope(:order).by_newest.with_user.first(25)
end end
# Returns an array of [id, days_ago] tuples. # Returns an array of [id, days_ago] tuples.
@ -590,8 +591,6 @@ class TopicView
def filtered_post_stream def filtered_post_stream
@filtered_post_stream ||= begin @filtered_post_stream ||= begin
posts = @filtered_posts posts = @filtered_posts
.order(:sort_order)
columns = [:id] columns = [:id]
if !is_mega_topic? if !is_mega_topic?
@ -632,7 +631,7 @@ class TopicView
end end
def last_post_id def last_post_id
@filtered_posts.order(sort_order: :desc).pluck_first(:id) @filtered_posts.reverse_order.pluck_first(:id)
end end
def current_post_number def current_post_number
@ -717,19 +716,15 @@ class TopicView
posts = posts =
if asc if asc
@filtered_posts @filtered_posts.where("sort_order > ?", sort_order)
.where("sort_order > ?", sort_order)
.order(sort_order: :asc)
else else
@filtered_posts @filtered_posts.reverse_order.where("sort_order < ?", sort_order)
.where("sort_order < ?", sort_order)
.order(sort_order: :desc)
end end
posts = posts.limit(@limit) if !@skip_limit posts = posts.limit(@limit) if !@skip_limit
filter_posts_by_ids(posts.pluck(:id)) filter_posts_by_ids(posts.pluck(:id))
@posts = @posts.unscope(:order).order(sort_order: :desc) if !asc @posts = @posts.reverse_order if !asc
end end
def filter_posts_by_ids(post_ids) def filter_posts_by_ids(post_ids)
@ -742,7 +737,8 @@ class TopicView
:topic, :topic,
:image_upload :image_upload
) )
.order('sort_order')
@posts = apply_default_order(@posts)
@posts = filter_post_types(@posts) @posts = filter_post_types(@posts)
@posts = @posts.with_deleted if @guardian.can_see_deleted_posts?(@topic.category) @posts = @posts.with_deleted if @guardian.can_see_deleted_posts?(@topic.category)
@posts @posts
@ -766,6 +762,10 @@ class TopicView
result result
end end
def apply_default_order(scope)
scope.order(sort_order: :asc)
end
def setup_filtered_posts def setup_filtered_posts
# Certain filters might leave gaps between posts. If that's true, we can return a gap structure # Certain filters might leave gaps between posts. If that's true, we can return a gap structure
@contains_gaps = false @contains_gaps = false
@ -869,7 +869,6 @@ class TopicView
@contains_gaps = true @contains_gaps = true
end end
end end
def check_and_raise_exceptions(skip_staff_action) def check_and_raise_exceptions(skip_staff_action)