From c64ca9750c02cd76db6da0baeb1f6926535eeb5e Mon Sep 17 00:00:00 2001 From: Molly Struve Date: Thu, 27 Feb 2020 08:12:36 -0500 Subject: [PATCH] Default search by active chat channel memberships only (#6325) [deploy] --- app/javascript/chat/actions.js | 1 - .../query_builders/chat_channel_membership.rb | 5 +++++ .../search/chat_channel_membership_spec.rb | 10 ++++----- .../chat_channel_membership_spec.rb | 22 ++++++++++++++----- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/app/javascript/chat/actions.js b/app/javascript/chat/actions.js index 95eb2fa8a..c48d6c03f 100644 --- a/app/javascript/chat/actions.js +++ b/app/javascript/chat/actions.js @@ -110,7 +110,6 @@ export function getChannels( dataHash[key] = value; } dataHash.per_page = 30; - dataHash.status = 'active'; dataHash.page = paginationNumber; dataHash.channel_text = query; diff --git a/app/services/search/query_builders/chat_channel_membership.rb b/app/services/search/query_builders/chat_channel_membership.rb index a10beda48..995fcbef1 100644 --- a/app/services/search/query_builders/chat_channel_membership.rb +++ b/app/services/search/query_builders/chat_channel_membership.rb @@ -23,6 +23,11 @@ module Search def initialize(params, user_id) @params = params.deep_symbolize_keys @params[:viewable_by] = user_id + + # TODO: @mstruve: When we want to allow people like admins to + # search ALL memberships this will need to change + @params[:status] = "active" + build_body end diff --git a/spec/services/search/chat_channel_membership_spec.rb b/spec/services/search/chat_channel_membership_spec.rb index e160847d9..4032bfb7c 100644 --- a/spec/services/search/chat_channel_membership_spec.rb +++ b/spec/services/search/chat_channel_membership_spec.rb @@ -159,11 +159,11 @@ RSpec.describe Search::ChatChannelMembership, type: :service, elasticsearch: tru expect(chat_channel_membership_docs.first["id"]).to eq(chat_channel_membership2.id) end - it "searches by status" do + it "only returns active status memberships" do chat_channel_membership1.update(status: "inactive") chat_channel_membership2.update(status: "active") index_documents([chat_channel_membership1, chat_channel_membership2]) - params = { size: 5, status: "active" } + params = { size: 5 } chat_channel_membership_docs = described_class.search_documents(params: params, user_id: user.id) expect(chat_channel_membership_docs.count).to eq(1) @@ -188,10 +188,10 @@ RSpec.describe Search::ChatChannelMembership, type: :service, elasticsearch: tru end it "sorts documents for given field" do - chat_channel_membership1.update(status: "inactive") - chat_channel_membership2.update(status: "active") + allow(chat_channel_membership1).to receive(:channel_type).and_return("not_direct") + allow(chat_channel_membership2).to receive(:channel_type).and_return("direct") index_documents([chat_channel_membership1, chat_channel_membership2]) - params = { size: 5, sort_by: "status", sort_direction: "asc" } + params = { size: 5, sort_by: "channel_type", sort_direction: "asc" } chat_channel_membership_docs = described_class.search_documents(params: params, user_id: user.id) expect(chat_channel_membership_docs.count).to eq(2) 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 670c640bd..f76c2ecde 100644 --- a/spec/services/search/query_builders/chat_channel_membership_spec.rb +++ b/spec/services/search/query_builders/chat_channel_membership_spec.rb @@ -16,12 +16,12 @@ RSpec.describe Search::QueryBuilders::ChatChannelMembership, type: :service do describe "#as_hash" do it "applies FILTER_KEYS from params" do - params = { channel_status: "active", channel_type: "direct", status: "open" } + params = { channel_status: "active", channel_type: "direct" } filter = described_class.new(params, 1) expected_filters = [ { "term" => { "channel_status" => "active" } }, { "term" => { "channel_type" => "direct" } }, - { "term" => { "status" => "open" } }, + { "term" => { "status" => "active" } }, { "term" => { "viewable_by" => 1 } }, ] expect(filter.as_hash.dig("query", "bool", "filter")).to match_array(expected_filters) @@ -46,16 +46,26 @@ RSpec.describe Search::QueryBuilders::ChatChannelMembership, type: :service do "query" => "a_name*", "fields" => [:channel_text], "lenient" => true, "analyze_wildcard" => true } }] - expected_filters = [{ "term" => { "channel_status" => "active" } }, { "term" => { "viewable_by" => 1 } }] + expected_filters = [{ "term" => { "channel_status" => "active" } }, { "term" => { "viewable_by" => 1 } }, { "term" => { "status" => "active" } }] 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 - params = { not_supported: "direct", status: "closed" } + it "always applies viewable_by and status params" do + params = {} filter = described_class.new(params, 1) expected_filters = [ - { "term" => { "status" => "closed" } }, + { "term" => { "status" => "active" } }, + { "term" => { "viewable_by" => 1 } }, + ] + expect(filter.as_hash.dig("query", "bool", "filter")).to match_array(expected_filters) + end + + it "ignores params we dont support" do + params = { not_supported: "direct" } + filter = described_class.new(params, 1) + expected_filters = [ + { "term" => { "status" => "active" } }, { "term" => { "viewable_by" => 1 } }, ] expect(filter.as_hash.dig("query", "bool", "filter")).to match_array(expected_filters)