From 8f0bfa5e08ab2962eb07b75ddf2af14f00b809d4 Mon Sep 17 00:00:00 2001 From: Molly Struve Date: Wed, 26 Feb 2020 11:06:38 -0500 Subject: [PATCH] Implement Search for Chat Channels with Elasticsearch (#6282) [deploy] --- app/controllers/search_controller.rb | 24 ++++++ app/javascript/chat/actions.js | 73 +++++++++++-------- app/models/chat_channel_membership.rb | 3 +- .../search/chat_channel_membership.rb | 20 ++++- .../query_builders/chat_channel_membership.rb | 21 ++++-- config/routes.rb | 1 + spec/models/chat_channel_membership_spec.rb | 9 +++ spec/requests/search_spec.rb | 16 ++++ .../search/chat_channel_membership_spec.rb | 24 ++++++ .../chat_channel_membership_spec.rb | 23 +++++- 10 files changed, 172 insertions(+), 42 deletions(-) diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 77a5a56c3..b13a1d782 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -8,4 +8,28 @@ class SearchController < ApplicationController rescue Search::Errors::Transport::BadRequest render json: { result: [] } end + + def chat_channels + ccm_docs = Search::ChatChannelMembership.search_documents( + params: chat_channel_params.to_h, user_id: current_user.id, + ) + + render json: { result: ccm_docs } + end + + private + + def chat_channel_params + accessible = %i[ + per_page + page + channel_text + channel_type + channel_status + status + ] + params[:page] = params[:page].to_i if params[:page].present? + params[:per_page] = params[:per_page].to_i if params[:per_page].present? + params.permit(accessible) + end end diff --git a/app/javascript/chat/actions.js b/app/javascript/chat/actions.js index e91482b66..c48d6c03f 100644 --- a/app/javascript/chat/actions.js +++ b/app/javascript/chat/actions.js @@ -104,38 +104,49 @@ export function getChannels( successCb, _failureCb, ) { - const client = algoliasearch(props.algoliaId, props.algoliaKey); - const index = client.initIndex(props.algoliaIndex); - const filters = { - ...{ - hitsPerPage: 30 + paginationNumber, - page: paginationNumber, + const dataHash = {}; + if (additionalFilters.filters) { + const [key, value] = additionalFilters.filters.split(':'); + dataHash[key] = value; + } + dataHash.per_page = 30; + dataHash.page = paginationNumber; + dataHash.channel_text = query; + + const searchParams = new URLSearchParams(dataHash).toString(); + return fetch(`/search/chat_channels?${searchParams}`, { + method: 'GET', + headers: { + Accept: 'application/json', + 'X-CSRF-Token': window.csrfToken, + 'Content-Type': 'application/json', }, - ...additionalFilters, - }; - index.search(query, filters).then(content => { - const channels = content.hits; - if ( - retrievalID === null || - content.hits.filter(e => e.chat_channel_id === retrievalID).length === 1 - ) { - successCb(channels, query); - } else { - fetch( - `/chat_channel_memberships/find_by_chat_channel_id?chat_channel_id=${retrievalID}`, - { - Accept: 'application/json', - 'Content-Type': 'application/json', - credentials: 'same-origin', - }, - ) - .then(response => response.json()) - .then(json => { - channels.unshift(json); - successCb(channels, query); - }); - } - }); + credentials: 'same-origin', + }) + .then(response => response.json()) + .then(response => { + const channels = response.result; + if ( + retrievalID === null || + channels.filter(e => e.chat_channel_id === retrievalID).length === 1 + ) { + successCb(channels, query); + } else { + fetch( + `/chat_channel_memberships/find_by_chat_channel_id?chat_channel_id=${retrievalID}`, + { + Accept: 'application/json', + 'Content-Type': 'application/json', + credentials: 'same-origin', + }, + ) + .then(individualResponse => individualResponse.json()) + .then(json => { + channels.unshift(json); + successCb(channels, query); + }); + } + }); } export function getUnopenedChannelIds(successCb) { diff --git a/app/models/chat_channel_membership.rb b/app/models/chat_channel_membership.rb index 67dadf2fe..b0c1bcbb3 100644 --- a/app/models/chat_channel_membership.rb +++ b/app/models/chat_channel_membership.rb @@ -37,7 +37,8 @@ class ChatChannelMembership < ApplicationRecord end def channel_text - "#{chat_channel.channel_name} #{chat_channel.slug} #{chat_channel.channel_human_names}" + parsed_channel_name = chat_channel.channel_name&.gsub("chat between", "")&.gsub("and", "") + "#{parsed_channel_name} #{chat_channel.slug} #{chat_channel.channel_human_names.join(' ')}" end def channel_name diff --git a/app/services/search/chat_channel_membership.rb b/app/services/search/chat_channel_membership.rb index 80a02d6db..a28cbbdf4 100644 --- a/app/services/search/chat_channel_membership.rb +++ b/app/services/search/chat_channel_membership.rb @@ -3,12 +3,17 @@ module Search INDEX_NAME = "chat_channel_memberships_#{Rails.env}".freeze INDEX_ALIAS = "chat_channel_memberships_#{Rails.env}_alias".freeze MAPPINGS = JSON.parse(File.read("config/elasticsearch/mappings/chat_channel_memberships.json"), symbolize_names: true).freeze + DEFAULT_PAGE = 0 + DEFAULT_PER_PAGE = 30 class << self def search_documents(params:, user_id:) + set_query_size(params) query_hash = Search::QueryBuilders::ChatChannelMembership.new(params, user_id).as_hash + results = search(body: query_hash) - results.dig("hits", "hits").map { |ccm_doc| ccm_doc.dig("_source") } + hits = results.dig("hits", "hits").map { |ccm_doc| ccm_doc.dig("_source") } + paginate_hits(hits, params) end private @@ -17,6 +22,19 @@ module Search SearchClient.search(index: INDEX_ALIAS, body: body) end + def set_query_size(params) + params[:page] ||= DEFAULT_PAGE + params[:per_page] ||= DEFAULT_PER_PAGE + + # pages start at 0 + params[:size] = params[:per_page].to_i * (params[:page].to_i + 1) + end + + def paginate_hits(hits, params) + start = (params[:per_page] + 1) * params[:page] + hits[start, params[:per_page]] + end + def index_settings if Rails.env.production? { diff --git a/app/services/search/query_builders/chat_channel_membership.rb b/app/services/search/query_builders/chat_channel_membership.rb index ece504eec..a10beda48 100644 --- a/app/services/search/query_builders/chat_channel_membership.rb +++ b/app/services/search/query_builders/chat_channel_membership.rb @@ -36,14 +36,13 @@ module Search @body = ActiveSupport::HashWithIndifferentAccess.new build_queries add_sort - # By default we will return 0 documents if size is not specified - @body[:size] = @params[:size] || DEFAULT_PARAMS[:size] + set_size end def build_queries @body[:query] = {} @body[:query][:bool] = { filter: filter_conditions } - @body[:query][:bool] = { must: query_conditions } if query_keys_present? + @body[:query][:bool][:must] = query_conditions if query_keys_present? end def add_sort @@ -54,9 +53,14 @@ module Search } end + def set_size + # By default we will return 0 documents if size is not specified + @body[:size] = @params[:size] || DEFAULT_PARAMS[:size] + end + def filter_conditions FILTER_KEYS.map do |filter_key| - next if @params[filter_key].blank? + next if @params[filter_key].blank? || @params[filter_key] == "all" { term: { filter_key => @params[filter_key] } } end.compact @@ -70,7 +74,14 @@ module Search QUERY_KEYS.map do |query_key| next if @params[query_key].blank? - { match: { query_key => { query: @params[query_key] } } } + { + simple_query_string: { + query: "#{@params[query_key]}*", + fields: [query_key], + lenient: true, + analyze_wildcard: true + } + } end.compact end end diff --git a/config/routes.rb b/config/routes.rb index 2e70bbade..51e130f39 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -216,6 +216,7 @@ Rails.application.routes.draw do resolve("ProMembership") { [:pro_membership] } # see https://guides.rubyonrails.org/routing.html#using-resolve get "/search/tags" => "search#tags" + get "/search/chat_channels" => "search#chat_channels" 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/models/chat_channel_membership_spec.rb b/spec/models/chat_channel_membership_spec.rb index 9044ab3ee..23979aa43 100644 --- a/spec/models/chat_channel_membership_spec.rb +++ b/spec/models/chat_channel_membership_spec.rb @@ -3,6 +3,15 @@ require "rails_helper" RSpec.describe ChatChannelMembership, type: :model do let(:chat_channel_membership) { FactoryBot.create(:chat_channel_membership) } + describe "#channel_text" do + it "sets channel text using name, slig, and human names" do + chat_channel = chat_channel_membership.chat_channel + parsed_channel_name = chat_channel_membership.channel_name&.gsub("chat between", "")&.gsub("and", "") + expected_text = "#{parsed_channel_name} #{chat_channel.slug} #{chat_channel.channel_human_names.join(' ')}" + expect(chat_channel_membership.channel_text).to eq(expected_text) + end + end + describe "#index_to_elasticsearch" do it "enqueues job to index tag to elasticsearch" do sidekiq_assert_enqueued_with(job: Search::IndexToElasticsearchWorker, args: [described_class.to_s, chat_channel_membership.id]) do diff --git a/spec/requests/search_spec.rb b/spec/requests/search_spec.rb index c1f70b878..5832590e2 100644 --- a/spec/requests/search_spec.rb +++ b/spec/requests/search_spec.rb @@ -23,4 +23,20 @@ RSpec.describe "Search", type: :request, proper_status: true do expect(response.parsed_body).to eq("result" => []) end end + + describe "GET /search/chat_channels" do + let(:authorized_user) { create(:user) } + let(:mock_documents) do + [{ "channel_name" => "channel1" }] + end + + it "returns json" do + sign_in authorized_user + allow(Search::ChatChannelMembership).to receive(:search_documents).and_return( + mock_documents, + ) + get "/search/chat_channels" + expect(response.parsed_body).to eq("result" => mock_documents) + end + end end diff --git a/spec/services/search/chat_channel_membership_spec.rb b/spec/services/search/chat_channel_membership_spec.rb index 3d63a3b2b..e160847d9 100644 --- a/spec/services/search/chat_channel_membership_spec.rb +++ b/spec/services/search/chat_channel_membership_spec.rb @@ -171,6 +171,22 @@ RSpec.describe Search::ChatChannelMembership, type: :service, elasticsearch: tru end end + context "with a query and filter" do + it "searches by channel_text and status" do + allow(chat_channel_membership1).to receive(:channel_text).and_return("a name") + allow(chat_channel_membership2).to receive(:channel_text).and_return("another name") + chat_channel_membership1.update(status: "active") + chat_channel_membership2.update(status: "inactive") + index_documents([chat_channel_membership1, chat_channel_membership2]) + name_params = { size: 5, channel_text: "name", status: "active" } + + chat_channel_membership_docs = described_class.search_documents(params: name_params, user_id: user.id) + expect(chat_channel_membership_docs.count).to eq(1) + doc_ids = chat_channel_membership_docs.map { |t| t.dig("id") } + expect(doc_ids).to include(chat_channel_membership1.id) + end + end + it "sorts documents for given field" do chat_channel_membership1.update(status: "inactive") chat_channel_membership2.update(status: "active") @@ -194,5 +210,13 @@ RSpec.describe Search::ChatChannelMembership, type: :service, elasticsearch: tru expect(chat_channel_membership_docs.first["id"]).to eq(chat_channel_membership1.id) expect(chat_channel_membership_docs.last["id"]).to eq(chat_channel_membership2.id) end + + it "will return a set number of docs based on pagination params" do + index_documents([chat_channel_membership1, chat_channel_membership2]) + params = { page: 0, per_page: 1 } + + chat_channel_membership_docs = described_class.search_documents(params: params, user_id: user.id) + expect(chat_channel_membership_docs.count).to eq(1) + end end end diff --git a/spec/services/search/query_builders/chat_channel_membership_spec.rb b/spec/services/search/query_builders/chat_channel_membership_spec.rb index c853bbebb..670c640bd 100644 --- a/spec/services/search/query_builders/chat_channel_membership_spec.rb +++ b/spec/services/search/query_builders/chat_channel_membership_spec.rb @@ -30,10 +30,25 @@ RSpec.describe Search::QueryBuilders::ChatChannelMembership, type: :service do it "applies QUERY_KEYS from params" do params = { channel_text: "a_name" } query = described_class.new(params, 1) - expected_musts = [ - { "match" => { "channel_text" => { "query" => "a_name" } } }, - ] - expect(query.as_hash.dig("query", "bool", "must")).to match_array(expected_musts) + expected_query = [{ + "simple_query_string" => { + "query" => "a_name*", "fields" => [:channel_text], "lenient" => true, "analyze_wildcard" => true + } + }] + expect(query.as_hash.dig("query", "bool", "must")).to match_array(expected_query) + end + + it "applies QUERY_KEYS and FILTER_KEYS from params" do + params = { channel_text: "a_name", channel_status: "active" } + query = described_class.new(params, 1) + expected_query = [{ + "simple_query_string" => { + "query" => "a_name*", "fields" => [:channel_text], "lenient" => true, "analyze_wildcard" => true + } + }] + expected_filters = [{ "term" => { "channel_status" => "active" } }, { "term" => { "viewable_by" => 1 } }] + expect(query.as_hash.dig("query", "bool", "must")).to match_array(expected_query) + expect(query.as_hash.dig("query", "bool", "filter")).to match_array(expected_filters) end it "ignores params we dont support" do