From 2c56f8df7c1eba7ff81bc66eaafd1e27a8c4ff19 Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Fri, 25 Aug 2017 11:52:18 -0400
Subject: [PATCH] FEATURE: show tags in search results

---
 .../javascripts/discourse/lib/search.js.es6   |  8 +++-
 .../widgets/search-menu-results.js.es6        |  9 ++++
 app/jobs/scheduled/reindex_search.rb          | 18 ++++++++
 app/models/tag.rb                             |  8 ++++
 app/models/tag_search_data.rb                 |  3 ++
 .../grouped_search_result_serializer.rb       |  5 +++
 app/serializers/tag_serializer.rb             |  3 ++
 app/services/search_indexer.rb                |  8 ++++
 .../20170823173427_create_tag_search_data.rb  | 16 +++++++
 lib/search.rb                                 | 17 ++++++-
 lib/search/grouped_search_results.rb          |  2 +
 spec/components/search_spec.rb                | 45 +++++++++++++++++++
 .../javascripts/acceptance/search-test.js.es6 | 12 +++++
 .../helpers/create-pretender.js.es6           | 10 +++++
 14 files changed, 162 insertions(+), 2 deletions(-)
 create mode 100644 app/models/tag_search_data.rb
 create mode 100644 app/serializers/tag_serializer.rb
 create mode 100644 db/migrate/20170823173427_create_tag_search_data.rb

diff --git a/app/assets/javascripts/discourse/lib/search.js.es6 b/app/assets/javascripts/discourse/lib/search.js.es6
index 9b9828deaab..8939c173299 100644
--- a/app/assets/javascripts/discourse/lib/search.js.es6
+++ b/app/assets/javascripts/discourse/lib/search.js.es6
@@ -18,6 +18,7 @@ export function translateResults(results, opts) {
   if (!results.users) { results.users = []; }
   if (!results.posts) { results.posts = []; }
   if (!results.categories) { results.categories = []; }
+  if (!results.tags) { results.tags = []; }
 
   const topicMap = {};
   results.topics = results.topics.map(function(topic){
@@ -44,12 +45,17 @@ export function translateResults(results, opts) {
     return Category.list().findBy('id', category.id);
   }).compact();
 
+  results.tags = results.tags.map(function(tag){
+    let tagName = Handlebars.Utils.escapeExpression(tag.name);
+    return Ember.Object.create({ id: tagName, url: Discourse.getURL("/tags/" + tagName) });
+  }).compact();
+
   const r = results.grouped_search_result;
   results.resultTypes = [];
 
   // TODO: consider refactoring front end to take a better structure
   if (r) {
-    [['topic','posts'],['user','users'],['category','categories']].forEach(function(pair){
+    [['topic','posts'],['user','users'],['category','categories'],['tag','tags']].forEach(function(pair){
       const type = pair[0], name = pair[1];
       if (results[name].length > 0) {
         var result = {
diff --git a/app/assets/javascripts/discourse/widgets/search-menu-results.js.es6 b/app/assets/javascripts/discourse/widgets/search-menu-results.js.es6
index 8b10b12e3da..bc974d7c853 100644
--- a/app/assets/javascripts/discourse/widgets/search-menu-results.js.es6
+++ b/app/assets/javascripts/discourse/widgets/search-menu-results.js.es6
@@ -91,6 +91,15 @@ createSearchResult({
   }
 });
 
+createSearchResult({
+  type: 'tag',
+  linkField: 'url',
+  builder(t) {
+    const tag = Handlebars.Utils.escapeExpression(t.get('id'));
+    return h('a', { attributes: { href: t.get('url') }, className: `tag-${tag} discourse-tag ${Discourse.SiteSettings.tag_style}`}, tag);
+  }
+});
+
 createWidget('search-menu-results', {
   tagName: 'div.results',
 
diff --git a/app/jobs/scheduled/reindex_search.rb b/app/jobs/scheduled/reindex_search.rb
index 51577f6f320..e354b36d007 100644
--- a/app/jobs/scheduled/reindex_search.rb
+++ b/app/jobs/scheduled/reindex_search.rb
@@ -8,6 +8,7 @@ module Jobs
       rebuild_problem_posts
       rebuild_problem_categories
       rebuild_problem_users
+      rebuild_problem_tags
     end
 
     def rebuild_problem_categories(limit = 500)
@@ -47,6 +48,15 @@ module Jobs
       end
     end
 
+    def rebuild_problem_tags(limit = 10000)
+      tag_ids = load_problem_tag_ids(limit)
+
+      tag_ids.each do |id|
+        tag = Tag.find_by(id: id)
+        SearchIndexer.index(tag, force: true) if tag
+      end
+    end
+
     private
 
     def load_problem_post_ids(limit)
@@ -83,5 +93,13 @@ module Jobs
         .limit(limit)
         .pluck(:id)
     end
+
+    def load_problem_tag_ids(limit)
+      Tag.joins(:tag_search_data)
+        .where('tag_search_data.locale != ?
+                OR tag_search_data.version != ?', SiteSetting.default_locale, Search::INDEX_VERSION)
+        .limit(limit)
+        .pluck(:id)
+    end
   end
 end
diff --git a/app/models/tag.rb b/app/models/tag.rb
index cf965c22d4a..60f18572717 100644
--- a/app/models/tag.rb
+++ b/app/models/tag.rb
@@ -1,4 +1,6 @@
 class Tag < ActiveRecord::Base
+  include Searchable
+
   validates :name, presence: true, uniqueness: true
 
   has_many :tag_users # notification settings
@@ -12,6 +14,8 @@ class Tag < ActiveRecord::Base
   has_many :tag_group_memberships
   has_many :tag_groups, through: :tag_group_memberships
 
+  after_save :index_search
+
   COUNT_ARG = "topics.id"
 
   # Apply more activerecord filters to the tags_by_count_query, and then
@@ -56,6 +60,10 @@ class Tag < ActiveRecord::Base
   def full_url
     "#{Discourse.base_url}/tags/#{self.name}"
   end
+
+  def index_search
+    SearchIndexer.index(self)
+  end
 end
 
 # == Schema Information
diff --git a/app/models/tag_search_data.rb b/app/models/tag_search_data.rb
new file mode 100644
index 00000000000..998ddffc45f
--- /dev/null
+++ b/app/models/tag_search_data.rb
@@ -0,0 +1,3 @@
+class TagSearchData < ActiveRecord::Base
+  include HasSearchData
+end
diff --git a/app/serializers/grouped_search_result_serializer.rb b/app/serializers/grouped_search_result_serializer.rb
index bc4a5a6a7e3..2ce8d6143f9 100644
--- a/app/serializers/grouped_search_result_serializer.rb
+++ b/app/serializers/grouped_search_result_serializer.rb
@@ -2,6 +2,7 @@ class GroupedSearchResultSerializer < ApplicationSerializer
   has_many :posts, serializer: SearchPostSerializer
   has_many :users, serializer: SearchResultUserSerializer
   has_many :categories, serializer: BasicCategorySerializer
+  has_many :tags, serializer: TagSerializer
   attributes :more_posts, :more_users, :more_categories, :term, :search_log_id, :more_full_page_results
 
   def search_log_id
@@ -12,4 +13,8 @@ class GroupedSearchResultSerializer < ApplicationSerializer
     search_log_id.present?
   end
 
+  def include_tags?
+    SiteSetting.tagging_enabled
+  end
+
 end
diff --git a/app/serializers/tag_serializer.rb b/app/serializers/tag_serializer.rb
new file mode 100644
index 00000000000..206a0256fa7
--- /dev/null
+++ b/app/serializers/tag_serializer.rb
@@ -0,0 +1,3 @@
+class TagSerializer < ApplicationSerializer
+  attributes :id, :name
+end
diff --git a/app/services/search_indexer.rb b/app/services/search_indexer.rb
index c29b7edd52c..b147e890838 100644
--- a/app/services/search_indexer.rb
+++ b/app/services/search_indexer.rb
@@ -84,6 +84,10 @@ class SearchIndexer
     update_index('category', category_id, name)
   end
 
+  def self.update_tags_index(tag_id, name)
+    update_index('tag', tag_id, name)
+  end
+
   def self.index(obj, force: false)
     return if @disabled
 
@@ -115,6 +119,10 @@ class SearchIndexer
     if obj.class == Category && (obj.name_changed? || force)
       SearchIndexer.update_categories_index(obj.id, obj.name)
     end
+
+    if obj.class == Tag && (obj.name_changed? || force)
+      SearchIndexer.update_tags_index(obj.id, obj.name)
+    end
   end
 
   class HtmlScrubber < Nokogiri::XML::SAX::Document
diff --git a/db/migrate/20170823173427_create_tag_search_data.rb b/db/migrate/20170823173427_create_tag_search_data.rb
new file mode 100644
index 00000000000..d35c488f965
--- /dev/null
+++ b/db/migrate/20170823173427_create_tag_search_data.rb
@@ -0,0 +1,16 @@
+class CreateTagSearchData < ActiveRecord::Migration
+  def up
+    create_table :tag_search_data, primary_key: :tag_id do |t|
+      t.tsvector "search_data"
+      t.text     "raw_data"
+      t.text     "locale"
+      t.integer  "version",     default: 0
+    end
+    execute 'create index idx_search_tag on tag_search_data using gin(search_data)'
+  end
+
+  def down
+    execute 'drop index idx_search_tag'
+    drop_table :tag_search_data
+  end
+end
diff --git a/lib/search.rb b/lib/search.rb
index ee89f89340f..dd70d9b5a2b 100644
--- a/lib/search.rb
+++ b/lib/search.rb
@@ -18,7 +18,7 @@ class Search
   end
 
   def self.facets
-    %w(topic category user private_messages)
+    %w(topic category user private_messages tags)
   end
 
   def self.ts_config(locale = SiteSetting.default_locale)
@@ -553,6 +553,7 @@ class Search
         unless @search_context
           user_search if @term.present?
           category_search if @term.present?
+          tags_search if @term.present?
         end
         topic_search
       end
@@ -631,6 +632,20 @@ class Search
       end
     end
 
+    def tags_search
+      return unless SiteSetting.tagging_enabled
+
+      tags = Tag.includes(:tag_search_data)
+        .where("tag_search_data.search_data @@ #{ts_query}")
+        .references(:tag_search_data)
+        .order("name asc")
+        .limit(limit)
+
+      tags.each do |tag|
+        @results.add(tag)
+      end
+    end
+
     def posts_query(limit, opts = nil)
       opts ||= {}
       posts = Post.where(post_type: Topic.visible_post_types(@guardian.user))
diff --git a/lib/search/grouped_search_results.rb b/lib/search/grouped_search_results.rb
index 787c279c04a..04ad93fa133 100644
--- a/lib/search/grouped_search_results.rb
+++ b/lib/search/grouped_search_results.rb
@@ -14,6 +14,7 @@ class Search
       :posts,
       :categories,
       :users,
+      :tags,
       :more_posts,
       :more_categories,
       :more_users,
@@ -34,6 +35,7 @@ class Search
       @posts = []
       @categories = []
       @users = []
+      @tags = []
     end
 
     def find_user_data(guardian)
diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb
index 2c2b3c32555..3576cb6a608 100644
--- a/spec/components/search_spec.rb
+++ b/spec/components/search_spec.rb
@@ -375,6 +375,51 @@ describe Search do
 
   end
 
+  context 'tags' do
+    def search
+      Search.execute(tag.name)
+    end
+
+    let!(:tag) { Fabricate(:tag) }
+    let(:tag_group) { Fabricate(:tag_group) }
+    let(:category) { Fabricate(:category) }
+
+    context 'tagging is disabled' do
+      before { SiteSetting.tagging_enabled = false }
+
+      it 'does not include tags' do
+        expect(search.tags).to_not be_present
+      end
+    end
+
+    context 'tagging is enabled' do
+      before { SiteSetting.tagging_enabled = true }
+
+      it 'returns the tag in the result' do
+        expect(search.tags).to eq([tag])
+      end
+
+      it 'shows staff tags' do
+        staff_tag = Fabricate(:tag, name: "#{tag.name}9")
+        SiteSetting.staff_tags = "#{staff_tag.name}"
+
+        expect(Search.execute(tag.name, guardian: Guardian.new(Fabricate(:admin))).tags).to contain_exactly(tag, staff_tag)
+        expect(search.tags).to contain_exactly(tag, staff_tag)
+      end
+
+      it 'includes category-restricted tags' do
+        category_tag = Fabricate(:tag, name: "#{tag.name}9")
+        tag_group.tags = [category_tag]
+        category.set_permissions(admins: :full)
+        category.allowed_tag_groups = [tag_group.name]
+        category.save!
+
+        expect(Search.execute(tag.name, guardian: Guardian.new(Fabricate(:admin))).tags).to contain_exactly(tag, category_tag)
+        expect(search.tags).to contain_exactly(tag, category_tag)
+      end
+    end
+  end
+
   context 'type_filter' do
 
     let!(:user) { Fabricate(:user, username: 'amazing', email: 'amazing@amazing.com') }
diff --git a/test/javascripts/acceptance/search-test.js.es6 b/test/javascripts/acceptance/search-test.js.es6
index fcd78afe189..346c53b7236 100644
--- a/test/javascripts/acceptance/search-test.js.es6
+++ b/test/javascripts/acceptance/search-test.js.es6
@@ -25,6 +25,18 @@ QUnit.test("search", (assert) => {
   });
 });
 
+QUnit.test("search for a tag", (assert) => {
+  visit("/");
+
+  click('#search-button');
+
+  fillIn('#search-term', 'evil');
+  keyEvent('#search-term', 'keyup', 16);
+  andThen(() => {
+    assert.ok(exists('.search-menu .results ul li'), 'it shows results');
+  });
+});
+
 QUnit.test("search scope checkbox", assert => {
   visit("/c/bug");
   click('#search-button');
diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6
index 6cb2f8464bd..9324d9840cf 100644
--- a/test/javascripts/helpers/create-pretender.js.es6
+++ b/test/javascripts/helpers/create-pretender.js.es6
@@ -108,6 +108,16 @@ export default function() {
             id: 1234
           }]
         });
+      } else if (request.queryParams.q === 'evil') {
+        return response({
+          posts: [{
+            id: 1234
+          }],
+          tags: [{
+            id: 6,
+            name: 'eviltrout'
+          }]
+        });
       }
 
       return response({});