FEATURE: add period filter in top topics route for tags. (#13415)

FEATURE: add period filter in top topics route for tags. (#13415)

And also move all the “top topics by period” routes to query string param.

/top/monthly => /top?period=monthly /c/:slug/:id/l/top/monthly => /c/:slug/:id/l/top?period=monthly /tag/:slug/l/top/daily => /tag/:slug/l/top?period=daily (new)

diff --git a/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js b/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js
index 8d02ca3..4fab6af 100644
--- a/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js
+++ b/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js
@@ -13,6 +13,7 @@ export const queryParams = {
   before: { replace: true, refreshModel: true },
   bumped_before: { replace: true, refreshModel: true },
   f: { replace: true, refreshModel: true },
+  period: { replace: true, refreshModel: true },
 };
 
 // Basic controller options
diff --git a/app/assets/javascripts/discourse/app/controllers/discovery.js b/app/assets/javascripts/discourse/app/controllers/discovery.js
index 9f2993a..4a09895 100644
--- a/app/assets/javascripts/discourse/app/controllers/discovery.js
+++ b/app/assets/javascripts/discourse/app/controllers/discovery.js
@@ -37,9 +37,10 @@ export default Controller.extend({
       }/l`;
     }
 
-    url += "/top/" + period;
+    url += "/top";
 
-    const queryParams = this.router.currentRoute.queryParams;
+    let queryParams = this.router.currentRoute.queryParams;
+    queryParams.period = period;
     if (Object.keys(queryParams).length) {
       url =
         `${url}?` +
diff --git a/app/assets/javascripts/discourse/app/controllers/discovery/topics.js b/app/assets/javascripts/discourse/app/controllers/discovery/topics.js
index 99afc1d..35ab378 100644
--- a/app/assets/javascripts/discourse/app/controllers/discovery/topics.js
+++ b/app/assets/javascripts/discourse/app/controllers/discovery/topics.js
@@ -1,12 +1,4 @@
-import {
-  alias,
-  empty,
-  equal,
-  gt,
-  not,
-  notEmpty,
-  readOnly,
-} from "@ember/object/computed";
+import { alias, empty, equal, gt, not, readOnly } from "@ember/object/computed";
 import BulkTopicSelection from "discourse/mixins/bulk-topic-selection";
 import DiscoveryController from "discourse/controllers/discovery";
 import I18n from "I18n";
@@ -140,7 +132,7 @@ const controllerOpts = {
   allLoaded: empty("model.more_topics_url"),
   latest: endWith("model.filter", "latest"),
   new: endWith("model.filter", "new"),
-  top: notEmpty("period"),
+  top: endWith("model.filter", "top"),
   yearly: equal("period", "yearly"),
   quarterly: equal("period", "quarterly"),
   monthly: equal("period", "monthly"),
diff --git a/app/assets/javascripts/discourse/app/controllers/tag-show.js b/app/assets/javascripts/discourse/app/controllers/tag-show.js
index 76b7d07..dae9fe2 100644
--- a/app/assets/javascripts/discourse/app/controllers/tag-show.js
+++ b/app/assets/javascripts/discourse/app/controllers/tag-show.js
@@ -8,6 +8,7 @@ import Topic from "discourse/models/topic";
 import { alias } from "@ember/object/computed";
 import bootbox from "bootbox";
 import { queryParams } from "discourse/controllers/discovery-sortable";
+import { endWith } from "discourse/lib/computed";
 
 export default Controller.extend(BulkTopicSelection, FilterModeMixin, {
   application: controller(),
@@ -27,6 +28,8 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, {
   max_posts: null,
   q: null,
   showInfo: false,
+  top: endWith("list.filter", "top"),
+  period: alias("list.for_period"),
 
   @discourseComputed(
     "canCreateTopic",
@@ -131,8 +134,30 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, {
         this.setProperties({ order, ascending: false });
       }
 
+      let params = { order, ascending: this.ascending };
+      if (this.period) {
+        params.period = this.period;
+      }
+
+      this.transitionToRoute({
+        queryParams: params,
+      });
+    },
+
+    changePeriod(p) {
+      this.set("period", p);
+
+      let params = { period: this.period };
+
+      if (!(this.order === "default" && this.ascending === false)) {
+        params = Object.assign(params, {
+          order: this.order,
+          ascending: this.ascending,
+        });
+      }
+
       this.transitionToRoute({
-        queryParams: { order, ascending: this.ascending },
+        queryParams: params,
       });
     },
 
diff --git a/app/assets/javascripts/discourse/app/mixins/filter-mode.js b/app/assets/javascripts/discourse/app/mixins/filter-mode.js
index 22f3060..ab5c23e 100644
--- a/app/assets/javascripts/discourse/app/mixins/filter-mode.js
+++ b/app/assets/javascripts/discourse/app/mixins/filter-mode.js
@@ -36,13 +36,7 @@ export default Mixin.create({
 
     set(key, value) {
       this.set("rawFilterMode", value);
-      const parts = value.split("/");
-
-      if (parts.length >= 2 && parts[parts.length - 2] === "top") {
-        this.set("filterType", "top");
-      } else {
-        this.set("filterType", parts.pop());
-      }
+      this.set("filterType", value.split("/").pop());
 
       return value;
     },
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 654d525..b408917 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
@@ -34,7 +34,18 @@ export default {
       app[
         `Discovery${filterCapitalized}CategoryNoneController`
       ] = DiscoverySortableController.extend();
-      if (filter !== "top") {
+
+      if (filter === "top") {
+        app.DiscoveryTopRoute = buildTopicRoute("top", {
+          actions: {
+            willTransition() {
+              User.currentProp("should_be_redirected_to_top", false);
+              User.currentProp("redirected_to_top.reason", null);
+              return this._super(...arguments);
+            },
+          },
+        });
+      } else {
         app[`Discovery${filterCapitalized}Route`] = buildTopicRoute(filter);
       }
 
@@ -46,38 +57,6 @@ export default {
       ] = buildCategoryRoute(filter, { no_subcategories: true });
     });
 
-    app.DiscoveryTopRoute = buildTopicRoute("top", {
-      actions: {
-        willTransition() {
-          User.currentProp("should_be_redirected_to_top", false);
-          User.currentProp("redirected_to_top.reason", null);
-          return this._super(...arguments);
-        },
-      },
-    });
-
-    site.get("periods").forEach((period) => {
-      const periodCapitalized = period.capitalize();
-      app[
-        `DiscoveryTop${periodCapitalized}Controller`
-      ] = DiscoverySortableController.extend();
-      app[
-        `DiscoveryTop${periodCapitalized}CategoryController`
-      ] = DiscoverySortableController.extend();
-      app[
-        `DiscoveryTop${periodCapitalized}CategoryNoneController`
-      ] = DiscoverySortableController.extend();
-      app[`DiscoveryTop${periodCapitalized}Route`] = buildTopicRoute(
-        "top/" + period
-      );
-      app[`DiscoveryTop${periodCapitalized}CategoryRoute`] = buildCategoryRoute(
-        "top/" + period
-      );
-      app[
-        `DiscoveryTop${periodCapitalized}CategoryNoneRoute`
-      ] = buildCategoryRoute("top/" + period, { no_subcategories: true });
-    });
-
     app["TagsShowCategoryRoute"] = TagShowRoute.extend();
     app["TagsShowCategoryNoneRoute"] = TagShowRoute.extend({
       noSubcategories: true,
diff --git a/app/assets/javascripts/discourse/app/routes/app-route-map.js b/app/assets/javascripts/discourse/app/routes/app-route-map.js
index 6ccf9c9..9bbb446 100644
--- a/app/assets/javascripts/discourse/app/routes/app-route-map.js
+++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js
@@ -27,14 +27,7 @@ export default function () {
   });
 
   this.route("discovery", { path: "/", resetNamespace: true }, function () {
-    // top
-    this.route("top");
-    this.route("topCategoryNone", {
-      path: "/c/*category_slug_path_with_id/none/l/top",
-    });
-    this.route("topCategory", { path: "/c/*category_slug_path_with_id/l/top" });
-

[... diff too long, it was truncated ...]

GitHub sha: 33eae4cbd8f14fa29f513b0882e3ee0b20938075

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