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

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

diff --git a/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js b/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js
index fdaf34c..5c17cdf 100644
--- a/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js
+++ b/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);
 };
diff --git a/app/assets/javascripts/discourse/app/controllers/discovery/topics.js b/app/assets/javascripts/discourse/app/controllers/discovery/topics.js
index 3ea86d3..2b55556 100644
--- a/app/assets/javascripts/discourse/app/controllers/discovery/topics.js
+++ b/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);
diff --git a/app/assets/javascripts/discourse/app/models/topic-list.js b/app/assets/javascripts/discourse/app/models/topic-list.js
index 16b2b67..c5239d4 100644
--- a/app/assets/javascripts/discourse/app/models/topic-list.js
+++ b/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
diff --git a/app/assets/javascripts/discourse/app/routes/build-category-route.js b/app/assets/javascripts/discourse/app/routes/build-category-route.js
index 78c08dd..b4a5475 100644
--- a/app/assets/javascripts/discourse/app/routes/build-category-route.js
+++ b/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
     }
   });
 };
diff --git a/app/assets/javascripts/discourse/app/routes/build-topic-route.js b/app/assets/javascripts/discourse/app/routes/build-topic-route.js
index 6b6b8da..e05fdcc 100644
--- a/app/assets/javascripts/discourse/app/routes/build-topic-route.js
+++ b/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
         };
 

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

GitHub sha: ba3ee344

1 Like

This commit appears in #10424 which was merged by eviltrout.