From e40c28117b95b9f3c95e2fce6d6d0150c4b02fa8 Mon Sep 17 00:00:00 2001
From: Alex <alexandersmith223@gmail.com>
Date: Fri, 28 Feb 2020 07:51:33 -0800
Subject: [PATCH] Search ClassifiedListings backend code (#6348)

* Add search methods

* Add spec for new search methods

* Add QueryBuilder for ClassifiedListing

* Add QueryBuilder spec for ClassifiedListing

* Refact build_query in ClassifiedListings

* Refactor logic for filter and query conditions

* Refactor index_documents to helper_method

* Update elasticsearch helper for single objects

* Refactor specs from stubbing to record update

* Update search and specs for query_builder

* Refactor set_size

* Update Search::ClassifiedListing

- Add DEFAULT_PAGE constant
- ADD DEFAULT_PER_PAGE constant
- Add pagination

* Fix paging

* Add pagination specs

* Remove manual Array conversion for Array.wrap()

* Change guard clause to .key? for readability
---
 app/services/search/classified_listing.rb     |  28 +++
 .../query_builders/classified_listing.rb      | 124 +++++++++++++
 spec/rails_helper.rb                          |   1 +
 .../search/classified_listing_spec.rb         | 169 ++++++++++++++++++
 .../query_builders/classified_listing_spec.rb |  87 +++++++++
 spec/support/elasticsearch_helpers.rb         |   7 +
 6 files changed, 416 insertions(+)
 create mode 100644 app/services/search/query_builders/classified_listing.rb
 create mode 100644 spec/services/search/query_builders/classified_listing_spec.rb
 create mode 100644 spec/support/elasticsearch_helpers.rb

diff --git a/app/services/search/classified_listing.rb b/app/services/search/classified_listing.rb
index 2b2fa9ff2..1e5d95082 100644
--- a/app/services/search/classified_listing.rb
+++ b/app/services/search/classified_listing.rb
@@ -3,10 +3,38 @@ module Search
     INDEX_NAME = "classified_listings_#{Rails.env}".freeze
     INDEX_ALIAS = "classified_listings_#{Rails.env}_alias".freeze
     MAPPINGS = JSON.parse(File.read("config/elasticsearch/mappings/classified_listings.json"), symbolize_names: true).freeze
+    DEFAULT_PAGE = 0
+    DEFAULT_PER_PAGE = 75
 
     class << self
+      def search_documents(params:)
+        set_query_size(params)
+        query_hash = Search::QueryBuilders::ClassifiedListing.new(params).as_hash
+
+        results = search(body: query_hash)
+        hits = results.dig("hits", "hits").map { |cl_doc| cl_doc.dig("_source") }
+        paginate_hits(hits, params)
+      end
+
       private
 
+      def search(body:)
+        SearchClient.search(index: INDEX_ALIAS, body: body)
+      end
+
+      def set_query_size(params)
+        params[:page] ||= DEFAULT_PAGE
+        params[:per_page] ||= DEFAULT_PER_PAGE
+
+        # pages start at 0
+        params[:size] = params[:per_page].to_i * (params[:page].to_i + 1)
+      end
+
+      def paginate_hits(hits, params)
+        start = params[:per_page] * params[:page]
+        hits[start, params[:per_page]] || []
+      end
+
       def index_settings
         if Rails.env.production?
           {
diff --git a/app/services/search/query_builders/classified_listing.rb b/app/services/search/query_builders/classified_listing.rb
new file mode 100644
index 000000000..f7ea065e0
--- /dev/null
+++ b/app/services/search/query_builders/classified_listing.rb
@@ -0,0 +1,124 @@
+module Search
+  module QueryBuilders
+    class ClassifiedListing
+      TERM_KEYS = %i[
+        category
+        contact_via_connect
+        location
+        slug
+        tags
+        title
+        user_id
+      ].freeze
+
+      RANGE_KEYS = %i[
+        bumped_at
+        expires_at
+      ].freeze
+
+      QUERY_KEYS = %i[
+        classified_listing_search
+      ].freeze
+
+      DEFAULT_PARAMS = {
+        sort_by: "bumped_at",
+        sort_direction: "desc",
+        size: 0
+      }.freeze
+
+      attr_accessor :params, :body
+
+      def initialize(params)
+        @params = params.deep_symbolize_keys
+        build_body
+      end
+
+      def as_hash
+        @body
+      end
+
+      private
+
+      def build_body
+        @body = ActiveSupport::HashWithIndifferentAccess.new
+        build_queries
+        add_sort
+        set_size
+      end
+
+      def build_queries
+        @body[:query] = { bool: {} }
+        @body[:query][:bool][:filter] = filter_conditions if filter_keys_present?
+        @body[:query][:bool][:must] = query_conditions if query_keys_present?
+      end
+
+      def add_sort
+        sort_key = @params[:sort_by] || DEFAULT_PARAMS[:sort_by]
+        sort_direction = @params[:sort_direction] || DEFAULT_PARAMS[:sort_direction]
+        @body[:sort] = {
+          sort_key => sort_direction
+        }
+      end
+
+      def set_size
+        # By default we will return 0 documents if size is not specified
+        @body[:size] = @params[:size] || DEFAULT_PARAMS[:size]
+      end
+
+      def filter_conditions
+        filter_conditions = []
+        filter_conditions.concat term_keys if term_keys_present?
+        filter_conditions.concat range_keys if range_keys_present?
+
+        filter_conditions
+      end
+
+      def term_keys
+        TERM_KEYS.map do |term_key|
+          next unless @params.key? term_key
+
+          { term: { term_key => @params[term_key] } }
+        end.compact
+      end
+
+      def range_keys
+        RANGE_KEYS.map do |range_key|
+          next unless @params.key? range_key
+
+          { range: { range_key => @params[range_key] } }
+        end.compact
+      end
+
+      def filter_keys_present?
+        term_keys_present? || range_keys_present?
+      end
+
+      def term_keys_present?
+        TERM_KEYS.detect { |key| @params[key].present? }
+      end
+
+      def range_keys_present?
+        RANGE_KEYS.detect { |key| @params[key].present? }
+      end
+
+      def query_keys_present?
+        QUERY_KEYS.detect { |key| @params[key].present? }
+      end
+
+      def query_conditions
+        QUERY_KEYS.map do |query_key|
+          next if @params[query_key].blank?
+
+          {
+            simple_query_string: {
+              query: "#{@params[query_key]}*",
+              fields: [query_key],
+              lenient: true,
+              analyze_wildcard: true
+            }
+          }
+        end.compact
+      end
+    end
+  end
+end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 3a580da96..3f8d979e6 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -77,6 +77,7 @@ RSpec.configure do |config|
   config.include FactoryBot::Syntax::Methods
   config.include OmniauthMacros
   config.include SidekiqTestHelpers
+  config.include ElasticsearchHelpers, elasticsearch: true
 
   config.before do
     ActiveRecord::Base.observers.disable :all # <-- Turn 'em all off!
diff --git a/spec/services/search/classified_listing_spec.rb b/spec/services/search/classified_listing_spec.rb
index 417657464..a2848f31b 100644
--- a/spec/services/search/classified_listing_spec.rb
+++ b/spec/services/search/classified_listing_spec.rb
@@ -100,4 +100,173 @@ RSpec.describe Search::ClassifiedListing, type: :service, elasticsearch: true do
       described_class.delete_index(index_name: other_name)
     end
   end
+
+  describe "::search_documents" do
+    let(:classified_listing) { create(:classified_listing) }
+
+    it "parses classified_listing document hits from search response" do
+      mock_search_response = { "hits" => { "hits" => {} } }
+      allow(described_class).to receive(:search) { mock_search_response }
+      described_class.search_documents(params: {})
+      expect(described_class).to have_received(:search).with(body: a_kind_of(Hash))
+    end
+
+    context "with a query" do
+      # classified_listing_search is a copy_to field including:
+      # body_markdown, location, slug, tags, and title
+      it "searches by classified_listing_search" do
+        classified_listing1 = create(:classified_listing, body_markdown: "# body_markdown with test")
+        classified_listing2 = create(:classified_listing, location: "a test location")
+        classified_listing3 = create(:classified_listing, title: "this test title is testing slug")
+        classified_listing4 = create(:classified_listing, tag_list: ["test"])
+        classified_listing5 = create(:classified_listing, title: "a test title")
+        classified_listings = [classified_listing1, classified_listing2, classified_listing3, classified_listing4, classified_listing5]
+        index_documents(classified_listings)
+
+        classified_listing_docs = described_class.search_documents(params: { size: 5, classified_listing_search: "test" })
+        expect(classified_listing_docs.count).to eq(5)
+        expect(classified_listing_docs.map { |t| t.dig("id") }).to match_array(classified_listings.map(&:id))
+      end
+    end
+
+    context "with a term filter" do
+      it "searches by category" do
+        classified_listing.update(category: "forhire")
+        index_documents(classified_listing)
+        params = { size: 5, category: "forhire" }
+
+        classified_listing_docs = described_class.search_documents(params: params)
+        expect(classified_listing_docs.count).to eq(1)
+        expect(classified_listing_docs.first["id"]).to eq(classified_listing.id)
+      end
+
+      it "searches by contact_via_connect" do
+        classified_listing.update(contact_via_connect: true)
+        index_documents(classified_listing)
+        params = { size: 5, contact_via_connect: true }
+
+        classified_listing_docs = described_class.search_documents(params: params)
+        expect(classified_listing_docs.count).to eq(1)
+        expect(classified_listing_docs.first["id"]).to eq(classified_listing.id)
+      end
+
+      it "searches by location" do
+        classified_listing.update(location: "a location")
+        index_documents(classified_listing)
+        params = { size: 5, location: "location" }
+
+        classified_listing_docs = described_class.search_documents(params: params)
+        expect(classified_listing_docs.count).to eq(1)
+        expect(classified_listing_docs.first["id"]).to eq(classified_listing.id)
+      end
+
+      it "searches by slug" do
+        slug_classified_listing = FactoryBot.create(:classified_listing, title: "A slug is created from this title in a callback")
+        index_documents(slug_classified_listing)
+        params = { size: 5, slug: "slug" }
+
+        classified_listing_docs = described_class.search_documents(params: params)
+        expect(classified_listing_docs.count).to eq(1)
+        expect(classified_listing_docs.first["id"]).to eq(slug_classified_listing.id)
+      end
+
+      it "searches by tags" do
+        classified_listing.update(tag_list: %w[beginners career])
+        index_documents(classified_listing)
+        params = { size: 5, tags: "career" }
+
+        classified_listing_docs = described_class.search_documents(params: params)
+        expect(classified_listing_docs.count).to eq(1)
+        expect(classified_listing_docs.first["id"]).to eq(classified_listing.id)
+      end
+
+      it "searches by title" do
+        classified_listing.update(title: "An Amazing Title")
+        index_documents(classified_listing)
+        params = { size: 5, title: "amazing" }
+
+        classified_listing_docs = described_class.search_documents(params: params)
+        expect(classified_listing_docs.count).to eq(1)
+        expect(classified_listing_docs.first["id"]).to eq(classified_listing.id)
+      end
+
+      it "searches by user_id" do
+        index_documents(classified_listing)
+        params = { size: 5, user_id: classified_listing.user_id }
+
+        classified_listing_docs = described_class.search_documents(params: params)
+        expect(classified_listing_docs.count).to eq(1)
+        expect(classified_listing_docs.first["id"]).to eq(classified_listing.id)
+      end
+
+      it "searches by bumped_at" do
+        classified_listing.update(bumped_at: 1.day.from_now)
+        index_documents(classified_listing)
+        params = { size: 5, bumped_at: { gt: Time.current } }
+
+        classified_listing_docs = described_class.search_documents(params: params)
+        expect(classified_listing_docs.count).to eq(1)
+        expect(classified_listing_docs.first["id"]).to eq(classified_listing.id)
+      end
+
+      it "searches by expires_at" do
+        classified_listing.update(expires_at: 1.day.ago)
+        index_documents(classified_listing)
+        params = { size: 5, expires_at: { lt: Time.current } }
+
+        classified_listing_docs = described_class.search_documents(params: params)
+        expect(classified_listing_docs.count).to eq(1)
+        expect(classified_listing_docs.first["id"]).to eq(classified_listing.id)
+      end
+    end
+
+    it "sorts documents for a given field" do
+      classified_listing.update(category: "forhire")
+      classified_listing2 = FactoryBot.create(:classified_listing, category: "cfp")
+      index_documents([classified_listing, classified_listing2])
+      params = { size: 5, sort_by: "category", sort_direction: "asc" }
+
+      classified_listing_docs = described_class.search_documents(params: params)
+      expect(classified_listing_docs.count).to eq(2)
+      expect(classified_listing_docs.first["id"]).to eq(classified_listing2.id)
+      expect(classified_listing_docs.last["id"]).to eq(classified_listing.id)
+    end
+
+    it "sorts documents by bumped_at by default" do
+      classified_listing.update(bumped_at: 1.year.ago)
+      classified_listing2 = FactoryBot.create(:classified_listing, bumped_at: Time.current)
+      index_documents([classified_listing, classified_listing2])
+      params = { size: 5 }
+
+      classified_listing_docs = described_class.search_documents(params: params)
+      expect(classified_listing_docs.count).to eq(2)
+      expect(classified_listing_docs.first["id"]).to eq(classified_listing2.id)
+      expect(classified_listing_docs.last["id"]).to eq(classified_listing.id)
+    end
+
+    it "paginates the results" do
+      classified_listing.update(bumped_at: 1.year.ago)
+      classified_listing2 = FactoryBot.create(:classified_listing, bumped_at: Time.current)
+      index_documents([classified_listing, classified_listing2])
+      first_page_params = { page: 0, per_page: 1, sort_by: "bumped_at", order: "dsc" }
+
+      classified_listing_docs = described_class.search_documents(params: first_page_params)
+      expect(classified_listing_docs.first["id"]).to eq(classified_listing2.id)
+
+      second_page_params = { page: 1, per_page: 1, sort_by: "bumped_at", order: "dsc" }
+
+      classified_listing_docs = described_class.search_documents(params: second_page_params)
+      expect(classified_listing_docs.first["id"]).to eq(classified_listing.id)
+    end
+
+    it "returns an empty Array if no results are found" do
+      classified_listing.update(category: "forhire")
+      classified_listing2 = FactoryBot.create(:classified_listing, category: "cfp")
+      index_documents([classified_listing, classified_listing2])
+      params = { page: 3, per_page: 1 }
+
+      classified_listing_docs = described_class.search_documents(params: params)
+      expect(classified_listing_docs).to eq([])
+    end
+  end
 end
diff --git a/spec/services/search/query_builders/classified_listing_spec.rb b/spec/services/search/query_builders/classified_listing_spec.rb
new file mode 100644
index 000000000..a2cae6230
--- /dev/null
+++ b/spec/services/search/query_builders/classified_listing_spec.rb
@@ -0,0 +1,87 @@
+require "rails_helper"
+
+RSpec.describe Search::QueryBuilders::ClassifiedListing, type: :service do
+  describe "::intialize" do
+    it "sets params" do
+      filter_params = { foo: "bar" }
+      filter = described_class.new(filter_params)
+      expect(filter.params).to include(filter_params)
+    end
+
+    it "builds query body" do
+      filter = described_class.new({})
+      expect(filter.body).not_to be_nil
+    end
+  end
+
+  describe "#as_hash" do
+    it "applies TERM_KEYS from params" do
+      params = { category: "cfp", tags: ["beginner"], contact_via_connect: false }
+      filter = described_class.new(params)
+      exepcted_filters = [
+        { "term" => { "category" => "cfp" } },
+        { "term" => { "tags" => ["beginner"] } },
+        { "term" => { "contact_via_connect" => false } },
+      ]
+      expect(filter.as_hash.dig("query", "bool", "filter")).to match_array(exepcted_filters)
+    end
+
+    it "applies RANGE_KEYS from params" do
+      Timecop.freeze(Time.current) do
+        params = { bumped_at: Time.current, expires_at: 1.day.from_now }
+        filter = described_class.new(params)
+        exepcted_filters = [
+          { "range" => { "bumped_at" => Time.current } },
+          { "range" => { "expires_at" => 1.day.from_now } },
+        ]
+        expect(filter.as_hash.dig("query", "bool", "filter")).to match_array(exepcted_filters)
+      end
+    end
+
+    it "applies QUERY_KEYS from params" do
+      params = { classified_listing_search: "test" }
+      filter = described_class.new(params)
+      exepcted_query = [{
+        "simple_query_string" => {
+          "query" => "test*", "fields" => [:classified_listing_search], "lenient" => true, "analyze_wildcard" => true
+        }
+      }]
+      expect(filter.as_hash.dig("query", "bool", "must")).to match_array(exepcted_query)
+    end
+
+    it "applies QUERY_KEYS, TERM_KEYS, and RANGE_KEYS from params" do
+      Timecop.freeze(Time.current) do
+        params = { classified_listing_search: "test", bumped_at: Time.current, category: "cfp" }
+        filter = described_class.new(params)
+        exepcted_query = [{
+          "simple_query_string" => { "query" => "test*", "fields" => [:classified_listing_search], "lenient" => true, "analyze_wildcard" => true }
+        }]
+        exepcted_filters = [{ "range" => { "bumped_at" => Time.current } }, { "term" => { "category" => "cfp" } }]
+        expect(filter.as_hash.dig("query", "bool", "must")).to match_array(exepcted_query)
+        expect(filter.as_hash.dig("query", "bool", "filter")).to match_array(exepcted_filters)
+      end
+    end
+
+    it "ignores params we don't support" do
+      params = { not_supported: "trash", category: "cfp" }
+      filter = described_class.new(params)
+      exepcted_filters = [
+        { "term" => { "category" => "cfp" } },
+      ]
+      expect(filter.as_hash.dig("query", "bool", "filter")).to match_array(exepcted_filters)
+    end
+
+    it "sets default params when not present" do
+      filter = described_class.new({})
+      expect(filter.as_hash.dig("sort")).to eq("bumped_at" => "desc")
+      expect(filter.as_hash.dig("size")).to eq(0)
+    end
+
+    it "allows default params to be overriden" do
+      params = { sort_by: "category", sort_direction: "asc", size: 20 }
+      filter = described_class.new(params)
+      expect(filter.as_hash.dig("sort")).to eq("category" => "asc")
+      expect(filter.as_hash.dig("size")).to eq(20)
+    end
+  end
+end
diff --git a/spec/support/elasticsearch_helpers.rb b/spec/support/elasticsearch_helpers.rb
new file mode 100644
index 000000000..447b39a46
--- /dev/null
+++ b/spec/support/elasticsearch_helpers.rb
@@ -0,0 +1,7 @@
+# Only included in specs with elasticsearch: true
+module ElasticsearchHelpers
+  def index_documents(resources)
+    Array.wrap(resources).each(&:index_to_elasticsearch_inline)
+    described_class.refresh_index
+  end
+end