FIX: ensures minimum tags logic is correct and shared (#14723)

Also fixes a bug where select-kit was not updating noneItem in multi-selects.
2024-11-26 02:40:53 -06:00 · 2021-11-12 14:04:48 +01:00 · 2021-11-12 14:04:48 +01:00 · 904d509cce
commit 904d509cce
parent 362c47ce6a
12 changed files with 124 additions and 87 deletions
--- a/app/assets/javascripts/discourse/app/controllers/topic.js
+++ b/app/assets/javascripts/discourse/app/controllers/topic.js
@ -1,3 +1,4 @@
+import Category from "discourse/models/category";
 import Controller, { inject as controller } from "@ember/controller";
 import DiscourseURL, { userPath } from "discourse/lib/url";
 import { alias, and, not, or } from "@ember/object/computed";
@ -212,11 +213,9 @@ export default Controller.extend(bufferedProperty("model"), {
    );
  },

-  @discourseComputed("model.category")
-  minimumRequiredTags(category) {
-    return category && category.minimum_required_tags > 0
-      ? category.minimum_required_tags
-      : null;
+  @discourseComputed("buffered.category_id")
+  minimumRequiredTags(categoryId) {
+    return Category.findById(categoryId)?.minimumRequiredTags || 0;
  },

  _removeDeleteOnOwnerReplyBookmarks() {
--- a/app/assets/javascripts/discourse/app/models/category.js
+++ b/app/assets/javascripts/discourse/app/models/category.js
@ -42,6 +42,19 @@ const Category = RestModel.extend({
    }
  },

+  @discourseComputed(
+    "required_tag_groups",
+    "min_tags_from_required_group",
+    "minimum_required_tags"
+  )
+  minimumRequiredTags() {
+    if (this.required_tag_groups) {
+      return this.min_tags_from_required_group;
+    } else {
+      return this.minimum_required_tags > 0 ? this.minimum_required_tags : null;
+    }
+  },
+
  @discourseComputed
  availablePermissions() {
    return [
--- a/app/assets/javascripts/discourse/app/models/composer.js
+++ b/app/assets/javascripts/discourse/app/models/composer.js
@ -157,26 +157,9 @@ const Composer = RestModel.extend({
    return categoryId ? this.site.categories.findBy("id", categoryId) : null;
  },

-  @discourseComputed("category")
-  minimumRequiredTags(category) {
-    if (category) {
-      if (category.required_tag_groups) {
-        return category.min_tags_from_required_group;
-      } else {
-        return category.minimum_required_tags > 0
-          ? category.minimum_required_tags
-          : null;
-      }
-    }
-
-    return null;
-  },
-
-  @discourseComputed("category")
-  requiredTagGroups(category) {
-    return category && category.required_tag_groups
-      ? category.required_tag_groups
-      : null;
+  @discourseComputed("category.minimumRequiredTags")
+  minimumRequiredTags(minimumRequiredTags) {
+    return minimumRequiredTags || 0;
  },

  creatingTopic: equal("action", CREATE_TOPIC),
--- a/app/assets/javascripts/discourse/app/templates/composer.hbs
+++ b/app/assets/javascripts/discourse/app/templates/composer.hbs
@ -99,7 +99,6 @@
                    options=(hash
                      categoryId=model.categoryId
                      minimum=model.minimumRequiredTags
-                      requiredTagGroups=model.requiredTagGroups
                    )
                  }}
                  {{popup-input-tip validation=tagValidation}}
--- a/app/assets/javascripts/discourse/tests/unit/models/category-test.js
+++ b/app/assets/javascripts/discourse/tests/unit/models/category-test.js
@ -221,6 +221,50 @@ module("Unit | Model | category", function () {
    assert.deepEqual(Category.findBySlugPathWithID("foo/baz/3"), baz);
  });

+  test("minimumRequiredTags", function (assert) {
+    const store = createStore();
+
+    let foo = store.createRecord("category", {
+      id: 1,
+      slug: "foo",
+      required_tag_groups: ["bar"],
+      min_tags_from_required_group: 2,
+    });
+
+    assert.equal(foo.minimumRequiredTags, 2);
+
+    foo = store.createRecord("category", {
+      id: 2,
+      slug: "foo",
+    });
+
+    assert.equal(foo.minimumRequiredTags, null);
+
+    foo = store.createRecord("category", {
+      id: 3,
+      slug: "foo",
+      minimum_required_tags: 0,
+    });
+
+    assert.equal(foo.minimumRequiredTags, null);
+
+    foo = store.createRecord("category", {
+      id: 4,
+      slug: "foo",
+      minimum_required_tags: 2,
+    });
+
+    assert.equal(foo.minimumRequiredTags, 2);
+
+    foo = store.createRecord("category", {
+      id: 5,
+      slug: "foo",
+      min_tags_from_required_group: 2,
+    });
+
+    assert.equal(foo.minimumRequiredTags, null);
+  });
+
  test("search with category name", function (assert) {
    const store = createStore(),
      category1 = store.createRecord("category", {
--- a/app/assets/javascripts/select-kit/addon/components/category-chooser.js
+++ b/app/assets/javascripts/select-kit/addon/components/category-chooser.js
@ -20,6 +20,7 @@ export default ComboBoxComponent.extend({
    permissionType: PermissionType.FULL,
    excludeCategoryId: null,
    scopedCategoryId: null,
+    prioritizedCategoryId: null,
  },

  modifyComponentForRow() {
--- a/app/assets/javascripts/select-kit/addon/components/dropdown-select-box.js
+++ b/app/assets/javascripts/select-kit/addon/components/dropdown-select-box.js
@ -12,6 +12,7 @@ export default SingleSelectComponent.extend({
    caretUpIcon: "caret-up",
    caretDownIcon: "caret-down",
    showCaret: false,
+    customStyle: null,
  },

  modifyComponentForRow() {
--- a/app/assets/javascripts/select-kit/addon/components/mini-tag-chooser.js
+++ b/app/assets/javascripts/select-kit/addon/components/mini-tag-chooser.js
@ -23,10 +23,8 @@ export default MultiSelectComponent.extend(TagsMixin, {
    termMatchesForbidden: false,
    categoryId: null,
    everyTag: false,
-    none: "tagging.choose_for_topic",
    closeOnChange: false,
-    maximum: "maximumSelectedTags",
-    minimum: "minimumSelectedTags",
+    maximum: "maxTagsPerTopic",
    autoInsertNoneItem: false,
  },

@ -38,31 +36,20 @@ export default MultiSelectComponent.extend(TagsMixin, {
    return "tag-row";
  },

-  allowAnyTag: or("allowCreate", "site.can_create_tag"),
-
-  maximumSelectedTags: computed(function () {
-    return parseInt(
-      this.options.limit ||
-        this.selectKit.options.maximum ||
-        this.maxTagsPerTopic,
-      10
-    );
-  }),
-
-  minimumSelectedTags: computed(function () {
-    if (
-      this.selectKit.options.minimum ||
-      this.selectKit.options.requiredTagGroups
-    ) {
-      const minimum = parseInt(this.selectKit.options.minimum, 10);
-      if (minimum > 0) {
-        return this.defaultItem(
-          null,
-          I18n.t("select_kit.min_content_not_reached", { count: minimum })
-        );
-      }
+  modifyNoSelection() {
+    if (this.selectKit.options.minimum > 0) {
+      return this.defaultItem(
+        null,
+        I18n.t("tagging.choose_for_topic_required", {
+          count: this.selectKit.options.minimum,
+        })
+      );
+    } else {
+      return this.defaultItem(null, I18n.t("tagging.choose_for_topic"));
    }
-  }),
+  },
+
+  allowAnyTag: or("allowCreate", "site.can_create_tag"),

  caretIcon: computed("value.[]", "content.[]", function () {
    const maximum = this.selectKit.options.maximum;
--- a/app/assets/javascripts/select-kit/addon/components/multi-select.js
+++ b/app/assets/javascripts/select-kit/addon/components/multi-select.js
@ -124,35 +124,40 @@ export default SelectKitComponent.extend({
    }
  },

-  selectedContent: computed("value.[]", "content.[]", function () {
-    const value = makeArray(this.value).map((v) =>
-      this.selectKit.options.castInteger && this._isNumeric(v) ? Number(v) : v
-    );
+  selectedContent: computed(
+    "value.[]",
+    "content.[]",
+    "selectKit.noneItem",
+    function () {
+      const value = makeArray(this.value).map((v) =>
+        this.selectKit.options.castInteger && this._isNumeric(v) ? Number(v) : v
+      );

-    if (value.length) {
-      let content = [];
+      if (value.length) {
+        let content = [];

-      value.forEach((v) => {
-        if (this.selectKit.valueProperty) {
-          const c = makeArray(this.content).findBy(
-            this.selectKit.valueProperty,
-            v
-          );
-          if (c) {
-            content.push(c);
+        value.forEach((v) => {
+          if (this.selectKit.valueProperty) {
+            const c = makeArray(this.content).findBy(
+              this.selectKit.valueProperty,
+              v
+            );
+            if (c) {
+              content.push(c);
+            }
+          } else {
+            if (makeArray(this.content).includes(v)) {
+              content.push(v);
+            }
          }
-        } else {
-          if (makeArray(this.content).includes(v)) {
-            content.push(v);
-          }
-        }
-      });
+        });

-      return this.selectKit.modifySelection(content);
+        return this.selectKit.modifySelection(content);
+      }
+
+      return this.selectKit.noneItem;
    }
-
-    return null;
-  }),
+  ),

  _onKeydown(event) {
    if (
--- a/app/assets/javascripts/select-kit/addon/components/select-kit.js
+++ b/app/assets/javascripts/select-kit/addon/components/select-kit.js
@ -209,8 +209,12 @@ export default Component.extend(
    didReceiveAttrs() {
      this._super(...arguments);

-      const computedOptions = {};
      Object.keys(this.selectKitOptions).forEach((key) => {
+        if (isPresent(this.options[key])) {
+          this.selectKit.options.set(key, this.options[key]);
+          return;
+        }
+
        const value = this.selectKitOptions[key];

        if (
@ -219,9 +223,9 @@ export default Component.extend(
          key === "componentForCollection"
        ) {
          if (typeof value === "string") {
-            computedOptions[key] = () => value;
+            this.selectKit.options.set(key, () => value);
          } else {
-            computedOptions[key] = bind(this, value);
+            this.selectKit.options.set(key, bind(this, value));
          }

          return;
@ -234,15 +238,13 @@ export default Component.extend(
        ) {
          const computedValue = get(this, value);
          if (typeof computedValue !== "function") {
-            computedOptions[key] = get(this, value);
+            this.selectKit.options.set(key, computedValue);
            return;
          }
        }
-        computedOptions[key] = value;
+
+        this.selectKit.options.set(key, value);
      });
-      this.selectKit.options.setProperties(
-        Object.assign(computedOptions, this.options || {})
-      );

      this.selectKit.setProperties({
        hasSelection: !isEmpty(this.value),
@ -264,6 +266,7 @@ export default Component.extend(
    },

    selectKitOptions: {
+      allowAny: false,
      showFullTitle: true,
      none: null,
      translatedNone: null,
@ -277,7 +280,6 @@ export default Component.extend(
      maximum: null,
      maximumLabel: null,
      minimum: null,
-      minimumLabel: null,
      autoInsertNoneItem: true,
      closeOnChange: true,
      limitMatches: null,
--- a/app/assets/javascripts/select-kit/addon/templates/components/multi-select.hbs
+++ b/app/assets/javascripts/select-kit/addon/templates/components/multi-select.hbs
@ -13,7 +13,7 @@
      id=(concat selectKit.uniqueID "-filter")
    }}

-    {{#if selectedContent}}
+    {{#if selectedContent.length}}
      <div class="selected-content">
        {{#each selectedContent as |item|}}
          {{component selectKit.options.selectedChoiceComponent
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@ -3768,6 +3768,9 @@ en:
      changed: "tags changed:"
      tags: "Tags"
      choose_for_topic: "optional tags"
+      choose_for_topic_required:
+        one: "select at least %{count} tag..."
+        other: "select at least %{count} tags..."
      info: "Info"
      default_info: "This tag isn't restricted to any categories, and has no synonyms. To add restrictions, put this tag in a <a href=%{basePath}/tag_groups>tag group</a>."
      category_restricted: "This tag is restricted to categories you don't have permission to access."