From e74a9efee1cd9a1410d15ca6d1c2d1b83a9ad7fb Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Thu, 29 Feb 2024 13:48:20 +0200 Subject: [PATCH] FIX: Show "no category" in category-chooser (#25917) CategoryChooser component usually displays just categories, but sometimes it can show two none values: a "no category" or Uncategorized. This commit makes sure that these are rendered correctly. The problem was that the "none" item was automatically inserted in the list of options, but that should not always happen. Toggling option `autoInsertNoneItem` requires setting `none` too. --- .../app/components/edit-category-general.hbs | 2 +- .../discourse/app/models/category.js | 19 ++++++++++++ .../discourse/app/routes/edit-category.js | 8 ++--- .../tests/acceptance/category-edit-test.js | 30 +++++++++++++++++++ .../addon/components/category-chooser.js | 17 ++++++----- .../addon/components/category-drop.js | 3 +- app/controllers/categories_controller.rb | 18 +++++++++-- spec/requests/categories_controller_spec.rb | 18 +++++++++++ 8 files changed, 97 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/edit-category-general.hbs b/app/assets/javascripts/discourse/app/components/edit-category-general.hbs index ab7d6ed51f2..eeb9ec837f9 100644 --- a/app/assets/javascripts/discourse/app/components/edit-category-general.hbs +++ b/app/assets/javascripts/discourse/app/components/edit-category-general.hbs @@ -19,13 +19,13 @@ diff --git a/app/assets/javascripts/discourse/app/models/category.js b/app/assets/javascripts/discourse/app/models/category.js index 64fcca5948a..b3b802dd0ee 100644 --- a/app/assets/javascripts/discourse/app/models/category.js +++ b/app/assets/javascripts/discourse/app/models/category.js @@ -176,6 +176,25 @@ export default class Category extends RestModel { return category; } + static async asyncFindBySlugPath(slugPath, opts = {}) { + const data = { slug_path: slugPath }; + if (opts.includePermissions) { + data.include_permissions = true; + } + + const result = await ajax("/categories/find", { data }); + + const categories = result["categories"].map((category) => { + category = Site.current().updateCategory(category); + if (opts.includePermissions) { + category.setupGroupsAndPermissions(); + } + return category; + }); + + return categories[categories.length - 1]; + } + static async asyncFindBySlugPathWithID(slugPathWithID) { const result = await ajax("/categories/find", { data: { slug_path_with_id: slugPathWithID }, diff --git a/app/assets/javascripts/discourse/app/routes/edit-category.js b/app/assets/javascripts/discourse/app/routes/edit-category.js index 9d9d818b6ef..8ada365d503 100644 --- a/app/assets/javascripts/discourse/app/routes/edit-category.js +++ b/app/assets/javascripts/discourse/app/routes/edit-category.js @@ -7,11 +7,9 @@ export default DiscourseRoute.extend({ router: service(), model(params) { - return Category.reloadCategoryWithPermissions( - params, - this.store, - this.site - ); + return this.site.lazy_load_categories + ? Category.asyncFindBySlugPath(params.slug, { includePermissions: true }) + : Category.reloadCategoryWithPermissions(params, this.store, this.site); }, afterModel(model) { diff --git a/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js b/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js index dfc8df543d7..14a5eda0a7b 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js @@ -145,6 +145,36 @@ acceptance("Category Edit", function (needs) { assert.deepEqual(removePayload.allowed_tag_groups, []); }); + test("Editing parent category (disabled Uncategorized)", async function (assert) { + this.siteSettings.allow_uncategorized_topics = false; + + await visit("/c/bug/edit"); + const categoryChooser = selectKit(".category-chooser"); + await categoryChooser.expand(); + await categoryChooser.selectRowByValue(6); + + await categoryChooser.expand(); + + const names = [...categoryChooser.rows()].map((row) => row.dataset.name); + assert.ok(names.includes("(no category)")); + assert.notOk(names.includes("Uncategorized")); + }); + + test("Editing parent category (enabled Uncategorized)", async function (assert) { + this.siteSettings.allow_uncategorized_topics = true; + + await visit("/c/bug/edit"); + const categoryChooser = selectKit(".category-chooser"); + await categoryChooser.expand(); + await categoryChooser.selectRowByValue(6); + + await categoryChooser.expand(); + + const names = [...categoryChooser.rows()].map((row) => row.dataset.name); + assert.ok(names.includes("(no category)")); + assert.notOk(names.includes("Uncategorized")); + }); + test("Index Route", async function (assert) { await visit("/c/bug/edit"); assert.strictEqual( 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 7b3fdd8a918..6eaeb5dd2b2 100644 --- a/app/assets/javascripts/select-kit/addon/components/category-chooser.js +++ b/app/assets/javascripts/select-kit/addon/components/category-chooser.js @@ -12,12 +12,13 @@ import ComboBoxComponent from "select-kit/components/combo-box"; export default ComboBoxComponent.extend({ pluginApiIdentifiers: ["category-chooser"], classNames: ["category-chooser"], - allowUncategorizedTopics: setting("allow_uncategorized_topics"), + allowUncategorized: setting("allow_uncategorized_topics"), fixedCategoryPositionsOnCreate: setting("fixed_category_positions_on_create"), selectKitOptions: { filterable: true, - allowUncategorized: false, + allowUncategorized: "allowUncategorized", + autoInsertNoneItem: false, allowSubCategories: true, permissionType: PermissionType.FULL, excludeCategoryId: null, @@ -30,6 +31,7 @@ export default ComboBoxComponent.extend({ if ( this.site.lazy_load_categories && + this.value && !Category.hasAsyncFoundAll([this.value]) ) { // eslint-disable-next-line no-console @@ -54,10 +56,7 @@ export default ComboBoxComponent.extend({ I18n.t(isString ? this.selectKit.options.none : "category.none") ) ); - } else if ( - this.allowUncategorizedTopics || - this.selectKit.options.allowUncategorized - ) { + } else if (this.selectKit.options.allowUncategorized) { return Category.findUncategorized(); } else { const defaultCategoryId = parseInt( @@ -94,8 +93,10 @@ export default ComboBoxComponent.extend({ search(filter) { if (this.site.lazy_load_categories) { return Category.asyncSearch(this._normalize(filter), { - scopedCategoryId: this.selectKit.options?.scopedCategoryId, - prioritizedCategoryId: this.selectKit.options?.prioritizedCategoryId, + includeUncategorized: this.selectKit.options.allowUncategorized, + rejectCategoryIds: [this.selectKit.options.excludeCategoryId], + scopedCategoryId: this.selectKit.options.scopedCategoryId, + prioritizedCategoryId: this.selectKit.options.prioritizedCategoryId, }); } 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 a68aa893f86..684fa8380d8 100644 --- a/app/assets/javascripts/select-kit/addon/components/category-drop.js +++ b/app/assets/javascripts/select-kit/addon/components/category-drop.js @@ -24,6 +24,7 @@ export default ComboBoxComponent.extend({ navigateToEdit: false, editingCategory: false, editingCategoryTab: null, + allowUncategorized: setting("allow_uncategorized_topics"), selectKitOptions: { filterable: true, @@ -40,7 +41,7 @@ export default ComboBoxComponent.extend({ displayCategoryDescription: "displayCategoryDescription", headerComponent: "category-drop/category-drop-header", parentCategory: false, - allowUncategorized: setting("allow_uncategorized_topics"), + allowUncategorized: "allowUncategorized", }, modifyComponentForRow() { diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index f6e24e905f9..ec63117b18a 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -303,9 +303,17 @@ class CategoriesController < ApplicationController def find categories = [] + serializer = params[:include_permissions] ? CategorySerializer : SiteCategorySerializer if params[:ids].present? categories = Category.secured(guardian).where(id: params[:ids]) + elsif params[:slug_path].present? + category = Category.find_by_slug_path(params[:slug_path].split("/")) + raise Discourse::NotFound if category.blank? + guardian.ensure_can_see!(category) + + ancestors = Category.secured(guardian).with_ancestors(category.id).where.not(id: category.id) + categories = [*ancestors, category] elsif params[:slug_path_with_id].present? category = Category.find_by_slug_path_with_id(params[:slug_path_with_id]) raise Discourse::NotFound if category.blank? @@ -319,7 +327,7 @@ class CategoriesController < ApplicationController Category.preload_user_fields!(guardian, categories) - render_serialized(categories, SiteCategorySerializer, root: :categories, scope: guardian) + render_serialized(categories, serializer, root: :categories, scope: guardian) end def search @@ -333,8 +341,12 @@ class CategoriesController < ApplicationController true end ) - select_category_ids = params[:select_category_ids].presence - reject_category_ids = params[:reject_category_ids].presence + if params[:select_category_ids].is_a?(Array) + select_category_ids = params[:select_category_ids].map(&:presence) + end + if params[:reject_category_ids].is_a?(Array) + reject_category_ids = params[:reject_category_ids].map(&:presence) + end include_subcategories = if params[:include_subcategories].present? ActiveModel::Type::Boolean.new.cast(params[:include_subcategories]) diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index aa12a3b4b30..1b182ec9a4a 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -1217,6 +1217,12 @@ RSpec.describe CategoriesController do expect(response.parsed_body["categories"].size).to eq(1) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly("Foo") end + + it "works with empty categories list" do + get "/categories/search.json", params: { select_category_ids: [""] } + + expect(response.parsed_body["categories"].size).to eq(0) + end end context "with reject_category_ids" do @@ -1230,6 +1236,18 @@ RSpec.describe CategoriesController do "Foobar", ) end + + it "works with empty categories list" do + get "/categories/search.json", params: { reject_category_ids: [""] } + + 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 include_subcategories" do