FIX: prevents false boolean param to be filtered as non existant (#9968)

FIX: prevents false boolean param to be filtered as non existant (#9968)

  • FIX: prevents false boolean param to be filtered as non existant

This was preventing to filter top category route to be filtered by replies.

  • if order is different ascending should be true on first click

  • test

  • fix

  • just pass params

  • more fixxes

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 18fe881..eac84d8 100644
--- a/app/assets/javascripts/discourse/app/routes/build-category-route.js
+++ b/app/assets/javascripts/discourse/app/routes/build-category-route.js
@@ -61,12 +61,12 @@ export default (filterArg, params) => {
           const record = this.store.createRecord("category", result.category);
           record.setupGroupsAndPermissions();
           this.site.updateCategory(record);
-          return { category: record };
+          return { category: record, modelParams };
         });
       }
 
       if (category) {
-        return { category };
+        return { category, modelParams };
       }
     },
 
@@ -79,7 +79,7 @@ export default (filterArg, params) => {
       this._setupNavigation(model.category);
       return all([
         this._createSubcategoryList(model.category),
-        this._retrieveTopicList(model.category, transition)
+        this._retrieveTopicList(model.category, transition, model.modelParams)
       ]);
     },
 
@@ -113,11 +113,11 @@ export default (filterArg, params) => {
       return Promise.resolve();
     },
 
-    _retrieveTopicList(category, transition) {
+    _retrieveTopicList(category, transition, modelParams) {
       const listFilter = `c/${Category.slugFor(category)}/${
           category.id
         }/l/${this.filter(category)}`,
-        findOpts = filterQueryParams(transition.to.queryParams, params),
+        findOpts = filterQueryParams(modelParams),
         extras = { cached: this.isPoppedState(transition) };
 
       return findTopicList(
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 8d34c83..786e9d5 100644
--- a/app/assets/javascripts/discourse/app/routes/build-topic-route.js
+++ b/app/assets/javascripts/discourse/app/routes/build-topic-route.js
@@ -1,3 +1,4 @@
+import { isEmpty } from "@ember/utils";
 import I18n from "I18n";
 import DiscourseRoute from "discourse/routes/discourse";
 import { queryParams } from "discourse/controllers/discovery-sortable";
@@ -12,7 +13,7 @@ function filterQueryParams(params, defaultParams) {
 
   if (params) {
     Object.keys(queryParams).forEach(function(opt) {
-      if (params[opt]) {
+      if (!isEmpty(params[opt])) {
         findOpts[opt] = params[opt];
       }
     });
@@ -50,10 +51,12 @@ function findTopicList(store, tracking, filter, filterParams, extras) {
 
     // Clean up any string parameters that might slip through
     filterParams = filterParams || {};
-    Object.keys(filterParams).forEach(function(k) {
-      const val = filterParams[k];
-      if (val === "undefined" || val === "null" || val === "false") {
-        filterParams[k] = undefined;
+    Object.keys(filterParams).forEach(k => {
+      let val = filterParams[k];
+      if (val === "false") val = false;
+      if (val === "true") val = true;
+      if (val === "undefined" || val === "null") {
+        filterParams[k] = null;
       }
     });
 

GitHub sha: d27b877a

This commit appears in #9968 which was merged by blake.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/reply-sorting-broken-with-in-a-top-date-range/151614/15

I don’t understand how this is working. Changing val here is a local variable and never put back into filterParams.

1 Like

oops yes I did endup removing the check after, and in the end it was not necessary anymore, will remove this :+1: good catch thanks

1 Like

I did followup here: https://github.com/discourse/discourse/commit/a23d31e4d629fb8f0d1745646c754955234ebeb9