From b6d140e4bd4ece15b9162f490ed36e9ce74cda69 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Mon, 4 Nov 2019 16:51:18 -0500 Subject: [PATCH] UX: tag input suggests required tags if none have been selected This is a follow-up to the new feature that allows a category to require a certain number of tags from a tag group. The tag input will shows results from the required group if none have been chosen yet. Once a require tag is selected, the tag input will include other results as usual. Staff users can ignore this restriction, so the input behaviour is unchanged for them. --- lib/discourse_tagging.rb | 7 +++ spec/components/discourse_tagging_spec.rb | 44 +++++++++++++++++ spec/integration/category_tag_spec.rb | 57 +++++++++++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index de6592c2654..1ef8da2abc2 100644 --- a/lib/discourse_tagging.rb +++ b/lib/discourse_tagging.rb @@ -165,6 +165,13 @@ module DiscourseTagging # Filters for category-specific tags: category = opts[:category] + if opts[:for_input] && !guardian.nil? && !guardian.is_staff? && category&.required_tag_group + required_tag_ids = category.required_tag_group.tags.pluck(:id) + if (required_tag_ids & selected_tag_ids).size < category.min_tags_from_required_group + query = query.where('tags.id IN (?)', required_tag_ids) + end + end + if category && (category.tags.count > 0 || category.tag_groups.count > 0) if category.allow_global_tags # Select tags that: diff --git a/spec/components/discourse_tagging_spec.rb b/spec/components/discourse_tagging_spec.rb index 8aeb04f9aee..7605b09fa58 100644 --- a/spec/components/discourse_tagging_spec.rb +++ b/spec/components/discourse_tagging_spec.rb @@ -57,6 +57,50 @@ describe DiscourseTagging do expect(tags.size).to eq(3) end end + + context 'with required tags from tag group' do + fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) } + fab!(:category) { Fabricate(:category, required_tag_group: tag_group, min_tags_from_required_group: 1) } + + it "returns the required tags if none have been selected" do + tags = DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), + for_input: true, + category: category, + term: 'fun' + ).to_a + expect(tags).to contain_exactly(tag1, tag2) + end + + it "returns all allowed tags if a required tag is selected" do + tags = DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), + for_input: true, + category: category, + selected_tags: [tag1.name], + term: 'fun' + ).to_a + expect(tags).to contain_exactly(tag2, tag3) + end + + it "returns required tags if not enough are selected" do + category.update!(min_tags_from_required_group: 2) + tags = DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), + for_input: true, + category: category, + selected_tags: [tag1.name], + term: 'fun' + ).to_a + expect(tags).to contain_exactly(tag2) + end + + it "let's staff ignore the requirement" do + tags = DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(admin), + for_input: true, + category: category, + term: 'fun' + ).to_a + expect(tags).to contain_exactly(tag1, tag2, tag3) + end + end end end diff --git a/spec/integration/category_tag_spec.rb b/spec/integration/category_tag_spec.rb index 9fddfb35ccd..60ea91bbb32 100644 --- a/spec/integration/category_tag_spec.rb +++ b/spec/integration/category_tag_spec.rb @@ -71,6 +71,17 @@ describe "category tag restrictions" do expect { other_category.update(allowed_tags: [tag1.name, 'tag-stuff', tag2.name, 'another-tag']) }.to change { Tag.count }.by(2) end + context 'required tags from tag group' do + 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) } + + it "search only returns the allowed tags" do + expect(filter_allowed_tags(for_input: true, category: category_with_tags)).to contain_exactly(tag1) + expect(filter_allowed_tags(for_input: true, category: category_with_tags, selected_tags: [tag1.name])).to contain_exactly(tag2) + expect(filter_allowed_tags(for_input: true, category: category_with_tags, selected_tags: [tag2.name])).to contain_exactly(tag1) + end + end + context 'category allows other tags to be used' do before do category_with_tags.update!(allow_global_tags: true) @@ -93,6 +104,17 @@ describe "category tag restrictions" do expect(filter_allowed_tags(for_input: true, category: other_category, selected_tags: [tag3.name])).to contain_exactly(tag4) expect(filter_allowed_tags(for_input: true, category: other_category, selected_tags: [tag3.name], term: 'tag')).to contain_exactly(tag4) end + + context 'required tags from tag group' do + 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) } + + it "search only returns the allowed tags" do + expect(filter_allowed_tags(for_input: true, category: category_with_tags)).to contain_exactly(tag1, tag3) + expect(filter_allowed_tags(for_input: true, category: category_with_tags, selected_tags: [tag1.name])).to contain_exactly(tag2, tag3, tag4) + expect(filter_allowed_tags(for_input: true, category: category_with_tags, selected_tags: [tag2.name])).to contain_exactly(tag1, tag3) + end + end end end @@ -131,6 +153,17 @@ describe "category tag restrictions" do expect(post.topic.tags.map(&:name)).to eq([tag1.name]) end + context 'required tags from tag group' do + fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag3]) } + before { category.update!(required_tag_group: tag_group, min_tags_from_required_group: 1) } + + it "search only returns the allowed tags" do + expect(filter_allowed_tags(for_input: true, category: category)).to contain_exactly(tag1) + expect(filter_allowed_tags(for_input: true, category: category, selected_tags: [tag1.name])).to contain_exactly(tag2) + expect(filter_allowed_tags(for_input: true, category: category, selected_tags: [tag2.name])).to contain_exactly(tag1) + end + end + context 'category allows other tags to be used' do before do category.update!(allow_global_tags: true) @@ -154,6 +187,17 @@ describe "category tag restrictions" do expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag1) end + context 'required tags from tag group' do + fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag3]) } + before { category.update!(required_tag_group: tag_group, min_tags_from_required_group: 1) } + + it "search only returns the allowed tags" do + expect(filter_allowed_tags(for_input: true, category: category)).to contain_exactly(tag1, tag3) + expect(filter_allowed_tags(for_input: true, category: category, selected_tags: [tag1.name])).to contain_exactly(tag2, tag3, tag4) + expect(filter_allowed_tags(for_input: true, category: category, selected_tags: [tag2.name])).to contain_exactly(tag1, tag3) + end + end + context 'another category has restricted tags using groups' do fab!(:category2) { Fabricate(:category) } fab!(:tag_group2) { Fabricate(:tag_group) } @@ -229,6 +273,19 @@ describe "category tag restrictions" do expect(filter_allowed_tags(for_input: true, selected_tags: [tag3.name])).to contain_exactly(tag4, tag1, common) end + context 'required tags from tag group' do + fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) } + fab!(:category) { Fabricate(:category, required_tag_group: tag_group, min_tags_from_required_group: 1) } + + it "search only returns the allowed tags" do + tag_group_with_parent = Fabricate(:tag_group, parent_tag_id: tag1.id, tags: [tag3, tag4]) + expect(filter_allowed_tags(for_input: true, category: category)).to contain_exactly(tag1, tag2) + expect(filter_allowed_tags(for_input: true, category: category, selected_tags: [tag2.name])).to contain_exactly(tag1) + expect(filter_allowed_tags(for_input: true, category: category, selected_tags: [tag1.name])).to contain_exactly(tag2, tag3, tag4) + expect(filter_allowed_tags(for_input: true, category: category, selected_tags: [tag1.name, tag2.name])).to contain_exactly(tag3, tag4) + end + end + context "and category restrictions" do fab!(:car_category) { Fabricate(:category) } fab!(:other_category) { Fabricate(:category) }