FIX: Move queryParams to each discovery controller rather than shared (#10424)

* REFACTOR: `refreshSort` doesn't cause it to sort again, it's misleading

* FIX: Move queryParams to each discovery controller rather than shared

This fixes issues where params previously would not reset between
routes. For example if you added `max_posts=1` to /latest and then went
to a category.

* Add backward compatibility for (action "changeSort") for themes

* FIX: refreshing was not working

* Update app/assets/javascripts/discourse/app/controllers/discovery/topics.js

Co-authored-by: Jarek Radosz <jradosz@gmail.com>

Co-authored-by: Jarek Radosz <jradosz@gmail.com>

app/assets/javascripts/discourse/app/controllers/discovery-sortable.js

@@ -1,11 +1,10 @@
-import { alias } from "@ember/object/computed";
 import { inject } from "@ember/controller";
 import Controller from "@ember/controller";
 
 // Just add query params here to have them automatically passed to topic list filters.
 export const queryParams = {
   order: { replace: true, refreshModel: true },
-  ascending: { replace: true, refreshModel: true },
+  ascending: { replace: true, refreshModel: true, default: false },
   status: { replace: true, refreshModel: true },
   state: { replace: true, refreshModel: true },
   search: { replace: true, refreshModel: true },
@@ -23,17 +22,36 @@ const controllerOpts = {
   queryParams: Object.keys(queryParams)
 };
 
-// Aliases for the values
-controllerOpts.queryParams.forEach(
-  p => (controllerOpts[p] = alias(`discoveryTopics.${p}`))
-);
+// Default to `null`
+controllerOpts.queryParams.forEach(p => {
+  controllerOpts[p] = queryParams[p].default;
+});
+
+export function changeSort(sortBy) {
+  let { controller } = this;
+  let model = this.controllerFor("discovery.topics").model;
+  if (sortBy === controller.order) {
+    controller.toggleProperty("ascending");
+    model.updateSortParams(sortBy, controller.ascending);
+  } else {
+    controller.setProperties({ order: sortBy, ascending: false });
+    model.updateSortParams(sortBy, false);
+  }
+}
+
+export function resetParams() {
+  let { controller } = this;
+  controllerOpts.queryParams.forEach(p => {
+    controller.set(p, queryParams[p].default);
+  });
+}
 
 const SortableController = Controller.extend(controllerOpts);
 
 export const addDiscoveryQueryParam = function(p, opts) {
   queryParams[p] = opts;
   const cOpts = {};
-  cOpts[p] = alias(`discoveryTopics.${p}`);
+  cOpts[p] = null;
   cOpts["queryParams"] = Object.keys(queryParams);
   SortableController.reopen(cOpts);
 };

app/assets/javascripts/discourse/app/controllers/discovery/topics.js

@@ -1,19 +1,30 @@
 import I18n from "I18n";
 import discourseComputed from "discourse-common/utils/decorators";
-import { alias, not, gt, empty, notEmpty, equal } from "@ember/object/computed";
+import {
+  alias,
+  not,
+  gt,
+  empty,
+  notEmpty,
+  equal,
+  readOnly
+} from "@ember/object/computed";
 import { inject as controller } from "@ember/controller";
 import DiscoveryController from "discourse/controllers/discovery";
-import { queryParams } from "discourse/controllers/discovery-sortable";
 import BulkTopicSelection from "discourse/mixins/bulk-topic-selection";
 import { endWith } from "discourse/lib/computed";
 import showModal from "discourse/lib/show-modal";
 import { userPath } from "discourse/lib/url";
 import TopicList from "discourse/models/topic-list";
 import Topic from "discourse/models/topic";
+import { routeAction } from "discourse/helpers/route-action";
+import { inject as service } from "@ember/service";
+import deprecated from "discourse-common/lib/deprecated";
 
 const controllerOpts = {
   discovery: controller(),
   discoveryTopics: controller("discovery/topics"),
+  router: service(),
 
   period: null,
 
@@ -21,27 +32,19 @@ const controllerOpts = {
   showTopicPostBadges: not("discoveryTopics.new"),
   redirectedReason: alias("currentUser.redirected_to_top.reason"),
 
-  order: null,
-  ascending: false,
   expandGloballyPinned: false,
   expandAllPinned: false,
 
-  resetParams() {
-    Object.keys(this.get("model.params") || {}).forEach(key => {
-      // controllerOpts contains the default values for parameters, so use them. They might be null.
-      this.set(key, controllerOpts[key]);
-    });
-  },
+  order: readOnly("model.params.order"),
+  ascending: readOnly("model.params.ascending"),
 
   actions: {
-    changeSort(sortBy) {
-      if (sortBy === this.order) {
-        this.toggleProperty("ascending");
-        this.model.refreshSort(sortBy, this.ascending);
-      } else {
-        this.setProperties({ order: sortBy, ascending: false });
-        this.model.refreshSort(sortBy, false);
-      }
+    changeSort() {
+      deprecated(
+        "changeSort has been changed from an (action) to a (route-action)",
+        { since: "2.6.0", dropFrom: "2.7.0" }
+      );
+      return routeAction("changeSort", this.router._router, ...arguments)();
     },
 
     // Show newly inserted topics
@@ -56,7 +59,7 @@ const controllerOpts = {
 
     refresh() {
       const filter = this.get("model.filter");
-      this.resetParams();
+      this.send("resetParams");
 
       // Don't refresh if we're still loading
       if (this.get("discovery.loading")) {
@@ -175,11 +178,4 @@ const controllerOpts = {
   }
 };
 
-Object.keys(queryParams).forEach(function(p) {
-  // If we don't have a default value, initialize it to null
-  if (typeof controllerOpts[p] === "undefined") {
-    controllerOpts[p] = null;
-  }
-});
-
 export default DiscoveryController.extend(controllerOpts, BulkTopicSelection);

app/assets/javascripts/discourse/app/models/topic-list.js

@@ -55,8 +55,8 @@ const TopicList = RestModel.extend({
     });
   },
 
-  refreshSort(order, ascending) {
-    let params = this.params || {};
+  updateSortParams(order, ascending) {
+    let params = Object.assign({}, this.params || {});
 
     if (params.q) {
       // search is unique, nothing else allowed with it

app/assets/javascripts/discourse/app/routes/build-category-route.js

@@ -4,7 +4,11 @@ import {
   filterQueryParams,
   findTopicList
 } from "discourse/routes/build-topic-route";
-import { queryParams } from "discourse/controllers/discovery-sortable";
+import {
+  changeSort,
+  resetParams,
+  queryParams
+} from "discourse/controllers/discovery-sortable";
 import TopicList from "discourse/models/topic-list";
 import PermissionType from "discourse/models/permission-type";
 import CategoryList from "discourse/models/category-list";
@@ -253,7 +257,10 @@ export default (filterArg, params) => {
 
       triggerRefresh() {
         this.refresh();
-      }
+      },
+
+      changeSort,
+      resetParams
     }
   });
 };

app/assets/javascripts/discourse/app/routes/build-topic-route.js

@@ -1,7 +1,11 @@
 import { isEmpty } from "@ember/utils";
 import I18n from "I18n";
 import DiscourseRoute from "discourse/routes/discourse";
-import { queryParams } from "discourse/controllers/discovery-sortable";
+import {
+  changeSort,
+  resetParams,
+  queryParams
+} from "discourse/controllers/discovery-sortable";
 import { defaultHomepage } from "discourse/lib/utilities";
 import Session from "discourse/models/session";
 import { Promise } from "rsvp";
@@ -131,15 +135,6 @@ export default function(filter, extras) {
           expandGloballyPinned: true
         };
 
-        const params = model.get("params");
-        if (params && Object.keys(params).length) {
-          if (params.order !== undefined) {
-            topicOpts.order = params.order;
-          }
-          if (params.ascending !== undefined) {
-            topicOpts.ascending = params.ascending;
-          }
-        }
         this.controllerFor("discovery/topics").setProperties(topicOpts);
         this.controllerFor("navigation/default").set(
           "canCreateTopic",
@@ -153,6 +148,11 @@ export default function(filter, extras) {
           controller: "discovery/topics",
           outlet: "list-container"
         });
+      },
+
+      actions: {
+        changeSort,
+        resetParams
       }
     },
     extras

app/assets/javascripts/discourse/app/templates/discovery/topics.hbs

@@ -59,7 +59,7 @@
       showTopicPostBadges=showTopicPostBadges
       showPosters=true
       canBulkSelect=canBulkSelect
-      changeSort=(action "changeSort")
+      changeSort=(route-action "changeSort")
       toggleBulkSelect=(action "toggleBulkSelect")
       hideCategory=model.hideCategory
       order=order
@@ -110,4 +110,4 @@
     {{/footer-message}}
 
   {{/if}}
-</footer>
+</footer>