FIX: Make edit categories sidebar modal work more intuitively (#27111)

* Load search results in displayed order so that when more categories are loaded on scroll, they appear at the end,
 * Limit the number of subcategories that are shown per category and display 'show more' links,

app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-menu/categories-modal.gjs

@@ -6,7 +6,8 @@ import { action } from "@ember/object";
 import didInsert from "@ember/render-modifiers/modifiers/did-insert";
 import { service } from "@ember/service";
 import { TrackedSet } from "@ember-compat/tracked-built-ins";
-import { gt, has, includes, not } from "truth-helpers";
+import { eq, gt, has } from "truth-helpers";
+import DButton from "discourse/components/d-button";
 import EditNavigationMenuModal from "discourse/components/sidebar/edit-navigation-menu/modal";
 import borderColor from "discourse/helpers/border-color";
 import categoryBadge from "discourse/helpers/category-badge";
@@ -18,6 +19,45 @@ import { INPUT_DELAY } from "discourse-common/config/environment";
 import i18n from "discourse-common/helpers/i18n";
 import discourseDebounce from "discourse-common/lib/debounce";
 
+class ActionSerializer {
+  constructor(perform) {
+    this.perform = perform;
+    this.processing = false;
+    this.queued = false;
+  }
+
+  async trigger() {
+    this.queued = true;
+
+    if (!this.processing) {
+      this.processing = true;
+
+      while (this.queued) {
+        this.queued = false;
+        await this.perform();
+      }
+
+      this.processing = false;
+    }
+  }
+}
+
+// Given an async method that takes no parameters, produce a method that
+// triggers the original method only if it is not currently executing it,
+// otherwise it will queue up to one execution of the method
+function serialized(target, key, descriptor) {
+  const originalMethod = descriptor.value;
+
+  descriptor.value = function () {
+    this[`_${key}_serializer`] ||= new ActionSerializer(() =>
+      originalMethod.apply(this)
+    );
+    this[`_${key}_serializer`].trigger();
+  };
+
+  return descriptor;
+}
+
 // Given a list, break into chunks starting a new chunk whenever the predicate
 // is true for an element.
 function splitWhere(elements, f) {
@@ -30,20 +70,39 @@ function splitWhere(elements, f) {
   }, []);
 }
 
-function findAncestors(categories) {
-  let categoriesToCheck = categories;
-  const ancestors = [];
+// categories must be topologically sorted so that the parents appear before
+// the children
+function findPartialCategories(categories) {
+  const categoriesById = new Map(
+    categories.map((category) => [category.id, category])
+  );
+  const subcategoryCounts = new Map();
+  const subcategoryCountsRecursive = new Map();
+  const partialCategoryInfos = new Map();
 
-  for (let i = 0; i < 3; i++) {
-    categoriesToCheck = categoriesToCheck
-      .map((c) => Category.findById(c.parent_category_id))
-      .filter(Boolean)
-      .uniqBy((c) => c.id);
+  for (const category of categories.slice().reverse()) {
+    const count = subcategoryCounts.get(category.parent_category_id) || 0;
+    subcategoryCounts.set(category.parent_category_id, count + 1);
 
-    ancestors.push(...categoriesToCheck);
+    const recursiveCount =
+      subcategoryCountsRecursive.get(category.parent_category_id) || 0;
+
+    subcategoryCountsRecursive.set(
+      category.parent_category_id,
+      recursiveCount + (subcategoryCountsRecursive.get(category.id) || 0) + 1
+    );
   }
 
-  return ancestors;
+  for (const [id, count] of subcategoryCounts) {
+    if (count === 5 && categoriesById.has(id)) {
+      partialCategoryInfos.set(id, {
+        level: categoriesById.get(id).level + 1,
+        offset: subcategoryCountsRecursive.get(id),
+      });
+    }
+  }
+
+  return partialCategoryInfos;
 }
 
 export default class SidebarEditNavigationMenuCategoriesModal extends Component {
@@ -52,20 +111,19 @@ export default class SidebarEditNavigationMenuCategoriesModal extends Component
   @service siteSettings;
 
   @tracked initialLoad = true;
-  @tracked filteredCategoriesGroupings = [];
-  @tracked filteredCategoryIds = [];
+  @tracked fetchedCategoriesGroupings = [];
   @tracked
-  selectedSidebarCategoryIds = new TrackedSet([
+  selectedCategoryIds = new TrackedSet([
     ...this.currentUser.sidebar_category_ids,
   ]);
-  hasMorePages;
+  selectedFilter = "";
+  selectedMode = "everything";
   loadedFilter;
   loadedMode;
   loadedPage;
-  processing = false;
-  requestedFilter;
-  requestedMode;
   saving = false;
+  loadAnotherPage = false;
+  unseenCategoryIdsChanged = false;
   observer = new IntersectionObserver(
     ([entry]) => {
       if (entry.isIntersecting) {
@@ -80,45 +138,102 @@ export default class SidebarEditNavigationMenuCategoriesModal extends Component
 
   constructor() {
     super(...arguments);
-    this.setFilterAndMode("", "everything");
+    this.subcategoryLoadList = [];
+    this.performSearch();
   }
 
-  setFilteredCategories(categories) {
-    this.filteredCategories = categories;
-    const ancestors = findAncestors(categories);
-    const allCategories = categories.concat(ancestors).uniqBy((c) => c.id);
+  recomputeGroupings() {
+    const categoriesWithShowMores = this.fetchedCategories.flatMap((el, i) => {
+      const result = [{ type: "category", category: el }];
 
-    this.filteredCategoriesGroupings = splitWhere(
-      Category.sortCategories(allCategories),
-      (category) => category.parent_category_id === undefined
-    );
+      const elID = el.id;
+      const elParentID = el.parent_category_id;
+      const nextParentID = this.fetchedCategories[i + 1]?.parent_category_id;
 
-    this.filteredCategoryIds = categories.map((c) => c.id);
-  }
+      const nextIsSibling = nextParentID === elParentID;
+      const nextIsChild = nextParentID === elID;
 
-  concatFilteredCategories(categories) {
-    this.setFilteredCategories(this.filteredCategories.concat(categories));
-  }
+      if (
+        !nextIsSibling &&
+        !nextIsChild &&
+        this.partialCategoryInfos.has(elParentID)
+      ) {
+        const { level, offset } = this.partialCategoryInfos.get(elParentID);
 
-  setFetchedCategories(mode, categories) {
-    this.setFilteredCategories(this.applyMode(mode, categories));
-  }
-
-  concatFetchedCategories(mode, categories) {
-    this.concatFilteredCategories(this.applyMode(mode, categories));
-  }
-
-  applyMode(mode, categories) {
-    return categories.filter((c) => {
-      switch (mode) {
-        case "everything":
-          return true;
-        case "only-selected":
-          return this.selectedSidebarCategoryIds.has(c.id);
-        case "only-unselected":
-          return !this.selectedSidebarCategoryIds.has(c.id);
+        result.push({
+          type: "show-more",
+          id: elParentID,
+          level,
+          offset,
+        });
       }
-    });
+
+      return result;
+    }, []);
+
+    this.fetchedCategoriesGroupings = splitWhere(
+      categoriesWithShowMores,
+      (c) =>
+        c.type === "category" && c.category.parent_category_id === undefined
+    );
+  }
+
+  setFetchedCategories(categories) {
+    this.fetchedCategories = categories;
+    this.partialCategoryInfos = findPartialCategories(categories);
+    this.recomputeGroupings();
+  }
+
+  concatFetchedCategories(categories) {
+    this.fetchedCategories = this.fetchedCategories.concat(categories);
+
+    // In order to find partially loaded categories correctly, we need to
+    // ensure that we account for categories that may have been partially
+    // loaded, because the total number of categories in the response clipped
+    // them off.
+    if (categories[0].parent_category_id !== undefined) {
+      const index = this.fetchedCategories.findLastIndex(
+        (element) => element.parent_category_id === undefined
+      );
+
+      categories = [...this.fetchedCategories.slice(index), ...categories];
+    }
+
+    this.partialCategoryInfos = new Map([
+      ...this.partialCategoryInfos,
+      ...findPartialCategories(categories),
+    ]);
+
+    this.recomputeGroupings();
+  }
+
+  substituteInFetchedCategories(id, subcategories, offset) {
+    this.partialCategoryInfos.delete(id);
+    this.recomputeGroupings();
+
+    if (subcategories.length !== 0) {
+      const index =
+        this.fetchedCategories.findLastIndex(
+          (c) => c.parent_category_id === id
+        ) + 1;
+
+      this.fetchedCategories = [
+        ...this.fetchedCategories.slice(0, index),
+        ...subcategories,
+        ...this.fetchedCategories.slice(index),
+      ];
+
+      this.partialCategoryInfos = new Map([
+        ...this.partialCategoryInfos,
+        ...findPartialCategories(subcategories),
+      ]);
+
+      this.partialCategoryInfos.set(id, {
+        offset: offset + subcategories.length,
+      });
+
+      this.recomputeGroupings();
+    }
   }
 
   @action
@@ -127,136 +242,149 @@ export default class SidebarEditNavigationMenuCategoriesModal extends Component
     this.observer.observe(element);
   }
 
-  async searchCategories(filter, mode) {
-    if (filter === "" && mode === "only-selected") {
-      this.setFilteredCategories(
-        await Category.asyncFindByIds([...this.selectedSidebarCategoryIds])
-      );
+  searchOpts() {
+    const requestedMode = this.selectedMode;
+    const requestedCategoryIds = [...this.selectedCategoryIds];
+    const opts = { includeUncategorized: false };
 
-      this.loadedPage = null;
-      this.hasMorePages = false;
-    } else {
-      const { categories } = await Category.asyncSearch(filter, {
-        includeAncestors: true,
-        includeUncategorized: false,
-      });
-
-      this.setFetchedCategories(mode, categories);
-
-      this.loadedPage = 1;
-      this.hasMorePages = true;
+    if (requestedMode === "only-selected") {
+      opts.only = requestedCategoryIds;
+    } else if (requestedMode === "only-unselected") {
+      opts.except = requestedCategoryIds;
     }
+
+    return opts;
   }
 
-  async setFilterAndMode(newFilter, newMode) {
-    this.requestedFilter = newFilter;
-    this.requestedMode = newMode;
+  @serialized
+  async performSearch() {
+    const requestedFilter = this.selectedFilter;
+    const requestedMode = this.selectedMode;
+    const selectedCategoriesNeedsUpdate =
+      this.unseenCategoryIdsChanged && requestedMode !== "everything";
 
-    if (!this.processing) {
-      this.processing = true;
+    // Is the current set of displayed categories up-to-date?
+    if (
+      requestedFilter === this.loadedFilter &&
+      requestedMode === this.loadedMode &&
+      !selectedCategoriesNeedsUpdate
+    ) {
+      // The shown categories are up-to-date, so we can do elaboration
+      if (this.loadAnotherPage && !this.lastPage) {
+        const requestedPage = this.loadedPage + 1;
+        const opts = { page: requestedPage, ...this.searchOpts() };
 
-      try {
-        while (
-          this.loadedFilter !== this.requestedFilter ||
-          this.loadedMode !== this.requestedMode
-        ) {
-          const filter = this.requestedFilter;
-          const mode = this.requestedMode;
+        const categories = await Category.asyncHierarchicalSearch(
+          requestedFilter,
+          opts
+        );
 
-          await this.searchCategories(filter, mode);
-
-          this.loadedFilter = filter;
-          this.loadedMode = mode;
-          this.initialLoad = false;
+        if (categories.length === 0) {
+          this.lastPage = true;
+        } else {
+          this.concatFetchedCategories(categories);
         }
-      } finally {
-        this.processing = false;
+
+        this.loadAnotherPage = false;
+        this.loadedPage = requestedPage;
+      } else if (this.subcategoryLoadList.length !== 0) {
+        const { id, offset } = this.subcategoryLoadList.shift();
+        const opts = { parentCategoryId: id, offset, ...this.searchOpts() };
+
+        let subcategories = await Category.asyncHierarchicalSearch(
+          requestedFilter,
+          opts
+        );
+
+        this.substituteInFetchedCategories(id, subcategories, offset);
       }
+    } else {
+      // The shown categories are stale, refresh everything
+      const requestedCategoryIds = [...this.selectedCategoryIds];
+      this.unseenCategoryIdsChanged = false;
+
+      this.setFetchedCategories(
+        await Category.asyncHierarchicalSearch(
+          requestedFilter,
+          this.searchOpts()
+        )
+      );
+
+      this.loadedFilter = requestedFilter;
+      this.loadedMode = requestedMode;
+      this.loadedCategoryIds = requestedCategoryIds;
+      this.loadedPage = 1;
+      this.lastPage = false;
+      this.initialLoad = false;
+      this.loadAnotherPage = false;
     }
   }
 
   async loadMore() {
-    if (!this.processing && this.hasMorePages) {
-      this.processing = true;
-
-      try {
-        const page = this.loadedPage + 1;
-        const { categories } = await Category.asyncSearch(
-          this.requestedFilter,
-          {
-            includeAncestors: true,
-            includeUncategorized: false,
-            page,
-          }
-        );
-        this.loadedPage = page;
-
-        if (categories.length === 0) {
-          this.hasMorePages = false;
-        } else {
-          this.concatFetchedCategories(this.requestedMode, categories);
-        }
-      } finally {
-        this.processing = false;
-      }
-
-      if (
-        this.loadedFilter !== this.requestedFilter ||
-        this.loadedMode !== this.requestedMode
-      ) {
-        await this.setFilterAndMode(this.requestedFilter, this.requestedMode);
-      }
-    }
+    this.loadAnotherPage = true;
+    this.debouncedSendRequest();
   }
 
-  debouncedSetFilterAndMode(filter, mode) {
-    discourseDebounce(this, this.setFilterAndMode, filter, mode, INPUT_DELAY);
+  @action
+  async loadSubcategories(id, offset) {
+    this.subcategoryLoadList.push({ id, offset });
+    this.debouncedSendRequest();
+  }
+
+  debouncedSendRequest() {
+    discourseDebounce(this, this.performSearch, INPUT_DELAY);
   }
 
   @action
   resetFilter() {
-    this.debouncedSetFilterAndMode(this.requestedFilter, "everything");
+    this.selectedMode = "everything";
+    this.debouncedSendRequest();
   }
 
   @action
   filterSelected() {
-    this.debouncedSetFilterAndMode(this.requestedFilter, "only-selected");
+    this.selectedMode = "only-selected";
+    this.debouncedSendRequest();
   }
 
   @action
   filterUnselected() {
-    this.debouncedSetFilterAndMode(this.requestedFilter, "only-unselected");
+    this.selectedMode = "only-unselected";
+    this.debouncedSendRequest();
   }
 
   @action
   onFilterInput(filter) {
-    this.debouncedSetFilterAndMode(
-      filter.toLowerCase().trim(),
-      this.requestedMode
-    );
+    this.selectedFilter = filter.toLowerCase().trim();
+    this.debouncedSendRequest();
   }
 
   @action
   deselectAll() {
-    this.selectedSidebarCategoryIds.clear();
+    this.selectedCategoryIds.clear();
+    this.unseenCategoryIdsChanged = true;
+    this.debouncedSendRequest();
   }
 
   @action
   toggleCategory(categoryId) {
-    if (this.selectedSidebarCategoryIds.has(categoryId)) {
-      this.selectedSidebarCategoryIds.delete(categoryId);
+    if (this.selectedCategoryIds.has(categoryId)) {
+      this.selectedCategoryIds.delete(categoryId);
     } else {
-      this.selectedSidebarCategoryIds.add(categoryId);
+      this.selectedCategoryIds.add(categoryId);
     }
   }
 
   @action
   resetToDefaults() {
-    this.selectedSidebarCategoryIds = new TrackedSet(
+    this.selectedCategoryIds = new TrackedSet(
       this.siteSettings.default_navigation_menu_categories
         .split("|")
         .map((id) => parseInt(id, 10))
     );
+
+    this.unseenCategoryIdsChanged = true;
+    this.debouncedSendRequest();
   }
 
   @action
@@ -264,9 +392,7 @@ export default class SidebarEditNavigationMenuCategoriesModal extends Component
     this.saving = true;
     const initialSidebarCategoryIds = this.currentUser.sidebar_category_ids;
 
-    this.currentUser.set("sidebar_category_ids", [
-      ...this.selectedSidebarCategoryIds,
-    ]);
+    this.currentUser.set("sidebar_category_ids", [...this.selectedCategoryIds]);
 
     try {
       await this.currentUser.save(["sidebar_category_ids"]);
@@ -307,60 +433,74 @@ export default class SidebarEditNavigationMenuCategoriesModal extends Component
             {{loadingSpinner size="small"}}
           </div>
         {{else}}
-          {{#each this.filteredCategoriesGroupings as |categories|}}
+          {{#each this.fetchedCategoriesGroupings as |categories|}}
             <div
-              {{didInsert this.didInsert}}
-              style={{borderColor (get categories "0.color") "left"}}
+              style={{borderColor (get categories "0.category.color") "left"}}
               class="sidebar-categories-form__row"
             >
-              {{#each categories as |category|}}
-                <div
-                  data-category-id={{category.id}}
-                  data-category-level={{category.level}}
-                  class="sidebar-categories-form__category-row"
-                >
-                  <label
-                    for={{concat
-                      "sidebar-categories-form__input--"
-                      category.id
-                    }}
-                    class="sidebar-categories-form__category-label"
+              {{#each categories as |c|}}
+                {{#if (eq c.type "category")}}
+                  <div
+                    {{didInsert this.didInsert}}
+                    data-category-id={{c.category.id}}
+                    data-category-level={{c.category.level}}
+                    class="sidebar-categories-form__category-row"
                   >
-                    <div class="sidebar-categories-form__category-wrapper">
-                      <div class="sidebar-categories-form__category-badge">
-                        {{categoryBadge category}}
+                    <label
+                      for={{concat
+                        "sidebar-categories-form__input--"
+                        c.category.id
+                      }}
+                      class="sidebar-categories-form__category-label"
+                    >
+                      <div class="sidebar-categories-form__category-wrapper">
+                        <div class="sidebar-categories-form__category-badge">
+                          {{categoryBadge c.category}}
+                        </div>
+
+                        {{#unless c.category.parentCategory}}
+                          <div
+                            class="sidebar-categories-form__category-description"
+                          >
+                            {{dirSpan
+                              c.category.description_excerpt
+                              htmlSafe="true"
+                            }}
+                          </div>
+                        {{/unless}}
                       </div>
 
-                      {{#unless category.parentCategory}}
-                        <div
-                          class="sidebar-categories-form__category-description"
-                        >
-                          {{dirSpan
-                            category.description_excerpt
-                            htmlSafe="true"
-                          }}
+                      <input
+                        {{on "click" (fn this.toggleCategory c.category.id)}}
+                        type="checkbox"
+                        checked={{has this.selectedCategoryIds c.category.id}}
+                        id={{concat
+                          "sidebar-categories-form__input--"
+                          c.category.id
+                        }}
+                        class="sidebar-categories-form__input"
+                      />
+                    </label>
+                  </div>
+                {{else}}
+                  <div
+                    {{didInsert this.didInsert}}
+                    data-category-level={{c.level}}
+                    class="sidebar-categories-form__category-row"
+                  >
+                    <label class="sidebar-categories-form__category-label">
+                      <div class="sidebar-categories-form__category-wrapper">
+                        <div class="sidebar-categories-form__category-badge">
+                          <DButton
+                            @label="sidebar.categories_form_modal.show_more"
+                            @action={{fn this.loadSubcategories c.id c.offset}}
+                            class="btn-flat"
+                          />
                         </div>
-                      {{/unless}}
-                    </div>
-
-                    <input
-                      {{on "click" (fn this.toggleCategory category.id)}}
-                      type="checkbox"
-                      checked={{has
-                        this.selectedSidebarCategoryIds
-                        category.id
-                      }}
-                      disabled={{not
-                        (includes this.filteredCategoryIds category.id)
-                      }}
-                      id={{concat
-                        "sidebar-categories-form__input--"
-                        category.id
-                      }}
-                      class="sidebar-categories-form__input"
-                    />
-                  </label>
-                </div>
+                      </div>
+                    </label>
+                  </div>
+                {{/if}}
               {{/each}}
             </div>
           {{else}}

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

@@ -14,6 +14,7 @@ import { MultiCache } from "discourse-common/utils/multi-cache";
 
 const STAFF_GROUP_NAME = "staff";
 const CATEGORY_ASYNC_SEARCH_CACHE = {};
+const CATEGORY_ASYNC_HIERARCHICAL_SEARCH_CACHE = {};
 
 export default class Category extends RestModel {
   // Sort subcategories directly under parents
@@ -385,6 +386,32 @@ export default class Category extends RestModel {
     return data.sortBy("read_restricted");
   }
 
+  static async asyncHierarchicalSearch(term, opts) {
+    opts ||= {};
+
+    const data = {
+      term,
+      parent_category_id: opts.parentCategoryId,
+      limit: opts.limit,
+      only: opts.only,
+      except: opts.except,
+      page: opts.page,
+      offset: opts.offset,
+      include_uncategorized: opts.includeUncategorized,
+    };
+
+    const result = (CATEGORY_ASYNC_HIERARCHICAL_SEARCH_CACHE[
+      JSON.stringify(data)
+    ] ||= await ajax("/categories/hierarchical_search", {
+      method: "GET",
+      data,
+    }));
+
+    return result["categories"].map((category) =>
+      Site.current().updateCategory(category)
+    );
+  }
+
   static async asyncSearch(term, opts) {
     opts ||= {};
 

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

@@ -91,8 +91,8 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
       return helper.response(cloneJSON(categoryFixture["/c/1/show.json"]));
     });
 
-    server.post("/categories/search", () => {
-      return helper.response({ categories: [], ancestors: [] });
+    server.get("/categories/hierarchical_search", () => {
+      return helper.response({ categories: [] });
     });
   });