[Search 2.0] Listings (#13133)

* Add PgSearch to Listing model

* Working MVP - pre-optimizations

* Remove includes

* Update search_spec

* Add more specs

* Fix specs

* Refactor filter_by_category

* Update block parameter name

* Add published index to classified_listings table

* Preload user, organization, and listing_category

* Working MVP - pre-optimizations

* Refactor filter_by_category

* Update schema

* Fix schema for real

* Remove constant specs and make them private

app/controllers/search_controller.rb

@@ -75,11 +75,19 @@ class SearchController < ApplicationController
   end
 
   def listings
-    cl_docs = Search::Listing.search_documents(
-      params: listing_params.to_h,
-    )
+    result =
+      if FeatureFlag.enabled?(:search_2_listings)
+        Search::Postgres::Listing.search_documents(
+          category: listing_params[:category],
+          page: listing_params[:page],
+          per_page: listing_params[:per_page],
+          term: listing_params[:listing_search],
+        )
+      else
+        Search::Listing.search_documents(params: listing_params.to_h)
+      end
 
-    render json: { result: cl_docs }
+    render json: { result: result }
   end
 
   def users

app/models/listing.rb

@@ -4,6 +4,7 @@ class Listing < ApplicationRecord
   self.table_name = "classified_listings"
 
   include Searchable
+  include PgSearch::Model
 
   SEARCH_SERIALIZER = Search::ListingSerializer
   SEARCH_CLASS = Search::Listing
@@ -32,6 +33,12 @@ class Listing < ApplicationRecord
   validate :restrict_markdown_input
   validate :validate_tags
 
+  # [@atsmith813] this is adapted from the `listing_search` property in
+  # `config/elasticsearch/mappings/listings.json`
+  pg_search_scope :search_listings,
+                  against: %i[body_markdown cached_tag_list location slug title],
+                  using: { tsearch: { prefix: true } }
+
   scope :published, -> { where(published: true) }
 
   # NOTE: we still need to use the old column name for the join query

app/serializers/search/listing_result_serializer.rb

@@ -0,0 +1,29 @@
+module Search
+  # TODO: [@atsmith813] Rename this to just ListingSerializer once Elasticsearch
+  # is fully removed
+  class ListingResultSerializer < ApplicationSerializer
+    attributes :id,
+               :body_markdown,
+               :bumped_at,
+               :category,
+               :contact_via_connect,
+               :expires_at,
+               :originally_published_at,
+               :location,
+               :processed_html,
+               :published,
+               :slug,
+               :title,
+               :user_id
+
+    attribute :tags do |listing|
+      listing.cached_tag_list.to_s.split(", ")
+    end
+
+    attribute :author do |listing|
+      ListingAuthorSerializer.new(listing.author)
+        .serializable_hash
+        .dig(:data, :attributes)
+    end
+  end
+end

app/services/search/postgres/listing.rb

@@ -0,0 +1,59 @@
+module Search
+  module Postgres
+    class Listing
+      ATTRIBUTES = %i[
+        id
+        body_markdown
+        bumped_at
+        cached_tag_list
+        classified_listing_category_id
+        contact_via_connect
+        expires_at
+        organization_id
+        originally_published_at
+        location
+        processed_html
+        published
+        slug
+        title
+        user_id
+      ].freeze
+      private_constant :ATTRIBUTES
+
+      DEFAULT_PER_PAGE = 75
+      private_constant :DEFAULT_PER_PAGE
+
+      MAX_PER_PAGE = 150 # to avoid querying too many items, we set a maximum amount for a page
+      private_constant :MAX_PER_PAGE
+
+      def self.search_documents(category: nil, page: 0, per_page: DEFAULT_PER_PAGE, term: nil)
+        # NOTE: [@rhymes/atsmith813] we should eventually update the frontend
+        # to start from page 1
+        page = page.to_i + 1
+        per_page = [(per_page || DEFAULT_PER_PAGE).to_i, MAX_PER_PAGE].min
+
+        relation = ::Listing
+          .includes(:user, :organization, :listing_category)
+          .where(published: true)
+
+        relation = relation.where("classified_listing_categories.slug": category) if category.present?
+
+        relation = relation.search_listings(term) if term.present?
+
+        relation = relation.select(*ATTRIBUTES).order(bumped_at: :desc)
+
+        results = relation.page(page).per(per_page)
+
+        serialize(results)
+      end
+
+      def self.serialize(results)
+        Search::ListingResultSerializer
+          .new(results, is_collection: true)
+          .serializable_hash[:data]
+          .pluck(:attributes)
+      end
+      private_class_method :serialize
+    end
+  end
+end

db/migrate/20210326172406_add_published_index_to_listings.rb

@@ -0,0 +1,15 @@
+class AddPublishedIndexToListings < ActiveRecord::Migration[6.0]
+  disable_ddl_transaction!
+
+  def up
+    unless index_exists?(:classified_listings, :published)
+      add_index :classified_listings, :published, algorithm: :concurrently
+    end
+  end
+
+  def down
+    if index_exists?(:classified_listings, :published)
+      remove_index :classified_listings, column: :published, algorithm: :concurrently
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_03_25_183834) do
+ActiveRecord::Schema.define(version: 2021_03_26_172406) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
@@ -344,6 +344,7 @@ ActiveRecord::Schema.define(version: 2021_03_25_183834) do
     t.bigint "user_id"
     t.index ["classified_listing_category_id"], name: "index_classified_listings_on_classified_listing_category_id"
     t.index ["organization_id"], name: "index_classified_listings_on_organization_id"
+    t.index ["published"], name: "index_classified_listings_on_published"
     t.index ["user_id"], name: "index_classified_listings_on_user_id"
   end
 

spec/requests/search_spec.rb

@@ -70,16 +70,43 @@ RSpec.describe "Search", type: :request, proper_status: true do
   end
 
   describe "GET /search/listings" do
-    let(:mock_documents) do
-      [{ "title" => "listing1" }]
+    context "when using Elasticsearch" do
+      let(:mock_documents) do
+        [{ "title" => "listing1" }]
+      end
+
+      it "returns json" do
+        allow(Search::Listing).to receive(:search_documents).and_return(
+          mock_documents,
+        )
+        get "/search/listings"
+        expect(response.parsed_body).to eq("result" => mock_documents)
+      end
     end
 
-    it "returns json" do
-      allow(Search::Listing).to receive(:search_documents).and_return(
-        mock_documents,
-      )
-      get "/search/listings"
-      expect(response.parsed_body).to eq("result" => mock_documents)
+    context "when using PostgreSQL" do
+      before do
+        allow(FeatureFlag).to receive(:enabled?).with(:search_2_listings).and_return(true)
+      end
+
+      it "returns the correct keys" do
+        create(:listing)
+        get search_listings_path
+        expect(response.parsed_body["result"]).to be_present
+      end
+
+      it "supports the search params" do
+        listing = create(:listing)
+
+        get search_listings_path(
+          category: listing.category,
+          page: 0,
+          per_page: 1,
+          term: listing.title.downcase,
+        )
+
+        expect(response.parsed_body["result"].first).to include("title" => listing.title)
+      end
     end
   end
 

spec/services/search/postgres/listing_spec.rb

@@ -0,0 +1,134 @@
+require "rails_helper"
+
+RSpec.describe Search::Postgres::Listing, type: :service do
+  let(:listing) { create(:listing) }
+
+  describe "::search_documents" do
+    it "does not include a listing that is unpublished", :aggregate_failures do
+      published_listing = create(:listing, title: "Published Listing", published: true)
+      unpublished_listing = create(:listing, title: "Unpublished Listing", published: false)
+      result = described_class.search_documents(term: "Listing")
+      titles = result.pluck(:title)
+
+      expect(titles).not_to include(unpublished_listing.title)
+      expect(titles).to include(published_listing.title)
+    end
+
+    context "when describing the result format" do
+      let(:result) { described_class.search_documents(term: listing.title) }
+
+      it "returns the correct attributes for the result" do
+        expected_keys = %i[
+          id body_markdown bumped_at category contact_via_connect expires_at
+          originally_published_at location processed_html published slug title
+          user_id tags author
+        ]
+
+        expect(result.first.keys).to match_array(expected_keys)
+      end
+
+      it "returns the correct attributes for the author" do
+        expected_keys = %i[username name profile_image_90]
+        expect(result.first[:author].keys).to match_array(expected_keys)
+      end
+
+      it "returns tag as an Array" do
+        expect(result.first[:tags]).to be_an_instance_of(Array)
+      end
+    end
+
+    context "when searching for a term" do
+      it "matches against the listing's body_markdown", :aggregate_failures do
+        listing.update_columns(body_markdown: "A Sweet New Opportunity")
+        result = described_class.search_documents(term: "new")
+
+        expect(result.first[:body_markdown]).to eq listing.body_markdown
+
+        result = described_class.search_documents(term: "old")
+        expect(result).to be_empty
+      end
+
+      it "matches against the listing's cached_tag_list", :aggregate_failures do
+        listing.update_columns(cached_tag_list: "javascript, beginners, ruby")
+        result = described_class.search_documents(term: "beginner")
+
+        expect(result.first[:tags].join(", ")).to eq listing.cached_tag_list
+
+        result = described_class.search_documents(term: "newbie")
+        expect(result).to be_empty
+      end
+
+      it "matches against the listing's location", :aggregate_failures do
+        listing.update_columns(location: "Tampa")
+        result = described_class.search_documents(term: "tampa")
+
+        expect(result.first[:location]).to eq listing.location
+
+        result = described_class.search_documents(term: "milan")
+        expect(result).to be_empty
+      end
+
+      it "matches against the listing's slug", :aggregate_failures do
+        listing.update_columns(slug: "some-cool-slug")
+        result = described_class.search_documents(term: "cool")
+
+        expect(result.first[:slug]).to eq listing.slug
+
+        result = described_class.search_documents(term: "lame")
+        expect(result).to be_empty
+      end
+
+      it "matches against the listing's title", :aggregate_failures do
+        listing.update_columns(title: "Awesome New Listing")
+        result = described_class.search_documents(term: "new")
+
+        expect(result.first[:title]).to eq listing.title
+
+        result = described_class.search_documents(term: "old")
+        expect(result).to be_empty
+      end
+    end
+
+    context "when searching for a term and filtering by category" do
+      it "selects results with the requested category" do
+        job_listings_category = create(:listing_category, name: "Job Listings", slug: "jobs")
+
+        job_listing = create(:listing,
+                             title: "Looking for a Ruby on Rails Developer!",
+                             listing_category: job_listings_category)
+
+        education_category = create(:listing_category, name: "Education/Courses", slug: "education")
+
+        listing.update_columns(
+          title: "New Ruby on Rails for Beginners Course!",
+          classified_listing_category_id: education_category.id,
+        )
+
+        result = described_class.search_documents(term: "Ruby on Rails", category: job_listing.category)
+        # rubocop:disable Rails/PluckId
+        ids = result.pluck(:id)
+        # rubocop:enable Rails/PluckId
+
+        expect(ids).to include(job_listing.id)
+        expect(ids).not_to include(listing.id)
+      end
+    end
+
+    context "when paginating" do
+      before { create_list(:listing, 2) }
+
+      it "returns no results when out of pagination bounds" do
+        result = described_class.search_documents(page: 99)
+        expect(result).to be_empty
+      end
+
+      it "returns paginated results", :aggregate_failures do
+        result = described_class.search_documents(page: 0, per_page: 1)
+        expect(result.length).to eq(1)
+
+        result = described_class.search_documents(page: 1, per_page: 1)
+        expect(result.length).to eq(1)
+      end
+    end
+  end
+end