diff --git a/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-menu/categories-modal.gjs b/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-menu/categories-modal.gjs index d512dfddda6..b924e889a05 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-menu/categories-modal.gjs +++ b/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}} diff --git a/app/assets/javascripts/discourse/app/models/category.js b/app/assets/javascripts/discourse/app/models/category.js index cd0016dfbc3..b9861b0c29a 100644 --- a/app/assets/javascripts/discourse/app/models/category.js +++ b/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 ||= {}; diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js index ef385a61b1d..d50d6032d19 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js +++ b/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: [] }); }); }); diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index b6ac1017ea4..590b7760ff0 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -334,6 +334,66 @@ class CategoriesController < ApplicationController render_serialized(categories, serializer, root: :categories, scope: guardian) end + def hierarchical_search + term = params[:term].to_s.strip + page = [1, params[:page].to_i].max + offset = params[:offset].to_i + parent_category_id = params[:parent_category_id].to_i if params[:parent_category_id].present? + only = Category.where(id: params[:only].to_a.map(&:to_i)) if params[:only].present? + except_ids = params[:except].to_a.map(&:to_i) + include_uncategorized = + ( + if params[:include_uncategorized].present? + ActiveModel::Type::Boolean.new.cast(params[:include_uncategorized]) + else + true + end + ) + + except_ids << SiteSetting.uncategorized_category_id unless include_uncategorized + + except = Category.where(id: except_ids) if except_ids.present? + + limit = + ( + if params[:limit].present? + params[:limit].to_i.clamp(1, MAX_CATEGORIES_LIMIT) + else + MAX_CATEGORIES_LIMIT + end + ) + + categories = + Category + .secured(guardian) + .limited_categories_matching(only, except, parent_category_id, term) + .preload( + :uploaded_logo, + :uploaded_logo_dark, + :uploaded_background, + :uploaded_background_dark, + :tags, + :tag_groups, + :form_templates, + category_required_tag_groups: :tag_group, + ) + .joins("LEFT JOIN topics t on t.id = categories.topic_id") + .select("categories.*, t.slug topic_slug") + .limit(limit) + .offset((page - 1) * limit + offset) + .to_a + + if Site.preloaded_category_custom_fields.present? + Category.preload_custom_fields(categories, Site.preloaded_category_custom_fields) + end + + Category.preload_user_fields!(guardian, categories) + + response = { categories: serialize_data(categories, SiteCategorySerializer, scope: guardian) } + + render_json_dump(response) + end + def search term = params[:term].to_s.strip parent_category_id = params[:parent_category_id].to_i if params[:parent_category_id].present? diff --git a/app/models/category.rb b/app/models/category.rb index 8ff20b03d0c..493d150e0ee 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -279,6 +279,130 @@ class Category < ActiveRecord::Base where(id: ancestor_ids) end + # Perform a search. If a category exists in the result, its ancestors do too. + # Also check for prefix matches. If a category has a prefix match, its + # ancestors report a match too. + scope :tree_search, + ->(only, except, term) do + term = term.strip + escaped_term = ActiveRecord::Base.connection.quote(term.downcase) + prefix_match = "starts_with(LOWER(categories.name), #{escaped_term})" + + word_match = <<~SQL + COALESCE( + ( + SELECT BOOL_AND(position(pattern IN LOWER(categories.name)) <> 0) + FROM unnest(regexp_split_to_array(#{escaped_term}, '\s+')) AS pattern + ), + true + ) + SQL + + if except + prefix_match = + "NOT categories.id IN (#{except.reselect(:id).to_sql}) AND #{prefix_match}" + word_match = "NOT categories.id IN (#{except.reselect(:id).to_sql}) AND #{word_match}" + end + + if only + prefix_match = "categories.id IN (#{only.reselect(:id).to_sql}) AND #{prefix_match}" + word_match = "categories.id IN (#{only.reselect(:id).to_sql}) AND #{word_match}" + end + + categories = + Category.select( + "categories.*", + "#{prefix_match} AS has_prefix_match", + "#{word_match} AS has_word_match", + ) + + (1...SiteSetting.max_category_nesting).each do + categories = Category.from("(#{categories.to_sql}) AS categories") + + subcategory_matches = + categories + .where.not(parent_category_id: nil) + .group("categories.parent_category_id") + .select( + "categories.parent_category_id AS id", + "BOOL_OR(categories.has_prefix_match) AS has_prefix_match", + "BOOL_OR(categories.has_word_match) AS has_word_match", + ) + + categories = + Category.joins( + "LEFT JOIN (#{subcategory_matches.to_sql}) AS subcategory_matches ON categories.id = subcategory_matches.id", + ).select( + "categories.*", + "#{prefix_match} OR COALESCE(subcategory_matches.has_prefix_match, false) AS has_prefix_match", + "#{word_match} OR COALESCE(subcategory_matches.has_word_match, false) AS has_word_match", + ) + end + + categories = + Category.from("(#{categories.to_sql}) AS categories").where(has_word_match: true) + + categories.select("has_prefix_match AS matches", :id) + end + + # Given a relation, 'matches', which contains category ids and a 'matches' + # boolean, and a limit (the maximum number of subcategories per category), + # produce a subset of the matches categories annotated with information about + # their ancestors. + scope :select_descendants, + ->(matches, limit) do + max_nesting = SiteSetting.max_category_nesting + + categories = + joins("INNER JOIN (#{matches.to_sql}) AS matches ON matches.id = categories.id").select( + "categories.id", + "categories.name", + "ARRAY[]::record[] AS ancestors", + "0 AS depth", + "matches.matches", + ) + + categories = Category.from("(#{categories.to_sql}) AS c1") + + (1...max_nesting).each { |i| categories = categories.joins(<<~SQL) } + INNER JOIN LATERAL ( + (SELECT c#{i}.id, c#{i}.name, c#{i}.ancestors, c#{i}.depth, c#{i}.matches) + UNION ALL + (SELECT + categories.id, + categories.name, + c#{i}.ancestors || ARRAY[ROW(NOT c#{i}.matches, c#{i}.name)] AS ancestors, + c#{i}.depth + 1 as depth, + matches.matches + FROM categories + INNER JOIN matches + ON matches.id = categories.id + WHERE categories.parent_category_id = c#{i}.id + AND c#{i}.depth = #{i - 1} + ORDER BY (NOT matches.matches, categories.name) + LIMIT #{limit}) + ) c#{i + 1} ON true + SQL + + categories.select( + "c#{max_nesting}.id", + "c#{max_nesting}.ancestors", + "c#{max_nesting}.name", + "c#{max_nesting}.matches", + ) + end + + scope :limited_categories_matching, + ->(only, except, parent_id, term) do + joins(<<~SQL).order("c.ancestors || ARRAY[ROW(NOT c.matches, c.name)]") + INNER JOIN ( + WITH matches AS (#{Category.tree_search(only, except, term).to_sql}) + #{Category.where(parent_category_id: parent_id).select_descendants(Category.from("matches").select(:matches, :id), 5).to_sql} + ) AS c + ON categories.id = c.id + SQL + end + def self.topic_id_cache @topic_id_cache ||= DistributedCache.new("category_topic_ids") end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index bf23d0f7574..d6ef527f7c4 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -4665,6 +4665,7 @@ en: text: "and we'll automatically show this site's most popular categories" filter_placeholder: "Filter categories" no_categories: "There are no categories matching the given term." + show_more: "Show more" tags_form_modal: title: "Edit tags navigation" filter_placeholder: "Filter tags" diff --git a/config/routes.rb b/config/routes.rb index 6058895335b..59f2a7d9ebc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1174,6 +1174,7 @@ Discourse::Application.routes.draw do post "categories/reorder" => "categories#reorder" get "categories/find" => "categories#find" post "categories/search" => "categories#search" + get "categories/hierarchical_search" => "categories#hierarchical_search" get "categories/:parent_category_id" => "categories#index" scope path: "category/:category_id" do diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index 006bf280595..751efb4724d 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -1550,4 +1550,19 @@ RSpec.describe Category do ) end end + + describe ".limited_categories_matching" do + before_all { SiteSetting.max_category_nesting = 3 } + + fab!(:foo) { Fabricate(:category, name: "foo") } + fab!(:bar) { Fabricate(:category, name: "bar", parent_category: foo) } + fab!(:baz) { Fabricate(:category, name: "baz", parent_category: bar) } + + it "produces results in depth-first pre-order" do + SiteSetting.max_category_nesting = 3 + expect(Category.limited_categories_matching(nil, nil, nil, "baz").pluck(:name)).to eq( + %w[foo bar baz], + ) + end + end end diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index 8ceb8cede51..1aecd088b26 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -1443,4 +1443,31 @@ RSpec.describe CategoriesController do expect(response.parsed_body["categories"].map { |c| c["id"] }).not_to include(category.id) end end + + describe "#hierachical_search" do + before { sign_in(user) } + + it "produces categories with an empty term" do + get "/categories/hierarchical_search.json", params: { term: "" } + + expect(response.status).to eq(200) + expect(response.parsed_body["categories"].length).not_to eq(0) + end + + it "doesn't produce categories with a very specific term" do + get "/categories/hierarchical_search.json", params: { term: "acategorythatdoesnotexist" } + + expect(response.status).to eq(200) + expect(response.parsed_body["categories"].length).to eq(0) + end + + it "doesn't expose secret categories" do + category.update!(read_restricted: true) + + get "/categories/hierarchical_search.json", params: { term: "" } + + expect(response.status).to eq(200) + expect(response.parsed_body["categories"].map { |c| c["id"] }).not_to include(category.id) + end + end end diff --git a/spec/system/editing_sidebar_categories_navigation_spec.rb b/spec/system/editing_sidebar_categories_navigation_spec.rb index de53872afa9..fb88f7f1144 100644 --- a/spec/system/editing_sidebar_categories_navigation_spec.rb +++ b/spec/system/editing_sidebar_categories_navigation_spec.rb @@ -55,7 +55,7 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do expect(modal).to have_no_reset_to_defaults_button expect(modal).to have_categories( - [category2, category2_subcategory, category, category_subcategory2, category_subcategory], + [category, category_subcategory, category_subcategory2, category2, category2_subcategory], ) modal @@ -102,18 +102,6 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do include_examples "a user can edit the sidebar categories navigation", true end - it "displays the categories in the modal based on the fixed position of the category when `fixed_category_positions` site setting is enabled" do - SiteSetting.fixed_category_positions = true - - visit "/latest" - - modal = sidebar.click_edit_categories_button - - expect(modal).to have_categories( - [category2, category2_subcategory, category, category_subcategory2, category_subcategory], - ) - end - it "allows a user to deselect all categories in the modal" do Fabricate(:category_sidebar_section_link, linkable: category, user: user) Fabricate(:category_sidebar_section_link, linkable: category_subcategory2, user: user) @@ -165,13 +153,13 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do modal.filter("subcategory") expect(modal).to have_categories( - [category2, category2_subcategory, category, category_subcategory2, category_subcategory], + [category, category_subcategory, category_subcategory2, category2, category2_subcategory], ) modal.filter("2") expect(modal).to have_categories( - [category2, category2_subcategory, category, category_subcategory2], + [category, category_subcategory2, category2, category2_subcategory], ) modal.filter("someinvalidterm") @@ -190,16 +178,11 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do modal = sidebar.click_edit_categories_button modal.filter_by_selected - expect(modal).to have_categories([category2, category, category_subcategory]) - expect(modal).to have_checkbox(category, disabled: true) - expect(modal).to have_checkbox(category_subcategory) - expect(modal).to have_checkbox(category2) + expect(modal).to have_categories([category, category_subcategory, category2]) modal.filter("category subcategory") expect(modal).to have_categories([category, category_subcategory]) - expect(modal).to have_checkbox(category, disabled: true) - expect(modal).to have_checkbox(category_subcategory) modal.filter("").filter_by_unselected @@ -207,22 +190,11 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do [category, category_subcategory2, category2, category2_subcategory], ) - expect(modal).to have_checkbox(category) - expect(modal).to have_checkbox(category_subcategory2) - expect(modal).to have_checkbox(category2, disabled: true) - expect(modal).to have_checkbox(category2_subcategory) - modal.filter_by_all expect(modal).to have_categories( - [category, category_subcategory2, category_subcategory, category2, category2_subcategory], + [category, category_subcategory, category_subcategory2, category2, category2_subcategory], ) - - expect(modal).to have_checkbox(category) - expect(modal).to have_checkbox(category_subcategory) - expect(modal).to have_checkbox(category_subcategory2) - expect(modal).to have_checkbox(category2) - expect(modal).to have_checkbox(category2_subcategory) end context "when there are more categories than the page limit" do @@ -265,6 +237,8 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do describe "when max_category_nesting has been set to 3" do before_all { SiteSetting.max_category_nesting = 3 } + before { SiteSetting.max_category_nesting = 3 } + fab!(:category_subcategory_subcategory) do Fabricate( :category, @@ -335,9 +309,9 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do category2_subcategory, category2_subcategory_subcategory, category, - category_subcategory2, category_subcategory, category_subcategory_subcategory2, + category_subcategory2, ], ) end