UX: Change ordering of categories shown in sidebar (#18803)

There are two possible ordering for categories shown in sidebar with
this commit.

When the `fixed_category_positions` site setting is enabled, the
categories are ordered based on `Category#position` which is a configurable
option by the user. When said site setting is disabled, the categories
are ordered based on `Category#name`.

The categories in Sidebar are also sorted in such a way where child
categories are always ordered right after their parents. When multiple
child categories are present, the child categories are ordered based on
the ordering described above.

app/assets/javascripts/discourse/app/components/sidebar/anonymous/categories-section.js

@@ -1,31 +1,32 @@
-import { inject as service } from "@ember/service";
 import { canDisplayCategory } from "discourse/lib/sidebar/helpers";
 import SidebarCommonCategoriesSection from "discourse/components/sidebar/common/categories-section";
+import Category from "discourse/models/category";
 
 export default class SidebarAnonymousCategoriesSection extends SidebarCommonCategoriesSection {
-  @service site;
+  constructor() {
+    super(...arguments);
+
+    if (!this.siteSettings.default_sidebar_categories) {
+      this.shouldSortCategoriesByDefault = false;
+    }
+  }
 
   get categories() {
-    let categories = this.site.categoriesList;
-
     if (this.siteSettings.default_sidebar_categories) {
-      const defaultCategoryIds = this.siteSettings.default_sidebar_categories
-        .split("|")
-        .map((categoryId) => parseInt(categoryId, 10));
-
-      categories = categories.filter((category) =>
-        defaultCategoryIds.includes(category.id)
+      return Category.findByIds(
+        this.siteSettings.default_sidebar_categories
+          .split("|")
+          .map((categoryId) => parseInt(categoryId, 10))
       );
     } else {
-      categories = categories
-        .filter(
-          (category) =>
-            canDisplayCategory(category, this.siteSettings) &&
-            !category.parent_category_id
-        )
+      return this.site.categoriesList
+        .filter((category) => {
+          return (
+            !category.parent_category_id &&
+            canDisplayCategory(category.id, this.siteSettings)
+          );
+        })
         .slice(0, 5);
     }
-
-    return categories;
   }
 }

app/assets/javascripts/discourse/app/components/sidebar/common/categories-section.js

@@ -2,29 +2,60 @@ import Component from "@glimmer/component";
 import { cached } from "@glimmer/tracking";
 import { inject as service } from "@ember/service";
 
+import Category from "discourse/models/category";
 import CategorySectionLink from "discourse/lib/sidebar/user/categories-section/category-section-link";
+import { canDisplayCategory } from "discourse/lib/sidebar/helpers";
 
 export default class SidebarCommonCategoriesSection extends Component {
   @service topicTrackingState;
   @service siteSettings;
+  @service site;
 
-  // Override in child
+  shouldSortCategoriesByDefault = true;
+
+  /**
+   * Override in child
+   *
+   * @returns {Object[]} An array of Category objects
+   */
   get categories() {}
 
+  get sortedCategories() {
+    if (!this.shouldSortCategoriesByDefault) {
+      return this.categories;
+    }
+
+    let categories = this.site.categories;
+
+    if (!this.siteSettings.fixed_category_positions) {
+      categories = categories.sort((a, b) => a.name.localeCompare(b.name));
+    }
+
+    const categoryIds = this.categories.map((category) => category.id);
+
+    return Category.sortCategories(categories).reduce(
+      (filteredCategories, category) => {
+        if (
+          categoryIds.includes(category.id) &&
+          canDisplayCategory(category.id, this.siteSettings)
+        ) {
+          filteredCategories.push(category);
+        }
+
+        return filteredCategories;
+      },
+      []
+    );
+  }
+
   @cached
   get sectionLinks() {
-    return this.categories
-      .sort((a, b) => a.name.localeCompare(b.name))
-      .reduce((links, category) => {
-        links.push(
-          new CategorySectionLink({
-            category,
-            topicTrackingState: this.topicTrackingState,
-            currentUser: this.currentUser,
-          })
-        );
-
-        return links;
-      }, []);
+    return this.sortedCategories.map((category) => {
+      return new CategorySectionLink({
+        category,
+        topicTrackingState: this.topicTrackingState,
+        currentUser: this.currentUser,
+      });
+    });
   }
 }

app/assets/javascripts/discourse/app/components/sidebar/user/categories-section.js

@@ -1,7 +1,8 @@
 import { inject as service } from "@ember/service";
 import { action } from "@ember/object";
+import Category from "discourse/models/category";
+import { cached } from "@glimmer/tracking";
 
-import { canDisplayCategory } from "discourse/lib/sidebar/helpers";
 import SidebarCommonCategoriesSection from "discourse/components/sidebar/common/categories-section";
 
 export default class SidebarUserCategoriesSection extends SidebarCommonCategoriesSection {
@@ -24,10 +25,9 @@ export default class SidebarUserCategoriesSection extends SidebarCommonCategorie
     this.topicTrackingState.offStateChange(this.callbackId);
   }
 
+  @cached
   get categories() {
-    return this.currentUser.sidebarCategories.filter((category) => {
-      return canDisplayCategory(category, this.siteSettings);
-    });
+    return Category.findByIds(this.currentUser.sidebarCategoryIds);
   }
 
   /**

app/assets/javascripts/discourse/app/lib/sidebar/helpers.js

@@ -1,7 +1,9 @@
-export function canDisplayCategory(category, siteSettings) {
+import Category from "discourse/models/category";
+
+export function canDisplayCategory(categoryId, siteSettings) {
   if (siteSettings.allow_uncategorized_topics) {
     return true;
   }
 
-  return !category.isUncategorizedCategory;
+  return !Category.isUncategorized(categoryId);
 }

app/assets/javascripts/discourse/app/models/category.js

@@ -337,13 +337,35 @@ const Category = RestModel.extend({
 
   @discourseComputed("id")
   isUncategorizedCategory(id) {
-    return id === Site.currentProp("uncategorized_category_id");
+    return Category.isUncategorized(id);
   },
 });
 
 let _uncategorized;
 
 Category.reopenClass({
+  // Sort subcategories directly under parents
+  sortCategories(categories) {
+    const children = new Map();
+
+    categories.forEach((category) => {
+      const parentId = parseInt(category.parent_category_id, 10) || -1;
+      const group = children.get(parentId) || [];
+      group.pushObject(category);
+
+      children.set(parentId, group);
+    });
+
+    const reduce = (values) =>
+      values.flatMap((c) => [c, reduce(children.get(c.id) || [])]).flat();
+
+    return reduce(children.get(-1));
+  },
+
+  isUncategorized(categoryId) {
+    return categoryId === Site.currentProp("uncategorized_category_id");
+  },
+
   slugEncoded() {
     let siteSettings = getOwner(this).lookup("service:site-settings");
     return siteSettings.slug_generation_method === "encoded";

app/assets/javascripts/discourse/app/models/site.js

@@ -1,6 +1,7 @@
 import EmberObject, { get } from "@ember/object";
 import { alias, sort } from "@ember/object/computed";
 import Archetype from "discourse/models/archetype";
+import Category from "discourse/models/category";
 import PostActionType from "discourse/models/post-action-type";
 import PreloadStore from "discourse/lib/preload-store";
 import RestModel from "discourse/models/rest";
@@ -59,27 +60,14 @@ const Site = RestModel.extend({
   // Sort subcategories under parents
   @discourseComputed("categoriesByCount", "categories.[]")
   sortedCategories(categories) {
-    const children = new Map();
-
-    categories.forEach((category) => {
-      const parentId = parseInt(category.parent_category_id, 10) || -1;
-      const group = children.get(parentId) || [];
-      group.pushObject(category);
-
-      children.set(parentId, group);
-    });
-
-    const reduce = (values) =>
-      values.flatMap((c) => [c, reduce(children.get(c.id) || [])]).flat();
-
-    return reduce(children.get(-1));
+    return Category.sortCategories(categories);
   },
 
   // Returns it in the correct order, by setting
   @discourseComputed("categories.[]")
-  categoriesList() {
+  categoriesList(categories) {
     return this.siteSettings.fixed_category_positions
-      ? this.categories
+      ? categories
       : this.sortedCategories;
   },
 
@@ -158,7 +146,7 @@ Site.reopenClass(Singleton, {
     if (result.categories) {
       let subcatMap = {};
 
-      result.categoriesById = {};
+      result.categoriesById = new Map();
       result.categories = result.categories.map((c) => {
         if (c.parent_category_id) {
           subcatMap[c.parent_category_id] =

app/assets/javascripts/discourse/app/models/user.js

@@ -338,18 +338,6 @@ const User = RestModel.extend({
   },
 
   sidebarTagNames: mapBy("sidebarTags", "name"),
-
-  @discourseComputed("sidebar_category_ids.[]")
-  sidebarCategories(sidebarCategoryIds) {
-    if (!sidebarCategoryIds || sidebarCategoryIds.length === 0) {
-      return [];
-    }
-
-    return Site.current().categoriesList.filter((category) =>
-      sidebarCategoryIds.includes(category.id)
-    );
-  },
-
   sidebarListDestination: readOnly("sidebar_list_destination"),
 
   changeUsername(new_username) {

app/assets/javascripts/discourse/app/routes/preferences-sidebar.js

@@ -1,4 +1,5 @@
 import RestrictedUserRoute from "discourse/routes/restricted-user";
+import Category from "discourse/models/category";
 
 export default RestrictedUserRoute.extend({
   showFooter: true,
@@ -6,7 +7,7 @@ export default RestrictedUserRoute.extend({
   setupController(controller, user) {
     const props = {
       model: user,
-      selectedSidebarCategories: user.sidebarCategories,
+      selectedSidebarCategories: Category.findByIds(user.sidebarCategoryIds),
     };
 
     if (this.siteSettings.tagging_enabled) {

app/assets/javascripts/discourse/tests/acceptance/sidebar-anonymous-categories-section-test.js

@@ -7,24 +7,51 @@ import {
 } from "discourse/tests/helpers/qunit-helpers";
 import Site from "discourse/models/site";
 
-acceptance("Sidebar - Anonymous Categories Section", function (needs) {
+acceptance("Sidebar - Anonymous - Categories Section", function (needs) {
   needs.settings({
     enable_experimental_sidebar_hamburger: true,
     enable_sidebar: true,
   });
 
-  test("category section links", async function (assert) {
+  test("category section links ordered by category's topic count when default_sidebar_categories has not been configured and site setting to fix categories positions is disabled", async function (assert) {
+    this.siteSettings.fixed_category_positions = false;
+
     await visit("/");
 
-    const categories = queryAll(
+    const categorySectionLinks = queryAll(
       ".sidebar-section-categories .sidebar-section-link-wrapper"
     );
-    assert.strictEqual(categories.length, 6);
-    assert.strictEqual(categories[0].textContent.trim(), "bug");
-    assert.strictEqual(categories[1].textContent.trim(), "dev");
-    assert.strictEqual(categories[2].textContent.trim(), "feature");
-    assert.strictEqual(categories[3].textContent.trim(), "support");
-    assert.strictEqual(categories[4].textContent.trim(), "ux");
+
+    const sidebarCategories = Site.current()
+      .categories.filter((category) => !category.parent_category_id)
+      .sort((a, b) => b.topic_count - a.topic_count);
+
+    assert.strictEqual(categorySectionLinks.length, 6);
+
+    assert.strictEqual(
+      categorySectionLinks[0].textContent.trim(),
+      sidebarCategories[0].name
+    );
+
+    assert.strictEqual(
+      categorySectionLinks[1].textContent.trim(),
+      sidebarCategories[1].name
+    );
+
+    assert.strictEqual(
+      categorySectionLinks[2].textContent.trim(),
+      sidebarCategories[2].name
+    );
+
+    assert.strictEqual(
+      categorySectionLinks[3].textContent.trim(),
+      sidebarCategories[3].name
+    );
+
+    assert.strictEqual(
+      categorySectionLinks[4].textContent.trim(),
+      sidebarCategories[4].name
+    );
 
     assert.ok(
       exists("a.sidebar-section-link-all-categories"),
@@ -32,8 +59,54 @@ acceptance("Sidebar - Anonymous Categories Section", function (needs) {
     );
   });
 
-  test("category section links in sidebar when default_sidebar_categories site setting has been configured", async function (assert) {
-    this.siteSettings.default_sidebar_categories = "3|13|1";
+  test("category section links ordered by default category's position when default_sidebar_categories has not been configured and site setting to fix categories positions is enabled", async function (assert) {
+    this.siteSettings.fixed_category_positions = true;
+
+    await visit("/");
+
+    const categories = queryAll(
+      ".sidebar-section-categories .sidebar-section-link-wrapper"
+    );
+
+    const siteCategories = Site.current().categories;
+
+    assert.strictEqual(categories.length, 6);
+
+    assert.strictEqual(
+      categories[0].textContent.trim(),
+      siteCategories[0].name
+    );
+
+    assert.strictEqual(
+      categories[1].textContent.trim(),
+      siteCategories[1].name
+    );
+
+    assert.strictEqual(
+      categories[2].textContent.trim(),
+      siteCategories[3].name
+    );
+
+    assert.strictEqual(
+      categories[3].textContent.trim(),
+      siteCategories[4].name
+    );
+
+    assert.strictEqual(
+      categories[4].textContent.trim(),
+      siteCategories[5].name
+    );
+
+    assert.ok(
+      exists("a.sidebar-section-link-all-categories"),
+      "all categories link is visible"
+    );
+  });
+
+  test("category section links in sidebar when default_sidebar_categories site setting has been configured and site setting to fix category position is enabled", async function (assert) {
+    this.siteSettings.fixed_category_positions = true;
+    this.siteSettings.default_sidebar_categories = "1|3|13";
+
     await visit("/");
 
     const categories = queryAll(
@@ -41,9 +114,9 @@ acceptance("Sidebar - Anonymous Categories Section", function (needs) {
     );
 
     assert.strictEqual(categories.length, 4);
-    assert.strictEqual(categories[0].textContent.trim(), "blog");
-    assert.strictEqual(categories[1].textContent.trim(), "bug");
-    assert.strictEqual(categories[2].textContent.trim(), "meta");
+    assert.strictEqual(categories[0].textContent.trim(), "meta");
+    assert.strictEqual(categories[1].textContent.trim(), "blog");
+    assert.strictEqual(categories[2].textContent.trim(), "bug");
 
     assert.ok(
       exists("a.sidebar-section-link-all-categories"),
@@ -56,7 +129,7 @@ acceptance("Sidebar - Anonymous Categories Section", function (needs) {
     this.siteSettings.fixed_category_positions = true;
     const site = Site.current();
 
-    const firstCategory = Site.current().categories.find((category) => {
+    const firstCategory = site.categories.find((category) => {
       return !category.parent_category_id;
     });
 

app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js

@@ -193,12 +193,45 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
     );
   });
 
-  test("category section links are sorted by category name alphabetically", async function (assert) {
-    const { category1, category2, category3 } = setupUserSidebarCategories();
+  test("category section links are ordered by category name with child category sorted after parent when site setting to fix category's position is disabled", async function (assert) {
+    this.siteSettings.fixed_category_positions = false;
 
-    category3.set("name", "aBC");
-    category2.set("name", "abc");
-    category1.set("name", "efg");
+    const site = Site.current();
+    const siteCategories = site.categories;
+
+    siteCategories[0].parent_category_id = -1001;
+    siteCategories[0].id = -1000;
+    siteCategories[0].name = "Parent B Child A";
+
+    siteCategories[1].parent_category_id = null;
+    siteCategories[1].id = -1001;
+    siteCategories[1].name = "Parent B";
+
+    siteCategories[2].parent_category_id = null;
+    siteCategories[2].id = -1002;
+    siteCategories[2].name = "Parent A";
+
+    siteCategories[3].parent_category_id = -1001;
+    siteCategories[3].id = -1003;
+    siteCategories[3].name = "Parent B Child B";
+
+    siteCategories[4].parent_category_id = -1002;
+    siteCategories[4].id = -1004;
+    siteCategories[4].name = "Parent A Child A";
+
+    siteCategories[5].parent_category_id = -1000;
+    siteCategories[5].id = -1005;
+    siteCategories[5].name = "Parent B Child A Child A";
+
+    site.categoriesById.clear();
+
+    siteCategories.forEach((category) => {
+      site.categoriesById[category.id] = category;
+    });
+
+    updateCurrentUser({
+      sidebar_category_ids: [-1005, -1004, -1003, -1002, -1000],
+    });
 
     await visit("/");
 
@@ -212,7 +245,139 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
 
     assert.deepEqual(
       categoryNames,
-      ["abc", "aBC", "efg", "Sub Category"],
+      [
+        "Parent A",
+        "Parent A Child A",
+        "Parent B Child A",
+        "Parent B Child A Child A",
+        "Parent B Child B",
+      ],
+      "category section links are displayed in the right order"
+    );
+  });
+
+  test("category section links are ordered by default order of site categories with child category sorted after parent category when site setting to fix category's position is enabled", async function (assert) {
+    this.siteSettings.fixed_category_positions = true;
+
+    const site = Site.current();
+    const siteCategories = site.categories;
+
+    siteCategories[0].parent_category_id = -1001;
+    siteCategories[0].id = -1000;
+    siteCategories[0].name = "Parent A Child A";
+
+    siteCategories[1].parent_category_id = null;
+    siteCategories[1].id = -1001;
+    siteCategories[1].name = "Parent A";
+
+    siteCategories[2].parent_category_id = null;
+    siteCategories[2].id = -1002;
+    siteCategories[2].name = "Parent B";
+
+    siteCategories[3].parent_category_id = -1001;
+    siteCategories[3].id = -1003;
+    siteCategories[3].name = "Parent A Child B";
+
+    siteCategories[4].parent_category_id = -1002;
+    siteCategories[4].id = -1004;
+    siteCategories[4].name = "Parent B Child A";
+
+    siteCategories[5].parent_category_id = -1000;
+    siteCategories[5].id = -1005;
+    siteCategories[5].name = "Parent A Child A Child A";
+
+    site.categoriesById.clear();
+
+    siteCategories.forEach((category) => {
+      site.categoriesById[category.id] = category;
+    });
+
+    updateCurrentUser({
+      sidebar_category_ids: [-1005, -1004, -1003, -1002, -1000],
+    });
+
+    await visit("/");
+
+    const categorySectionLinks = queryAll(
+      ".sidebar-section-categories .sidebar-section-link:not(.sidebar-section-link-all-categories)"
+    );
+
+    const categoryNames = [...categorySectionLinks].map((categorySectionLink) =>
+      categorySectionLink.textContent.trim()
+    );
+
+    assert.deepEqual(
+      categoryNames,
+      [
+        "Parent A Child A",
+        "Parent A Child A Child A",
+        "Parent A Child B",
+        "Parent B",
+        "Parent B Child A",
+      ],
+      "category section links are displayed in the right order"
+    );
+  });
+
+  test("category section links are ordered by position when site setting to fix category's position is enabled", async function (assert) {
+    this.siteSettings.fixed_category_positions = true;
+
+    const site = Site.current();
+    const siteCategories = site.categories;
+
+    siteCategories[0].parent_category_id = -1001;
+    siteCategories[0].id = -1000;
+    siteCategories[0].name = "Parent A Child A";
+
+    siteCategories[1].parent_category_id = null;
+    siteCategories[1].id = -1001;
+    siteCategories[1].name = "Parent A";
+
+    siteCategories[2].parent_category_id = null;
+    siteCategories[2].id = -1002;
+    siteCategories[2].name = "Parent B";
+
+    siteCategories[3].parent_category_id = -1001;
+    siteCategories[3].id = -1003;
+    siteCategories[3].name = "Parent A Child B";
+
+    siteCategories[4].parent_category_id = -1002;
+    siteCategories[4].id = -1004;
+    siteCategories[4].name = "Parent B Child A";
+
+    siteCategories[5].parent_category_id = -1000;
+    siteCategories[5].id = -1005;
+    siteCategories[5].name = "Parent A Child A Child A";
+
+    site.categoriesById.clear();
+
+    siteCategories.forEach((category) => {
+      site.categoriesById[category.id] = category;
+    });
+
+    updateCurrentUser({
+      sidebar_category_ids: [-1005, -1004, -1003, -1002, -1000],
+    });
+
+    await visit("/");
+
+    const categorySectionLinks = queryAll(
+      ".sidebar-section-categories .sidebar-section-link:not(.sidebar-section-link-all-categories)"
+    );
+
+    const categoryNames = [...categorySectionLinks].map((categorySectionLink) =>
+      categorySectionLink.textContent.trim()
+    );
+
+    assert.deepEqual(
+      categoryNames,
+      [
+        "Parent A Child A",
+        "Parent A Child A Child A",
+        "Parent A Child B",
+        "Parent B",
+        "Parent B Child A",
+      ],
       "category section links are displayed in the right order"
     );
   });

app/assets/javascripts/discourse/tests/unit/models/category-test.js

@@ -401,4 +401,30 @@ module("Unit | Model | category", function () {
       "ignores case of category slug and search term"
     );
   });
+
+  test("sortCategories returns categories with child categories sorted after parent categories", function (assert) {
+    const categories = [
+      { id: 1003, name: "Test Sub Sub", parent_category_id: 1002 },
+      { id: 1001, name: "Test" },
+      { id: 1004, name: "Test Sub Sub Sub", parent_category_id: 1003 },
+      { id: 1002, name: "Test Sub", parent_category_id: 1001 },
+      { id: 1005, name: "Test Sub Sub Sub2", parent_category_id: 1003 },
+      { id: 1006, name: "Test2" },
+      { id: 1000, name: "Test2 Sub", parent_category_id: 1006 },
+      { id: 997, name: "Test2 Sub Sub2", parent_category_id: 1000 },
+      { id: 999, name: "Test2 Sub Sub", parent_category_id: 1000 },
+    ];
+
+    assert.deepEqual(Category.sortCategories(categories).mapBy("name"), [
+      "Test",
+      "Test Sub",
+      "Test Sub Sub",
+      "Test Sub Sub Sub",
+      "Test Sub Sub Sub2",
+      "Test2",
+      "Test2 Sub",
+      "Test2 Sub Sub2",
+      "Test2 Sub Sub",
+    ]);
+  });
 });

app/assets/javascripts/discourse/tests/unit/models/site-test.js

@@ -77,26 +77,47 @@ module("Unit | Model | site", function () {
     );
   });
 
-  test("deeply nested categories", function (assert) {
+  test("sortedCategories returns categories sorted by topic counts and sorts child categories after parent", function (assert) {
     const store = createStore();
     const site = store.createRecord("site", {
       categories: [
-        { id: 1003, name: "Test Sub Sub", parent_category_id: 1002 },
-        { id: 1001, name: "Test" },
+        {
+          id: 1003,
+          name: "Test Sub Sub",
+          parent_category_id: 1002,
+          topic_count: 0,
+        },
+        { id: 1001, name: "Test", topic_count: 1 },
         { id: 1004, name: "Test Sub Sub Sub", parent_category_id: 1003 },
-        { id: 1002, name: "Test Sub", parent_category_id: 1001 },
-        { id: 1005, name: "Test Sub Sub Sub2", parent_category_id: 1003 },
-        { id: 1006, name: "Test2" },
+        {
+          id: 1002,
+          name: "Test Sub",
+          parent_category_id: 1001,
+          topic_count: 0,
+        },
+        {
+          id: 1005,
+          name: "Test Sub Sub Sub2",
+          parent_category_id: 1003,
+          topic_count: 1,
+        },
+        { id: 1006, name: "Test2", topic_count: 2 },
+        { id: 1000, name: "Test2 Sub", parent_category_id: 1006 },
+        { id: 997, name: "Test2 Sub Sub2", parent_category_id: 1000 },
+        { id: 999, name: "Test2 Sub Sub", parent_category_id: 1000 },
       ],
     });
 
     assert.deepEqual(site.sortedCategories.mapBy("name"), [
+      "Test2",
+      "Test2 Sub",
+      "Test2 Sub Sub2",
+      "Test2 Sub Sub",
       "Test",
       "Test Sub",
       "Test Sub Sub",
-      "Test Sub Sub Sub",
       "Test Sub Sub Sub2",
-      "Test2",
+      "Test Sub Sub Sub",
     ]);
   });
 });