[Hotfix] [Search 2.0] Optimize performance of listings and articles and fix bugs (#13577)

* Add tsvector index on listings

* Fix sorting order when fetching tag flares

* Add published_at as a sorting condition for Homepage::ArticlesQuery

* Re-added param needed by ES, this got lost somewhere down the line

app/controllers/search_controller.rb

@@ -51,6 +51,7 @@ class SearchController < ApplicationController
     :search_fields,
     :sort_by,
     :sort_direction,
+    :tag,
     :user_id,
     {
       tag_names: [],

app/queries/homepage/articles_query.rb

@@ -17,7 +17,7 @@ module Homepage
     ].freeze
     DEFAULT_PER_PAGE = 60
     MAX_PER_PAGE = 100
-    SORT_PARAMS = %i[hotness_score public_reactions_count].freeze
+    SORT_PARAMS = %i[hotness_score public_reactions_count published_at].freeze
 
     def self.call(...)
       new(...).call

app/services/search/postgres/article.rb

@@ -15,10 +15,10 @@ module Search
 
         relation = relation.search_articles(term) if term.present?
 
-        tag_flares = Homepage::FetchTagFlares.call(relation)
-
         relation = sort(relation, sort_by, sort_direction)
 
+        tag_flares = Homepage::FetchTagFlares.call(relation)
+
         # including user and organization as the last step as they are not needed
         # by the query that fetches tag flares, they are only needed by the serializer
         relation = relation.includes(:user, :organization)

db/migrate/20210429094116_add_tsvector_index_on_searchable_columns_to_listings.rb

@@ -0,0 +1,35 @@
+class AddTsvectorIndexOnSearchableColumnnsToListings < ActiveRecord::Migration[6.1]
+  disable_ddl_transaction!
+
+  def up
+    return if index_name_exists?(:classified_listings, :index_classified_listings_on_search_fields_as_tsvector)
+
+    query = <<-SQL
+    (
+      to_tsvector('simple'::regconfig, COALESCE((body_markdown)::text, ''::text)) ||
+      to_tsvector('simple'::regconfig, COALESCE((cached_tag_list)::text, ''::text)) ||
+      to_tsvector('simple'::regconfig, COALESCE((location)::text, ''::text)) ||
+      to_tsvector('simple'::regconfig, COALESCE((slug)::text, ''::text)) ||
+      to_tsvector('simple'::regconfig, COALESCE((title)::text, ''::text))
+    )
+    SQL
+
+    add_index(
+      :classified_listings,
+      query,
+      using: :gin,
+      name: :index_classified_listings_on_search_fields_as_tsvector,
+      algorithm: :concurrently,
+    )
+  end
+
+  def down
+    return unless index_name_exists?(:classified_listings, :index_classified_listings_on_search_fields_as_tsvector)
+
+    remove_index(
+      :classified_listings,
+      name: :index_classified_listings_on_search_fields_as_tsvector,
+      algorithm: :concurrently,
+    )
+  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_04_28_190634) do
+ActiveRecord::Schema.define(version: 2021_04_29_094116) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
@@ -343,6 +343,7 @@ ActiveRecord::Schema.define(version: 2021_04_28_190634) do
     t.string "title"
     t.datetime "updated_at", null: false
     t.bigint "user_id"
+    t.index "(((((to_tsvector('simple'::regconfig, COALESCE(body_markdown, ''::text)) || to_tsvector('simple'::regconfig, COALESCE((cached_tag_list)::text, ''::text))) || to_tsvector('simple'::regconfig, COALESCE((location)::text, ''::text))) || to_tsvector('simple'::regconfig, COALESCE((slug)::text, ''::text))) || to_tsvector('simple'::regconfig, COALESCE((title)::text, ''::text))))", name: "index_classified_listings_on_search_fields_as_tsvector", using: :gin
     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"

spec/queries/homepage/articles_query_spec.rb

@@ -154,7 +154,7 @@ RSpec.describe Homepage::ArticlesQuery, type: :query do
     end
 
     describe "sorting" do
-      it "sorts by the hotness_score", :aggregate_failures do
+      it "sorts by hotness_score", :aggregate_failures do
         article1, article2 = create_list(:article, 2)
 
         article1.update_columns(hotness_score: 1)
@@ -167,7 +167,7 @@ RSpec.describe Homepage::ArticlesQuery, type: :query do
         expect(result).to eq([article1.id, article2.id])
       end
 
-      it "sorts by the public_reactions_count" do
+      it "sorts by public_reactions_count", :aggregate_failures do
         article1, article2 = create_list(:article, 2)
 
         article1.update_columns(public_reactions_count: 1)
@@ -180,14 +180,30 @@ RSpec.describe Homepage::ArticlesQuery, type: :query do
         expect(result).to eq([article1.id, article2.id])
       end
 
+      it "sorts by published_at", :aggregate_failures do
+        article1, article2 = create_list(:article, 2)
+
+        article1.update_columns(published_at: 2.weeks.ago)
+        article2.update_columns(published_at: 1.week.ago)
+
+        result = described_class.call(sort_by: :published_at, sort_direction: :desc).ids
+        expect(result).to eq([article2.id, article1.id])
+
+        result = described_class.call(sort_by: :published_at, sort_direction: :asc).ids
+        expect(result).to eq([article1.id, article2.id])
+      end
+
       it "does not sort by unknown parameters" do
+        allow(Article).to receive(:order)
+
         article1, article2 = create_list(:article, 2)
 
         article1.update_columns(comments_count: 1)
         article2.update_columns(comments_count: 2)
 
-        expect(Article).not_to receive(:order) # rubocop:disable RSpec/MessageSpies
-        described_class.call(sort_by: :comments_count, sort_direction: :desc).ids
+        described_class.call(sort_by: :comments_count, sort_direction: :desc)
+
+        expect(Article).not_to have_received(:order)
       end
     end
   end