FIX: Fetch categories for "+subcategories" option (#26622)

Selecting the +subcategories option does not work sometimes when "lazy
load categories" is enabled because the subcategories may not be
fetched. This ensures that subcategories are loaded by requesting them
before being used.

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

@@ -399,6 +399,7 @@ export default class Category extends RestModel {
       include_ancestors: opts.includeAncestors,
       prioritized_category_id: opts.prioritizedCategoryId,
       limit: opts.limit,
+      page: opts.page,
     };
 
     const result = (CATEGORY_ASYNC_SEARCH_CACHE[JSON.stringify(data)] ||=

app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js

@@ -521,6 +521,7 @@ export default {
           notification_level: null,
           background_url: null,
           has_children: true,
+          subcategory_count: 2,
         },
         {
           id: 1002,

app/assets/javascripts/select-kit/addon/components/category-row.gjs

@@ -111,8 +111,8 @@ export default class CategoryRow extends Component {
           this.allowUncategorizedTopics || this.allowUncategorized,
         hideParent: !!this.parentCategory,
         topicCount: this.topicCount,
-        subcategoryCount: this.args.item?.categories
-          ? this.args.item.categories.length - 1
+        subcategoryCount: this.args.item?.category
+          ? this.category.subcategory_count
           : 0,
       })
     );

app/assets/javascripts/select-kit/addon/components/category-selector.js

@@ -72,18 +72,11 @@ export default MultiSelectComponent.extend({
     // If there is a single match or an exact match and it has subcategories,
     // add a row for selecting all subcategories
     if (
-      categories.length === 1 ||
-      (categories.length > 0 && categories[0].name.localeCompare(filter) === 0)
+      (categories.length === 1 ||
+        (categories.length > 0 &&
+          categories[0].name.localeCompare(filter) === 0)) &&
+      categories[0].subcategory_count > 0
     ) {
-      // Descendants may not be loaded if lazy loading is enabled. Search for
-      // subcategories will make sure these are loaded
-      if (this.site.lazy_load_categories) {
-        await Category.asyncSearch("", {
-          parentCategoryId: categories[0].id,
-        });
-      }
-
-      if (categories[0].descendants.length > 1) {
       categories.splice(
         1,
         0,
@@ -92,20 +85,32 @@ export default MultiSelectComponent.extend({
           // that parseInt still returns a valid ID in order to generate the
           // label
           id: `${categories[0].id}+subcategories`,
-            categories: categories[0].descendants,
+          category: categories[0],
         })
       );
     }
-    }
 
     return categories;
   },
 
-  select(value, item) {
-    if (item.categories) {
+  async select(value, item) {
+    // item is usually a category, but if the "category" property is set, then
+    // it is the special row for selecting all subcategories
+    if (item.category) {
+      if (this.site.lazy_load_categories) {
+        // Descendants may not be loaded if lazy loading is enabled. Searching
+        // for subcategories will make sure these are loaded
+        for (let page = 1, categories = [null]; categories.length > 0; page++) {
+          categories = await Category.asyncSearch("", {
+            parentCategoryId: item.category.id,
+            page,
+          });
+        }
+      }
+
       this.selectKit.change(
-        makeArray(this.value).concat(item.categories.mapBy("id")),
-        makeArray(this.selectedContent).concat(item.categories)
+        makeArray(this.value).concat(item.category.descendants.mapBy("id")),
+        makeArray(this.selectedContent).concat(item.category.descendants)
       );
     } else {
       this._super(value, item);

app/controllers/categories_controller.rb

@@ -362,7 +362,15 @@ class CategoriesController < ApplicationController
     prioritized_category_id = params[:prioritized_category_id].to_i if params[
       :prioritized_category_id
     ].present?
-    limit = params[:limit].to_i.clamp(1, MAX_CATEGORIES_LIMIT) if params[:limit].present?
+    limit =
+      (
+        if params[:limit].present?
+          params[:limit].to_i.clamp(1, MAX_CATEGORIES_LIMIT)
+        else
+          MAX_CATEGORIES_LIMIT
+        end
+      )
+    page = [1, params[:page].to_i].max
 
     categories = Category.secured(guardian)
 
@@ -404,7 +412,8 @@ class CategoriesController < ApplicationController
         )
         .joins("LEFT JOIN topics t on t.id = categories.topic_id")
         .select("categories.*, t.slug topic_slug")
-        .limit(limit || MAX_CATEGORIES_LIMIT)
+        .limit(limit)
+        .offset((page - 1) * limit)
 
     if Site.preloaded_category_custom_fields.present?
       Category.preload_custom_fields(categories, Site.preloaded_category_custom_fields)

app/models/category.rb

@@ -226,7 +226,8 @@ class Category < ActiveRecord::Base
                 :subcategory_ids,
                 :subcategory_list,
                 :notification_level,
-                :has_children
+                :has_children,
+                :subcategory_count
 
   # Allows us to skip creating the category definition topic in tests.
   attr_accessor :skip_category_definition
@@ -244,13 +245,9 @@ class Category < ActiveRecord::Base
         Category.topic_create_allowed(guardian).where(id: category_ids).pluck(:id).to_set
       end
 
-    # Categories with children
-    with_children =
-      Category
-        .secured(guardian)
-        .where(parent_category_id: category_ids)
-        .pluck(:parent_category_id)
-        .to_set
+    # Load subcategory counts (used to fill has_children property)
+    subcategory_count =
+      Category.secured(guardian).where.not(parent_category_id: nil).group(:parent_category_id).count
 
     # Update category attributes
     categories.each do |category|
@@ -259,7 +256,9 @@ class Category < ActiveRecord::Base
       category.permission = CategoryGroup.permission_types[:full] if guardian.is_admin? ||
         allowed_topic_create_ids&.include?(category[:id])
 
-      category.has_children = with_children.include?(category[:id])
+      category.has_children = subcategory_count.key?(category[:id])
+
+      category.subcategory_count = subcategory_count[category[:id]] if category.has_children
     end
   end
 

app/serializers/basic_category_serializer.rb

@@ -20,6 +20,7 @@ class BasicCategorySerializer < ApplicationSerializer
              :can_edit,
              :topic_template,
              :has_children,
+             :subcategory_count,
              :sort_order,
              :sort_ascending,
              :show_subcategory_list,

spec/requests/api/schemas/json/category_create_response.json

@@ -84,6 +84,12 @@
             "null"
           ]
         },
+        "subcategory_count": {
+          "type": [
+            "integer",
+            "null"
+          ]
+        },
         "sort_order": {
           "type": [
             "string",
@@ -287,6 +293,7 @@
         "can_edit",
         "topic_template",
         "has_children",
+        "subcategory_count",
         "sort_order",
         "sort_ascending",
         "show_subcategory_list",

spec/requests/api/schemas/json/category_list_response.json

@@ -78,6 +78,12 @@
                 "has_children": {
                   "type": "boolean"
                 },
+                "subcategory_count": {
+                  "type": [
+                    "integer",
+                    "null"
+                  ]
+                },
                 "sort_order": {
                   "type": [
                     "string",
@@ -194,6 +200,7 @@
                 "can_edit",
                 "topic_template",
                 "has_children",
+                "subcategory_count",
                 "sort_order",
                 "sort_ascending",
                 "show_subcategory_list",

spec/requests/api/schemas/json/category_update_response.json

@@ -87,6 +87,12 @@
             "null"
           ]
         },
+        "subcategory_count": {
+          "type": [
+            "integer",
+            "null"
+          ]
+        },
         "sort_order": {
           "type": [
             "string",
@@ -291,6 +297,7 @@
         "topic_template",
         "form_template_ids",
         "has_children",
+        "subcategory_count",
         "sort_order",
         "sort_ascending",
         "show_subcategory_list",

spec/requests/api/schemas/json/site_response.json

@@ -603,6 +603,12 @@
             "has_children": {
               "type": "boolean"
             },
+            "subcategory_count": {
+              "type": [
+                "integer",
+                "null"
+              ]
+            },
             "sort_order": {
               "type": [
                 "string",
@@ -740,6 +746,7 @@
             "notification_level",
             "topic_template",
             "has_children",
+            "subcategory_count",
             "sort_order",
             "sort_ascending",
             "show_subcategory_list",

spec/requests/categories_controller_spec.rb

@@ -1115,6 +1115,7 @@ RSpec.describe CategoriesController do
         expect(serialized["notification_level"]).to eq(CategoryUser.default_notification_level)
         expect(serialized["permission"]).to eq(nil)
         expect(serialized["has_children"]).to eq(false)
+        expect(serialized["subcategory_count"]).to eq(nil)
       end
 
       it "does not return hidden category" do
@@ -1168,6 +1169,7 @@ RSpec.describe CategoriesController do
       expect(category["notification_level"]).to eq(NotificationLevels.all[:regular])
       expect(category["permission"]).to eq(CategoryGroup.permission_types[:full])
       expect(category["has_children"]).to eq(true)
+      expect(category["subcategory_count"]).to eq(1)
     end
 
     context "with a read restricted child category" do
@@ -1179,6 +1181,7 @@ RSpec.describe CategoriesController do
         get "/categories/find.json", params: { ids: [category.id] }
         category = response.parsed_body["categories"].first
         expect(category["has_children"]).to eq(true)
+        expect(category["subcategory_count"]).to eq(1)
       end
 
       it "indicates to a normal user that the category has no child" do
@@ -1187,6 +1190,7 @@ RSpec.describe CategoriesController do
         get "/categories/find.json", params: { ids: [category.id] }
         category = response.parsed_body["categories"].first
         expect(category["has_children"]).to eq(false)
+        expect(category["subcategory_count"]).to eq(nil)
       end
     end
   end
@@ -1415,6 +1419,7 @@ RSpec.describe CategoriesController do
       expect(category["notification_level"]).to eq(NotificationLevels.all[:regular])
       expect(category["permission"]).to eq(CategoryGroup.permission_types[:full])
       expect(category["has_children"]).to eq(true)
+      expect(category["subcategory_count"]).to eq(1)
     end
   end
 end