[deploy] Feature: Serve Reading List from FeedContent Index (#10294)

* Feature:Serve Reading List from FeedContent Index
* Fix paging, order ID desc, improve comments

app/controllers/search_controller.rb

@@ -103,8 +103,8 @@ class SearchController < ApplicationController
   end
 
   def reactions
-    result = Search::Reaction.search_documents(
-      params: reaction_params.merge(user_id: current_user.id).to_h,
+    result = Search::ReadingList.search_documents(
+      params: reaction_params.to_h, user: current_user,
     )
 
     render json: { result: result["reactions"], total: result["total"] }

app/javascript/readingList/__tests__/readingList.test.jsx

@@ -18,7 +18,7 @@ describe('<ReadingList />', () => {
             body_text: 'Some body text',
             class_name: 'Article',
             path: '/bobbytables/what-s-in-your-database-2d3f',
-            published_date_string: 'Jun 22',
+            readable_publish_date_string: 'Jun 22',
             reading_time: 0,
             tags: [
               {

app/javascript/readingList/components/ItemListItem.jsx

@@ -7,7 +7,7 @@ export const ItemListItem = ({ item, children }) => {
     path: item.reactable.path,
     title: item.reactable.title,
     user: item.reactable.user,
-    publishedDate: item.reactable.published_date_string,
+    publishedDate: item.reactable.readable_publish_date_string,
     readingTime: item.reactable.reading_time,
     tags: item.reactable.tags,
   };
@@ -59,7 +59,7 @@ const readingListItemPropTypes = PropTypes.shape({
     path: PropTypes.string.isRequired,
     title: PropTypes.string.isRequired,
     reading_time: PropTypes.number.isRequired,
-    published_date_string: PropTypes.string.isRequired,
+    readable_publish_date_string: PropTypes.string.isRequired,
     user: PropTypes.shape({
       username: PropTypes.string.isRequired,
       profile_image_90: PropTypes.string.isRequired,

app/javascript/readingList/components/__tests__/ItemListItem.test.jsx

@@ -9,7 +9,7 @@ function getItem() {
     reactable: {
       path: '/article',
       title: 'Title',
-      published_date_string: 'Jun 29',
+      readable_publish_date_string: 'Jun 29',
       reading_time: 1,
       user: {
         username: 'bob',

app/javascript/searchableItemList/searchableItemList.js

@@ -139,6 +139,7 @@ export function loadNextPage() {
   const { query, selectedTags, page, statusView } = component.state;
   component.setState({ page: page + 1 });
   component.search(query, {
+    page: page + 1,
     tags: selectedTags,
     statusView,
     appendItems: true,

app/services/search/query_builders/feed_content.rb

@@ -138,8 +138,14 @@ module Search
         TERM_KEYS.filter_map do |term_key, search_key|
           next unless @params.key? term_key
 
-          { terms: { search_key => Array.wrap(@params[term_key]) } }
-        end
+          values = Array.wrap(@params[term_key])
+
+          if params[:tag_boolean_mode] == "all" && term_key == :tag_names
+            values.map { |val| { term: { search_key => val } } }
+          else
+            { terms: { search_key => values } }
+          end
+        end.compact
       end
 
       def range_keys

app/services/search/reading_list.rb

@@ -0,0 +1,78 @@
+module Search
+  # This class does not inherit from Search::Base like our other search classes bc it
+  # is using the FeedContent index to search for ReadingList reaction articles rather than its
+  # own separate index. The primary function of this class is to help combine and parse reaction
+  # data with article Elasticsearch data to populate the Reading List view.
+  class ReadingList
+    DEFAULT_PAGE = 0
+    DEFAULT_PER_PAGE = 60
+    DEFAULT_STATUS = %w[valid confirmed].freeze
+
+    # Using a class method here to follow the pattern of the other Search classes
+    def self.search_documents(params:, user:)
+      new(params: params, user: user).reading_list_reactions
+    end
+
+    def initialize(params:, user:)
+      self.status = params.delete(:status) || DEFAULT_STATUS
+      self.view_page = params.delete(:page) || DEFAULT_PAGE
+      self.view_per_page = params.delete(:per_page) || DEFAULT_PER_PAGE
+      self.user = user
+      self.search_params = params
+    end
+
+    def reading_list_reactions
+      ordered_articles = parse_and_order_articles(article_docs)
+      { "reactions" => paginate_articles(ordered_articles), "total" => total }
+    end
+
+    private
+
+    attr_accessor :search_params, :user, :status, :view_page, :view_per_page, :total
+
+    def paginate_articles(ordered_articles)
+      start = view_per_page * view_page
+      ordered_articles[start, view_per_page] || []
+    end
+
+    def article_docs
+      return @article_docs if @article_docs
+
+      # Gather articles from Elasticsearch based on search criteria containing
+      # tags, text search, status, and the list of IDs of all articles in a user's
+      # reading list
+      docs = FeedContent.search_documents(
+        params: search_params.merge(
+          id: search_ids,
+          class_name: "Article",
+          page: 0,
+          per_page: reading_list_article_ids.count,
+        ),
+      )
+      self.total = docs.count
+      @article_docs = docs.index_by { |doc| doc["id"] }
+    end
+
+    def reading_list_article_ids
+      # Collect all reading list IDs and article IDs for a user
+      @reading_list_article_ids ||= user.reactions.readinglist.where(status: status).order(id: :desc).pluck(
+        :reactable_id, :id
+      ).to_h
+    end
+
+    def search_ids
+      reading_list_article_ids.keys.map { |id| "article_#{id}" }
+    end
+
+    def parse_and_order_articles(articles)
+      # Combines reaction and article data to create hashes that contain the fields
+      # the reading list view needs. Ensures articles are returned in order of reaction ID
+      reading_list_article_ids.map do |article_id, reaction_id|
+        found_article_doc = articles[article_id]
+        next unless found_article_doc
+
+        { "id" => reaction_id, "user_id" => user.id, "reactable" => articles[article_id] }
+      end.compact
+    end
+  end
+end

spec/requests/search_spec.rb

@@ -133,7 +133,7 @@ RSpec.describe "Search", type: :request, proper_status: true do
 
     it "returns json with reactions and total" do
       sign_in authorized_user
-      allow(Search::Reaction).to receive(:search_documents).and_return(
+      allow(Search::ReadingList).to receive(:search_documents).and_return(
         mock_response,
       )
       get "/search/reactions"
@@ -142,13 +142,13 @@ RSpec.describe "Search", type: :request, proper_status: true do
 
     it "accepts array of tag names" do
       sign_in authorized_user
-      allow(Search::Reaction).to receive(:search_documents).and_return(
+      allow(Search::ReadingList).to receive(:search_documents).and_return(
         mock_response,
       )
       get "/search/reactions?tag_names[]=1&tag_names[]=2"
-      expect(Search::Reaction).to have_received(
+      expect(Search::ReadingList).to have_received(
         :search_documents,
-      ).with(params: { "tag_names" => %w[1 2], "user_id" => authorized_user.id })
+      ).with(params: { "tag_names" => %w[1 2] }, user: authorized_user)
     end
   end
 end

spec/services/search/feed_content_spec.rb

@@ -116,6 +116,24 @@ RSpec.describe Search::FeedContent, type: :service do
         expect(doc_ids).to include(article1.id)
       end
 
+      it "filters by multiple tag names when tag_boolean_mode is set to all" do
+        tag_one = create(:tag)
+        tag_two = create(:tag)
+        article1.tags << tag_one
+        article2.tags << tag_two
+        article2.tags << tag_one
+        index_documents([article1, article2])
+        query_params = {
+          tag_names: [tag_one.name, tag_two.name],
+          tag_boolean_mode: "all"
+        }
+
+        feed_docs = described_class.search_documents(params: query_params)
+        expect(feed_docs.count).to eq(1)
+        doc_ids = feed_docs.map { |t| t["id"] }
+        expect(doc_ids).to include(article2.id)
+      end
+
       it "filters by user_id" do
         index_documents([article1, article2])
         query_params = { size: 5, user_id: article1.user_id }

spec/services/search/reading_list_spec.rb

@@ -0,0 +1,115 @@
+require "rails_helper"
+
+RSpec.describe Search::ReadingList, type: :service do
+  describe "::search_documents", elasticsearch: "FeedContent" do
+    let(:user) { create(:user) }
+    let(:article0) { create(:article) }
+    let(:article1) { create(:article) }
+    let(:article2) { create(:article) }
+    let(:reaction1) { create(:reaction, user: user, category: "readinglist", reactable: article1) }
+    let(:reaction2) { create(:reaction, user: user, category: "readinglist", reactable: article2) }
+    let(:reaction3) { create(:reaction, user: user, category: "readinglist", reactable: article0) }
+    let(:query_params) { {} }
+
+    before do
+      reaction1
+      reaction2
+    end
+
+    def index_documents(docs)
+      index_documents_for_search_class(Array.wrap(docs), Search::FeedContent)
+    end
+
+    it "returns fields necessary for the view" do
+      view_keys = %w[id reactable user_id]
+      reactable_keys = %w[title path readable_publish_date_string tags user]
+      user_keys = %w[username name profile_image_90]
+      index_documents(article1)
+
+      result = described_class.search_documents(params: {}, user: user)
+      reaction_doc = result["reactions"].first
+      expect(result["total"]).to be_present
+      expect(reaction_doc.keys).to include(*view_keys)
+      expect(reaction_doc["reactable"].keys).to include(*reactable_keys)
+      expect(reaction_doc.dig("reactable", "user").keys).to include(*user_keys)
+    end
+
+    context "with a query" do
+      it "searches by search_fields" do
+        allow(article1).to receive(:title).and_return("ruby")
+        allow(article2).to receive(:body_text).and_return("Ruby Tuesday")
+        index_documents([article1, article2])
+        query_params[:search_fields] = "ruby"
+
+        reaction_docs = described_class.search_documents(params: query_params, user: user)["reactions"]
+        expect(reaction_docs.count).to eq(2)
+        doc_ids = reaction_docs.map { |t| t["id"] }
+        expect(doc_ids).to include(reaction1.id, reaction2.id)
+      end
+    end
+
+    context "with a filter term" do
+      let(:tag_one) { create(:tag) }
+      let(:tag_two) { create(:tag) }
+
+      it "filters by tag names" do
+        article1.tags << tag_one
+        article2.tags << tag_two
+        index_documents([article1, article2])
+        query_params[:tag_names] = [tag_one.name]
+
+        reaction_docs = described_class.search_documents(params: query_params, user: user)["reactions"]
+        expect(reaction_docs.count).to eq(1)
+        doc_ids = reaction_docs.map { |t| t["id"] }
+        expect(doc_ids).to include(reaction1.id)
+        expect(doc_ids).not_to include(reaction2.id)
+      end
+
+      it "filters by multiple tag names when tag_boolean_mode is set to all" do
+        article1.tags << tag_one
+        article2.tags << tag_two
+        article2.tags << tag_one
+        index_documents([article1, article2])
+        query_params[:tag_names] = [tag_one.name, tag_two.name]
+        query_params[:tag_boolean_mode] = "all"
+
+        reaction_docs = described_class.search_documents(params: query_params, user: user)["reactions"]
+        expect(reaction_docs.count).to eq(1)
+        doc_ids = reaction_docs.map { |t| t["id"] }
+        expect(doc_ids).to include(reaction2.id)
+        expect(doc_ids).not_to include(reaction1.id)
+      end
+
+      it "filters by status" do
+        reaction1.update(status: "invalid")
+        index_documents([article1, article2])
+        query_params[:status] = %w[valid confirmed]
+
+        reaction_docs = described_class.search_documents(params: query_params, user: user)["reactions"]
+        expect(reaction_docs.count).to eq(1)
+        doc_ids = reaction_docs.map { |t| t["id"] }
+        expect(doc_ids).to include(reaction2.id)
+      end
+
+      it "only returns readinglist reactions" do
+        reaction1.update(category: "like")
+        index_documents([article1, article2])
+
+        reaction_docs = described_class.search_documents(params: query_params, user: user)["reactions"]
+        expect(reaction_docs.count).to eq(1)
+        doc_ids = reaction_docs.map { |t| t["id"] }
+        expect(doc_ids).to include(reaction2.id)
+        expect(doc_ids).not_to include(reaction1.id)
+      end
+    end
+
+    it "sorts by reaction ID DESC" do
+      reaction3
+      index_documents([article0, article1, article2])
+
+      reaction_docs = described_class.search_documents(params: query_params, user: user)["reactions"]
+      doc_ids = reaction_docs.map { |t| t["id"] }
+      expect(doc_ids).to eq([reaction3.id, reaction2.id, reaction1.id])
+    end
+  end
+end