From fd9ef14ec03dfd48339e0fd4a403801fe4fb61fd Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Wed, 2 Jun 2021 12:43:34 -0400
Subject: [PATCH] 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).
---
 lib/discourse_tagging.rb                  | 18 ++++++++--
 spec/components/discourse_tagging_spec.rb | 41 ++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb
index 0699cf7e3c3..96dd3274a44 100644
--- a/lib/discourse_tagging.rb
+++ b/lib/discourse_tagging.rb
@@ -311,7 +311,13 @@ module DiscourseTagging
       sql.gsub!("/*and_name_like*/", "")
     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)
       if (required_tag_ids & selected_tag_ids).size < category.min_tags_from_required_group
         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)")
     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]
       builder.order_by("topic_count DESC, name")
     elsif opts[:order_search_results] && !term.blank?
diff --git a/spec/components/discourse_tagging_spec.rb b/spec/components/discourse_tagging_spec.rb
index 30e51bcad00..815e399d263 100644
--- a/spec/components/discourse_tagging_spec.rb
+++ b/spec/components/discourse_tagging_spec.rb
@@ -162,14 +162,53 @@ describe DiscourseTagging do
           expect(sorted_tag_names(tags)).to contain_exactly(tag2.name)
         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),
             for_input: true,
             category: category,
             term: 'fun'
           ).to_a
+
           expect(sorted_tag_names(tags)).to eq(sorted_tag_names([tag1, tag2, tag3]))
         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
 
       context 'empty term' do