[Search 2.0] Articles (#13540)

* Rename the search scope to reflect its generic usage

* Add and use Search::Postgres::Article

app/controllers/search_controller.rb

@@ -122,21 +122,27 @@ class SearchController < ApplicationController
   # TODO: [@rhymes] the homepage feed uses `feed_content_search` as an index,
   # we should eventually move it to a JSON result
   # in ArticlesController#Homepage or HomepageController#show
-  # rubocop:disable Metric/PerceivedComplexity
+  # rubocop:disable Metrics/CyclomaticComplexity
+  # rubocop:disable Metrics/PerceivedComplexity
   def feed_content
     class_name = feed_params[:class_name].to_s.inquiry
 
     enable_search_2_homepage = (
       class_name.Article? &&
       feed_params[:search_fields].blank? &&
+      feed_params[:sort_by].present? &&
       FeatureFlag.enabled?(:search_2_homepage, current_user)
     )
 
     result =
       if class_name.blank?
-        # If we are in the main feed and not filtering by type return
-        # all articles, podcast episodes, and users
-        feed_content_search.concat(user_search)
+        if FeatureFlag.enabled?(:search_2_articles, current_user)
+          search_postgres_article
+        else
+          # If we are in the main feed and not filtering by type return
+          # all articles, podcast episodes, and users
+          feed_content_search.concat(user_search)
+        end
       elsif enable_search_2_homepage
         # NOTE: published_at is sent from the frontend in the following ES-friendly format:
         # => {"published_at"=>{"gte"=>"2021-04-06T14:53:23Z"}}
@@ -180,13 +186,16 @@ class SearchController < ApplicationController
         else
           user_search
         end
-      else # search page
+      elsif class_name.Article? && FeatureFlag.enabled?(:search_2_articles, current_user)
+        search_postgres_article
+      else
         feed_content_search
       end
 
     render json: { result: result }
   end
-  # rubocop:enable Metric/PerceivedComplexity
+  # rubocop:enable Metrics/PerceivedComplexity
+  # rubocop:enable Metrics/CyclomaticComplexity
 
   def reactions
     if FeatureFlag.enabled?(:search_2_reading_list)
@@ -213,6 +222,17 @@ class SearchController < ApplicationController
 
   private
 
+  def search_postgres_article
+    Search::Postgres::Article.search_documents(
+      term: feed_params[:search_fields],
+      user_id: feed_params[:user_id],
+      sort_by: feed_params[:sort_by],
+      sort_direction: feed_params[:sort_direction],
+      page: feed_params[:page],
+      per_page: feed_params[:per_page],
+    )
+  end
+
   def feed_content_search
     Search::FeedContent.search_documents(params: feed_params.to_h)
   end

app/models/article.rb

@@ -156,7 +156,11 @@ class Article < ApplicationRecord
   serialize :cached_user
   serialize :cached_organization
 
-  pg_search_scope :search_reading_list,
+  # TODO: [@rhymes] Rename the article column and the trigger name.
+  # What was initially meant just for the reading list (filtered using the `reactions` table),
+  # is also used for the article search page.
+  # The name of the `tsvector` column and its related trigger should be adapted.
+  pg_search_scope :search_articles,
                   against: :reading_list_document,
                   using: {
                     tsearch: {

app/services/search/postgres/article.rb

@@ -0,0 +1,45 @@
+module Search
+  module Postgres
+    class Article
+      DEFAULT_SORT_BY = "hotness_score DESC, comments_count DESC".freeze
+
+      def self.search_documents(
+        term: nil,
+        user_id: nil,
+        sort_by: nil,
+        sort_direction: nil,
+        page: nil,
+        per_page: nil
+      )
+        relation = Homepage::ArticlesQuery.call(user_id: user_id, page: page, per_page: per_page)
+
+        relation = relation.search_articles(term) if term.present?
+
+        tag_flares = Homepage::FetchTagFlares.call(relation)
+
+        relation = sort(relation, sort_by, sort_direction)
+
+        # 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)
+
+        serialize(relation, tag_flares)
+      end
+
+      def self.sort(relation, sort_by, sort_direction)
+        return relation.reorder(sort_by => sort_direction) if sort_by&.to_sym == :published_at
+
+        relation.reorder(DEFAULT_SORT_BY)
+      end
+      private_class_method :sort
+
+      def self.serialize(articles, tag_flares)
+        Homepage::ArticleSerializer
+          .new(articles, params: { tag_flares: tag_flares }, is_collection: true)
+          .serializable_hash[:data]
+          .pluck(:attributes)
+      end
+      private_class_method :serialize
+    end
+  end
+end

app/services/search/postgres/reading_list.rb

@@ -72,9 +72,11 @@ module Search
           .select(*REACTION_ATTRIBUTES)
           .to_sql
 
-        relation = Article.joins("INNER JOIN (#{reaction_query_sql}) reactions ON reactions.reactable_id = articles.id")
+        relation = ::Article.joins(
+          "INNER JOIN (#{reaction_query_sql}) reactions ON reactions.reactable_id = articles.id",
+        )
 
-        relation = relation.search_reading_list(term) if term.present?
+        relation = relation.search_articles(term) if term.present?
 
         # NOTE: [@rhymes] A previous version was implemented with:
         # `.tagged_with(tags, any: false).reselect(*ATTRIBUTES)`

spec/requests/search_spec.rb

@@ -234,7 +234,10 @@ RSpec.describe "Search", type: :request, proper_status: true do
     end
 
     context "when using PostgreSQL for the homepage" do
+      let(:homepage_params) { { class_name: "Article", sort_by: "published_at", sort_direction: "desc" } }
+
       before do
+        allow(FeatureFlag).to receive(:enabled?).with(:search_2_articles, anything).and_return(true)
         allow(FeatureFlag).to receive(:enabled?).with(:search_2_homepage, anything).and_return(true)
       end
 
@@ -251,7 +254,7 @@ RSpec.describe "Search", type: :request, proper_status: true do
       it "calls Homepage::FetchArticles when class_name is Article" do
         allow(Homepage::FetchArticles).to receive(:call)
 
-        get search_feed_content_path(class_name: "Article")
+        get search_feed_content_path(homepage_params)
 
         expect(Homepage::FetchArticles).to have_received(:call)
       end
@@ -259,7 +262,7 @@ RSpec.describe "Search", type: :request, proper_status: true do
       it "returns the correct keys", :aggregate_failures do
         create(:article)
 
-        get search_feed_content_path(class_name: "Article")
+        get search_feed_content_path(homepage_params)
 
         expect(response.parsed_body["result"]).to be_present
       end
@@ -267,18 +270,18 @@ RSpec.describe "Search", type: :request, proper_status: true do
       it "parses published_at correctly", :aggregate_failures do
         article = create(:article)
 
-        get search_feed_content_path(class_name: "Article", published_at: { gte: article.published_at.iso8601 })
+        get search_feed_content_path(homepage_params.merge(published_at: { gte: article.published_at.iso8601 }))
         expect(response.parsed_body["result"].first["id"]).to eq(article.id)
 
         datetime = article.published_at + 1.minute
-        get search_feed_content_path(class_name: "Article", published_at: { gte: datetime.iso8601 })
+        get search_feed_content_path(homepage_params.merge(published_at: { gte: datetime.iso8601 }))
         expect(response.parsed_body["result"]).to be_empty
       end
 
       it "supports the user_id parameter" do
         allow(Homepage::FetchArticles).to receive(:call)
 
-        get search_feed_content_path(class_name: "Article", user_id: 1)
+        get search_feed_content_path(homepage_params.merge(user_id: 1))
 
         expect(Homepage::FetchArticles).to have_received(:call).with(hash_including(user_id: "1"))
       end
@@ -286,7 +289,7 @@ RSpec.describe "Search", type: :request, proper_status: true do
       it "supports the organization_id parameter" do
         allow(Homepage::FetchArticles).to receive(:call)
 
-        get search_feed_content_path(class_name: "Article", organization_id: 1)
+        get search_feed_content_path(homepage_params.merge(organization_id: 1))
 
         expect(Homepage::FetchArticles).to have_received(:call).with(hash_including(organization_id: "1"))
       end
@@ -294,12 +297,59 @@ RSpec.describe "Search", type: :request, proper_status: true do
       it "supports the tag_names parameter" do
         allow(Homepage::FetchArticles).to receive(:call)
 
-        get search_feed_content_path(class_name: "Article", tag_names: %i[ruby])
+        get search_feed_content_path(homepage_params.merge(tag_names: %i[ruby]))
 
         expect(Homepage::FetchArticles).to have_received(:call).with(hash_including(tags: %w[ruby]))
       end
     end
 
+    context "when using PostgreSQL for articles" do
+      before do
+        allow(FeatureFlag).to receive(:enabled?).with(:search_2_articles, anything).and_return(true)
+      end
+
+      it "calls Search::Postgres::Article without a class_name" do
+        allow(Search::Postgres::Article).to receive(:search_documents)
+
+        get search_feed_content_path
+
+        expect(Search::Postgres::Article).to have_received(:search_documents)
+      end
+
+      it "calls Search::Postgres::Article without a class_name with :search_2_homepage active" do
+        allow(FeatureFlag).to receive(:enabled?).with(:search_2_homepage, anything).and_return(true)
+        allow(Search::Postgres::Article).to receive(:search_documents)
+
+        get search_feed_content_path
+
+        expect(Search::Postgres::Article).to have_received(:search_documents)
+      end
+
+      it "calls Search::Postgres::Article with class_name=Article with :search_2_homepage active" do
+        allow(FeatureFlag).to receive(:enabled?).with(:search_2_homepage, anything).and_return(true)
+        allow(Search::Postgres::Article).to receive(:search_documents)
+
+        get search_feed_content_path(class_name: "Article")
+
+        expect(Search::Postgres::Article).to have_received(:search_documents)
+      end
+
+      it "supports the search params", :aggregate_failures do
+        allow(Search::Postgres::Article).to receive(:search_documents).and_call_original
+
+        article = create(:article)
+
+        get search_feed_content_path(
+          class_name: "Article", page: 0, per_page: 1, search_fields: article.title,
+          sort_by: :published_at, sort_direction: :desc
+        )
+
+        expect(response.parsed_body["result"].first["id"]).to eq(article.id)
+
+        expect(Search::Postgres::Article).to have_received(:search_documents)
+      end
+    end
+
     context "when using PostgreSQL for comments" do
       before do
         allow(FeatureFlag).to receive(:enabled?).with(:search_2_comments).and_return(true)

spec/services/search/postgres/article_spec.rb

@@ -0,0 +1,152 @@
+require "rails_helper"
+
+# rubocop:disable Rails/PluckId
+# This spec uses `pluck` on an array of hashes, but Rubocop can't tell the difference.
+RSpec.describe Search::Postgres::Article, type: :service do
+  describe "::search_documents" do
+    it "returns an empty result if there are no articles" do
+      expect(described_class.search_documents).to be_empty
+    end
+
+    it "does not return unpublished articles" do
+      article = create(:article, published: false)
+
+      expect(described_class.search_documents.pluck(:id)).not_to include(article.id)
+    end
+
+    it "returns published articles" do
+      article = create(:article)
+
+      expect(described_class.search_documents.pluck(:id)).to include(article.id)
+    end
+
+    context "when describing the result format" do
+      it "does not include a highlight attribute" do
+        create(:article)
+
+        expect(described_class.search_documents.first.key?(:highlight)).to be(false)
+      end
+    end
+
+    context "when filtering by user_id" do
+      it "returns articles belonging to a specific user", :aggregate_failures do
+        user = create(:user)
+        articles = create_list(:article, 2, user: user)
+        article = create(:article)
+
+        results = described_class.search_documents(user_id: user.id).pluck(:id)
+        expect(results).not_to include(article.id)
+        expect(results).to match_array(articles.pluck(:id))
+      end
+    end
+
+    context "when searching for a term" do
+      let(:article) { create(:article) }
+
+      it "matches against the article's body_markdown", :aggregate_failures do
+        article.update_columns(body_markdown: "Life of the party")
+
+        result = described_class.search_documents(term: "part").first
+        expect(result[:path]).to eq(article.path)
+
+        results = described_class.search_documents(term: "fiesta")
+        expect(results).to be_empty
+      end
+
+      it "matches against the article's title", :aggregate_failures do
+        article.update_columns(title: "Life of the party")
+
+        result = described_class.search_documents(term: "part").first
+        expect(result[:path]).to eq(article.path)
+
+        results = described_class.search_documents(term: "fiesta")
+        expect(results).to be_empty
+      end
+
+      it "matches against the article's tags", :aggregate_failures do
+        article.update_columns(cached_tag_list: "javascript, beginners, ruby")
+
+        result = described_class.search_documents(term: "beginner").first
+        expect(result[:path]).to eq(article.path)
+
+        results = described_class.search_documents(term: "newbie")
+        expect(results).to be_empty
+      end
+
+      it "matches against the article's organization's name", :aggregate_failures do
+        article.organization = create(:organization, name: "ACME corp")
+        article.save!
+
+        result = described_class.search_documents(term: "ACME").first
+        expect(result[:path]).to eq(article.path)
+
+        results = described_class.search_documents(term: "ECMA")
+        expect(results).to be_empty
+      end
+
+      it "matches against the article's user's name", :aggregate_failures do
+        result = described_class.search_documents(term: article.user_name.first(3)).first
+        expect(result[:path]).to eq(article.path)
+
+        results = described_class.search_documents(term: "notaname")
+        expect(results).to be_empty
+      end
+
+      it "matches against the article's user's username", :aggregate_failures do
+        result = described_class.search_documents(term: article.user_username.first(3)).first
+        expect(result[:path]).to eq(article.path)
+
+        results = described_class.search_documents(term: "notausername")
+        expect(results).to be_empty
+      end
+    end
+
+    context "when sorting" do
+      it "sorts by 'hotness_score' and 'comments_count' in descending order by default" do
+        article1, article2, article3 = create_list(:article, 3)
+
+        article1.update_columns(hotness_score: 10, comments_count: 10)
+        article2.update_columns(hotness_score: 10, comments_count: 11)
+        article3.update_columns(hotness_score: 9, comments_count: 20)
+
+        results = described_class.search_documents
+        expect(results.pluck(:id)).to eq([article2.id, article1.id, article3.id])
+      end
+
+      it "supports sorting by published_at in ascending and descending order", :aggregate_failures do
+        article1 = create(:article)
+
+        article2 = nil
+        Timecop.travel(1.week.ago) do
+          article2 = create(:article)
+        end
+
+        results = described_class.search_documents(sort_by: :published_at, sort_direction: :asc)
+        expect(results.pluck(:id)).to eq([article2.id, article1.id])
+
+        results = described_class.search_documents(sort_by: :published_at, sort_direction: :desc)
+        expect(results.pluck(:id)).to eq([article1.id, article2.id])
+      end
+    end
+
+    context "when paginating" do
+      it "returns no items when out of pagination bounds" do
+        create(:article)
+
+        result = described_class.search_documents(page: 99)
+        expect(result).to be_empty
+      end
+
+      it "returns paginated items", :aggregate_failures do
+        create_list(:article, 2)
+
+        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
+# rubocop:enable Rails/PluckId