From 2e68ead45b4cb94fa65c46694716c0ccf25c949d Mon Sep 17 00:00:00 2001 From: Bianca Nenciu <nbianca@users.noreply.github.com> Date: Tue, 17 Oct 2023 19:46:54 +0300 Subject: [PATCH] FEATURE: Use async search for category dropdowns (#23774) This commit introduces a new endpoint to search categories and uses it instead of the categories map that is preloaded using SiteSerializer. This feature is enabled only when the hidden site setting lazy_load_categories is enabled and should be used only on sites with many categories. --- .../discourse/app/helpers/category-link.js | 6 - .../discourse/app/models/category.js | 21 +++ .../addon/components/category-chooser.js | 7 + .../addon/components/category-drop.js | 30 +++-- .../addon/components/category-selector.js | 56 +++----- .../common/select-kit/category-selector.scss | 4 - app/controllers/categories_controller.rb | 65 +++++++++ config/locales/client.en.yml | 6 - config/routes.rb | 1 + config/site_settings.yml | 1 + spec/requests/categories_controller_spec.rb | 124 ++++++++++++++++++ 11 files changed, 257 insertions(+), 64 deletions(-) diff --git a/app/assets/javascripts/discourse/app/helpers/category-link.js b/app/assets/javascripts/discourse/app/helpers/category-link.js index a4cd1418227..5b42a40509f 100644 --- a/app/assets/javascripts/discourse/app/helpers/category-link.js +++ b/app/assets/javascripts/discourse/app/helpers/category-link.js @@ -185,11 +185,5 @@ export function defaultCategoryLinkRenderer(category, opts) { if (opts.topicCount && categoryStyle === "box") { afterBadgeWrapper += buildTopicCount(opts.topicCount); } - if (opts.plusSubcategories && opts.lastSubcategory) { - afterBadgeWrapper += `<span class="plus-subcategories">${I18n.t( - "category_row.plus_subcategories", - { count: opts.plusSubcategories } - )}</span>`; - } return `<${tagName} class="badge-wrapper ${extraClasses}" ${href}>${html}</${tagName}>${afterBadgeWrapper}`; } diff --git a/app/assets/javascripts/discourse/app/models/category.js b/app/assets/javascripts/discourse/app/models/category.js index e107040459b..cfdc6927f94 100644 --- a/app/assets/javascripts/discourse/app/models/category.js +++ b/app/assets/javascripts/discourse/app/models/category.js @@ -652,6 +652,27 @@ Category.reopenClass({ return data.sortBy("read_restricted"); }, + + async asyncSearch(term, opts) { + opts ||= {}; + + const result = await ajax("/categories/search", { + data: { + term, + parent_category_id: opts.parentCategoryId, + include_uncategorized: opts.includeUncategorized, + select_category_ids: opts.selectCategoryIds, + reject_category_ids: opts.rejectCategoryIds, + include_subcategories: opts.includeSubcategories, + prioritized_category_id: opts.prioritizedCategoryId, + limit: opts.limit, + }, + }); + + return result["categories"].map((category) => + Site.current().updateCategory(category) + ); + }, }); export default Category; diff --git a/app/assets/javascripts/select-kit/addon/components/category-chooser.js b/app/assets/javascripts/select-kit/addon/components/category-chooser.js index 190d50a949a..41a5dbac7ed 100644 --- a/app/assets/javascripts/select-kit/addon/components/category-chooser.js +++ b/app/assets/javascripts/select-kit/addon/components/category-chooser.js @@ -76,6 +76,13 @@ export default ComboBoxComponent.extend({ }, search(filter) { + if (this.siteSettings.lazy_load_categories) { + return Category.asyncSearch(this._normalize(filter), { + scopedCategoryId: this.selectKit.options?.scopedCategoryId, + prioritizedCategoryId: this.selectKit.options?.prioritizedCategoryId, + }); + } + if (filter) { filter = this._normalize(filter); return this.content.filter((item) => { diff --git a/app/assets/javascripts/select-kit/addon/components/category-drop.js b/app/assets/javascripts/select-kit/addon/components/category-drop.js index 28778ab0f54..08bb9500a5f 100644 --- a/app/assets/javascripts/select-kit/addon/components/category-drop.js +++ b/app/assets/javascripts/select-kit/addon/components/category-drop.js @@ -132,13 +132,28 @@ export default ComboBoxComponent.extend({ } ), - search(filter) { - if (filter) { - let opts = { - parentCategoryId: this.options.parentCategory?.id, - }; - let results = Category.search(filter, opts); - results = this._filterUncategorized(results).sort((a, b) => { + async search(filter) { + const opts = { + parentCategoryId: this.options.parentCategory?.id, + includeUncategorized: this.siteSettings.allow_uncategorized_topics, + }; + + if (this.siteSettings.lazy_load_categories) { + const results = await Category.asyncSearch(filter, { ...opts, limit: 5 }); + return results.sort((a, b) => { + if (a.parent_category_id && !b.parent_category_id) { + return 1; + } else if (!a.parent_category_id && b.parent_category_id) { + return -1; + } else { + return 0; + } + }); + } + + if (filter) { + let results = Category.search(filter, opts); + return this._filterUncategorized(results).sort((a, b) => { if (a.parent_category_id && !b.parent_category_id) { return 1; } else if (!a.parent_category_id && b.parent_category_id) { @@ -147,7 +162,6 @@ export default ComboBoxComponent.extend({ return 0; } }); - return results; } else { return this._filterUncategorized(this.content); } diff --git a/app/assets/javascripts/select-kit/addon/components/category-selector.js b/app/assets/javascripts/select-kit/addon/components/category-selector.js index f8e7e59176a..cb90d629d82 100644 --- a/app/assets/javascripts/select-kit/addon/components/category-selector.js +++ b/app/assets/javascripts/select-kit/addon/components/category-selector.js @@ -1,10 +1,7 @@ -import EmberObject, { computed } from "@ember/object"; +import { computed } from "@ember/object"; import { mapBy } from "@ember/object/computed"; -import { htmlSafe } from "@ember/template"; -import { categoryBadgeHTML } from "discourse/helpers/category-link"; import Category from "discourse/models/category"; import { makeArray } from "discourse-common/lib/helpers"; -import I18n from "I18n"; import MultiSelectComponent from "select-kit/components/multi-select"; export default MultiSelectComponent.extend({ @@ -56,42 +53,21 @@ export default MultiSelectComponent.extend({ return "category-row"; }, - search(filter) { - const result = this._super(filter); - if (result.length === 1) { - const subcategoryIds = new Set([result[0].id]); - for (let i = 0; i < this.siteSettings.max_category_nesting; ++i) { - subcategoryIds.forEach((categoryId) => { - this.content.forEach((category) => { - if (category.parent_category_id === categoryId) { - subcategoryIds.add(category.id); - } - }); - }); - } - - if (subcategoryIds.size > 1) { - result.push( - EmberObject.create({ - multiCategory: [...subcategoryIds], - category: result[0], - title: I18n.t("category_row.plus_subcategories_title", { - name: result[0].name, - count: subcategoryIds.size - 1, - }), - label: htmlSafe( - categoryBadgeHTML(result[0], { - link: false, - recursive: true, - plusSubcategories: subcategoryIds.size - 1, - }) - ), - }) - ); - } - } - - return result; + async search(filter) { + return this.siteSettings.lazy_load_categories + ? await Category.asyncSearch(filter, { + includeUncategorized: + this.attrs.options?.allowUncategorized !== undefined + ? this.attrs.options.allowUncategorized + : this.selectKit.options.allowUncategorized, + selectCategoryIds: this.categories + ? this.categories.map((x) => x.id) + : null, + rejectCategoryIds: this.blockedCategories + ? this.blockedCategories.map((x) => x.id) + : null, + }) + : this._super(filter); }, select(value, item) { diff --git a/app/assets/stylesheets/common/select-kit/category-selector.scss b/app/assets/stylesheets/common/select-kit/category-selector.scss index 45835a558b2..972be8bae4a 100644 --- a/app/assets/stylesheets/common/select-kit/category-selector.scss +++ b/app/assets/stylesheets/common/select-kit/category-selector.scss @@ -16,10 +16,6 @@ .topic-count { margin-left: 0.25em; } - - .plus-subcategories { - font-size: var(--font-down-2); - } } } } diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 28b312cfb8b..10baa4cce6d 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -11,6 +11,7 @@ class CategoriesController < ApplicationController redirect find_by_slug visible_groups + search ] before_action :fetch_category, only: %i[show update destroy visible_groups] @@ -19,6 +20,7 @@ class CategoriesController < ApplicationController SYMMETRICAL_CATEGORIES_TO_TOPICS_FACTOR = 1.5 MIN_CATEGORIES_TOPICS = 5 + MAX_CATEGORIES_LIMIT = 25 def redirect return if handle_permalink("/category/#{params[:path]}") @@ -297,6 +299,69 @@ class CategoriesController < ApplicationController render json: success_json.merge(groups: groups || []) end + def search + term = params[:term].to_s.strip + parent_category_id = params[:parent_category_id].to_i if params[:parent_category_id].present? + include_uncategorized = + ( + if params[:include_uncategorized].present? + ActiveModel::Type::Boolean.new.cast(params[:include_uncategorized]) + else + true + end + ) + select_category_ids = params[:select_category_ids].presence + reject_category_ids = params[:reject_category_ids].presence + include_subcategories = + if params[:include_subcategories].present? + ActiveModel::Type::Boolean.new.cast(params[:include_subcategories]) + else + true + end + 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? + + categories = Category.secured(guardian) + + categories = + categories + .includes(:category_search_data) + .references(:category_search_data) + .where( + "category_search_data.search_data @@ #{Search.ts_query(term: term)}", + ) if term.present? + + categories = + categories.where( + "id = :id OR parent_category_id = :id", + id: parent_category_id, + ) if parent_category_id.present? + + categories = + categories.where.not(id: SiteSetting.uncategorized_category_id) if !include_uncategorized + + categories = categories.where(id: select_category_ids) if select_category_ids + + categories = categories.where.not(id: reject_category_ids) if reject_category_ids + + categories = categories.where(parent_category_id: nil) if !include_subcategories + + categories = categories.limit(limit || MAX_CATEGORIES_LIMIT) + + categories = categories.order(<<~SQL) if prioritized_category_id.present? + CASE + WHEN id = #{prioritized_category_id} OR parent_category_id = #{prioritized_category_id} THEN 0 + ELSE 1 + END + SQL + + categories = categories.order(:read_restricted) + + render json: categories, each_serializer: SiteCategorySerializer + end + private def self.topics_per_page diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 41a566c4f07..1d58ccdb590 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2312,12 +2312,6 @@ en: topic_count: one: "%{count} topic in this category" other: "%{count} topics in this category" - plus_subcategories_title: - one: "%{name} and one subcategory" - other: "%{name} and %{count} subcategories" - plus_subcategories: - one: "+ %{count} subcategory" - other: "+ %{count} subcategories" select_kit: delete_item: "Delete %{name}" diff --git a/config/routes.rb b/config/routes.rb index 0931b0538a7..c5055da4e67 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1155,6 +1155,7 @@ Discourse::Application.routes.draw do resources :categories, except: %i[show new edit] post "categories/reorder" => "categories#reorder" + get "categories/search" => "categories#search" scope path: "category/:category_id" do post "/move" => "categories#move" diff --git a/config/site_settings.yml b/config/site_settings.yml index bab5c11bae8..a456971669f 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2176,6 +2176,7 @@ developer: hidden: true lazy_load_categories: default: false + client: true hidden: true navigation: diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index 062dc86917c..286ca7e585f 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -1039,4 +1039,128 @@ RSpec.describe CategoriesController do expect(response.parsed_body["groups"]).to eq([]) end end + + describe "#search" do + fab!(:category) { Fabricate(:category, name: "Foo") } + fab!(:subcategory) { Fabricate(:category, name: "Foobar", parent_category: category) } + fab!(:category2) { Fabricate(:category, name: "Notfoo") } + + before do + SearchIndexer.enable + [category, category2, subcategory].each { |c| SearchIndexer.index(c, force: true) } + end + + context "with term" do + it "returns categories" do + get "/categories/search.json", params: { term: "Foo" } + + expect(response.parsed_body["categories"].size).to eq(2) + expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( + "Foo", + "Foobar", + ) + end + end + + context "with parent_category_id" do + it "returns categories" do + get "/categories/search.json", params: { parent_category_id: category.id } + + expect(response.parsed_body["categories"].size).to eq(2) + expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( + "Foo", + "Foobar", + ) + end + end + + context "with include_uncategorized" do + it "returns Uncategorized" do + get "/categories/search.json", params: { include_uncategorized: true } + + expect(response.parsed_body["categories"].size).to eq(4) + expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( + "Uncategorized", + "Foo", + "Foobar", + "Notfoo", + ) + end + + it "does not return Uncategorized" do + get "/categories/search.json", params: { include_uncategorized: false } + + expect(response.parsed_body["categories"].size).to eq(3) + expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( + "Foo", + "Foobar", + "Notfoo", + ) + end + end + + context "with select_category_ids" do + it "returns categories" do + get "/categories/search.json", params: { select_category_ids: [category.id] } + + expect(response.parsed_body["categories"].size).to eq(1) + expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly("Foo") + end + end + + context "with reject_category_ids" do + it "returns categories" do + get "/categories/search.json", params: { reject_category_ids: [category2.id] } + + expect(response.parsed_body["categories"].size).to eq(3) + expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( + "Uncategorized", + "Foo", + "Foobar", + ) + end + end + + context "with include_subcategories" do + it "returns categories" do + get "/categories/search.json", params: { include_subcategories: false } + + expect(response.parsed_body["categories"].size).to eq(3) + expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( + "Uncategorized", + "Foo", + "Notfoo", + ) + end + + it "returns categories and subcategories" do + get "/categories/search.json", params: { include_subcategories: true } + + expect(response.parsed_body["categories"].size).to eq(4) + expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( + "Uncategorized", + "Foo", + "Foobar", + "Notfoo", + ) + end + end + + context "with prioritized_category_id" do + it "returns categories" do + get "/categories/search.json", params: { prioritized_category_id: category2.id } + + expect(response.parsed_body["categories"].size).to eq(4) + expect(response.parsed_body["categories"][0]["name"]).to eq("Notfoo") + end + end + + context "with limit" do + it "returns categories" do + get "/categories/search.json", params: { limit: 2 } + + expect(response.parsed_body["categories"].size).to eq(2) + end + end + end end