From 7ff79f77b861f230c4c62bb785e6527dd22bba4f Mon Sep 17 00:00:00 2001 From: Molly Struve Date: Mon, 17 Feb 2020 08:16:45 -0500 Subject: [PATCH] Search Tags in Elasticsearch from Classified Listing and Post Creation (#6024) [deploy] --- app/controllers/search_controller.rb | 9 ++++ .../__snapshots__/tags.test.jsx.snap | 48 +----------------- .../elements/__tests__/tags.test.jsx | 17 ++++++- app/javascript/shared/components/tags.jsx | 30 +++++------- app/services/search/tag.rb | 27 ++++++++++ config/routes.rb | 1 + spec/factories/tags.rb | 4 ++ spec/requests/search_spec.rb | 19 +++++++ spec/services/search/tag_spec.rb | 49 +++++++++++++++++++ 9 files changed, 139 insertions(+), 65 deletions(-) create mode 100644 app/controllers/search_controller.rb create mode 100644 spec/requests/search_spec.rb diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb new file mode 100644 index 000000000..1091e09f1 --- /dev/null +++ b/app/controllers/search_controller.rb @@ -0,0 +1,9 @@ +class SearchController < ApplicationController + before_action :authenticate_user! + + def tags + tag_docs = Search::Tag.search_documents("name:#{params[:name]}* AND supported:true") + + render json: { result: tag_docs } + end +end diff --git a/app/javascript/article-form/elements/__tests__/__snapshots__/tags.test.jsx.snap b/app/javascript/article-form/elements/__tests__/__snapshots__/tags.test.jsx.snap index 30127b183..a248caba8 100644 --- a/app/javascript/article-form/elements/__tests__/__snapshots__/tags.test.jsx.snap +++ b/app/javascript/article-form/elements/__tests__/__snapshots__/tags.test.jsx.snap @@ -60,32 +60,10 @@ Object { "prevLen": 0, "searchResults": Array [ Object { - "_highlightResult": Object { - "bg_color_hex": Object { - "matchLevel": "none", - "matchedWords": Array [], - "value": "#888751", - }, - "name": Object { - "fullyHighlighted": false, - "matchLevel": "full", - "matchedWords": Array [ - "gi", - ], - "value": "git", - }, - "text_color_hex": Object { - "matchLevel": "none", - "matchedWords": Array [], - "value": "#56c938", - }, - }, - "bg_color_hex": "#888751", "hotness_score": 0, "name": "git", - "objectID": "4", + "short_summary": null, "supported": true, - "text_color_hex": "#56c938", }, ], "selectedIndex": 0, @@ -100,32 +78,10 @@ Object { "prevLen": 0, "searchResults": Array [ Object { - "_highlightResult": Object { - "bg_color_hex": Object { - "matchLevel": "none", - "matchedWords": Array [], - "value": "#888751", - }, - "name": Object { - "fullyHighlighted": false, - "matchLevel": "full", - "matchedWords": Array [ - "gi", - ], - "value": "git", - }, - "text_color_hex": Object { - "matchLevel": "none", - "matchedWords": Array [], - "value": "#56c938", - }, - }, - "bg_color_hex": "#888751", "hotness_score": 0, "name": "git", - "objectID": "4", + "short_summary": null, "supported": true, - "text_color_hex": "#56c938", }, ], "selectedIndex": 0, diff --git a/app/javascript/article-form/elements/__tests__/tags.test.jsx b/app/javascript/article-form/elements/__tests__/tags.test.jsx index 3e8de74bd..9a0660dd6 100644 --- a/app/javascript/article-form/elements/__tests__/tags.test.jsx +++ b/app/javascript/article-form/elements/__tests__/tags.test.jsx @@ -2,15 +2,28 @@ import { h, render as preactRender } from 'preact'; import render from 'preact-render-to-json'; import { shallow } from 'preact-render-spy'; import { JSDOM } from 'jsdom'; +import fetch from 'jest-fetch-mock'; import Tags from '../../../shared/components/tags'; -import algoliasearch from '../__mocks__/algoliasearch'; + +global.fetch = fetch; + +const sampleResponse = JSON.stringify({ + result: [ + { + name: 'git', + hotness_score: 0, + supported: true, + short_summary: null, + }, + ], +}); describe('', () => { beforeEach(() => { const doc = new JSDOM(''); global.document = doc; global.window = doc.defaultView; - global.window.algoliasearch = algoliasearch; + fetch.mockResponse(sampleResponse); }); it('renders properly', () => { diff --git a/app/javascript/shared/components/tags.jsx b/app/javascript/shared/components/tags.jsx index b1496926e..f5470cc24 100644 --- a/app/javascript/shared/components/tags.jsx +++ b/app/javascript/shared/components/tags.jsx @@ -36,14 +36,6 @@ class Tags extends Component { prevLen: 0, showingRulesForTag: null, }; - - const algoliaId = document.querySelector("meta[name='algolia-public-id']") - .content; - const algoliaKey = document.querySelector("meta[name='algolia-public-key']") - .content; - const env = document.querySelector("meta[name='environment']").content; - const client = algoliasearch(algoliaId, algoliaKey); - this.index = client.initIndex(`Tag_${env}`); } componentDidMount() { @@ -321,20 +313,24 @@ class Tags extends Component { }); } const { listing } = this.props; - return this.index - .search(query, { - hitsPerPage: 8, - attributesToHighlight: [], - filters: 'supported:true', - }) - .then(content => { + return fetch(`/search/tags?name=${query}`, { + method: 'GET', + headers: { + Accept: 'application/json', + 'X-CSRF-Token': window.csrfToken, + 'Content-Type': 'application/json', + }, + credentials: 'same-origin', + }) + .then(response => response.json()) + .then(response => { if (listing === true) { const { additionalTags } = this.state; const { category } = this.props; const additionalItems = (additionalTags[category] || []).filter(t => t.includes(query), ); - const resultsArray = content.hits; + const resultsArray = response.result; additionalItems.forEach(t => { if (!resultsArray.includes(t)) { resultsArray.push({ name: t }); @@ -344,7 +340,7 @@ class Tags extends Component { // updates searchResults array according to what is being typed by user // allows user to choose a tag when they've typed the partial or whole word this.setState({ - searchResults: content.hits, + searchResults: response.result, }); }); } diff --git a/app/services/search/tag.rb b/app/services/search/tag.rb index 455c5a050..a8239fe6c 100644 --- a/app/services/search/tag.rb +++ b/app/services/search/tag.rb @@ -17,6 +17,29 @@ module Search SearchClient.get(id: tag_id, index: INDEX_ALIAS) end + def search(query_string) + SearchClient.search( + index: INDEX_ALIAS, + body: { + query: { + query_string: { + query: query_string, + analyze_wildcard: true, + allow_leading_wildcard: false + } + }, + sort: { + hotness_score: "desc" + } + }, + ) + end + + def search_documents(query_string) + results = search(query_string) + results.dig("hits", "hits").map { |tag_doc| tag_doc.dig("_source") } + end + def create_index(index_name: INDEX_NAME) SearchClient.indices.create(index: index_name, body: settings) end @@ -25,6 +48,10 @@ module Search SearchClient.indices.delete(index: index_name) end + def refresh_index(index_name: INDEX_ALIAS) + SearchClient.indices.refresh(index: index_name) + end + def add_alias(index_name: INDEX_NAME, index_alias: INDEX_ALIAS) SearchClient.indices.put_alias(index: index_name, name: index_alias) end diff --git a/config/routes.rb b/config/routes.rb index 2ab6b9006..f22113b8e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -209,6 +209,7 @@ Rails.application.routes.draw do resources :podcasts, only: %i[new create] resolve("ProMembership") { [:pro_membership] } # see https://guides.rubyonrails.org/routing.html#using-resolve + get "/search/tags" => "search#tags" get "/chat_channel_memberships/find_by_chat_channel_id" => "chat_channel_memberships#find_by_chat_channel_id" get "/listings/dashboard" => "classified_listings#dashboard" get "/listings/:category" => "classified_listings#index" diff --git a/spec/factories/tags.rb b/spec/factories/tags.rb index 9e82608b8..5c395d5f2 100644 --- a/spec/factories/tags.rb +++ b/spec/factories/tags.rb @@ -4,5 +4,9 @@ FactoryBot.define do factory :tag do name { generate :name } supported { true } + + trait :search_indexed do + after(:create, &:index_to_elasticsearch_inline) + end end end diff --git a/spec/requests/search_spec.rb b/spec/requests/search_spec.rb new file mode 100644 index 000000000..a507a1cff --- /dev/null +++ b/spec/requests/search_spec.rb @@ -0,0 +1,19 @@ +require "rails_helper" + +RSpec.describe "Search", type: :request, proper_status: true do + describe "GET /search/tags" do + let(:authorized_user) { create(:user) } + let(:mock_documents) do + [{ "name" => "tag1" }, { "name" => "tag2" }, { "name" => "tag3" }] + end + + it "returns json" do + sign_in authorized_user + allow(Search::Tag).to receive(:search_documents).and_return( + mock_documents, + ) + get "/search/tags" + expect(response.parsed_body).to eq("result" => mock_documents) + end + end +end diff --git a/spec/services/search/tag_spec.rb b/spec/services/search/tag_spec.rb index 9a492bc51..46194ee51 100644 --- a/spec/services/search/tag_spec.rb +++ b/spec/services/search/tag_spec.rb @@ -10,6 +10,55 @@ RSpec.describe Search::Tag, type: :service, elasticsearch: true do end end + describe "::search" do + it "searches with a given query string" do + tag1 = FactoryBot.create(:tag, :search_indexed, name: "tag1") + described_class.refresh_index + hits = described_class.search("name:tag1").dig("hits", "hits") + tag_names = hits.map { |t| t.dig("_source", "name") } + expect(tag_names.count).to eq(1) + expect(tag_names).to include(tag1.name) + end + + it "analyzes wildcards" do + tag1 = FactoryBot.create(:tag, :search_indexed, name: "tag1") + tag2 = FactoryBot.create(:tag, :search_indexed, name: "tag2") + tag3 = FactoryBot.create(:tag, :search_indexed, name: "3tag") + described_class.refresh_index + hits = described_class.search("name:tag*").dig("hits", "hits") + tag_names = hits.map { |t| t.dig("_source", "name") } + expect(tag_names).to include(tag1.name, tag2.name) + expect(tag_names).not_to include(tag3.name) + end + + it "does not allow leading wildcards" do + bad_request_error = Elasticsearch::Transport::Transport::Errors::BadRequest + expect { described_class.search("name:*tag") }.to raise_error(bad_request_error) + end + end + + describe "::search_documents" do + let(:tag_doc_1) { { "name" => "tag1" } } + let(:tag_doc_2) { { "name" => "tag2" } } + let(:mock_search_response) do + { + "hits" => { + "hits" => [ + { "_source" => tag_doc_1 }, + { "_source" => tag_doc_2 }, + ] + } + } + end + + it "parses tag document hits from search response" do + allow(SearchClient).to receive(:search) { mock_search_response } + tag_docs = described_class.search_documents("query") + expect(tag_docs.count).to eq(2) + expect(tag_docs).to include(tag_doc_1, tag_doc_2) + end + end + describe "::find_document" do it "fetches a document for a given ID from elasticsearch" do tag = FactoryBot.create(:tag)