From 3891c7d46871bb9560309851e88db4bfa452c75e Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 6 Apr 2021 13:03:31 -0400 Subject: [PATCH] =?UTF-8?q?=E2=9C=82=EF=B8=8F=20=20Remove=20search=20from?= =?UTF-8?q?=20Connect=20(#13235)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove search from connect * Remove alternate approach * Alphabetize CHAT_CHANNEL_PARAMS * Remove default value for user_ids * Add spec for result ordering --- app/controllers/search_controller.rb | 41 +++++++------ app/javascript/chat/chat.jsx | 7 --- app/javascript/chat/util.js | 1 - .../chat_channel_membership_serializer.rb | 5 +- .../postgres/chat_channel_membership.rb | 61 +++++++++++++++++++ spec/requests/search_spec.rb | 2 +- .../postgres/chat_channel_membership_spec.rb | 48 +++++++++++++++ 7 files changed, 135 insertions(+), 30 deletions(-) create mode 100644 app/services/search/postgres/chat_channel_membership.rb create mode 100644 spec/services/search/postgres/chat_channel_membership_spec.rb diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index d9b742e24..6cb765b7d 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -3,6 +3,15 @@ class SearchController < ApplicationController before_action :format_integer_params before_action :sanitize_params, only: %i[listings reactions feed_content] + CHAT_CHANNEL_PARAMS = %i[ + channel_status + channel_type + page + per_page + status + user_id + ].freeze + LISTINGS_PARAMS = [ :category, :listing_search, @@ -62,16 +71,20 @@ class SearchController < ApplicationController end def chat_channels - search_user_id = if chat_channel_params[:user_id].present? - [current_user.id, SiteConfig.mascot_user_id, chat_channel_params[:user_id]].reject(&:blank?) - else - [current_user.id] - end - ccm_docs = Search::ChatChannelMembership.search_documents( - params: chat_channel_params.merge(user_id: search_user_id).to_h, + user_ids = + if chat_channel_params[:user_id].present? + [current_user.id, SiteConfig.mascot_user_id, chat_channel_params[:user_id]].reject(&:blank?) + else + [current_user.id] + end + + result = Search::Postgres::ChatChannelMembership.search_documents( + user_ids: user_ids, + page: chat_channel_params[:page], + per_page: chat_channel_params[:per_page], ) - render json: { result: ccm_docs } + render json: { result: result } end def listings @@ -160,17 +173,7 @@ class SearchController < ApplicationController end def chat_channel_params - accessible = %i[ - per_page - page - channel_text - channel_type - channel_status - status - user_id - ] - - params.permit(accessible) + params.permit(CHAT_CHANNEL_PARAMS) end def listing_params diff --git a/app/javascript/chat/chat.jsx b/app/javascript/chat/chat.jsx index ebbc4cb29..57e3b0df6 100644 --- a/app/javascript/chat/chat.jsx +++ b/app/javascript/chat/chat.jsx @@ -1271,13 +1271,6 @@ export class Chat extends Component {
{notificationsButton} - {invitesButton} {joiningRequestButton}
diff --git a/app/javascript/chat/util.js b/app/javascript/chat/util.js index b759d6515..f0991fff1 100644 --- a/app/javascript/chat/util.js +++ b/app/javascript/chat/util.js @@ -118,7 +118,6 @@ export const createDataHash = (additionalFilters, searchParams) => { } dataHash.per_page = 30; dataHash.page = searchParams.paginationNumber; - dataHash.channel_text = searchParams.query; if (searchParams.searchType === 'discoverable') { dataHash.user_id = 'all'; } diff --git a/app/serializers/search/chat_channel_membership_serializer.rb b/app/serializers/search/chat_channel_membership_serializer.rb index 9cd13db84..41b7f2c80 100644 --- a/app/serializers/search/chat_channel_membership_serializer.rb +++ b/app/serializers/search/chat_channel_membership_serializer.rb @@ -2,7 +2,8 @@ module Search class ChatChannelMembershipSerializer < ApplicationSerializer attributes :id, :status, :viewable_by, :chat_channel_id, :last_opened_at, :channel_text, :channel_last_message_at, :channel_status, - :channel_status, :channel_type, :channel_username, :channel_name, - :channel_image, :channel_modified_slug, :channel_discoverable, :channel_messages_count + :channel_type, :channel_username, :channel_name, :channel_image, + :channel_modified_slug, :channel_discoverable, + :channel_messages_count end end diff --git a/app/services/search/postgres/chat_channel_membership.rb b/app/services/search/postgres/chat_channel_membership.rb new file mode 100644 index 000000000..01b686d49 --- /dev/null +++ b/app/services/search/postgres/chat_channel_membership.rb @@ -0,0 +1,61 @@ +module Search + module Postgres + class ChatChannelMembership + ATTRIBUTES = %w[ + chat_channel_memberships.id + chat_channel_memberships.chat_channel_id + chat_channel_memberships.last_opened_at + chat_channel_memberships.status + chat_channel_memberships.user_id + chat_channels.channel_name + chat_channels.discoverable + chat_channels.last_message_at + chat_channels.slug + chat_channels.status + users.name + users.profile_image + users.username + ].freeze + private_constant :ATTRIBUTES + + # TODO: @mstruve: When we want to allow people like admins to search ALL + # memberships this will need to change + PERMITTED_STATUSES = %w[ + active + joining_request + ].freeze + + DEFAULT_PER_PAGE = 30 + private_constant :DEFAULT_PER_PAGE + + MAX_PER_PAGE = 60 # to avoid querying too many items, we set a maximum amount for a page + private_constant :MAX_PER_PAGE + + def self.search_documents(user_ids:, page: 0, per_page: DEFAULT_PER_PAGE) + # NOTE: [@rhymes/atsmith813] 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 + + relation = ::ChatChannelMembership + .includes(:user, chat_channel: :messages) + .where("chat_channel_memberships.status": PERMITTED_STATUSES) + .where("chat_channel_memberships.user_id": user_ids) + .select(*ATTRIBUTES) + .order("chat_channels.last_message_at desc") + + results = relation.page(page).per(per_page) + + serialize(results) + end + + def self.serialize(results) + Search::ChatChannelMembershipSerializer + .new(results, is_collection: true) + .serializable_hash[:data] + .pluck(:attributes) + end + private_class_method :serialize + end + end +end diff --git a/spec/requests/search_spec.rb b/spec/requests/search_spec.rb index c829947ad..c9b91dd5f 100644 --- a/spec/requests/search_spec.rb +++ b/spec/requests/search_spec.rb @@ -61,7 +61,7 @@ RSpec.describe "Search", type: :request, proper_status: true do it "returns json" do sign_in authorized_user - allow(Search::ChatChannelMembership).to receive(:search_documents).and_return( + allow(Search::Postgres::ChatChannelMembership).to receive(:search_documents).and_return( mock_documents, ) get "/search/chat_channels" diff --git a/spec/services/search/postgres/chat_channel_membership_spec.rb b/spec/services/search/postgres/chat_channel_membership_spec.rb new file mode 100644 index 000000000..efde58254 --- /dev/null +++ b/spec/services/search/postgres/chat_channel_membership_spec.rb @@ -0,0 +1,48 @@ +require "rails_helper" + +RSpec.describe Search::Postgres::ChatChannelMembership, type: :service do + let(:user) { create(:user) } + + describe "::search_documents" do + it "does not include chat channel memberships that are not included in permitted statuses", :aggregate_failures do + ccm = create(:chat_channel_membership, user: user) + rejected_ccm = create(:chat_channel_membership, user: user, status: "rejected") + result = described_class.search_documents(user_ids: [user.id]) + # rubocop:disable Rails/PluckId + ids = result.pluck(:id) + # rubocop:enable Rails/PluckId + + expect(ids).not_to include(rejected_ccm.id) + expect(ids).to include(ccm.id) + end + + context "when describing the result format" do + before { create(:chat_channel_membership, status: "active", user: user) } + + it "returns the correct attributes for the result" do + result = described_class.search_documents(user_ids: [user.id]) + expected_keys = %i[ + id status viewable_by chat_channel_id last_opened_at channel_text channel_last_message_at + channel_status channel_type channel_username channel_name channel_image + channel_modified_slug channel_discoverable channel_messages_count + ] + + expect(result.first.keys).to match_array(expected_keys) + end + end + end + + it "orders the results by chat_channel.last_message at in descending order" do + cc_older = create(:chat_channel, last_message_at: 1.hour.ago) + cc_newer = create(:chat_channel, last_message_at: Time.zone.now) + ccm_older = create(:chat_channel_membership, user: user, chat_channel: cc_older) + ccm_newer = create(:chat_channel_membership, user: user, chat_channel: cc_newer) + result = described_class.search_documents(user_ids: [user.id]) + + # rubocop:disable Rails/PluckId + ids = result.pluck(:id) + # rubocop:enable Rails/PluckId + + expect(ids).to eq([ccm_newer.id, ccm_older.id]) + end +end