diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 6814631d2..2a1b2893c 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -119,11 +119,26 @@ class SearchController < ApplicationController end def reactions - result = Search::ReadingList.search_documents( - params: reaction_params.to_h, user: current_user, - ) + if FeatureFlag.enabled?(:search_2_reading_list) + # [@rhymes] we're recyling the existing params as we want to change the frontend as + # little as possible, we might simplify in the future + result = Search::Postgres::ReadingList.search_documents( + current_user, + page: reaction_params[:page], + per_page: reaction_params[:per_page], + statuses: reaction_params[:status], + tags: reaction_params[:tag_names], + term: reaction_params[:search_fields], + ) - render json: { result: result["reactions"], total: result["total"] } + render json: { result: result[:items], total: result[:total] } + else + result = Search::ReadingList.search_documents( + params: reaction_params.to_h, user: current_user, + ) + + render json: { result: result["reactions"], total: result["total"] } + end end private diff --git a/app/javascript/readingList/readingList.jsx b/app/javascript/readingList/readingList.jsx index b60194469..135947f1d 100644 --- a/app/javascript/readingList/readingList.jsx +++ b/app/javascript/readingList/readingList.jsx @@ -158,6 +158,7 @@ export class ReadingList extends Component { render() { const { items = [], + itemsTotal, availableTags, selectedTags, showLoadMoreButton, @@ -190,7 +191,7 @@ export class ReadingList extends Component {

{isStatusViewValid ? 'Reading list' : 'Archive'} - {` (${items.length})`} + {` (${itemsTotal})`}

diff --git a/app/javascript/searchableItemList/searchableItemList.js b/app/javascript/searchableItemList/searchableItemList.js index f8eb1f8b8..5c5b9b80b 100644 --- a/app/javascript/searchableItemList/searchableItemList.js +++ b/app/javascript/searchableItemList/searchableItemList.js @@ -12,6 +12,7 @@ export function defaultState(options) { items: [], itemsLoaded: false, + itemsTotal: 0, availableTags: [], selectedTags: [], @@ -74,6 +75,8 @@ export function performInitialSearch({ searchOptions = {} }) { const responsePromise = fetchSearch('reactions', dataHash); return responsePromise.then((response) => { const reactions = response.result; + // FIXME: [@rhymes] the list of tags in the left column of the reading list + // is populated with only the tags belonging to items in the first page const availableTags = [ ...new Set(reactions.flatMap((rxn) => rxn.reactable.tag_list)), ].sort(); @@ -81,6 +84,7 @@ export function performInitialSearch({ searchOptions = {} }) { page: 0, items: reactions, itemsLoaded: true, + itemsTotal: response.total, showLoadMoreButton: hitsPerPage < response.total, availableTags, }); @@ -128,6 +132,7 @@ export function search(query, { page, tags, statusView, appendItems = false }) { query, page: newPage, items, + itemsTotal: response.total, showLoadMoreButton: items.length < response.total, }); }); diff --git a/app/models/article.rb b/app/models/article.rb index b4e7b1b29..448478ddf 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -7,6 +7,7 @@ class Article < ApplicationRecord include Reactable include Searchable include UserSubscriptionSourceable + include PgSearch::Model SEARCH_SERIALIZER = Search::ArticleSerializer SEARCH_CLASS = Search::FeedContent @@ -111,6 +112,16 @@ class Article < ApplicationRecord serialize :cached_user serialize :cached_organization + # [@rhymes] this is adapted from the `search_fields` property in + # `config/elasticsearch/mappings/feed_content.json` + pg_search_scope :search_reading_list, + against: %i[body_markdown title cached_tag_list], + associated_against: { + organization: %i[name], + user: %i[name username] + }, + using: { tsearch: { prefix: true } } + # [@jgaskins] We use an index on `published`, but since it's a boolean value # the Postgres query planner often skips it due to lack of diversity of the # data in the column. However, since `published_at` is a *very* diverse diff --git a/app/models/tag.rb b/app/models/tag.rb index ef535cbee..2edc59293 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -13,9 +13,6 @@ class Tag < ActsAsTaggableOn::Tag include Searchable include PgSearch::Model - pg_search_scope :search_by_name, - against: :name, - using: { tsearch: { prefix: true } } ALLOWED_CATEGORIES = %w[uncategorized language library tool site_mechanic location subcommunity].freeze HEX_COLOR_REGEXP = /\A#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})\z/.freeze @@ -48,6 +45,10 @@ class Tag < ActsAsTaggableOn::Tag after_commit :sync_related_elasticsearch_docs, on: [:update] after_commit :remove_from_elasticsearch, on: [:destroy] + pg_search_scope :search_by_name, + against: :name, + using: { tsearch: { prefix: true } } + scope :eager_load_serialized_data, -> {} SEARCH_SERIALIZER = Search::TagSerializer diff --git a/app/serializers/search/reading_list_article_serializer.rb b/app/serializers/search/reading_list_article_serializer.rb new file mode 100644 index 000000000..86009a719 --- /dev/null +++ b/app/serializers/search/reading_list_article_serializer.rb @@ -0,0 +1,36 @@ +module Search + class ReadingListArticleSerializer < ApplicationSerializer + attribute :id, &:reaction_id + attribute :user_id, &:reaction_user_id + + # NOTE: the list of attributes for `reactable` comes from two places: + # => the `` Preact component: https://github.com/forem/forem/blob/33d0e03dbd94fc6797693b84fcafb5040ea399d0/app/javascript/readingList/components/ItemListItem.jsx#L72-L85 + # => the `performInitialSearch` function: https://github.com/forem/forem/blob/33d0e03dbd94fc6797693b84fcafb5040ea399d0/app/javascript/searchableItemList/searchableItemList.js#L78 + attribute :reactable do |article, params| + user = params[:users][article.user_id] + tags = article.cached_tag_list.to_s.split(", ") + + { + path: article.path, + readable_publish_date_string: article.readable_publish_date, + reading_time: article.reading_time, + + # TODO: once we switch to PG we should revisit how we handle tags in the + # frontend, we're sending back tags twice in slightly different formats + tag_list: tags, + tags: tags.map { |tag| { name: tag } }, + + title: article.title, + + # NOTE: not using the `NestedUserSerializer` because we don't need the + # the `pro` flag on the frontend, and we also avoid hitting Redis to + # fetch the cached value + user: { + name: user.name, + profile_image_90: user.profile_image_90, + username: user.username + } + } + end + end +end diff --git a/app/services/search/postgres/reading_list.rb b/app/services/search/postgres/reading_list.rb new file mode 100644 index 000000000..dca6aef71 --- /dev/null +++ b/app/services/search/postgres/reading_list.rb @@ -0,0 +1,123 @@ +module Search + module Postgres + class ReadingList + ATTRIBUTES = [ + "articles.cached_tag_list", + "articles.crossposted_at", + "articles.path", + "articles.published_at", + "articles.reading_time", + "articles.title", + "articles.user_id", + "reactions.id AS reaction_id", + "reactions.user_id AS reaction_user_id", + ].freeze + USER_ATTRIBUTES = %i[id name profile_image username].freeze + + DEFAULT_STATUSES = %w[confirmed valid].freeze + + DEFAULT_PER_PAGE = 60 + MAX_PER_PAGE = 100 # to avoid querying too many items, we set a maximum amount for a page + + def self.search_documents(user, term: nil, statuses: [], tags: [], page: 0, per_page: DEFAULT_PER_PAGE) + return {} unless user + + statuses = statuses.presence || DEFAULT_STATUSES + tags = tags.presence || [] + + # NOTE: [@rhymes] 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 + + result = find_articles( + user_id: user.id, + term: term, + statuses: statuses, + tags: tags, + page: page, + per_page: per_page, + ) + + # NOTE: [@rhymes] an earlier version used `Article.includes(:user)` + # to preload users, unfortunately it's not possible in Rails to specify + # which fields of the included relation's table to select ahead of time. + # The `users` table is massive (115 columns on March 2021) and thus we + # shouldn't load it all in memory just to select a few fields. + # For these reasons I decided to avoid preloading altogether and issue + # an additional SQL query to load User objects + # (see https://github.com/forem/forem/pull/4744#discussion_r345698674 + # and https://github.com/rails/rails/issues/15185#issuecomment-351868335 + # for additional context) + user_ids = result[:items].pluck(:user_id) + users = find_users(user_ids) + + { + items: serialize(result[:items], users), + total: result[:total] + } + end + + def self.find_articles(user_id:, term:, statuses:, tags:, page:, per_page:) + relation = ::Article + .joins(:reactions) + .where("reactions.category": :readinglist) + .where("reactions.user_id": user_id) + .where("reactions.status": statuses) + + relation = relation.search_reading_list(term) if term.present? + + # NOTE: [@rhymes] A previous version was implemented with: + # `.tagged_with(tags, any: false).reselect(*ATTRIBUTES)` + # + # =>`.tagged_with()` merges `articles.*` to the SQL, thus we needed to + # use `reselect()`, see https://github.com/forem/forem/pull/12420 + # => `.tagged_with()` with multiple tags constructs a monster query, + # see https://explain.depesz.com/s/CqQV / https://explain.dalibo.com/plan/1Lm + # This is because the `acts-as-taggable-on` query creates a separate INNER JOIN + # per each tag that is added to the list, each new clause uses the `LIKE` operator on `tags.name`. + # That could have been improved by by adding a GIN index on `tags.name`, see + # https://www.cybertec-postgresql.com/en/postgresql-more-performance-for-like-and-ilike-statements/ + # and a similar discussion https://github.com/forem/forem/pull/12584#discussion_r570756176 + # + # An alternative solution, as we don't need the `Tag` model itself, is to use + # `articles.cached_tag_list` and the `LIKE` operator on it, this could be further + # improved, if needed, by adding a GIN index on `cached_tag_list` + # It seems not to be needed as this approach is roughly 1850 times faster than the previous + # see https://explain.depesz.com/s/ajoP / https://explain.dalibo.com/plan/PZb + tags.each do |tag| + relation = relation.where("articles.cached_tag_list LIKE ?", "%#{tag}%") + end + + # here we issue a COUNT(*) after all the conditions are applied, + # because we need to fetch the total number of articles, pre pagination + total = relation.count + + relation = relation.select(*ATTRIBUTES).order("reactions.created_at": :desc) + relation = relation.page(page).per(per_page) + + { + items: relation, + total: total + } + end + private_class_method :find_articles + + def self.find_users(user_ids) + ::User + .where(id: user_ids) + .select(*USER_ATTRIBUTES) + .index_by(&:id) + end + private_class_method :find_users + + def self.serialize(articles, users) + Search::ReadingListArticleSerializer + .new(articles, params: { users: users }, is_collection: true) + .serializable_hash[:data] + .pluck(:attributes) + end + private_class_method :serialize + end + end +end diff --git a/db/migrate/20210319185315_add_index_to_reactions_status.rb b/db/migrate/20210319185315_add_index_to_reactions_status.rb new file mode 100644 index 000000000..1cd745fcb --- /dev/null +++ b/db/migrate/20210319185315_add_index_to_reactions_status.rb @@ -0,0 +1,7 @@ +class AddIndexToReactionsStatus < ActiveRecord::Migration[6.0] + disable_ddl_transaction! + + def change + add_index :reactions, :status, algorithm: :concurrently + end +end diff --git a/db/migrate/20210322152837_add_index_to_articles_cached_tag_list.rb b/db/migrate/20210322152837_add_index_to_articles_cached_tag_list.rb new file mode 100644 index 000000000..07d2a5976 --- /dev/null +++ b/db/migrate/20210322152837_add_index_to_articles_cached_tag_list.rb @@ -0,0 +1,9 @@ +class AddIndexToArticlesCachedTagList < ActiveRecord::Migration[6.0] + disable_ddl_transaction! + + def change + # From + # We need a GIN index on `cached_tag_list` to speed up `LIKE` queries + add_index :articles, :cached_tag_list, using: :gin, opclass: :gin_trgm_ops, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index b9de1234b..861e2186f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -156,6 +156,7 @@ ActiveRecord::Schema.define(version: 2021_03_23_190443) do t.string "video_thumbnail_url" t.index "user_id, title, digest(body_markdown, 'sha512'::text)", name: "index_articles_on_user_id_and_title_and_digest_body_markdown", unique: true t.index ["boost_states"], name: "index_articles_on_boost_states", using: :gin + t.index ["cached_tag_list"], name: "index_articles_on_cached_tag_list", opclass: :gin_trgm_ops, using: :gin t.index ["canonical_url"], name: "index_articles_on_canonical_url", unique: true t.index ["collection_id"], name: "index_articles_on_collection_id" t.index ["comment_score"], name: "index_articles_on_comment_score" @@ -1036,6 +1037,7 @@ ActiveRecord::Schema.define(version: 2021_03_23_190443) do t.index ["points"], name: "index_reactions_on_points" t.index ["reactable_id", "reactable_type"], name: "index_reactions_on_reactable_id_and_reactable_type" t.index ["reactable_type"], name: "index_reactions_on_reactable_type" + t.index ["status"], name: "index_reactions_on_status" t.index ["user_id", "reactable_id", "reactable_type", "category"], name: "index_reactions_on_user_id_reactable_id_reactable_type_category", unique: true end diff --git a/spec/requests/search_spec.rb b/spec/requests/search_spec.rb index 11b7b3292..8cedb4bf6 100644 --- a/spec/requests/search_spec.rb +++ b/spec/requests/search_spec.rb @@ -198,29 +198,57 @@ RSpec.describe "Search", type: :request, proper_status: true do end describe "GET /search/reactions" do - let(:authorized_user) { create(:user) } - let(:mock_response) do - { "reactions" => [{ id: 123 }], "total" => 100 } + before do + sign_in authorized_user end - it "returns json with reactions and total" do - sign_in authorized_user - allow(Search::ReadingList).to receive(:search_documents).and_return( - mock_response, - ) - get "/search/reactions" - expect(response.parsed_body).to eq("result" => [{ "id" => 123 }], "total" => 100) + context "when using Elasticsearch" do + let(:mock_response) do + { "reactions" => [{ id: 123 }], "total" => 100 } + end + + before do + allow(Search::ReadingList).to receive(:search_documents).and_return(mock_response) + end + + it "returns json with reactions and total" do + get search_reactions_path + + expect(response.parsed_body).to eq("result" => [{ "id" => 123 }], "total" => 100) + end + + it "accepts array of tag names" do + get search_reactions_path(tag_names: [1, 2]) + + expect(Search::ReadingList).to( + have_received(:search_documents) + .with(params: { "tag_names" => %w[1 2] }, user: authorized_user), + ) + end end - it "accepts array of tag names" do - sign_in authorized_user - allow(Search::ReadingList).to receive(:search_documents).and_return( - mock_response, - ) - get "/search/reactions?tag_names[]=1&tag_names[]=2" - expect(Search::ReadingList).to have_received( - :search_documents, - ).with(params: { "tag_names" => %w[1 2] }, user: authorized_user) + context "when using PostgreSQL" do + let(:article) { create(:article) } + + before do + allow(FeatureFlag).to receive(:enabled?).with(:search_2_reading_list).and_return(true) + create(:reaction, category: :readinglist, reactable: article, user: authorized_user) + end + + it "returns the correct keys", :aggregate_failures do + get search_reactions_path + + expect(response.parsed_body["result"]).to be_present + expect(response.parsed_body["total"]).to eq(1) + end + + it "supports the search params" do + article.update_columns(title: "Title", cached_tag_list: "ruby, python") + + get search_reactions_path(page: 0, per_page: 1, status: %w[valid], tags: %w[ruby], term: "title") + + expect(response.parsed_body["result"].first["reactable"]).to include("title" => "Title") + end end end end diff --git a/spec/services/search/postgres/reading_list_spec.rb b/spec/services/search/postgres/reading_list_spec.rb new file mode 100644 index 000000000..c99576b33 --- /dev/null +++ b/spec/services/search/postgres/reading_list_spec.rb @@ -0,0 +1,327 @@ +require "rails_helper" + +RSpec.describe Search::Postgres::ReadingList, type: :service do + let(:user) { create(:user) } + let(:article) { create(:article) } + let(:article_not_in_reading_list) { create(:article) } + let(:article_1) { create(:article, with_tags: false) } + let(:article_2) { create(:article, with_tags: false) } + + def extract_from_results(result, attribute) + result[:items].pluck(:reactable).pluck(attribute) + end + + describe "::search_documents" do + before do + create(:reaction, user: user, reactable: article, category: :readinglist, status: :valid) + end + + it "returns an empty result without a user" do + expect(described_class.search_documents(nil)).to be_empty + end + + it "does not include an article not in the reading list" do + result = described_class.search_documents(user) + expect(extract_from_results(result, :path)).not_to include(article_not_in_reading_list.path) + end + + it "does not return an article belonging to another user's reading list" do + create( + :reaction, + reactable: article_not_in_reading_list, + category: :readinglist, + user: article_not_in_reading_list.user, + ) + + result = described_class.search_documents(user) + expect(extract_from_results(result, :path)).not_to include(article_not_in_reading_list.path) + end + + it "returns results of articles in the reading list" do + result = described_class.search_documents(user) + expect(extract_from_results(result, :path)).to include(article.path) + end + + it "returns the total count of the articles in the reading list" do + articles = create_list(:article, 2) + articles.each { |article| create(:reaction, category: :readinglist, reactable: article, user: user) } + + result = described_class.search_documents(user) + expect(result[:total]).to eq(user.reactions.readinglist.count) + end + + context "when describing the result format" do + let(:result) { described_class.search_documents(user) } + + it "returns the correct attributes for the result", :aggregate_failures do + expect(result.keys).to match_array(%i[items total]) + expect(result[:total]).to be_present + expect(result[:items]).to be_present + end + + it "returns the correct attributes for a reading list item", :aggregate_failures do + item = result[:items].first + + expect(item.keys).to match_array(%i[id user_id reactable]) + expect(item[:id]).to eq(user.reactions.readinglist.first.id) + expect(item[:user_id]).to eq(user.id) + end + + it "returns the correct attributes for a reading list item's reactable", :aggregate_failures do + item = result[:items].first + reactable = item[:reactable] + + expect(reactable.keys).to match_array( + %i[ + path readable_publish_date_string reading_time + tag_list tags title user + ], + ) + expect(reactable[:path]).to eq(article.path) + expect(reactable[:readable_publish_date_string]).to eq(article.readable_publish_date) + expect(reactable[:reading_time]).to eq(article.reading_time) + tags = article.cached_tag_list.to_s.split(", ") + expect(reactable[:tag_list]).to eq(tags) + expect(reactable[:tags]).to eq(tags.map { |tag| { name: tag } }) + expect(reactable[:title]).to eq(article.title) + end + + it "returns the correct attributes for a reading list item's reactable's user", :aggregate_failures do + item = result[:items].first + reactable = item[:reactable] + + reactable_user = reactable[:user] + expect(reactable_user.keys).to eq(%i[name profile_image_90 username]) + expect(reactable_user[:name]).to eq(article.user.name) + expect(reactable_user[:profile_image_90]).to eq(article.user.profile_image_90) + expect(reactable_user[:username]).to eq(article.user.username) + end + end + + context "when filtering by statuses" do + it "returns confirmed items by default" do + item = user.reactions.readinglist.last + item.update_columns(status: :confirmed) + + expect(described_class.search_documents(user)[:items].first[:id]).to eq(item.id) + end + + it "returns valid items by default" do + item = user.reactions.readinglist.last + item.update_columns(status: :valid) + + expect(described_class.search_documents(user)[:items].first[:id]).to eq(item.id) + end + + it "selects archived items upon request", :aggregate_failures do + valid_item = user.reactions.readinglist.last + archived_item = create(:reaction, user: user, category: :readinglist, status: :archived) + + # rubocop:disable Rails/PluckId + ids = described_class.search_documents(user, statuses: :archived)[:items].pluck(:id) + # rubocop:enable Rails/PluckId + + expect(ids).to include(archived_item.id) + expect(ids).not_to include(valid_item.id) + end + end + + context "when filtering by tags" do + after do + user.reactions.readinglist.delete_all + end + + it "filters out items belonging to an article without the requested tag" do + article_1.tag_list.add(:beginners) + article_1.save! + + create(:reaction, reactable: article_1, user: user, category: :readinglist) + + result = described_class.search_documents(user, tags: [:foobar]) + expect(extract_from_results(result, :path)).to be_empty + end + + it "selects items belonging to an article with the requested tag" do + article_1.tag_list.add(:beginners) + article_1.save! + + create(:reaction, reactable: article_1, user: user, category: :readinglist) + + result = described_class.search_documents(user, tags: [:beginners]) + expect(extract_from_results(result, :path)).to match_array([article_1.path]) + end + + it "selects items belonging to an article with all the requested tags", :aggregate_failures do + article_1.tag_list.add(:beginners) + article_1.tag_list.add(:ruby) + article_1.save! + + article_2.tag_list.add(:beginners) + article_2.save! + + create(:reaction, reactable: article_1, user: user, category: :readinglist) + create(:reaction, reactable: article_2, user: user, category: :readinglist) + + result = described_class.search_documents(user, tags: %i[beginners ruby]) + expect(extract_from_results(result, :path)).to include(article_1.path) + expect(extract_from_results(result, :path)).not_to include(article_2.path) + end + end + + context "when filtering by statuses and tags" do + it "selects items with the requested statuses and articles tags", :aggregate_failures do + article_1.tag_list.add(:beginners) + article_2.tag_list.add(:beginners) + article_1.save! + article_2.save! + + valid_item = create(:reaction, category: :readinglist, user: user, reactable: article_1, status: :valid) + archived_item = create(:reaction, category: :readinglist, user: user, reactable: article_2, status: :archived) + + result = described_class.search_documents(user, statuses: %i[valid], tags: %i[beginners]) + + # rubocop:disable Rails/PluckId + item_ids = result[:items].pluck(:id) + # rubocop:enable Rails/PluckId + expect(item_ids).to include(valid_item.id) + expect(item_ids).not_to include(archived_item.id) + end + end + + context "when searching for a term" do + 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(user, term: "part") + expect(extract_from_results(result, :path)).to include(article.path) + + result = described_class.search_documents(user, term: "fiesta") + expect(extract_from_results(result, :path)).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(user, term: "part") + expect(extract_from_results(result, :path)).to include(article.path) + + result = described_class.search_documents(user, term: "fiesta") + expect(extract_from_results(result, :path)).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(user, term: "beginner") + expect(extract_from_results(result, :path)).to include(article.path) + + result = described_class.search_documents(user, term: "newbie") + expect(extract_from_results(result, :path)).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(user, term: "ACME") + expect(extract_from_results(result, :path)).to include(article.path) + + result = described_class.search_documents(user, term: "ECMA") + expect(extract_from_results(result, :path)).to be_empty + end + + it "matches against the article's user's name", :aggregate_failures do + article_user = article.user + article_user.update_columns(name: "Friday Sunday") + + result = described_class.search_documents(user, term: "Frida") + expect(extract_from_results(result, :path)).to include(article.path) + + result = described_class.search_documents(user, term: "Sat") + expect(extract_from_results(result, :path)).to be_empty + end + + it "matches against the article's user's username", :aggregate_failures do + article_user = article.user + article_user.update_columns(username: "fridaysunday") + + result = described_class.search_documents(user, term: "Frida") + expect(extract_from_results(result, :path)).to include(article.path) + + result = described_class.search_documents(user, term: "Sat") + expect(extract_from_results(result, :path)).to be_empty + end + end + + context "when searching for a term and filtering by statuses" do + it "selects items with the requested status belonging to articles matching the term", :aggregate_failures do + article.update_columns(body_markdown: "Life of the party") + + result = described_class.search_documents(user, term: "part", statuses: %i[valid]) + expect(extract_from_results(result, :path)).to include(article.path) + + result = described_class.search_documents(user, term: "part", statuses: %i[archived]) + expect(extract_from_results(result, :path)).to be_empty + + result = described_class.search_documents(user, term: "fiesta", statuses: %i[valid]) + expect(extract_from_results(result, :path)).to be_empty + end + end + + context "when searching for a term and filtering by tags" do + it "selects items with the requested status belonging to articles matching the term", :aggregate_failures do + article.update_columns(body_markdown: "Life of the party", cached_tag_list: "javascript, beginners, ruby") + + result = described_class.search_documents(user, term: "part", tags: %i[javascript]) + expect(extract_from_results(result, :path)).to include(article.path) + + result = described_class.search_documents(user, term: "part", tags: %i[python]) + expect(extract_from_results(result, :path)).to be_empty + + result = described_class.search_documents(user, term: "fiesta", tags: %i[javascript]) + expect(extract_from_results(result, :path)).to be_empty + end + end + + context "when searching for a term and filtering by statuses and tags" do + it "selects items with the requested status belonging to articles matching the term", :aggregate_failures do + article.update_columns(body_markdown: "Life of the party", cached_tag_list: "javascript, beginners, ruby") + + result = described_class.search_documents(user, term: "part", statuses: %i[valid], tags: %i[javascript]) + expect(extract_from_results(result, :path)).to include(article.path) + + result = described_class.search_documents(user, term: "part", statuses: %i[archived], tags: %i[javascript]) + expect(extract_from_results(result, :path)).to be_empty + + result = described_class.search_documents(user, term: "part", statuses: %i[valid], tags: %i[python]) + expect(extract_from_results(result, :path)).to be_empty + end + end + + context "when paginating" do + let(:articles) { create_list(:article, 2) } + + before do + articles.each { |article| create(:reaction, category: :readinglist, reactable: article, user: user) } + end + + it "returns the total count of the articles pre-pagination" do + result = described_class.search_documents(user, page: 1, per_page: 1) + expect(result[:total]).to eq(user.reactions.readinglist.count) + end + + it "returns no items when out of pagination bounds" do + result = described_class.search_documents(user, page: 99) + expect(result[:items]).to be_empty + end + + it "returns paginated items", :aggregate_failures do + result = described_class.search_documents(user, page: 1, per_page: 1) + expect(result[:items].length).to eq(1) + + result = described_class.search_documents(user, page: 2, per_page: 1) + expect(result[:items].length).to eq(1) + end + end + end +end