From c12e7112bf52009f54afcd5e79510fa5a8090aed Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 3 May 2023 12:40:00 +0800 Subject: [PATCH] DEV: Fix `order:` filter not working on `/filter` route (#21330) `TopicQuery#latest_results` which was being used by `TopicQuery#list_filter` defaults to ordering by `Topic#bumped_at` in descending order and that was taking precedent over the order scopes being applied by `TopicsFilter`. --- lib/topic_query.rb | 11 +++++++--- spec/requests/list_controller_spec.rb | 30 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 98c54597066..ddb15e1ce40 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -270,7 +270,10 @@ class TopicQuery def list_filter topics_filter = - TopicsFilter.new(guardian: @guardian, scope: latest_results(include_muted: false)) + TopicsFilter.new( + guardian: @guardian, + scope: latest_results(include_muted: false, skip_ordering: true), + ) results = topics_filter.filter_from_query_string(@options[:q]) @@ -278,6 +281,8 @@ class TopicQuery results = remove_muted_topics(results, @user) end + results = apply_ordering(results) if results.order_values.empty? + create_list(:filter, {}, results) end @@ -618,7 +623,7 @@ class TopicQuery result.where("topics.category_id != ?", drafts_category_id) end - def apply_ordering(result, options) + def apply_ordering(result, options = {}) sort_column = SORTABLE_MAPPING[options[:order]] || "default" sort_dir = (options[:ascending] == "true") ? "ASC" : "DESC" @@ -732,7 +737,7 @@ class TopicQuery result = filter_by_tags(result) end - result = apply_ordering(result, options) + result = apply_ordering(result, options) if !options[:skip_ordering] all_listable_topics = @guardian.filter_allowed_categories( diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 66f7fb1a37e..04f3aa5e19e 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -1308,6 +1308,36 @@ RSpec.describe ListController do end end + describe "when ordering using the `order:` filter" do + fab!(:topic2) { Fabricate(:topic, views: 2) } + fab!(:topic3) { Fabricate(:topic, views: 3) } + fab!(:topic4) { Fabricate(:topic, views: 1) } + + it "return topics ordered by topic bumped at date in descending order when `q` query param is not present" do + sign_in(user) + + get "/filter.json" + + expect(response.status).to eq(200) + + expect(response.parsed_body["topic_list"]["topics"].map { |topic| topic["id"] }).to eq( + [topic4.id, topic3.id, topic2.id, topic.id], + ) + end + + it "return topics ordered by views when `q` query param is `order:views`" do + sign_in(user) + + get "/filter.json", params: { q: "order:views" } + + expect(response.status).to eq(200) + + expect(response.parsed_body["topic_list"]["topics"].map { |topic| topic["id"] }).to eq( + [topic3.id, topic2.id, topic4.id, topic.id], + ) + end + end + describe "when filtering by status" do fab!(:group) { Fabricate(:group) } fab!(:private_category) { Fabricate(:private_category, group: group) }