From 8f7a387aa7553c05fbab478e3d9f3a16e2cd3ca2 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 27 Jun 2019 17:53:26 +1000 Subject: [PATCH] FEATURE: add support for tag group search The behaviour of #TERM in search has been amended 1. We try category or subcategory slugs 2. We try tags 3. We try tag-groups The term `hello #my-group` will search for all posts tagged with any of the tags in the tag group `My Group` Future work may be introducing a slug cache here or caching it in the table but the assumption is that the number of tag groups will not be huge --- app/models/tag_group.rb | 15 ++++++++--- lib/search.rb | 35 ++++++++++++++++++------- spec/components/search_spec.rb | 48 +++++++++++++++++++++++----------- 3 files changed, 70 insertions(+), 28 deletions(-) diff --git a/app/models/tag_group.rb b/app/models/tag_group.rb index 3771b80a797..1fd5711f820 100644 --- a/app/models/tag_group.rb +++ b/app/models/tag_group.rb @@ -36,11 +36,18 @@ class TagGroup < ActiveRecord::Base @permissions = TagGroup.resolve_permissions(permissions) end - def self.resolve_permissions(permissions) - everyone_group_id = Group::AUTO_GROUPS[:everyone] - full = TagGroupPermission.permission_types[:full] + # TODO: long term we can cache this if TONs of tag groups exist + def self.find_id_by_slug(slug) + self.pluck(:id, :name).each do |id, name| + if Slug.for(name) == slug + return id + end + end + nil + end - mapped = permissions.map do |group, permission| + def self.resolve_permissions(permissions) + permissions.map do |group, permission| group_id = Group.group_id_from_param(group) permission = TagGroupPermission.permission_types[permission] unless permission.is_a?(Integer) [group_id, permission] diff --git a/lib/search.rb b/lib/search.rb index 12af2135949..aa3bdb02b9c 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -453,17 +453,34 @@ class Search # try a possible tag match tag_id = Tag.where_name(category_slug).pluck(:id).first if (tag_id) - posts.where("topics.id IN ( - SELECT DISTINCT(tt.topic_id) - FROM topic_tags tt - WHERE tt.tag_id = ? - )", tag_id) + posts.where(<<~SQL, tag_id) + topics.id IN ( + SELECT DISTINCT(tt.topic_id) + FROM topic_tags tt + WHERE tt.tag_id = ? + ) + SQL else + if tag_group_id = TagGroup.find_id_by_slug(category_slug) + posts.where(<<~SQL, tag_group_id) + topics.id IN ( + SELECT DISTINCT(tt.topic_id) + FROM topic_tags tt + WHERE tt.tag_id in ( + SELECT tag_id + FROM tag_group_memberships + WHERE tag_group_id = ? + ) + ) + SQL + # a bit yucky but we got to add the term back in - if match.to_s.length >= SiteSetting.min_search_term_length - posts.where("posts.id IN ( - SELECT post_id FROM post_search_data pd1 - WHERE pd1.search_data @@ #{Search.ts_query(term: "##{match}")})") + elsif match.to_s.length >= SiteSetting.min_search_term_length + posts.where <<~SQL + posts.id IN ( + SELECT post_id FROM post_search_data pd1 + WHERE pd1.search_data @@ #{Search.ts_query(term: "##{match}")}) + SQL end end end diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 3e8f6b1eea7..7e6e45c2b77 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -1085,19 +1085,39 @@ describe Search do end context 'tags' do - let(:tag1) { Fabricate(:tag, name: 'lunch') } - let(:tag2) { Fabricate(:tag, name: 'eggs') } - let(:tag3) { Fabricate(:tag, name: 'sandwiches') } - let(:topic1) { Fabricate(:topic, tags: [tag2, Fabricate(:tag)]) } - let(:topic2) { Fabricate(:topic, tags: [tag2]) } - let(:topic3) { Fabricate(:topic, tags: [tag1, tag2]) } - let(:topic4) { Fabricate(:topic, tags: [tag1, tag2, tag3]) } - let(:topic5) { Fabricate(:topic, tags: [tag2, tag3]) } - let!(:post1) { Fabricate(:post, topic: topic1) } - let!(:post2) { Fabricate(:post, topic: topic2) } - let!(:post3) { Fabricate(:post, topic: topic3) } - let!(:post4) { Fabricate(:post, topic: topic4) } - let!(:post5) { Fabricate(:post, topic: topic5) } + fab!(:tag1) { Fabricate(:tag, name: 'lunch') } + fab!(:tag2) { Fabricate(:tag, name: 'eggs') } + fab!(:tag3) { Fabricate(:tag, name: 'sandwiches') } + + fab!(:tag_group) do + group = TagGroup.create!(name: 'mid day') + TagGroupMembership.create!(tag_id: tag1.id, tag_group_id: group.id) + TagGroupMembership.create!(tag_id: tag3.id, tag_group_id: group.id) + group + end + + fab!(:topic1) { Fabricate(:topic, tags: [tag2, Fabricate(:tag)]) } + fab!(:topic2) { Fabricate(:topic, tags: [tag2]) } + fab!(:topic3) { Fabricate(:topic, tags: [tag1, tag2]) } + fab!(:topic4) { Fabricate(:topic, tags: [tag1, tag2, tag3]) } + fab!(:topic5) { Fabricate(:topic, tags: [tag2, tag3]) } + + def indexed_post(*args) + SearchIndexer.enable + Fabricate(:post, *args) + end + + fab!(:post1) { indexed_post(topic: topic1) } + fab!(:post2) { indexed_post(topic: topic2) } + fab!(:post3) { indexed_post(topic: topic3) } + fab!(:post4) { indexed_post(topic: topic4) } + fab!(:post5) { indexed_post(topic: topic5) } + + it 'can find posts by tag group' do + expect(Search.execute('#mid-day').posts.map(&:id)).to ( + contain_exactly(post3.id, post4.id, post5.id) + ) + end it 'can find posts with tag' do post4 = Fabricate(:post, topic: topic3, raw: "It probably doesn't help that they're green...") @@ -1115,8 +1135,6 @@ describe Search do end it 'can find posts with any tag from multiple tags' do - Fabricate(:post) - expect(Search.execute('tags:eggs,lunch').posts.map(&:id).sort).to eq([post1.id, post2.id, post3.id, post4.id, post5.id].sort) end