DEV: Async category search for sidebar modal (#25686)

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

@@ -9,6 +9,7 @@ import EditNavigationMenuModal from "discourse/components/sidebar/edit-navigatio
 import borderColor from "discourse/helpers/border-color";
 import categoryBadge from "discourse/helpers/category-badge";
 import dirSpan from "discourse/helpers/dir-span";
+import loadingSpinner from "discourse/helpers/loading-spinner";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import Category from "discourse/models/category";
 import { INPUT_DELAY } from "discourse-common/config/environment";
@@ -18,126 +19,145 @@ import gt from "truth-helpers/helpers/gt";
 import includes from "truth-helpers/helpers/includes";
 import not from "truth-helpers/helpers/not";
 
+// Given a list, break into chunks starting a new chunk whenever the predicate
+// is true for an element.
+function splitWhere(elements, f) {
+  return elements.reduce((acc, el, i) => {
+    if (i === 0 || f(el)) {
+      acc.push([]);
+    }
+    acc[acc.length - 1].push(el);
+    return acc;
+  }, []);
+}
+
+function findAncestors(categories) {
+  let categoriesToCheck = categories;
+  const ancestors = [];
+
+  for (let i = 0; i < 3; i++) {
+    categoriesToCheck = categoriesToCheck
+      .map((c) => Category.findById(c.parent_category_id))
+      .filter(Boolean)
+      .uniqBy((c) => c.id);
+
+    ancestors.push(...categoriesToCheck);
+  }
+
+  return ancestors;
+}
+
 export default class extends Component {
   @service currentUser;
   @service site;
   @service siteSettings;
 
-  @tracked filter = "";
-  @tracked filteredCategoryIds;
-  @tracked onlySelected = false;
-  @tracked onlyUnselected = false;
+  @tracked initialLoad = true;
+  @tracked filteredCategoriesGroupings = [];
+  @tracked filteredCategoryIds = [];
 
   @tracked
   selectedSidebarCategoryIds = [...this.currentUser.sidebar_category_ids];
 
-  categoryGroupings = [];
-
   constructor() {
     super(...arguments);
 
-    let categories = [...this.site.categories];
-
-    if (!this.siteSettings.fixed_category_positions) {
-      categories.sort((a, b) => a.name.localeCompare(b.name));
-    }
-
-    Category.sortCategories(categories).reduce(
-      (categoryGrouping, category, index, arr) => {
-        if (category.isUncategorizedCategory) {
-          return categoryGrouping;
-        }
-
-        categoryGrouping.push(category);
-
-        const nextCategory = arr[index + 1];
-
-        if (!nextCategory || nextCategory.level === 0) {
-          this.categoryGroupings.push(categoryGrouping);
-          return [];
-        }
-
-        return categoryGrouping;
-      },
-      []
-    );
+    this.processing = false;
+    this.setFilterAndMode("", "everything");
   }
 
-  get filteredCategoriesGroupings() {
-    const filteredCategoryIds = new Set();
+  setFilteredCategories(categories) {
+    const ancestors = findAncestors(categories);
+    const allCategories = categories.concat(ancestors).uniqBy((c) => c.id);
 
-    const groupings = this.categoryGroupings.reduce((acc, categoryGrouping) => {
-      const filteredCategories = new Set();
+    if (this.siteSettings.fixed_category_positions) {
+      allCategories.sort((a, b) => a.position - b.position);
+    } else {
+      allCategories.sort((a, b) => a.name.localeCompare(b.name));
+    }
 
-      const addCategory = (category) => {
-        if (this.#matchesFilter(category)) {
-          if (category.parentCategory?.parentCategory) {
-            filteredCategories.add(category.parentCategory.parentCategory);
-          }
+    this.filteredCategoriesGroupings = splitWhere(
+      Category.sortCategories(allCategories),
+      (category) => category.parent_category_id === undefined
+    );
 
-          if (category.parentCategory) {
-            filteredCategories.add(category.parentCategory);
-          }
+    this.filteredCategoryIds = categories.map((c) => c.id);
+  }
 
-          filteredCategoryIds.add(category.id);
-          filteredCategories.add(category);
-        }
-      };
+  async searchCategories(filter, mode) {
+    if (filter === "" && mode === "only-selected") {
+      this.setFilteredCategories(
+        await Category.asyncFindByIds(this.selectedSidebarCategoryIds)
+      );
+    } else {
+      const { categories } = await Category.asyncSearch(filter, {
+        includeAncestors: true,
+        includeUncategorized: false,
+      });
 
-      categoryGrouping.forEach((category) => {
-        if (this.onlySelected) {
-          if (this.selectedSidebarCategoryIds.includes(category.id)) {
-            addCategory(category);
-          }
-        } else if (this.onlyUnselected) {
-          if (!this.selectedSidebarCategoryIds.includes(category.id)) {
-            addCategory(category);
-          }
-        } else {
-          addCategory(category);
+      const filteredFetchedCategories = categories.filter((c) => {
+        switch (mode) {
+          case "everything":
+            return true;
+          case "only-selected":
+            return this.selectedSidebarCategoryIds.includes(c.id);
+          case "only-unselected":
+            return !this.selectedSidebarCategoryIds.includes(c.id);
         }
       });
 
-      if (filteredCategories.size > 0) {
-        acc.push(Array.from(filteredCategories));
-      }
-
-      return acc;
-    }, []);
-
-    this.filteredCategoryIds = Array.from(filteredCategoryIds);
-    return groupings;
+      this.setFilteredCategories(filteredFetchedCategories);
+    }
   }
 
-  #matchesFilter(category) {
-    return this.filter.length === 0 || category.nameLower.includes(this.filter);
+  async setFilterAndMode(newFilter, newMode) {
+    this.filter = newFilter;
+    this.mode = newMode;
+
+    if (!this.processing) {
+      this.processing = true;
+
+      try {
+        while (true) {
+          const filter = this.filter;
+          const mode = this.mode;
+
+          await this.searchCategories(filter, mode);
+
+          this.initialLoad = false;
+
+          if (filter === this.filter && mode === this.mode) {
+            break;
+          }
+        }
+      } finally {
+        this.processing = false;
+      }
+    }
+  }
+
+  debouncedSetFilterAndMode(filter, mode) {
+    discourseDebounce(this, this.setFilterAndMode, filter, mode, INPUT_DELAY);
   }
 
   @action
   resetFilter() {
-    this.onlySelected = false;
-    this.onlyUnselected = false;
+    this.debouncedSetFilterAndMode(this.filter, "everything");
   }
 
   @action
   filterSelected() {
-    this.onlySelected = true;
-    this.onlyUnselected = false;
+    this.debouncedSetFilterAndMode(this.filter, "only-selected");
   }
 
   @action
   filterUnselected() {
-    this.onlySelected = false;
-    this.onlyUnselected = true;
+    this.debouncedSetFilterAndMode(this.filter, "only-unselected");
   }
 
   @action
   onFilterInput(filter) {
-    discourseDebounce(this, this.#performFiltering, filter, INPUT_DELAY);
-  }
-
-  #performFiltering(filter) {
-    this.filter = filter.toLowerCase();
+    this.debouncedSetFilterAndMode(filter.toLowerCase().trim(), this.mode);
   }
 
   @action
@@ -209,7 +229,11 @@ export default class extends Component {
       class="sidebar__edit-navigation-menu__categories-modal"
     >
       <form class="sidebar-categories-form">
-        {{#if (gt this.filteredCategoriesGroupings.length 0)}}
+        {{#if this.initialLoad}}
+          <div class="sidebar-categories-form__loading">
+            {{loadingSpinner size="small"}}
+          </div>
+        {{else if (gt this.filteredCategoriesGroupings.length 0)}}
           {{#each this.filteredCategoriesGroupings as |categories|}}
             <div
               class="sidebar-categories-form__row"

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

@@ -365,6 +365,7 @@ export default class Category extends RestModel {
       select_category_ids: opts.selectCategoryIds,
       reject_category_ids: opts.rejectCategoryIds,
       include_subcategories: opts.includeSubcategories,
+      include_ancestors: opts.includeAncestors,
       prioritized_category_id: opts.prioritizedCategoryId,
       limit: opts.limit,
     };
@@ -372,9 +373,20 @@ export default class Category extends RestModel {
     const result = (CATEGORY_ASYNC_SEARCH_CACHE[JSON.stringify(data)] ||=
       await ajax("/categories/search", { data }));
 
-    return result["categories"].map((category) =>
-      Site.current().updateCategory(category)
-    );
+    if (opts.includeAncestors) {
+      return {
+        ancestors: result["ancestors"].map((category) =>
+          Site.current().updateCategory(category)
+        ),
+        categories: result["categories"].map((category) =>
+          Site.current().updateCategory(category)
+        ),
+      };
+    } else {
+      return result["categories"].map((category) =>
+        Site.current().updateCategory(category)
+      );
+    }
   }
 
   permissions = null;

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

@@ -90,6 +90,10 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
     server.get("/c/:categorySlug/:categoryId/find_by_slug.json", () => {
       return helper.response(cloneJSON(categoryFixture["/c/1/show.json"]));
     });
+
+    server.get("/categories/search", () => {
+      return helper.response({ categories: [], ancestors: [] });
+    });
   });
 
   const setupUserSidebarCategories = function () {

app/controllers/categories_controller.rb

@@ -341,6 +341,12 @@ class CategoriesController < ApplicationController
       else
         true
       end
+    include_ancestors =
+      if params[:include_ancestors].present?
+        ActiveModel::Type::Boolean.new.cast(params[:include_ancestors])
+      else
+        false
+      end
     prioritized_category_id = params[:prioritized_category_id].to_i if params[
       :prioritized_category_id
     ].present?
@@ -388,7 +394,18 @@ class CategoriesController < ApplicationController
 
     Category.preload_user_fields!(guardian, categories)
 
-    render_serialized(categories, SiteCategorySerializer, root: :categories, scope: guardian)
+    if include_ancestors
+      ancestors = Category.secured(guardian).ancestors_of(categories.map(&:id))
+
+      render_json_dump(
+        {
+          categories: serialize_data(categories, SiteCategorySerializer, scope: guardian),
+          ancestors: serialize_data(ancestors, SiteCategorySerializer, scope: guardian),
+        },
+      )
+    else
+      render_serialized(categories, SiteCategorySerializer, root: :categories, scope: guardian)
+    end
   end
 
   private

app/jobs/base.rb

@@ -21,6 +21,14 @@ module Jobs
     @run_immediately = false
   end
 
+  def self.with_immediate_jobs
+    prior = @run_immediately
+    run_immediately!
+    yield
+  ensure
+    @run_immediately = prior
+  end
+
   def self.last_job_performed_at
     Sidekiq.redis do |r|
       int = r.get("last_job_perform_at")

app/models/category.rb

@@ -257,6 +257,23 @@ class Category < ActiveRecord::Base
     end
   end
 
+  def self.ancestors_of(category_ids)
+    ancestor_ids = []
+
+    SiteSetting.max_category_nesting.times do
+      category_ids =
+        where(id: category_ids)
+          .where.not(parent_category_id: nil)
+          .pluck("DISTINCT parent_category_id")
+
+      ancestor_ids.concat(category_ids)
+
+      break if category_ids.empty?
+    end
+
+    where(id: ancestor_ids)
+  end
+
   def self.topic_id_cache
     @topic_id_cache ||= DistributedCache.new("category_topic_ids")
   end

app/services/search_indexer.rb

@@ -21,6 +21,14 @@ class SearchIndexer
     @disabled = false
   end
 
+  def self.with_indexing
+    prior = @disabled
+    enable
+    yield
+  ensure
+    @disabled = prior
+  end
+
   def self.update_index(table:, id:, a_weight: nil, b_weight: nil, c_weight: nil, d_weight: nil)
     raw_data = { a: a_weight, b: b_weight, c: c_weight, d: d_weight }
 

spec/models/category_spec.rb

@@ -1483,4 +1483,31 @@ RSpec.describe Category do
       end
     end
   end
+
+  describe ".ancestors_of" do
+    fab!(:category)
+    fab!(:subcategory) { Fabricate(:category, parent_category: category) }
+
+    fab!(:sub_subcategory) do
+      SiteSetting.max_category_nesting = 3
+      Fabricate(:category, parent_category: subcategory)
+    end
+
+    it "finds the parent" do
+      expect(Category.ancestors_of([subcategory.id]).to_a).to eq([category])
+    end
+
+    it "finds the grandparent" do
+      expect(Category.ancestors_of([sub_subcategory.id]).to_a).to contain_exactly(
+        category,
+        subcategory,
+      )
+    end
+
+    it "respects the relation it's called on" do
+      expect(Category.where.not(id: category.id).ancestors_of([sub_subcategory.id]).to_a).to eq(
+        [subcategory],
+      )
+    end
+  end
 end

spec/requests/categories_controller_spec.rb

@@ -1127,6 +1127,30 @@ RSpec.describe CategoriesController do
       [category, category2, subcategory].each { |c| SearchIndexer.index(c, force: true) }
     end
 
+    context "without include_ancestors" do
+      it "doesn't return ancestors" do
+        get "/categories/search.json", params: { term: "Notfoo" }
+
+        expect(response.parsed_body).not_to have_key("ancestors")
+      end
+    end
+
+    context "with include_ancestors=false" do
+      it "returns ancestors" do
+        get "/categories/search.json", params: { term: "Notfoo", include_ancestors: false }
+
+        expect(response.parsed_body).not_to have_key("ancestors")
+      end
+    end
+
+    context "with include_ancestors=true" do
+      it "returns ancestors" do
+        get "/categories/search.json", params: { term: "Notfoo", include_ancestors: true }
+
+        expect(response.parsed_body).to have_key("ancestors")
+      end
+    end
+
     context "with term" do
       it "returns categories" do
         get "/categories/search.json", params: { term: "Foo" }

spec/system/editing_sidebar_categories_navigation_spec.rb

@@ -3,10 +3,10 @@
 RSpec.describe "Editing sidebar categories navigation", type: :system do
   fab!(:user)
 
-  fab!(:category2) { Fabricate(:category, name: "category2") }
+  fab!(:category2) { Fabricate(:category, name: "category 2") }
 
   fab!(:category2_subcategory) do
-    Fabricate(:category, parent_category_id: category2.id, name: "category2 subcategory")
+    Fabricate(:category, parent_category_id: category2.id, name: "category 2 subcategory")
   end
 
   fab!(:category) { Fabricate(:category, name: "category") }
@@ -21,6 +21,19 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do
 
   let(:sidebar) { PageObjects::Components::NavigationMenu::Sidebar.new }
 
+  before_all do
+    Jobs.with_immediate_jobs do
+      SearchIndexer.with_indexing do
+        category2.index_search
+        category2_subcategory.index_search
+
+        category.index_search
+        category_subcategory2.index_search
+        category_subcategory.index_search
+      end
+    end
+  end
+
   before { sign_in(user) }
 
   shared_examples "a user can edit the sidebar categories navigation" do |mobile|
@@ -149,9 +162,11 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do
 
     modal = sidebar.click_edit_categories_button
 
-    modal.filter("category subcategory 2")
+    modal.filter("subcategory")
 
-    expect(modal).to have_categories([category, category_subcategory2])
+    expect(modal).to have_categories(
+      [category, category_subcategory, category_subcategory2, category2, category2_subcategory],
+    )
 
     modal.filter("2")
 
@@ -233,10 +248,20 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do
       Fabricate(
         :category,
         parent_category_id: category2_subcategory.id,
-        name: "category2 subcategory subcategory",
+        name: "category 2 subcategory subcategory",
       )
     end
 
+    before_all do
+      Jobs.with_immediate_jobs do
+        SearchIndexer.with_indexing do
+          category_subcategory_subcategory.index_search
+          category_subcategory_subcategory2.index_search
+          category2_subcategory_subcategory.index_search
+        end
+      end
+    end
+
     it "allows a user to edit sub-subcategories to be included in the sidebar categories section" do
       visit "/latest"
 
@@ -265,10 +290,18 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do
       expect(sidebar).to have_categories_section
 
       modal = sidebar.click_edit_categories_button
-      modal.filter("category2 subcategory subcategory")
+      modal.filter("category 2 subcategory subcategory")
 
       expect(modal).to have_categories(
-        [category2, category2_subcategory, category2_subcategory_subcategory],
+        [
+          category,
+          category_subcategory,
+          category_subcategory_subcategory2,
+          category_subcategory2,
+          category2,
+          category2_subcategory,
+          category2_subcategory_subcategory,
+        ],
       )
     end
   end