From 0884d570b1999152eb16f846d296c260a1ee461c Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 22 Jul 2020 19:26:36 +0530 Subject: [PATCH] FEATURE: add support for `top` filter in tag page. (#10281) Currently, tag pages only have the `latest` filter. --- .../dynamic-route-builders.js | 17 ++++------- app/controllers/tags_controller.rb | 7 ++++- config/routes.rb | 5 ---- lib/discourse.rb | 4 +-- spec/requests/tags_controller_spec.rb | 30 +++++++++++++++++++ 5 files changed, 43 insertions(+), 20 deletions(-) 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 47626918f03..d4866e8bbb1 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 136a2e84676..471d3b29911 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 a6c8b81c953..c62265d0431 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 feef8f054dc..5d328826337 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 38528e5d497..95714bc871a 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