[Search 2.0] Users (#13453)

* Add Search::Postgres::User

* Add sorting params

* Use a better algorithm to filter out suspended users

* Add tsvector index on users.name

* Add specs

* Add UserRole spec

* Update spec/services/search/postgres/user_spec.rb

Co-authored-by: Michael Kohl <citizen428@dev.to>

* Fix search with quotes in users's names

Co-authored-by: Michael Kohl <citizen428@dev.to>

app/controllers/search_controller.rb

@@ -159,8 +159,17 @@ class SearchController < ApplicationController
           term: feed_params[:search_fields],
         )
       elsif class_name.User?
-        # No need to check for articles or podcast episodes if we know we only want users
-        user_search
+        if FeatureFlag.enabled?(:search_2_users)
+          Search::Postgres::User.search_documents(
+            term: feed_params[:search_fields],
+            page: feed_params[:page],
+            per_page: feed_params[:per_page],
+            sort_by: feed_params[:sort_by] == "published_at" ? :created_at : nil,
+            sort_direction: feed_params[:sort_direction],
+          )
+        else
+          user_search
+        end
       else # search page
         feed_content_search
       end

app/models/user.rb

@@ -266,19 +266,25 @@ class User < ApplicationRecord
   # used in expression indexes as it's a mutable function and depends on server settings
   # => https://stackoverflow.com/a/11007216/4186181
   #
-  # rubocop:disable Layout/LineLength
   scope :search_by_name_and_username, lambda { |term|
     where(
-      %{to_tsvector('simple', coalesce(name::text, '')) @@ to_tsquery('simple', ''' ' || unaccent('simple', ?) || ' ''' || ':*')},
-      term,
+      sanitize_sql_array(
+        [
+          "to_tsvector('simple', coalesce(name::text, '')) @@ to_tsquery('simple', ? || ':*')",
+          connection.quote(term),
+        ],
+      ),
     ).or(
       where(
-        %{to_tsvector('simple', coalesce(username::text, '')) @@ to_tsquery('simple', ''' ' || unaccent('simple', ?) || ' ''' || ':*')},
-        term,
+        sanitize_sql_array(
+          [
+            "to_tsvector('simple', coalesce(username::text, '')) @@ to_tsquery('simple', ? || ':*')",
+            connection.quote(term),
+          ],
+        ),
       ),
     )
   }
-  # rubocop:enable Layout/LineLength
   scope :with_feed, -> { where.not(feed_url: [nil, ""]) }
 
   before_validation :check_for_username_change
@@ -650,6 +656,7 @@ class User < ApplicationRecord
     "#{employer_name}#{mostly_work_with}#{available_for}"
   end
 
+  # TODO: this can be removed once we migrate away from ES
   def search_score
     counts_score = (articles_count + comments_count + reactions_count + badge_achievements_count) * 10
     score = (counts_score + tag_keywords_for_search.size) * reputation_modifier

app/models/user_role.rb

@@ -0,0 +1,7 @@
+# Exposes the `rolify` gem's `users_roles` table via ActiveRecord
+class UserRole < ApplicationRecord
+  self.table_name = "users_roles"
+
+  belongs_to :user
+  belongs_to :role
+end

app/serializers/search/postgres_user_serializer.rb

@@ -0,0 +1,17 @@
+module Search
+  # to be renamed when ES is gone
+  class PostgresUserSerializer < ApplicationSerializer
+    attribute :class_name, -> { "User" }
+    attributes :id, :path
+    attribute :title, &:name
+
+    attribute :user do |user|
+      {
+        # currently the frontend expects this to be the username, despite the attribute name
+        name: user.username,
+        profile_image_90: user.profile_image_90,
+        username: user.username
+      }
+    end
+  end
+end

app/services/search/postgres/user.rb

@@ -0,0 +1,74 @@
+module Search
+  module Postgres
+    class User
+      ATTRIBUTES = %i[
+        id
+        name
+        profile_image
+        username
+      ].freeze
+
+      DEFAULT_PER_PAGE = 60
+      MAX_PER_PAGE = 100
+
+      # User.search_score used to take employer related fields into account, but they have since been moved to profile
+      # and removed from fields that are searched against.
+      HOTNESS_SCORE_ORDER = Arel.sql(%{
+        (((articles_count + comments_count + reactions_count + badge_achievements_count) * 10) * reputation_modifier)
+        DESC
+      }.squish).freeze
+
+      def self.search_documents(term: nil, sort_by: :nil, sort_direction: :desc, page: 0, per_page: DEFAULT_PER_PAGE)
+        # NOTE: 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 = ::User
+
+        relation = filter_suspended_users(relation)
+
+        relation = relation.search_by_name_and_username(term) if term.present?
+
+        relation = relation.select(*ATTRIBUTES)
+
+        relation = sort(relation, sort_by, sort_direction)
+
+        relation = relation.page(page).per(per_page)
+
+        serialize(relation)
+      end
+
+      # `User.without_role` generates a subquery + 2 inner joins.
+      # Given that the number of suspended users will, hopefully, be a tiny percentage
+      # of regular users, and the `rolify`'s gem approach is not particularly efficient,
+      # we simplified the subquery and added a precondition to skip that query entirely,
+      # when a community has no suspended users.
+      # NOTE: An alternative approach that could be explored is to
+      # preload the user ids of all suspended users and use those with `.where.not(id: ...)`
+      def self.filter_suspended_users(relation)
+        suspended = UserRole.joins(:role).where(roles: { name: :suspended })
+
+        return relation unless suspended.exists?
+
+        relation.where.not(id: suspended.select(:user_id))
+      end
+      private_class_method :filter_suspended_users
+
+      def self.sort(relation, sort_by, sort_direction)
+        return relation.reorder(sort_by => sort_direction) if sort_by&.to_sym == :created_at
+
+        relation.reorder(HOTNESS_SCORE_ORDER)
+      end
+      private_class_method :sort
+
+      def self.serialize(users)
+        Search::PostgresUserSerializer
+          .new(users, is_collection: true)
+          .serializable_hash[:data]
+          .pluck(:attributes)
+      end
+      private_class_method :serialize
+    end
+  end
+end

spec/models/user_role_spec.rb

@@ -0,0 +1,6 @@
+require "rails_helper"
+
+RSpec.describe UserRole, type: :model do
+  it { is_expected.to belong_to(:user) }
+  it { is_expected.to belong_to(:role) }
+end

spec/requests/search_spec.rb

@@ -301,6 +301,31 @@ RSpec.describe "Search", type: :request, proper_status: true do
         expect(response.parsed_body["result"].first).to include("body_text" => comment.body_markdown)
       end
     end
+
+    context "when using PostgreSQL for users" do
+      before do
+        allow(FeatureFlag).to receive(:enabled?).with(:search_2_users).and_return(true)
+      end
+
+      it "returns the correct keys", :aggregate_failures do
+        create(:user)
+
+        get search_feed_content_path(class_name: "User")
+
+        expect(response.parsed_body["result"]).to be_present
+      end
+
+      it "supports the search params" do
+        user = create(:user)
+
+        get search_feed_content_path(
+          class_name: "User", page: 0, per_page: 1, search_fields: user.name,
+          sort_by: :created_at, sort_direction: :desc
+        )
+
+        expect(response.parsed_body["result"].first["id"]).to eq(user.id)
+      end
+    end
   end
 
   describe "GET /search/reactions" do

spec/services/search/postgres/user_spec.rb

@@ -0,0 +1,118 @@
+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::User, type: :service do
+  describe "::search_documents" do
+    it "returns an empty result if there are no users" do
+      expect(described_class.search_documents).to be_empty
+    end
+
+    it "does not return suspended users" do
+      user = create(:user, :suspended)
+
+      expect(described_class.search_documents.pluck(:id)).not_to include(user.id)
+    end
+
+    it "returns regular users" do
+      user = create(:user)
+
+      expect(described_class.search_documents.pluck(:id)).to include(user.id)
+    end
+
+    it "returns admins" do
+      user = create(:user, :super_admin)
+
+      expect(described_class.search_documents.pluck(:id)).to include(user.id)
+    end
+
+    context "when describing the result format" do
+      let(:results) { described_class.search_documents }
+
+      it "returns the correct attributes for a single result", :aggregate_failures do
+        user = create(:user)
+
+        result = results.first
+
+        expect(result.keys).to match_array(%i[class_name id path title user])
+        expect(result[:class_name]).to eq("User")
+        expect(result[:id]).to eq(user.id)
+        expect(result[:path]).to eq(user.path)
+        expect(result[:title]).to eq(user.name)
+
+        expect(result[:user][:name]).to eq(user.username)
+        expect(result[:user][:profile_image_90]).to eq(user.profile_image_90)
+        expect(result[:user][:username]).to eq(user.username)
+      end
+    end
+
+    context "when searching for a term" do
+      let(:user) { create(:user) }
+
+      it "matches against the user's name", :aggregate_failures do
+        user.update_columns(name: "Langston Hughes")
+
+        result = described_class.search_documents(term: "lang")
+        expect(result.first[:id]).to eq(user.id)
+
+        result = described_class.search_documents(term: "fiesta")
+        expect(result).to be_empty
+      end
+
+      it "matches against the user's username", :aggregate_failures do
+        result = described_class.search_documents(term: user.username.first(3))
+        expect(result.first[:id]).to eq(user.id)
+
+        result = described_class.search_documents(term: "fiesta")
+        expect(result).to be_empty
+      end
+    end
+
+    context "when sorting" do
+      it "sorts by 'hotness_score' in descending order by default" do
+        user1, user2 = create_list(:user, 2)
+
+        user1.update_columns(articles_count: 10, reputation_modifier: 1.0)
+        user2.update_columns(articles_count: 10, reputation_modifier: 2.2)
+
+        results = described_class.search_documents
+        expect(results.pluck(:id)).to eq([user2.id, user1.id])
+      end
+
+      it "supports sorting by created_at in ascending and descending order", :aggregate_failures do
+        user1 = create(:user)
+
+        user2 = nil
+        Timecop.travel(1.week.ago) do
+          user2 = create(:user)
+        end
+
+        results = described_class.search_documents(sort_by: :created_at, sort_direction: :asc)
+        expect(results.pluck(:id)).to eq([user2.id, user1.id])
+
+        results = described_class.search_documents(sort_by: :created_at, sort_direction: :desc)
+        expect(results.pluck(:id)).to eq([user1.id, user2.id])
+      end
+    end
+
+    context "when paginating" do
+      it "returns no items when out of pagination bounds" do
+        create_list(:user, 2)
+
+        result = described_class.search_documents(page: 99)
+        expect(result).to be_empty
+      end
+
+      it "returns paginated items", :aggregate_failures do
+        create_list(:user, 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

spec/services/search/postgres/username_spec.rb

@@ -47,6 +47,14 @@ RSpec.describe Search::Postgres::Username, type: :service do
       expect(described_class.search_documents(user.name.first(3))).to be_present
     end
 
+    it "finds a user if their name contains quotes", :aggregate_failures do
+      user = create(:user, name: "McNamara O'Hara")
+      expect(described_class.search_documents(user.name)).to be_present
+
+      user = create(:user, name: 'McNamara O"Hara')
+      expect(described_class.search_documents(user.name)).to be_present
+    end
+
     it "finds multiple users whose names have common parts", :aggregate_failures do
       alex = create(:user, username: "alex")
       alexsmith = create(:user, name: "alexsmith")