FEATURE: Allow multiple required tag groups for a category (#16381)

Previously we only supported a single 'required tag group' for a category. This commit allows admins to specify multiple required tag groups, each with their own minimum tag count.

A new category_required_tag_groups database table replaces the existing columns on the categories table. Data is automatically migrated.
This commit is contained in:
David Taylor
2022-04-06 14:08:06 +01:00
committed by GitHub
parent 8f03baaf8e
commit 68c74e9b93
32 changed files with 387 additions and 156 deletions

View File

@ -1,8 +1,29 @@
import { and, empty } from "@ember/object/computed"; import { and, empty } from "@ember/object/computed";
import { buildCategoryPanel } from "discourse/components/edit-category-panel"; import { buildCategoryPanel } from "discourse/components/edit-category-panel";
import { action, set } from "@ember/object";
export default buildCategoryPanel("tags", { export default buildCategoryPanel("tags", {
allowedTagsEmpty: empty("category.allowed_tags"), allowedTagsEmpty: empty("category.allowed_tags"),
allowedTagGroupsEmpty: empty("category.allowed_tag_groups"), allowedTagGroupsEmpty: empty("category.allowed_tag_groups"),
disableAllowGlobalTags: and("allowedTagsEmpty", "allowedTagGroupsEmpty"), disableAllowGlobalTags: and("allowedTagsEmpty", "allowedTagGroupsEmpty"),
@action
onTagGroupChange(rtg, valueArray) {
// A little strange, but we're using a multi-select component
// to select a single tag group. This action takes the array
// and extracts the first value in it.
set(rtg, "name", valueArray[0]);
},
@action
addRequiredTagGroup() {
this.category.required_tag_groups.pushObject({
min_count: 1,
});
},
@action
deleteRequiredTagGroup(rtg) {
this.category.required_tag_groups.removeObject(rtg);
},
}); });

View File

@ -1435,17 +1435,12 @@ export default Controller.extend({
tagValidation(category, tags, lastValidatedAt) { tagValidation(category, tags, lastValidatedAt) {
const tagsArray = tags || []; const tagsArray = tags || [];
if (this.site.can_tag_topics && !this.currentUser.staff && category) { if (this.site.can_tag_topics && !this.currentUser.staff && category) {
if ( // category.minimumRequiredTags incorporates both minimum_required_tags, and required_tag_groups
category.minimum_required_tags > tagsArray.length || if (category.minimumRequiredTags > tagsArray.length) {
(category.required_tag_groups &&
category.min_tags_from_required_group > tagsArray.length)
) {
return EmberObject.create({ return EmberObject.create({
failed: true, failed: true,
reason: I18n.t("composer.error.tags_missing", { reason: I18n.t("composer.error.tags_missing", {
count: count: category.minimumRequiredTags,
category.minimum_required_tags ||
category.min_tags_from_required_group,
}), }),
lastShownAt: lastValidatedAt, lastShownAt: lastValidatedAt,
}); });

View File

@ -35,21 +35,13 @@ const Category = RestModel.extend({
} }
}, },
@on("init") @discourseComputed("required_tag_groups", "minimum_required_tags")
setupRequiredTagGroups() {
if (this.required_tag_group_name) {
this.set("required_tag_groups", [this.required_tag_group_name]);
}
},
@discourseComputed(
"required_tag_groups",
"min_tags_from_required_group",
"minimum_required_tags"
)
minimumRequiredTags() { minimumRequiredTags() {
if (this.required_tag_groups) { if (this.required_tag_groups?.length > 0) {
return this.min_tags_from_required_group; return this.required_tag_groups.reduce(
(sum, rtg) => sum + rtg.min_count,
0
);
} else { } else {
return this.minimum_required_tags > 0 ? this.minimum_required_tags : null; return this.minimum_required_tags > 0 ? this.minimum_required_tags : null;
} }
@ -200,7 +192,8 @@ const Category = RestModel.extend({
const url = id ? `/categories/${id}` : "/categories"; const url = id ? `/categories/${id}` : "/categories";
return ajax(url, { return ajax(url, {
data: { contentType: "application/json",
data: JSON.stringify({
name: this.name, name: this.name,
slug: this.slug, slug: this.slug,
color: this.color, color: this.color,
@ -234,11 +227,7 @@ const Category = RestModel.extend({
? this.allowed_tag_groups ? this.allowed_tag_groups
: null, : null,
allow_global_tags: this.allow_global_tags, allow_global_tags: this.allow_global_tags,
required_tag_group_name: required_tag_groups: this.required_tag_groups,
this.required_tag_groups && this.required_tag_groups.length > 0
? this.required_tag_groups[0]
: null,
min_tags_from_required_group: this.min_tags_from_required_group,
sort_order: this.sort_order, sort_order: this.sort_order,
sort_ascending: this.sort_ascending, sort_ascending: this.sort_ascending,
topic_featured_link_allowed: this.topic_featured_link_allowed, topic_featured_link_allowed: this.topic_featured_link_allowed,
@ -255,7 +244,7 @@ const Category = RestModel.extend({
reviewable_by_group_name: this.reviewable_by_group_name, reviewable_by_group_name: this.reviewable_by_group_name,
read_only_banner: this.read_only_banner, read_only_banner: this.read_only_banner,
default_list_filter: this.default_list_filter, default_list_filter: this.default_list_filter,
}, }),
type: id ? "PUT" : "POST", type: id ? "PUT" : "POST",
}); });
}, },

View File

@ -18,6 +18,7 @@
{{tag-group-chooser {{tag-group-chooser
id="category-allowed-tag-groups" id="category-allowed-tag-groups"
tagGroups=category.allowed_tag_groups tagGroups=category.allowed_tag_groups
onChange=(action (mut category.allowed_tag_groups))
}} }}
{{#link-to "tagGroups" class="manage-tag-groups"}}{{i18n "category.manage_tag_groups_link"}}{{/link-to}} {{#link-to "tagGroups" class="manage-tag-groups"}}{{i18n "category.manage_tag_groups_link"}}{{/link-to}}
</section> </section>
@ -34,23 +35,33 @@
</section> </section>
<section class="field required-tag-group-description"> <section class="field required-tag-group-description">
{{i18n "category.required_tag_group_description"}} {{i18n "category.required_tag_group.description"}}
</section> </section>
<section class="field with-items"> <section class="field with-items">
<section class="field-item min-tags-from-required-group"> <section class="field-item required-tag-groups">
<label for="category-min-tags-from-group">{{i18n "category.min_tags_from_required_group_label"}}</label> {{#each category.required_tag_groups as |rtg|}}
{{text-field value=category.min_tags_from_required_group id="category-min-tags-from-group" type="number" min="1"}} <div class="required-tag-group-row">
</section> {{text-field value=rtg.min_count type="number" min="1"}}
<section class="field-item required-tag-group">
<label>{{i18n "category.required_tag_group_label"}}</label>
{{tag-group-chooser {{tag-group-chooser
id="category-required-tag-group" tagGroups=(if rtg.name (array rtg.name) (array))
tagGroups=category.required_tag_groups onChange=(action "onTagGroupChange" rtg)
options=(hash options=(hash
maximum=1 maximum=1
filterPlaceholder="category.tag_group_selector_placeholder" filterPlaceholder="category.required_tag_group.placeholder"
) )
}} }}
{{d-button
label="category.required_tag_group.delete"
action=(action "deleteRequiredTagGroup" rtg)
icon="trash-alt"
class="delete-required-tag-group"}}
</div>
{{/each}}
{{d-button
label="category.required_tag_group.add"
action=(action "addRequiredTagGroup")
icon="plus"
class="add-required-tag-group"}}
</section> </section>
</section> </section>

View File

@ -1,5 +1,7 @@
import { import {
acceptance, acceptance,
count,
exists,
queryAll, queryAll,
visible, visible,
} from "discourse/tests/helpers/qunit-helpers"; } from "discourse/tests/helpers/qunit-helpers";
@ -11,7 +13,7 @@ import { test } from "qunit";
acceptance("Category Edit", function (needs) { acceptance("Category Edit", function (needs) {
needs.user(); needs.user();
needs.settings({ email_in: true }); needs.settings({ email_in: true, tagging_enabled: true });
test("Editing the category", async function (assert) { test("Editing the category", async function (assert) {
await visit("/c/bug"); await visit("/c/bug");
@ -70,6 +72,31 @@ acceptance("Category Edit", function (needs) {
); );
}); });
test("Editing required tag groups", async function (assert) {
await visit("/c/bug/edit/tags");
assert.ok(exists(".required-tag-groups"));
assert.strictEqual(count(".required-tag-group-row"), 0);
await click(".add-required-tag-group");
assert.strictEqual(count(".required-tag-group-row"), 1);
await click(".add-required-tag-group");
assert.strictEqual(count(".required-tag-group-row"), 2);
await click(".delete-required-tag-group");
assert.strictEqual(count(".required-tag-group-row"), 1);
const tagGroupChooser = selectKit(
".required-tag-group-row .tag-group-chooser"
);
await tagGroupChooser.expand();
await tagGroupChooser.selectRowByValue("TagGroup1");
await click("#save-category");
assert.strictEqual(count(".required-tag-group-row"), 1);
});
test("Index Route", async function (assert) { test("Index Route", async function (assert) {
await visit("/c/bug/edit"); await visit("/c/bug/edit");
assert.strictEqual( assert.strictEqual(

View File

@ -74,8 +74,7 @@ acceptance("Composer - Tags", function (needs) {
await fillIn(".d-editor-input", "this is the *content* of a post"); await fillIn(".d-editor-input", "this is the *content* of a post");
Category.findById(2).setProperties({ Category.findById(2).setProperties({
required_tag_groups: ["support tags"], required_tag_groups: [{ name: "support tags", min_count: 1 }],
min_tags_from_required_group: 1,
}); });
const categoryChooser = selectKit(".category-chooser"); const categoryChooser = selectKit(".category-chooser");

View File

@ -366,6 +366,7 @@ export default {
can_edit: true, can_edit: true,
show_subcategory_list: false, show_subcategory_list: false,
default_view: "latest", default_view: "latest",
required_tag_groups: [],
}, },
{ {
id: 17, id: 17,

View File

@ -0,0 +1,8 @@
export default {
"/tag_groups/filter/search": {
results: [
{ name: "TagGroup1", tag_names: ["alpha", "bravo", "charlie"] },
{ name: "TagGroup2", tag_names: ["delta", "echo"] },
],
},
};

View File

@ -356,7 +356,7 @@ export function applyDefaultHandlers(pretender) {
); );
pretender.put("/categories/:category_id", (request) => { pretender.put("/categories/:category_id", (request) => {
const category = parsePostData(request.requestBody); const category = JSON.parse(request.requestBody);
category.id = parseInt(request.params.category_id, 10); category.id = parseInt(request.params.category_id, 10);
if (category.email_in === "duplicate@example.com") { if (category.email_in === "duplicate@example.com") {
@ -1121,6 +1121,10 @@ export function applyDefaultHandlers(pretender) {
], ],
}); });
}); });
pretender.get("/tag_groups/filter/search", () =>
response(fixturesByUrl["/tag_groups/filter/search"])
);
} }
export function resetPretender() { export function resetPretender() {

View File

@ -227,8 +227,7 @@ module("Unit | Model | category", function () {
let foo = store.createRecord("category", { let foo = store.createRecord("category", {
id: 1, id: 1,
slug: "foo", slug: "foo",
required_tag_groups: ["bar"], required_tag_groups: [{ name: "bar", min_count: 2 }],
min_tags_from_required_group: 2,
}); });
assert.equal(foo.minimumRequiredTags, 2); assert.equal(foo.minimumRequiredTags, 2);
@ -259,7 +258,7 @@ module("Unit | Model | category", function () {
foo = store.createRecord("category", { foo = store.createRecord("category", {
id: 5, id: 5,
slug: "foo", slug: "foo",
min_tags_from_required_group: 2, required_tag_groups: [],
}); });
assert.equal(foo.minimumRequiredTags, null); assert.equal(foo.minimumRequiredTags, null);

View File

@ -28,12 +28,6 @@ export default MultiSelectComponent.extend(TagsMixin, {
.map((t) => this.defaultItem(t, t)); .map((t) => this.defaultItem(t, t));
}), }),
actions: {
onChange(value) {
this.set("tagGroups", value);
},
},
search(query) { search(query) {
const data = { const data = {
q: query, q: query,

View File

@ -162,6 +162,23 @@ div.edit-category {
.category-default-slow-mode-seconds { .category-default-slow-mode-seconds {
width: 200px; width: 200px;
} }
.required-tag-groups {
.required-tag-group-row {
display: flex;
gap: 0.5em;
> * {
margin: 0;
}
input[type="number"] {
width: 4em;
}
margin-bottom: 1em;
}
}
} }
.category-permissions-table { .category-permissions-table {

View File

@ -312,7 +312,7 @@ class CategoriesController < ApplicationController
if SiteSetting.tagging_enabled if SiteSetting.tagging_enabled
params[:allowed_tags] = params[:allowed_tags].presence || [] if params[:allowed_tags] params[:allowed_tags] = params[:allowed_tags].presence || [] if params[:allowed_tags]
params[:allowed_tag_groups] = params[:allowed_tag_groups].presence || [] if params[:allowed_tag_groups] params[:allowed_tag_groups] = params[:allowed_tag_groups].presence || [] if params[:allowed_tag_groups]
params[:required_tag_group_name] = params[:required_tag_group_name].presence || '' if params[:required_tag_group_name] params[:required_tag_groups] = params[:required_tag_groups].presence || [] if params[:required_tag_groups]
end end
if SiteSetting.enable_category_group_moderation? if SiteSetting.enable_category_group_moderation?
@ -351,8 +351,6 @@ class CategoriesController < ApplicationController
:navigate_to_first_post_after_read, :navigate_to_first_post_after_read,
:search_priority, :search_priority,
:allow_global_tags, :allow_global_tags,
:required_tag_group_name,
:min_tags_from_required_group,
:read_only_banner, :read_only_banner,
:default_list_filter, :default_list_filter,
:reviewable_by_group_id, :reviewable_by_group_id,
@ -360,8 +358,13 @@ class CategoriesController < ApplicationController
permissions: [*p.try(:keys)], permissions: [*p.try(:keys)],
allowed_tags: [], allowed_tags: [],
allowed_tag_groups: [], allowed_tag_groups: [],
required_tag_groups: [:name, :min_count]
) )
if result[:required_tag_groups] && !result[:required_tag_groups].is_a?(Array)
raise Discourse::InvalidParameters.new(:required_tag_groups)
end
result result
end end
end end

View File

@ -5,10 +5,11 @@ class Category < ActiveRecord::Base
'none' 'none'
] ]
# TODO(2020-11-18): remove self.ignored_columns = [
self.ignored_columns = %w{ :suppress_from_latest, # TODO(2020-11-18): remove
suppress_from_latest :required_tag_group_id, # TODO(2023-04-01): remove
} :min_tags_from_required_group, # TODO(2023-04-01): remove
]
include Searchable include Searchable
include Positionable include Positionable
@ -56,7 +57,6 @@ class Category < ActiveRecord::Base
validates :num_featured_topics, numericality: { only_integer: true, greater_than: 0 } validates :num_featured_topics, numericality: { only_integer: true, greater_than: 0 }
validates :search_priority, inclusion: { in: Searchable::PRIORITIES.values } validates :search_priority, inclusion: { in: Searchable::PRIORITIES.values }
validates :min_tags_from_required_group, numericality: { only_integer: true, greater_than: 0 }
validate :parent_category_validator validate :parent_category_validator
validate :email_in_validator validate :email_in_validator
@ -103,7 +103,8 @@ class Category < ActiveRecord::Base
has_many :tags, through: :category_tags has_many :tags, through: :category_tags
has_many :category_tag_groups, dependent: :destroy has_many :category_tag_groups, dependent: :destroy
has_many :tag_groups, through: :category_tag_groups has_many :tag_groups, through: :category_tag_groups
belongs_to :required_tag_group, class_name: 'TagGroup'
has_many :category_required_tag_groups, -> { order(order: :asc) }, dependent: :destroy
belongs_to :reviewable_by_group, class_name: 'Group' belongs_to :reviewable_by_group, class_name: 'Group'
@ -639,8 +640,14 @@ class Category < ActiveRecord::Base
self.tag_groups = TagGroup.where(name: group_names).all.to_a self.tag_groups = TagGroup.where(name: group_names).all.to_a
end end
def required_tag_group_name=(group_name) def required_tag_groups=(required_groups)
self.required_tag_group = group_name.blank? ? nil : TagGroup.where(name: group_name).first map = Array(required_groups).map.with_index { |rg, i| [rg["name"], { min_count: rg["min_count"].to_i, order: i }] }.to_h
tag_groups = TagGroup.where(name: map.keys)
self.category_required_tag_groups = tag_groups.map do |tag_group|
attrs = map[tag_group.name]
CategoryRequiredTagGroup.new(tag_group: tag_group, **attrs)
end.sort_by(&:order)
end end
def downcase_email def downcase_email
@ -1044,8 +1051,6 @@ end
# search_priority :integer default(0) # search_priority :integer default(0)
# allow_global_tags :boolean default(FALSE), not null # allow_global_tags :boolean default(FALSE), not null
# reviewable_by_group_id :integer # reviewable_by_group_id :integer
# required_tag_group_id :integer
# min_tags_from_required_group :integer default(1), not null
# read_only_banner :string # read_only_banner :string
# default_list_filter :string(20) default("all") # default_list_filter :string(20) default("all")
# allow_unlimited_owner_edits_on_first_post :boolean default(FALSE), not null # allow_unlimited_owner_edits_on_first_post :boolean default(FALSE), not null

View File

@ -0,0 +1,29 @@
# frozen_string_literal: true
class CategoryRequiredTagGroup < ActiveRecord::Base
belongs_to :category
belongs_to :tag_group
validates :min_count, numericality: { only_integer: true, greater_than: 0 }
after_commit do
Site.clear_cache
end
end
# == Schema Information
#
# Table name: category_required_tag_groups
#
# id :bigint not null, primary key
# category_id :bigint not null
# tag_group_id :bigint not null
# min_count :integer default(1), not null
# order :integer default(1), not null
# created_at :datetime not null
# updated_at :datetime not null
#
# Indexes
#
# idx_category_required_tag_groups (category_id,tag_group_id) UNIQUE
#

View File

@ -65,7 +65,7 @@ class Site
# corresponding ActiveRecord callback to clear the categories cache. # corresponding ActiveRecord callback to clear the categories cache.
Discourse.cache.fetch(categories_cache_key, expires_in: 30.minutes) do Discourse.cache.fetch(categories_cache_key, expires_in: 30.minutes) do
categories = Category categories = Category
.includes(:uploaded_logo, :uploaded_background, :tags, :tag_groups, :required_tag_group) .includes(:uploaded_logo, :uploaded_background, :tags, :tag_groups, category_required_tag_groups: :tag_group)
.joins('LEFT JOIN topics t on t.id = categories.topic_id') .joins('LEFT JOIN topics t on t.id = categories.topic_id')
.select('categories.*, t.slug topic_slug') .select('categories.*, t.slug topic_slug')
.order(:position) .order(:position)

View File

@ -0,0 +1,9 @@
# frozen_string_literal: true
class CategoryRequiredTagGroupSerializer < ApplicationSerializer
attributes :name, :min_count
def name
object.tag_group.name
end
end

View File

@ -5,10 +5,10 @@ class SiteCategorySerializer < BasicCategorySerializer
attributes :allowed_tags, attributes :allowed_tags,
:allowed_tag_groups, :allowed_tag_groups,
:allow_global_tags, :allow_global_tags,
:min_tags_from_required_group,
:required_tag_group_name,
:read_only_banner :read_only_banner
has_many :category_required_tag_groups, key: :required_tag_groups, embed: :objects
def include_allowed_tags? def include_allowed_tags?
SiteSetting.tagging_enabled SiteSetting.tagging_enabled
end end
@ -29,7 +29,7 @@ class SiteCategorySerializer < BasicCategorySerializer
SiteSetting.tagging_enabled SiteSetting.tagging_enabled
end end
def required_tag_group_name def include_required_tag_groups?
object.required_tag_group&.name SiteSetting.tagging_enabled
end end
end end

View File

@ -3324,10 +3324,11 @@ en:
tag_groups_placeholder: "(Optional) list of allowed tag groups" tag_groups_placeholder: "(Optional) list of allowed tag groups"
manage_tag_groups_link: "Manage tag groups" manage_tag_groups_link: "Manage tag groups"
allow_global_tags_label: "Also allow other tags" allow_global_tags_label: "Also allow other tags"
tag_group_selector_placeholder: "(Optional) Tag group" required_tag_group:
required_tag_group_description: "Require new topics to have tags from a tag group:" description: "Require new topics to have tags from tag groups:"
min_tags_from_required_group_label: "Num Tags:" delete: "Delete"
required_tag_group_label: "Tag group:" add: "Add required tag group"
placeholder: "select tag group..."
topic_featured_link_allowed: "Allow featured links in this category" topic_featured_link_allowed: "Allow featured links in this category"
delete: "Delete Category" delete: "Delete Category"
create: "New Category" create: "New Category"

View File

@ -0,0 +1,27 @@
# frozen_string_literal: true
class CreateCategoryRequiredTagGroups < ActiveRecord::Migration[6.1]
def up
create_table :category_required_tag_groups do |t|
t.bigint :category_id, null: false
t.bigint :tag_group_id, null: false
t.integer :min_count, null: false, default: 1
t.integer :order, null: false, default: 1
t.timestamps
end
add_index :category_required_tag_groups, [:category_id, :tag_group_id], name: "idx_category_required_tag_groups", unique: true
execute <<~SQL
INSERT INTO category_required_tag_groups
(category_id, tag_group_id, min_count, updated_at, created_at)
SELECT c.id, c.required_tag_group_id, c.min_tags_from_required_group, NOW(), NOW()
FROM categories c
WHERE c.required_tag_group_id IS NOT NULL
SQL
end
def down
drop_table :category_required_tag_groups
end
end

View File

@ -0,0 +1,20 @@
# frozen_string_literal: true
class DropCategoryRequiredTagGroupColumns < ActiveRecord::Migration[6.1]
DROPPED_COLUMNS ||= {
categories: %i{
required_tag_group_id
min_tags_from_required_group
}
}
def up
DROPPED_COLUMNS.each do |table, columns|
Migration::ColumnDropper.execute_drop(table, columns)
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -155,26 +155,29 @@ module DiscourseTagging
end end
def self.validate_required_tags_from_group(guardian, model, category, tags = []) def self.validate_required_tags_from_group(guardian, model, category, tags = [])
if !guardian.is_staff? && return true if guardian.is_staff? || category.nil?
category &&
category.required_tag_group && success = true
(tags.length < category.min_tags_from_required_group || category.category_required_tag_groups.each do |crtg|
category.required_tag_group.tags.where("tags.id in (?)", tags.map(&:id)).count < category.min_tags_from_required_group) if tags.length < crtg.min_count ||
crtg.tag_group.tags.where("tags.id in (?)", tags.map(&:id)).count < crtg.min_count
success = false
model.errors.add(:base, model.errors.add(:base,
I18n.t( I18n.t(
"tags.required_tags_from_group", "tags.required_tags_from_group",
count: category.min_tags_from_required_group, count: crtg.min_count,
tag_group_name: category.required_tag_group.name, tag_group_name: crtg.tag_group.name,
tags: category.required_tag_group.tags.order(:id).pluck(:name).join(", ") tags: crtg.tag_group.tags.order(:id).pluck(:name).join(", ")
) )
) )
false
else
true
end end
end end
success
end
def self.validate_category_restricted_tags(guardian, model, category, tags = []) def self.validate_category_restricted_tags(guardian, model, category, tags = [])
return true if tags.blank? || category.blank? return true if tags.blank? || category.blank?
@ -363,13 +366,17 @@ module DiscourseTagging
# or for staff when # or for staff when
# - there are more available tags than the query limit # - there are more available tags than the query limit
# - and no search term has been included # - and no search term has been included
filter_required_tags = category&.required_tag_group && (filter_for_non_staff || (term.blank? && category&.required_tag_group&.tags.size >= opts[:limit].to_i)) required_tag_ids = nil
if opts[:for_input] && category&.category_required_tag_groups.present? && (filter_for_non_staff || term.blank?)
if opts[:for_input] && filter_required_tags category.category_required_tag_groups.each do |crtg|
required_tag_ids = category.required_tag_group.tags.pluck(:id) group_tags = crtg.tag_group.tags.pluck(:id)
if (required_tag_ids & selected_tag_ids).size < category.min_tags_from_required_group next if (group_tags & selected_tag_ids).size >= crtg.min_count
if filter_for_non_staff || group_tags.size >= opts[:limit].to_i
required_tag_ids = group_tags
builder.where("id IN (?)", required_tag_ids) builder.where("id IN (?)", required_tag_ids)
end end
break
end
end end
if filter_for_non_staff if filter_for_non_staff

View File

@ -96,7 +96,7 @@ describe "category tag restrictions" do
context 'required tags from tag group' do context 'required tags from tag group' do
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag3]) } fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag3]) }
before { category_with_tags.update!(required_tag_group: tag_group, min_tags_from_required_group: 1) } before { category_with_tags.update!(category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1)]) }
it "search only returns the allowed tags" do it "search only returns the allowed tags" do
expect_same_tag_names(filter_allowed_tags(for_input: true, category: category_with_tags), [tag1]) expect_same_tag_names(filter_allowed_tags(for_input: true, category: category_with_tags), [tag1])
@ -130,7 +130,7 @@ describe "category tag restrictions" do
context 'required tags from tag group' do context 'required tags from tag group' do
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag3]) } fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag3]) }
before { category_with_tags.update!(required_tag_group: tag_group, min_tags_from_required_group: 1) } before { category_with_tags.update!(category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1)]) }
it "search only returns the allowed tags" do it "search only returns the allowed tags" do
expect_same_tag_names(filter_allowed_tags(for_input: true, category: category_with_tags), [tag1, tag3]) expect_same_tag_names(filter_allowed_tags(for_input: true, category: category_with_tags), [tag1, tag3])
@ -190,7 +190,7 @@ describe "category tag restrictions" do
context 'required tags from tag group' do context 'required tags from tag group' do
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag3]) } fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag3]) }
before { category.update!(required_tag_group: tag_group, min_tags_from_required_group: 1) } before { category.update!(category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1)]) }
it "search only returns the allowed tags" do it "search only returns the allowed tags" do
expect_same_tag_names(filter_allowed_tags(for_input: true, category: category), [tag1]) expect_same_tag_names(filter_allowed_tags(for_input: true, category: category), [tag1])
@ -224,7 +224,7 @@ describe "category tag restrictions" do
context 'required tags from tag group' do context 'required tags from tag group' do
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag3]) } fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag3]) }
before { category.update!(required_tag_group: tag_group, min_tags_from_required_group: 1) } before { category.update!(category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1)]) }
it "search only returns the allowed tags" do it "search only returns the allowed tags" do
expect_same_tag_names(filter_allowed_tags(for_input: true, category: category), [tag1, tag3]) expect_same_tag_names(filter_allowed_tags(for_input: true, category: category), [tag1, tag3])
@ -320,7 +320,7 @@ describe "category tag restrictions" do
context 'required tags from tag group' do context 'required tags from tag group' do
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) } fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) }
fab!(:category) { Fabricate(:category, required_tag_group: tag_group, min_tags_from_required_group: 1) } fab!(:category) { Fabricate(:category, category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1)]) }
it "search only returns the allowed tags" do it "search only returns the allowed tags" do
tag_group_with_parent = Fabricate(:tag_group, parent_tag_id: tag1.id, tags: [tag3, tag4]) tag_group_with_parent = Fabricate(:tag_group, parent_tag_id: tag1.id, tags: [tag3, tag4])

View File

@ -129,7 +129,7 @@ describe DiscourseTagging do
context 'with required tags from tag group' do context 'with required tags from tag group' do
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) } fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) }
fab!(:category) { Fabricate(:category, required_tag_group: tag_group, min_tags_from_required_group: 1) } fab!(:category) { Fabricate(:category, category_required_tag_groups: [ CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1) ]) }
it "returns the required tags if none have been selected" do it "returns the required tags if none have been selected" do
tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user), tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user),
@ -151,7 +151,7 @@ describe DiscourseTagging do
end end
it "returns required tags if not enough are selected" do it "returns required tags if not enough are selected" do
category.update!(min_tags_from_required_group: 2) category.category_required_tag_groups.first.update!(min_count: 2)
tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user), tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user),
for_input: true, for_input: true,
category: category, category: category,
@ -171,6 +171,36 @@ describe DiscourseTagging do
expect(sorted_tag_names(tags)).to eq(sorted_tag_names([tag1, tag2, tag3])) expect(sorted_tag_names(tags)).to eq(sorted_tag_names([tag1, tag2, tag3]))
end end
it "handles multiple required tag groups in sequence" do
tag4 = Fabricate(:tag)
tag_group_2 = Fabricate(:tag_group, tags: [tag4])
CategoryRequiredTagGroup.create!(category: category, tag_group: tag_group_2, min_count: 1, order: 2)
category.reload
# In the beginning, show tags for tag_group
tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user),
for_input: true,
category: category,
).to_a
expect(sorted_tag_names(tags)).to eq(sorted_tag_names([tag1, tag2]))
# Once a tag_group tag has been selected, move on to tag_group_2 tags
tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user),
for_input: true,
category: category,
selected_tags: [tag1.name],
).to_a
expect(sorted_tag_names(tags)).to eq(sorted_tag_names([tag4]))
# Once all requirements are satisfied, show all tags
tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user),
for_input: true,
category: category,
selected_tags: [tag1.name, tag4.name],
).to_a
expect(sorted_tag_names(tags)).to eq(sorted_tag_names([tag2, tag3]))
end
end end
context 'with many required tags in a tag group' do context 'with many required tags in a tag group' do
@ -179,7 +209,7 @@ describe DiscourseTagging do
fab!(:tag6) { Fabricate(:tag, name: "T6") } fab!(:tag6) { Fabricate(:tag, name: "T6") }
fab!(:tag7) { Fabricate(:tag, name: "T7") } fab!(:tag7) { Fabricate(:tag, name: "T7") }
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2, tag4, tag5, tag6, tag7]) } fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2, tag4, tag5, tag6, tag7]) }
fab!(:category) { Fabricate(:category, required_tag_group: tag_group, min_tags_from_required_group: 1) } fab!(:category) { Fabricate(:category, category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1)]) }
it "returns required tags for staff by default" do it "returns required tags for staff by default" do
tags = DiscourseTagging.filter_allowed_tags(Guardian.new(admin), tags = DiscourseTagging.filter_allowed_tags(Guardian.new(admin),
@ -500,8 +530,7 @@ describe DiscourseTagging do
before do before do
tag_group.tags = [tag1, tag2] tag_group.tags = [tag1, tag2]
category.update( category.update(
required_tag_group: tag_group, category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1)],
min_tags_from_required_group: 1
) )
end end

View File

@ -577,7 +577,7 @@ describe NewPostManager do
let(:tag) { Fabricate(:tag) } let(:tag) { Fabricate(:tag) }
before do before do
TagGroupMembership.create(tag: tag, tag_group: tag_group) TagGroupMembership.create(tag: tag, tag_group: tag_group)
category.update(min_tags_from_required_group: 1, required_tag_group_id: tag_group.id) category.update(category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1)])
end end
it "errors when there are no tags from the group provided" do it "errors when there are no tags from the group provided" do
@ -593,8 +593,8 @@ describe NewPostManager do
expect(result.errors.full_messages).to include( expect(result.errors.full_messages).to include(
I18n.t( I18n.t(
"tags.required_tags_from_group", "tags.required_tags_from_group",
count: category.min_tags_from_required_group, count: category.category_required_tag_groups.first.min_count,
tag_group_name: category.required_tag_group.name, tag_group_name: category.category_required_tag_groups.first.tag_group.name,
tags: tag.name tags: tag.name
) )
) )

View File

@ -1123,7 +1123,7 @@ describe PostRevisor do
fab!(:tag2) { Fabricate(:tag) } fab!(:tag2) { Fabricate(:tag) }
fab!(:tag3) { Fabricate(:tag) } fab!(:tag3) { Fabricate(:tag) }
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) } fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) }
fab!(:category) { Fabricate(:category, name: "beta", required_tag_group: tag_group, min_tags_from_required_group: 1) } fab!(:category) { Fabricate(:category, name: "beta", category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1)]) }
before do before do
post.topic.update(category: category) post.topic.update(category: category)

View File

@ -159,7 +159,7 @@ describe TopicCreator do
context 'required tag group' do context 'required tag group' do
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1]) } fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1]) }
fab!(:category) { Fabricate(:category, name: "beta", required_tag_group: tag_group, min_tags_from_required_group: 1) } fab!(:category) { Fabricate(:category, name: "beta", category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group, min_count: 1)]) }
it "when no tags are not present" do it "when no tags are not present" do
expect( expect(

View File

@ -128,9 +128,6 @@
] ]
}, },
"min_tags_from_required_group": {
"type": "integer"
},
"allowed_tags": { "allowed_tags": {
"type": "array" "type": "array"
}, },
@ -140,10 +137,25 @@
"allow_global_tags": { "allow_global_tags": {
"type": "boolean" "type": "boolean"
}, },
"required_tag_group_name": { "required_tag_groups": {
"type": [ "type": "array",
"string", "items": [
"null" {
"type": "object",
"additionalProperties": false,
"properties": {
"name": {
"type": "string"
},
"min_count": {
"type": "integer"
}
},
"required": [
"name",
"min_count"
]
}
] ]
}, },
"read_only_banner": { "read_only_banner": {
@ -267,8 +279,7 @@
"minimum_required_tags", "minimum_required_tags",
"navigate_to_first_post_after_read", "navigate_to_first_post_after_read",
"custom_fields", "custom_fields",
"min_tags_from_required_group", "required_tag_groups",
"required_tag_group_name",
"read_only_banner", "read_only_banner",
"available_groups", "available_groups",
"auto_close_hours", "auto_close_hours",

View File

@ -131,9 +131,6 @@
] ]
}, },
"min_tags_from_required_group": {
"type": "integer"
},
"allowed_tags": { "allowed_tags": {
"type": "array" "type": "array"
}, },
@ -143,10 +140,25 @@
"allow_global_tags": { "allow_global_tags": {
"type": "boolean" "type": "boolean"
}, },
"required_tag_group_name": { "required_tag_groups": {
"type": [ "type": "array",
"string", "items": [
"null" {
"type": "object",
"additionalProperties": false,
"properties": {
"name": {
"type": "string"
},
"min_count": {
"type": "integer"
}
},
"required": [
"name",
"min_count"
]
}
] ]
}, },
"read_only_banner": { "read_only_banner": {
@ -270,8 +282,7 @@
"minimum_required_tags", "minimum_required_tags",
"navigate_to_first_post_after_read", "navigate_to_first_post_after_read",
"custom_fields", "custom_fields",
"min_tags_from_required_group", "required_tag_groups",
"required_tag_group_name",
"read_only_banner", "read_only_banner",
"available_groups", "available_groups",
"auto_close_hours", "auto_close_hours",

View File

@ -593,13 +593,25 @@
"allow_global_tags": { "allow_global_tags": {
"type": "boolean" "type": "boolean"
}, },
"min_tags_from_required_group": { "required_tag_groups": {
"type": "integer" "type": "array",
"items": [
{
"type": "object",
"additionalProperties": false,
"properties": {
"name": {
"type": "string"
}, },
"required_tag_group_name": { "min_count": {
"type": [ "type": "integer"
"string", }
"null" },
"required": [
"name",
"min_count"
]
}
] ]
}, },
"read_only_banner": { "read_only_banner": {
@ -655,8 +667,7 @@
"allowed_tags", "allowed_tags",
"allowed_tag_groups", "allowed_tag_groups",
"allow_global_tags", "allow_global_tags",
"min_tags_from_required_group", "required_tag_groups",
"required_tag_group_name",
"read_only_banner", "read_only_banner",
"uploaded_logo", "uploaded_logo",
"uploaded_background", "uploaded_background",

View File

@ -493,8 +493,10 @@ describe CategoriesController do
}, },
minimum_required_tags: "", minimum_required_tags: "",
allow_global_tags: 'true', allow_global_tags: 'true',
required_tag_group_name: tag_group.name, required_tag_groups: [{
min_tags_from_required_group: 2 name: tag_group.name,
min_count: 2
}]
} }
expect(response.status).to eq(200) expect(response.status).to eq(200)
@ -509,8 +511,9 @@ describe CategoriesController do
expect(category.custom_fields).to eq("dancing" => "frogs") expect(category.custom_fields).to eq("dancing" => "frogs")
expect(category.minimum_required_tags).to eq(0) expect(category.minimum_required_tags).to eq(0)
expect(category.allow_global_tags).to eq(true) expect(category.allow_global_tags).to eq(true)
expect(category.required_tag_group_id).to eq(tag_group.id) expect(category.category_required_tag_groups.count).to eq(1)
expect(category.min_tags_from_required_group).to eq(2) expect(category.category_required_tag_groups.first.tag_group.id).to eq(tag_group.id)
expect(category.category_required_tag_groups.first.min_count).to eq(2)
end end
it 'logs the changes correctly' do it 'logs the changes correctly' do
@ -558,19 +561,19 @@ describe CategoriesController do
it "can remove required tag group" do it "can remove required tag group" do
SiteSetting.tagging_enabled = true SiteSetting.tagging_enabled = true
category.update!(required_tag_group: Fabricate(:tag_group)) category.update!(category_required_tag_groups: [ CategoryRequiredTagGroup.new(tag_group: Fabricate(:tag_group)) ])
put "/categories/#{category.id}.json", params: { put "/categories/#{category.id}.json", params: {
name: category.name, name: category.name,
color: category.color, color: category.color,
text_color: category.text_color, text_color: category.text_color,
allow_global_tags: 'false', allow_global_tags: 'false',
min_tags_from_required_group: 1, min_tags_from_required_group: 1,
required_tag_group_name: '' required_tag_groups: []
} }
expect(response.status).to eq(200) expect(response.status).to eq(200)
category.reload category.reload
expect(category.required_tag_group).to be_nil expect(category.category_required_tag_groups).to be_empty
end end
it "does not update other fields" do it "does not update other fields" do
@ -581,7 +584,7 @@ describe CategoriesController do
category.update!( category.update!(
allowed_tags: ["hello", "world"], allowed_tags: ["hello", "world"],
allowed_tag_groups: [tag_group_1.name], allowed_tag_groups: [tag_group_1.name],
required_tag_group_name: tag_group_2.name, category_required_tag_groups: [ CategoryRequiredTagGroup.new(tag_group: tag_group_2) ],
custom_fields: { field_1: 'hello', field_2: 'hello' } custom_fields: { field_1: 'hello', field_2: 'hello' }
) )
@ -590,7 +593,7 @@ describe CategoriesController do
category.reload category.reload
expect(category.tags.pluck(:name)).to contain_exactly("hello", "world") expect(category.tags.pluck(:name)).to contain_exactly("hello", "world")
expect(category.tag_groups.pluck(:name)).to contain_exactly(tag_group_1.name) expect(category.tag_groups.pluck(:name)).to contain_exactly(tag_group_1.name)
expect(category.required_tag_group).to eq(tag_group_2) expect(category.category_required_tag_groups.first.tag_group).to eq(tag_group_2)
expect(category.custom_fields).to eq({ 'field_1' => 'hello', 'field_2' => 'hello' }) expect(category.custom_fields).to eq({ 'field_1' => 'hello', 'field_2' => 'hello' })
put "/categories/#{category.id}.json", params: { allowed_tags: [], custom_fields: { field_1: nil } } put "/categories/#{category.id}.json", params: { allowed_tags: [], custom_fields: { field_1: nil } }
@ -598,15 +601,15 @@ describe CategoriesController do
category.reload category.reload
expect(category.tags).to be_blank expect(category.tags).to be_blank
expect(category.tag_groups.pluck(:name)).to contain_exactly(tag_group_1.name) expect(category.tag_groups.pluck(:name)).to contain_exactly(tag_group_1.name)
expect(category.required_tag_group).to eq(tag_group_2) expect(category.category_required_tag_groups.first.tag_group).to eq(tag_group_2)
expect(category.custom_fields).to eq({ 'field_2' => 'hello' }) expect(category.custom_fields).to eq({ 'field_2' => 'hello' })
put "/categories/#{category.id}.json", params: { allowed_tags: [], allowed_tag_groups: [], required_tag_group_name: nil, custom_fields: { field_1: 'hi', field_2: nil } } put "/categories/#{category.id}.json", params: { allowed_tags: [], allowed_tag_groups: [], required_tag_groups: [], custom_fields: { field_1: 'hi', field_2: nil } }
expect(response.status).to eq(200) expect(response.status).to eq(200)
category.reload category.reload
expect(category.tags).to be_blank expect(category.tags).to be_blank
expect(category.tag_groups).to be_blank expect(category.tag_groups).to be_blank
expect(category.required_tag_group).to eq(nil) expect(category.category_required_tag_groups).to eq([])
expect(category.custom_fields).to eq({ 'field_1' => 'hi' }) expect(category.custom_fields).to eq({ 'field_1' => 'hi' })
end end
end end

View File

@ -35,14 +35,14 @@ describe SiteSerializer do
category.tags << tag category.tags << tag
category.tag_groups << tag_group category.tag_groups << tag_group
category.update!(required_tag_group: tag_group_2) category.update!(category_required_tag_groups: [CategoryRequiredTagGroup.new(tag_group: tag_group_2, min_count: 1)])
serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
c1 = serialized[:categories].find { |c| c[:id] == category.id } c1 = serialized[:categories].find { |c| c[:id] == category.id }
expect(c1[:allowed_tags]).to contain_exactly(tag.name) expect(c1[:allowed_tags]).to contain_exactly(tag.name)
expect(c1[:allowed_tag_groups]).to contain_exactly(tag_group.name) expect(c1[:allowed_tag_groups]).to contain_exactly(tag_group.name)
expect(c1[:required_tag_group_name]).to eq(tag_group_2.name) expect(c1[:required_tag_groups]).to eq([{ name: tag_group_2.name, min_count: 1 }])
end end
it "returns correct notification level for categories" do it "returns correct notification level for categories" do