From 5e410dc5e0eff133d0b2e396d022fa1bf2ef85e4 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <gxtan1990@gmail.com>
Date: Mon, 18 Mar 2019 15:25:45 +0800
Subject: [PATCH] FEATURE: Ability to exclude category from search results.
 (#7194)

This commit also adds `Category#search_priority` which sets the ground
work to enable prioritizing of posts for certain categories when searching.
---
 .../category_search_priorities.js.es6.erb     |  1 +
 .../components/edit-category-settings.js.es6  | 15 ++++
 .../discourse/models/category.js.es6          |  3 +-
 .../components/edit-category-settings.hbs     | 11 +++
 app/controllers/categories_controller.rb      |  1 +
 app/models/category.rb                        |  7 +-
 app/models/concerns/searchable.rb             |  5 ++
 app/serializers/category_serializer.rb        |  3 +-
 config/locales/client.en.yml                  |  5 ++
 ...82018_add_search_priority_to_categories.rb |  6 ++
 lib/search.rb                                 | 48 +++++++-----
 spec/components/search_spec.rb                | 74 ++++++++++++++++---
 spec/models/category_spec.rb                  | 13 ++++
 spec/requests/categories_controller_spec.rb   |  2 +
 .../acceptance/category-edit-test.js.es6      | 21 ++----
 15 files changed, 170 insertions(+), 45 deletions(-)
 create mode 100644 app/assets/javascripts/discourse/components/concerns/category_search_priorities.js.es6.erb
 create mode 100644 db/migrate/20190314082018_add_search_priority_to_categories.rb

diff --git a/app/assets/javascripts/discourse/components/concerns/category_search_priorities.js.es6.erb b/app/assets/javascripts/discourse/components/concerns/category_search_priorities.js.es6.erb
new file mode 100644
index 00000000000..643b1777f96
--- /dev/null
+++ b/app/assets/javascripts/discourse/components/concerns/category_search_priorities.js.es6.erb
@@ -0,0 +1 @@
+export const searchPriorities = <%= Searchable::PRIORITIES.to_json %>;
diff --git a/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 b/app/assets/javascripts/discourse/components/edit-category-settings.js.es6
index 712aa67be17..82af15b8f0d 100644
--- a/app/assets/javascripts/discourse/components/edit-category-settings.js.es6
+++ b/app/assets/javascripts/discourse/components/edit-category-settings.js.es6
@@ -1,6 +1,7 @@
 import { setting } from "discourse/lib/computed";
 import { buildCategoryPanel } from "discourse/components/edit-category-panel";
 import computed from "ember-addons/ember-computed-decorators";
+import { searchPriorities } from "discourse/components/concerns/category_search_priorities";
 
 const categorySortCriteria = [];
 export function addCategorySortCriteria(criteria) {
@@ -57,6 +58,20 @@ export default buildCategoryPanel("settings", {
     );
   },
 
+  @computed
+  searchPrioritiesOptions() {
+    const options = [];
+
+    for (const [name, value] of Object.entries(searchPriorities)) {
+      options.push({
+        name: I18n.t(`category.search_priority.options.${name}`),
+        value: value
+      });
+    }
+
+    return options.sort((a, b) => a.value <= b.value);
+  },
+
   @computed
   availableSorts() {
     return [
diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6
index 2e6928abfe4..d31fcb72b45 100644
--- a/app/assets/javascripts/discourse/models/category.js.es6
+++ b/app/assets/javascripts/discourse/models/category.js.es6
@@ -129,7 +129,8 @@ const Category = RestModel.extend({
         minimum_required_tags: this.get("minimum_required_tags"),
         navigate_to_first_post_after_read: this.get(
           "navigate_to_first_post_after_read"
-        )
+        ),
+        search_priority: this.get("search_priority")
       },
       type: id ? "PUT" : "POST"
     });
diff --git a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs
index f9628a3fa46..1aab6209b31 100644
--- a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs
+++ b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs
@@ -43,6 +43,17 @@
   </label>
 </section>
 
+<section class="field">
+  <label for="category-search-priority">
+    {{i18n "category.search_priority.label"}}
+  </label>
+
+  {{combo-box valueAttribute="value"
+      id="category-search-priority"
+      content=searchPrioritiesOptions
+      value=category.search_priority}}
+</section>
+
 {{#if isParentCategory}}
   <section class="field show-subcategory-list-field">
     <label>
diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb
index 4e6a6da7c84..53236f406e1 100644
--- a/app/controllers/categories_controller.rb
+++ b/app/controllers/categories_controller.rb
@@ -293,6 +293,7 @@ class CategoriesController < ApplicationController
                       :default_top_period,
                       :minimum_required_tags,
                       :navigate_to_first_post_after_read,
+                      :search_priority,
                       custom_fields: [params[:custom_fields].try(:keys)],
                       permissions: [*p.try(:keys)],
                       allowed_tags: [],
diff --git a/app/models/category.rb b/app/models/category.rb
index 96c9a09ce3b..a1e5771d6cd 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -44,17 +44,18 @@ class Category < ActiveRecord::Base
   has_and_belongs_to_many :web_hooks
 
   validates :user_id, presence: true
+
   validates :name, if: Proc.new { |c| c.new_record? || c.will_save_change_to_name? },
                    presence: true,
                    uniqueness: { scope: :parent_category_id, case_sensitive: false },
                    length: { in: 1..50 }
+
   validates :num_featured_topics, numericality: { only_integer: true, greater_than: 0 }
+  validates :search_priority, inclusion: { in: Searchable::PRIORITIES.values }
+
   validate :parent_category_validator
-
   validate :email_in_validator
-
   validate :ensure_slug
-
   validate :permissions_compatibility_validator
 
   validates :auto_close_hours, numericality: { greater_than: 0, less_than_or_equal_to: 87600 }, allow_nil: true
diff --git a/app/models/concerns/searchable.rb b/app/models/concerns/searchable.rb
index dd67af05474..e655a2e4784 100644
--- a/app/models/concerns/searchable.rb
+++ b/app/models/concerns/searchable.rb
@@ -1,6 +1,11 @@
 module Searchable
   extend ActiveSupport::Concern
 
+  PRIORITIES = Enum.new(
+    normal: 0,
+    ignore: 1
+  )
+
   included do
     has_one "#{self.name.underscore}_search_data".to_sym, dependent: :destroy
   end
diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb
index 30540b14e54..1621397165e 100644
--- a/app/serializers/category_serializer.rb
+++ b/app/serializers/category_serializer.rb
@@ -18,7 +18,8 @@ class CategorySerializer < BasicCategorySerializer
              :custom_fields,
              :allowed_tags,
              :allowed_tag_groups,
-             :topic_featured_link_allowed
+             :topic_featured_link_allowed,
+             :search_priority
 
   def group_permissions
     @group_permissions ||= begin
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 0221f6faa77..783d6ce28a8 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -2453,6 +2453,11 @@ en:
         muted:
           title: "Muted"
           description: "You will never be notified of anything about new topics in these categories, and they will not appear in latest."
+      search_priority:
+        label: "Search Priority"
+        options:
+          normal: "Normal"
+          ignore: "Ignore"
       sort_options:
         default: "default"
         likes: "Likes"
diff --git a/db/migrate/20190314082018_add_search_priority_to_categories.rb b/db/migrate/20190314082018_add_search_priority_to_categories.rb
new file mode 100644
index 00000000000..af2c67365f8
--- /dev/null
+++ b/db/migrate/20190314082018_add_search_priority_to_categories.rb
@@ -0,0 +1,6 @@
+class AddSearchPriorityToCategories < ActiveRecord::Migration[5.2]
+  def change
+    add_column :categories, :search_priority, :integer, default: 0
+    add_index :categories, :search_priority
+  end
+end
diff --git a/lib/search.rb b/lib/search.rb
index 2535b6b9938..bf3a771158f 100644
--- a/lib/search.rb
+++ b/lib/search.rb
@@ -243,7 +243,7 @@ class Search
       end
     end
 
-    find_grouped_results unless @results.posts.present?
+    find_grouped_results if @results.posts.blank?
 
     if preloaded_topic_custom_fields.present? && @results.posts.present?
       topics = @results.posts.map(&:topic)
@@ -608,7 +608,6 @@ class Search
   end
 
   def find_grouped_results
-
     if @results.type_filter.present?
       raise Discourse::InvalidAccess.new("invalid type filter") unless Search.facets.include?(@results.type_filter)
       send("#{@results.type_filter}_search")
@@ -727,6 +726,7 @@ class Search
 
   def posts_query(limit, opts = nil)
     opts ||= {}
+
     posts = Post.where(post_type: Topic.visible_post_types(@guardian.user))
       .joins(:post_search_data, :topic)
       .joins("LEFT JOIN categories ON categories.id = topics.category_id")
@@ -768,6 +768,7 @@ class Search
         weights = @in_title ? 'A' : (SiteSetting.tagging_enabled ? 'ABCD' : 'ABD')
         posts = posts.where("post_search_data.search_data @@ #{ts_query(weight_filter: weights)}")
         exact_terms = @term.scan(/"([^"]+)"/).flatten
+
         exact_terms.each do |exact|
           posts = posts.where("posts.raw ilike :exact OR topics.title ilike :exact", exact: "%#{exact}%")
         end
@@ -783,26 +784,29 @@ class Search
     end if @filters
 
     # If we have a search context, prioritize those posts first
-    if @search_context.present?
+    posts =
+      if @search_context.present?
+        if @search_context.is_a?(User)
+          if opts[:private_messages]
+            posts.private_posts_for_user(@search_context)
+          else
+            posts.where("posts.user_id = #{@search_context.id}")
+          end
+        elsif @search_context.is_a?(Category)
+          category_ids = Category
+            .where(parent_category_id: @search_context.id)
+            .pluck(:id)
+            .push(@search_context.id)
 
-      if @search_context.is_a?(User)
-
-        if opts[:private_messages]
-          posts = posts.private_posts_for_user(@search_context)
-        else
-          posts = posts.where("posts.user_id = #{@search_context.id}")
+          posts.where("topics.category_id in (?)", category_ids)
+        elsif @search_context.is_a?(Topic)
+          posts.where("topics.id = #{@search_context.id}")
+            .order("posts.post_number #{@order == :latest ? "DESC" : ""}")
         end
-
-      elsif @search_context.is_a?(Category)
-        category_ids = [@search_context.id] + Category.where(parent_category_id: @search_context.id).pluck(:id)
-        posts = posts.where("topics.category_id in (?)", category_ids)
-      elsif @search_context.is_a?(Topic)
-        posts = posts.where("topics.id = #{@search_context.id}")
-          .order("posts.post_number #{@order == :latest ? "DESC" : ""}")
+      else
+        categories_ignored(posts)
       end
 
-    end
-
     if @order == :latest || (@term.blank? && !@order)
       if opts[:aggregate_search]
         posts = posts.order("MAX(posts.created_at) DESC")
@@ -847,6 +851,14 @@ class Search
     posts.limit(limit)
   end
 
+  def categories_ignored(posts)
+    posts.where(<<~SQL, Searchable::PRIORITIES[:ignore])
+    categories.id NOT IN (
+      SELECT categories.id WHERE categories.search_priority = ?
+    )
+    SQL
+  end
+
   def self.default_ts_config
     "'#{Search.ts_config}'"
   end
diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb
index 287fa8e7f65..c8621309062 100644
--- a/spec/components/search_spec.rb
+++ b/spec/components/search_spec.rb
@@ -413,19 +413,72 @@ describe Search do
   end
 
   context 'categories' do
+    let(:category) { Fabricate(:category, name: "monkey Category 2") }
+    let(:topic) { Fabricate(:topic, category: category) }
+    let!(:post) { Fabricate(:post, topic: topic, raw: "snow monkey") }
 
-    let!(:category) { Fabricate(:category) }
-    def search
-      Search.execute(category.name)
+    let!(:ignored_category) do
+      Fabricate(:category,
+        name: "monkey Category 1",
+        search_priority: Searchable::PRIORITIES[:ignore]
+      )
     end
 
-    it 'returns the correct result' do
-      expect(search.categories).to be_present
+    it "should return the right categories" do
+      search = Search.execute("monkey")
 
-      category.set_permissions({})
-      category.save
+      expect(search.categories).to contain_exactly(
+        category, ignored_category
+      )
 
-      expect(search.categories).not_to be_present
+      expect(search.posts).to contain_exactly(category.topic.first_post, post)
+    end
+
+    describe "with child categories" do
+      let!(:child_of_ignored_category) do
+        Fabricate(:category,
+          name: "monkey Category 3",
+          parent_category: ignored_category
+        )
+      end
+
+      let!(:post2) do
+        Fabricate(:post,
+          topic: Fabricate(:topic, category: child_of_ignored_category),
+          raw: "snow monkey park"
+        )
+      end
+
+      it 'returns the right results' do
+        search = Search.execute("monkey")
+
+        expect(search.categories).to contain_exactly(
+          category, ignored_category, child_of_ignored_category
+        )
+
+        expect(search.posts).to contain_exactly(
+          category.topic.first_post,
+          post,
+          child_of_ignored_category.topic.first_post,
+          post2
+        )
+
+        search = Search.execute("snow")
+        expect(search.posts).to contain_exactly(post, post2)
+
+        category.set_permissions({})
+        category.save
+        search = Search.execute("monkey")
+
+        expect(search.categories).to contain_exactly(
+          ignored_category, child_of_ignored_category
+        )
+
+        expect(search.posts).to contain_exactly(
+          child_of_ignored_category.topic.first_post,
+          post2
+        )
+      end
     end
 
   end
@@ -576,7 +629,10 @@ describe Search do
     end
 
     it 'can use category as a search context' do
-      category = Fabricate(:category)
+      category = Fabricate(:category,
+        search_priority: Searchable::PRIORITIES[:ignore]
+      )
+
       topic = Fabricate(:topic, category: category)
       topic_no_cat = Fabricate(:topic)
 
diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb
index ea641205cef..152878d9ef5 100644
--- a/spec/models/category_spec.rb
+++ b/spec/models/category_spec.rb
@@ -4,6 +4,8 @@ require 'rails_helper'
 require_dependency 'post_creator'
 
 describe Category do
+  let(:user) { Fabricate(:user) }
+
   it { is_expected.to validate_presence_of :user_id }
   it { is_expected.to validate_presence_of :name }
 
@@ -12,6 +14,17 @@ describe Category do
     is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_category_id)
   end
 
+  it 'validates inclusion of search_priority' do
+    category = Fabricate.build(:category, user: user)
+
+    expect(category.valid?).to eq(true)
+
+    category.search_priority = Searchable::PRIORITIES.values.last + 1
+
+    expect(category.valid?).to eq(false)
+    expect(category.errors.keys).to contain_exactly(:search_priority)
+  end
+
   it 'validates uniqueness in case insensitive way' do
     Fabricate(:category, name: "Cats")
     cats = Fabricate.build(:category, name: "cats")
diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb
index e8568963cef..6bfca76b4d2 100644
--- a/spec/requests/categories_controller_spec.rb
+++ b/spec/requests/categories_controller_spec.rb
@@ -141,6 +141,7 @@ describe CategoriesController do
             text_color: "fff",
             slug: "hello-cat",
             auto_close_hours: 72,
+            search_priority: Searchable::PRIORITIES[:ignore],
             permissions: {
               "everyone" => readonly,
               "staff" => create_post
@@ -156,6 +157,7 @@ describe CategoriesController do
           expect(category.slug).to eq("hello-cat")
           expect(category.color).to eq("ff0")
           expect(category.auto_close_hours).to eq(72)
+          expect(category.search_priority).to eq(Searchable::PRIORITIES[:ignore])
           expect(UserHistory.count).to eq(4) # 1 + 3 (bootstrap mode)
         end
       end
diff --git a/test/javascripts/acceptance/category-edit-test.js.es6 b/test/javascripts/acceptance/category-edit-test.js.es6
index afd0df3618d..7a772ebdcef 100644
--- a/test/javascripts/acceptance/category-edit-test.js.es6
+++ b/test/javascripts/acceptance/category-edit-test.js.es6
@@ -16,27 +16,22 @@ QUnit.test("Can open the category modal", async assert => {
   assert.ok(!visible(".d-modal"), "it closes the modal");
 });
 
-QUnit.test("Change the category color", async assert => {
+QUnit.test("Editing the category", async assert => {
   await visit("/c/bug");
 
   await click(".edit-category");
   await fillIn("#edit-text-color", "#ff0000");
-  await click("#save-category");
-  assert.ok(!visible(".d-modal"), "it closes the modal");
-  assert.equal(
-    DiscourseURL.redirectedTo,
-    "/c/bug",
-    "it does one of the rare full page redirects"
-  );
-});
 
-QUnit.test("Change the topic template", async assert => {
-  await visit("/c/bug");
-
-  await click(".edit-category");
   await click(".edit-category-topic-template");
   await fillIn(".d-editor-input", "this is the new topic template");
+
+  await click(".edit-category-settings");
+  const searchPriorityChooser = selectKit("#category-search-priority");
+  await searchPriorityChooser.expand();
+  await searchPriorityChooser.selectRowByValue(1);
+
   await click("#save-category");
+
   assert.ok(!visible(".d-modal"), "it closes the modal");
   assert.equal(
     DiscourseURL.redirectedTo,