FIX: Show required tags to staff by default and override limit (#13242)

This improves the display of available tags in categories that are
configured to require at least (x) tags from a tag group.

There are two changes included:
- regular users will now see all the available tags in the required tag
group (previously they could see a max. of 5 tags)
- staff users will now see the tags from the required tag group when
the tag group contains more tags than the default limit (also set to 5)

Both changes only apply to the default query (i.e. no search terms).
This commit is contained in:
Penar Musaraj 2021-06-02 12:43:34 -04:00 committed by GitHub
parent e81a5182b3
commit fd9ef14ec0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 56 additions and 3 deletions

View File

@ -311,7 +311,13 @@ module DiscourseTagging
sql.gsub!("/*and_name_like*/", "") sql.gsub!("/*and_name_like*/", "")
end end
if opts[:for_input] && filter_for_non_staff && category&.required_tag_group # show required tags for non-staff
# or for staff when
# - there are more available tags than the query limit
# - 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))
if opts[:for_input] && filter_required_tags
required_tag_ids = category.required_tag_group.tags.pluck(:id) required_tag_ids = category.required_tag_group.tags.pluck(:id)
if (required_tag_ids & selected_tag_ids).size < category.min_tags_from_required_group if (required_tag_ids & selected_tag_ids).size < category.min_tags_from_required_group
builder.where("id IN (?)", required_tag_ids) builder.where("id IN (?)", required_tag_ids)
@ -364,7 +370,15 @@ module DiscourseTagging
builder.where("id NOT IN (SELECT target_tag_id FROM tags WHERE target_tag_id IS NOT NULL)") builder.where("id NOT IN (SELECT target_tag_id FROM tags WHERE target_tag_id IS NOT NULL)")
end end
builder.limit(opts[:limit]) if opts[:limit] if opts[:limit]
if required_tag_ids && term.blank?
# override limit so all required tags are shown by default
builder.limit(required_tag_ids.size)
else
builder.limit(opts[:limit])
end
end
if opts[:order_popularity] if opts[:order_popularity]
builder.order_by("topic_count DESC, name") builder.order_by("topic_count DESC, name")
elsif opts[:order_search_results] && !term.blank? elsif opts[:order_search_results] && !term.blank?

View File

@ -162,14 +162,53 @@ describe DiscourseTagging do
expect(sorted_tag_names(tags)).to contain_exactly(tag2.name) expect(sorted_tag_names(tags)).to contain_exactly(tag2.name)
end end
it "let's staff ignore the requirement" do it "lets staff ignore the requirement" do
tags = DiscourseTagging.filter_allowed_tags(Guardian.new(admin),
for_input: true,
category: category,
limit: 5
).to_a
expect(sorted_tag_names(tags)).to eq(sorted_tag_names([tag1, tag2, tag3]))
end
end
context 'with many required tags in a tag group' do
fab!(:tag4) { Fabricate(:tag, name: "T4") }
fab!(:tag5) { Fabricate(:tag, name: "T5") }
fab!(:tag6) { Fabricate(:tag, name: "T6") }
fab!(:tag7) { Fabricate(:tag, name: "T7") }
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) }
it "returns required tags for staff by default" do
tags = DiscourseTagging.filter_allowed_tags(Guardian.new(admin),
for_input: true,
category: category
).to_a
expect(sorted_tag_names(tags)).to eq(sorted_tag_names([tag1, tag2, tag4, tag5, tag6, tag7]))
end
it "ignores required tags for staff when searching using a term" do
tags = DiscourseTagging.filter_allowed_tags(Guardian.new(admin), tags = DiscourseTagging.filter_allowed_tags(Guardian.new(admin),
for_input: true, for_input: true,
category: category, category: category,
term: 'fun' term: 'fun'
).to_a ).to_a
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 "returns required tags for nonstaff and overrides limit" do
tags = DiscourseTagging.filter_allowed_tags(Guardian.new(user),
for_input: true,
limit: 5,
category: category
).to_a
expect(sorted_tag_names(tags)).to eq(sorted_tag_names([tag1, tag2, tag4, tag5, tag6, tag7]))
end
end end
context 'empty term' do context 'empty term' do