FEATURE: add support for `top` filter in tag page. (#10281)

FEATURE: add support for top filter in tag page. (#10281)

Currently, tag pages only have the latest filter.

diff --git a/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js b/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js
index 4762691..d4866e8 100644
--- a/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js
+++ b/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js
@@ -41,7 +41,10 @@ export default {
       app[
         `Discovery${filterCapitalized}CategoryNoneController`
       ] = DiscoverySortableController.extend();
-      app[`Discovery${filterCapitalized}Route`] = buildTopicRoute(filter);
+      if (filter !== "top") {
+        app[`Discovery${filterCapitalized}Route`] = buildTopicRoute(filter);
+      }
+
       app[`Discovery${filterCapitalized}CategoryRoute`] = buildCategoryRoute(
         filter
       );
@@ -53,12 +56,7 @@ export default {
       ] = buildCategoryRoute(filter, { no_subcategories: true });
     });
 
-    Discourse.DiscoveryTopController = DiscoverySortableController.extend();
-    Discourse.DiscoveryTopCategoryController = DiscoverySortableController.extend();
-    Discourse.DiscoveryTopParentCategoryController = DiscoverySortableController.extend();
-    Discourse.DiscoveryTopCategoryNoneController = DiscoverySortableController.extend();
-
-    Discourse.DiscoveryTopRoute = buildTopicRoute("top", {
+    app.DiscoveryTopRoute = buildTopicRoute("top", {
       actions: {
         willTransition() {
           User.currentProp("should_be_redirected_to_top", false);
@@ -67,11 +65,6 @@ export default {
         }
       }
     });
-    Discourse.DiscoveryTopCategoryRoute = buildCategoryRoute("top");
-    Discourse.DiscoveryTopParentCategoryRoute = buildCategoryRoute("top");
-    Discourse.DiscoveryTopCategoryNoneRoute = buildCategoryRoute("top", {
-      no_subcategories: true
-    });
 
     site.get("periods").forEach(period => {
       const periodCapitalized = period.capitalize();
diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb
index 136a2e8..471d3b2 100644
--- a/app/controllers/tags_controller.rb
+++ b/app/controllers/tags_controller.rb
@@ -78,8 +78,13 @@ class TagsController < ::ApplicationController
       @additional_tags = params[:additional_tag_ids].to_s.split('/').map { |t| t.force_encoding("UTF-8") }
 
       list_opts = build_topic_list_options
+      @list = nil
 
-      @list = TopicQuery.new(current_user, list_opts).public_send("list_#{filter}")
+      if filter == :top
+        @list = TopicQuery.new(current_user, list_opts).public_send("list_top_for", SiteSetting.top_page_default_timeframe.to_sym)
+      else
+        @list = TopicQuery.new(current_user, list_opts).public_send("list_#{filter}")
+      end
 
       @list.draft_key = Draft::NEW_TOPIC
       @list.draft_sequence = DraftSequence.current(current_user, Draft::NEW_TOPIC)
diff --git a/config/routes.rb b/config/routes.rb
index a6c8b81..c62265d 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -679,8 +679,6 @@ Discourse::Application.routes.draw do
     get "c/*category_slug_path_with_id.rss" => "list#category_feed", format: :rss
     scope path: 'c/*category_slug_path_with_id' do
       get "/none" => "list#category_none_latest"
-      get "/none/l/top" => "list#category_none_top", as: "category_none_top"
-      get "/l/top" => "list#category_top", as: "category_top"
 
       TopTopic.periods.each do |period|
         get "/none/l/top/#{period}" => "list#category_none_top_#{period}", as: "category_none_top_#{period}"
@@ -711,7 +709,6 @@ Discourse::Application.routes.draw do
       get "#{filter}" => "list##{filter}"
     end
 
-    get "top" => "list#top"
     get "search/query" => "search#query"
     get "search" => "search#show"
     post "search/click" => "search#click"
@@ -943,8 +940,6 @@ Discourse::Application.routes.draw do
     end
     # special case for categories
     root to: "categories#index", constraints: HomePageConstraint.new("categories"), as: "categories_index"
-    # special case for top
-    root to: "list#top", constraints: HomePageConstraint.new("top"), as: "top_lists"
 
     root to: 'finish_installation#index', constraints: HomePageConstraint.new("finish_installation"), as: 'installation_redirect'
 
diff --git a/lib/discourse.rb b/lib/discourse.rb
index feef8f0..5d32882 100644
--- a/lib/discourse.rb
+++ b/lib/discourse.rb
@@ -186,7 +186,7 @@ module Discourse
   class ScssError < StandardError; end
 
   def self.filters
-    @filters ||= [:latest, :unread, :new, :read, :posted, :bookmarks]
+    @filters ||= [:latest, :unread, :new, :top, :read, :posted, :bookmarks]
   end
 
   def self.anonymous_filters
@@ -194,7 +194,7 @@ module Discourse
   end
 
   def self.top_menu_items
-    @top_menu_items ||= Discourse.filters + [:categories, :top]
+    @top_menu_items ||= Discourse.filters + [:categories]
   end
 
   def self.anonymous_top_menu_items
diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb
index 38528e5..95714bc 100644
--- a/spec/requests/tags_controller_spec.rb
+++ b/spec/requests/tags_controller_spec.rb
@@ -567,6 +567,36 @@ describe TagsController do
     end
   end
 
+  describe '#show_top' do
+    fab!(:tag)       { Fabricate(:tag) }
+
+    fab!(:category) { Fabricate(:category) }
+    fab!(:topic) { Fabricate(:topic, category: category) }
+    fab!(:tag_topic)  { Fabricate(:topic, category: category, tags: [tag]) }
+
+    before do
+      SiteSetting.top_page_default_timeframe = 'all'
+      TopTopic.create!(topic: topic, all_score: 1)
+      TopTopic.create!(topic: tag_topic, all_score: 1)
+    end
+
+    it "can filter by tag" do
+      get "/tag/#{tag.name}/l/top.json"
+      expect(response.status).to eq(200)
+
+      topic_ids = response.parsed_body["topic_list"]["topics"].map { |topic| topic["id"] }
+      expect(topic_ids).to eq([tag_topic.id])
+    end
+
+    it "can filter by both category and tag" do
+      get "/tags/c/#{category.slug}/#{category.id}/#{tag.name}/l/top.json"
+      expect(response.status).to eq(200)
+
+      topic_ids = response.parsed_body["topic_list"]["topics"].map { |topic| topic["id"] }
+      expect(topic_ids).to eq([tag_topic.id])
+    end
+  end
+
   describe '#search' do
     context 'tagging disabled' do
       it "returns 404" do

GitHub sha: 0884d570

1 Like

This commit appears in #10281 which was approved by eviltrout. It was merged by vinothkannans.