Disabled Uncategorized shows in category dropdowns

Bug
13

Oh I see, just did a bit of local debugging, I confirm this is 100% reproducible on default discourse installs.

Let me recap here:

There are 2 settings when it comes to uncategorized:

allow_uncategorized_topics default off
suppress_uncategorized_badge default on

when allow allow_uncategorized_topics is disabled (default setup) we leak its presence into certain places.

image
image953×710 59.2 KB

If you try to work around by enabling uncategorized so you can delete it, you are presented with:

image
image935×1180 114 KB

In Discourse this category is super weird in that:

  1. It must be there
  2. It can not be deleted
  3. We inject it into lots of logic into lots of places:

We can fix the leakage by just adding more and more conditionals, we are probably up to at least 10 now both on client and server.

Or we can fix this at the core, just allow admins to delete the category, then it goes away and we never need to check for it anyway.

My recommendation here is

  1. Delete both settings and delete the hidden setting uncategorized_category_id
  2. Have a concept of default category (which we already do) - we already have default_composer_category
  3. 'uncategorized` then is no longer a special concept, less stuff to reason about.
  4. Have a theme component for people that must have a “category without a badge”

The current search bug can be fixed with something like

diff --git a/app/assets/javascripts/select-kit/addon/components/search-advanced-category-chooser.js b/app/assets/javascripts/select-kit/addon/components/search-advanced-category-chooser.js
index a678919d16..83a9ed27db 100644
--- a/app/assets/javascripts/select-kit/addon/components/search-advanced-category-chooser.js
+++ b/app/assets/javascripts/select-kit/addon/components/search-advanced-category-chooser.js
@@ -1,4 +1,5 @@
 import { classNames } from "@ember-decorators/component";
+import { setting } from "discourse/lib/computed";
 import CategoryChooserComponent from "select-kit/components/category-chooser";
 import {
   pluginApiIdentifiers,
@@ -7,11 +8,13 @@ import {
 
 @classNames("search-advanced-category-chooser")
 @selectKitOptions({
-  allowUncategorized: true,
+  allowUncategorized: "allowUncategorized",
   clearable: true,
   none: "category.all",
   displayCategoryDescription: false,
   permissionType: null,
 })
 @pluginApiIdentifiers("search-advanced-category-chooser")
-export default class SearchAdvancedCategoryChooser extends CategoryChooserComponent {}
+export default class SearchAdvancedCategoryChooser extends CategoryChooserComponent {
+  @setting("allow_uncategorized_topics") allowUncategorized;
+}

import { render } from "@ember/test-helpers";
import { module, test } from "qunit";
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
import selectKit from "discourse/tests/helpers/select-kit-helper";
import SearchAdvancedCategoryChooser from "select-kit/components/search-advanced-category-chooser";

module(
  "Integration | Component | select-kit/search-advanced-category-chooser",
  function (hooks) {
    setupRenderingTest(hooks);

    hooks.beforeEach(function () {
      this.set("subject", selectKit());
    });

    test("respects allow_uncategorized_topics setting when false", async function (assert) {
      this.siteSettings.allow_uncategorized_topics = false;

      await render(<template><SearchAdvancedCategoryChooser /></template>);

      await this.subject.expand();

      // Uncategorized category (ID 17 in test data) should not be present when setting is false
      assert.false(
        this.subject.rowByValue(17).exists(),
        "uncategorized category is not available when allow_uncategorized_topics is false"
      );
    });

    test("shows uncategorized category when allow_uncategorized_topics is true", async function (assert) {
      this.siteSettings.allow_uncategorized_topics = true;

      await render(<template><SearchAdvancedCategoryChooser /></template>);

      await this.subject.expand();

      // Uncategorized category (ID 17 in test data) should be present when setting is true
      assert.true(
        this.subject.rowByValue(17).exists(),
        "uncategorized category is available when allow_uncategorized_topics is true"
      );
    });

    test("has correct default options", async function (assert) {
      await render(<template><SearchAdvancedCategoryChooser /></template>);

      assert.strictEqual(
        this.subject.header().label(),
        "All categories",
        "has correct default none label"
      );
    });
  }
);

But that is just the advanced search bug, we are playing wack-a-mole here…

3 Likes