maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[Search 2.0] Remove remaining search_2 feature flags (#13591)

Browse source 
* Remove remaining search_2 feature flags

* Fix specs

* Fix more specs

* Remove leftovers

Co-authored-by: rhymes <rhymes@hey.com>
This commit is contained in:
Alex 2021-04-30 09:02:35 -04:00 • committed by GitHub
parent 5a1034cab6
commit c88fc5d233
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 101 additions and 269 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

82
app/controllers/search_controller.rb
View file

@ -103,31 +103,19 @@ class SearchController < ApplicationController
render json: { result: result }
end
# TODO: [@rhymes] the homepage feed uses `feed_content_search` as an index,
# we should eventually move it to a JSON result
# in ArticlesController#Homepage or HomepageController#show
# rubocop:disable Metrics/CyclomaticComplexity
# rubocop:disable Metrics/PerceivedComplexity
def feed_content
class_name = feed_params[:class_name].to_s.inquiry
enable_search_2_homepage = (
is_homepage_search = (
class_name.Article? &&
feed_params[:search_fields].blank? &&
feed_params[:sort_by].present? &&
FeatureFlag.enabled?(:search_2_homepage, current_user)
feed_params[:sort_by].present?
)
result =
if class_name.blank?
if FeatureFlag.enabled?(:search_2_articles, current_user)
search_postgres_article
else
# If we are in the main feed and not filtering by type return
# all articles, podcast episodes, and users
feed_content_search.concat(user_search)
end
elsif enable_search_2_homepage
search_postgres_article
elsif is_homepage_search
# NOTE: published_at is sent from the frontend in the following ES-friendly format:
# => {"published_at"=>{"gte"=>"2021-04-06T14:53:23Z"}}
published_at_gte = feed_params.dig(:published_at, :gte)
@ -150,7 +138,7 @@ class SearchController < ApplicationController
page: params[:page],
per_page: params[:per_page],
)
elsif class_name.Comment? && FeatureFlag.enabled?(:search_2_comments)
elsif class_name.Comment?
Search::Postgres::Comment.search_documents(
page: feed_params[:page],
per_page: feed_params[:per_page],
@ -158,7 +146,7 @@ class SearchController < ApplicationController
sort_direction: feed_params[:sort_direction],
term: feed_params[:search_fields],
)
elsif class_name.PodcastEpisode? && FeatureFlag.enabled?(:search_2_podcast_episodes)
elsif class_name.PodcastEpisode?
Search::Postgres::PodcastEpisode.search_documents(
page: feed_params[:page],
per_page: feed_params[:per_page],
@ -167,49 +155,33 @@ class SearchController < ApplicationController
term: feed_params[:search_fields],
)
elsif class_name.User?
if FeatureFlag.enabled?(:search_2_users)
Search::Postgres::User.search_documents(
term: feed_params[:search_fields],
page: feed_params[:page],
per_page: feed_params[:per_page],
sort_by: feed_params[:sort_by] == "published_at" ? :created_at : nil,
sort_direction: feed_params[:sort_direction],
)
else
user_search
end
elsif class_name.Article? && FeatureFlag.enabled?(:search_2_articles, current_user)
Search::Postgres::User.search_documents(
term: feed_params[:search_fields],
page: feed_params[:page],
per_page: feed_params[:per_page],
sort_by: feed_params[:sort_by] == "published_at" ? :created_at : nil,
sort_direction: feed_params[:sort_direction],
)
elsif class_name.Article?
search_postgres_article
else
feed_content_search
end
render json: { result: result }
end
# rubocop:enable Metrics/PerceivedComplexity
# rubocop:enable Metrics/CyclomaticComplexity
def reactions
if FeatureFlag.enabled?(:search_2_reading_list)
# [@rhymes] we're recyling the existing params as we want to change the frontend as
# little as possible, we might simplify in the future
result = Search::Postgres::ReadingList.search_documents(
current_user,
page: reaction_params[:page],
per_page: reaction_params[:per_page],
statuses: reaction_params[:status],
tags: reaction_params[:tag_names],
term: reaction_params[:search_fields],
)
# [@rhymes] we're recyling the existing params as we want to change the frontend as
# little as possible, we might simplify in the future
result = Search::Postgres::ReadingList.search_documents(
current_user,
page: reaction_params[:page],
per_page: reaction_params[:per_page],
statuses: reaction_params[:status],
tags: reaction_params[:tag_names],
term: reaction_params[:search_fields],
)
render json: { result: result[:items], total: result[:total] }
else
result = Search::ReadingList.search_documents(
params: reaction_params.to_h, user: current_user,
)
render json: { result: result["reactions"], total: result["total"] }
end
render json: { result: result[:items], total: result[:total] }
end
private
@ -225,10 +197,6 @@ class SearchController < ApplicationController
)
end
def feed_content_search
Search::FeedContent.search_documents(params: feed_params.to_h)
end
def user_search
Search::User.search_documents(params: user_params.to_h)
end

253
spec/requests/search_spec.rb
View file

@ -99,140 +99,62 @@ RSpec.describe "Search", type: :request, proper_status: true do
end
describe "GET /search/feed_content" do
context "when using Elasticsearch" do
let(:mock_documents) { [{ "title" => "article1" }] }
let(:homepage_params) { { class_name: "Article", sort_by: "published_at", sort_direction: "desc" } }
it "returns json" do
allow(Search::FeedContent).to receive(:search_documents).and_return(
mock_documents,
)
it "does not call Homepage::FetchArticles when class_name is Article with a search term", :aggregate_failures do
allow(Homepage::FetchArticles).to receive(:call)
get search_feed_content_path
expect(response.parsed_body["result"]).to eq(mock_documents)
end
get search_feed_content_path
expect(Homepage::FetchArticles).not_to have_received(:call)
it "queries only the user index if class_name=User" do
allow(Search::FeedContent).to receive(:search_documents)
allow(Search::User).to receive(:search_documents).and_return(
mock_documents,
)
get search_feed_content_path(class_name: "User")
expect(Search::User).to have_received(:search_documents)
expect(Search::FeedContent).not_to have_received(:search_documents)
end
it "queries for Articles, Podcast Episodes and Users if no class_name filter is present" do
allow(Search::FeedContent).to receive(:search_documents).and_return(
mock_documents,
)
allow(Search::User).to receive(:search_documents).and_return(
mock_documents,
)
get search_feed_content_path
expect(Search::User).to have_received(:search_documents)
expect(Search::FeedContent).to have_received(:search_documents)
end
it "queries for only Articles and Podcast Episodes if class_name!=User" do
allow(Search::FeedContent).to receive(:search_documents).and_return(
mock_documents,
)
allow(Search::User).to receive(:search_documents)
get search_feed_content_path(class_name: "Article")
expect(Search::User).not_to have_received(:search_documents)
expect(Search::FeedContent).to have_received(:search_documents)
end
it "queries for approved" do
allow(Search::FeedContent).to receive(:search_documents).and_return(
mock_documents,
)
get search_feed_content_path(class_name: "Article", approved: "true")
expect(Search::FeedContent).to have_received(:search_documents).with(
params: { "approved" => "true", "class_name" => "Article" },
)
end
get search_feed_content_path(class_name: "Article", search_fields: "keyword")
expect(Homepage::FetchArticles).not_to have_received(:call)
end
context "when using PostgreSQL for the homepage" do
let(:homepage_params) { { class_name: "Article", sort_by: "published_at", sort_direction: "desc" } }
it "returns the correct keys", :aggregate_failures do
create(:article)
before do
allow(FeatureFlag).to receive(:enabled?).with(:search_2_articles, anything).and_return(true)
allow(FeatureFlag).to receive(:enabled?).with(:search_2_homepage, anything).and_return(true)
end
get search_feed_content_path(homepage_params)
it "does not call Homepage::FetchArticles when class_name is Article with a search term", :aggregate_failures do
allow(Homepage::FetchArticles).to receive(:call)
get search_feed_content_path
expect(Homepage::FetchArticles).not_to have_received(:call)
get search_feed_content_path(class_name: "Article", search_fields: "keyword")
expect(Homepage::FetchArticles).not_to have_received(:call)
end
it "calls Homepage::FetchArticles when class_name is Article" do
allow(Homepage::FetchArticles).to receive(:call)
get search_feed_content_path(homepage_params)
expect(Homepage::FetchArticles).to have_received(:call)
end
it "returns the correct keys", :aggregate_failures do
create(:article)
get search_feed_content_path(homepage_params)
expect(response.parsed_body["result"]).to be_present
end
it "parses published_at correctly", :aggregate_failures do
article = create(:article)
get search_feed_content_path(homepage_params.merge(published_at: { gte: article.published_at.iso8601 }))
expect(response.parsed_body["result"].first["id"]).to eq(article.id)
datetime = article.published_at + 1.minute
get search_feed_content_path(homepage_params.merge(published_at: { gte: datetime.iso8601 }))
expect(response.parsed_body["result"]).to be_empty
end
it "supports the user_id parameter" do
allow(Homepage::FetchArticles).to receive(:call)
get search_feed_content_path(homepage_params.merge(user_id: 1))
expect(Homepage::FetchArticles).to have_received(:call).with(hash_including(user_id: "1"))
end
it "supports the organization_id parameter" do
allow(Homepage::FetchArticles).to receive(:call)
get search_feed_content_path(homepage_params.merge(organization_id: 1))
expect(Homepage::FetchArticles).to have_received(:call).with(hash_including(organization_id: "1"))
end
it "supports the tag_names parameter" do
allow(Homepage::FetchArticles).to receive(:call)
get search_feed_content_path(homepage_params.merge(tag_names: %i[ruby]))
expect(Homepage::FetchArticles).to have_received(:call).with(hash_including(tags: %w[ruby]))
end
expect(response.parsed_body["result"]).to be_present
end
context "when using PostgreSQL for articles" do
before do
allow(FeatureFlag).to receive(:enabled?).with(:search_2_articles, anything).and_return(true)
end
it "parses published_at correctly", :aggregate_failures do
article = create(:article)
get search_feed_content_path(homepage_params.merge(published_at: { gte: article.published_at.iso8601 }))
expect(response.parsed_body["result"].first["id"]).to eq(article.id)
datetime = article.published_at + 1.minute
get search_feed_content_path(homepage_params.merge(published_at: { gte: datetime.iso8601 }))
expect(response.parsed_body["result"]).to be_empty
end
it "supports the user_id parameter" do
allow(Homepage::FetchArticles).to receive(:call)
get search_feed_content_path(homepage_params.merge(user_id: 1))
expect(Homepage::FetchArticles).to have_received(:call).with(hash_including(user_id: "1"))
end
it "supports the organization_id parameter" do
allow(Homepage::FetchArticles).to receive(:call)
get search_feed_content_path(homepage_params.merge(organization_id: 1))
expect(Homepage::FetchArticles).to have_received(:call).with(hash_including(organization_id: "1"))
end
it "supports the tag_names parameter" do
allow(Homepage::FetchArticles).to receive(:call)
get search_feed_content_path(homepage_params.merge(tag_names: %i[ruby]))
expect(Homepage::FetchArticles).to have_received(:call).with(hash_including(tags: %w[ruby]))
end
context "when searching for articles" do
it "calls Search::Postgres::Article without a class_name" do
allow(Search::Postgres::Article).to receive(:search_documents)
@ -241,17 +163,7 @@ RSpec.describe "Search", type: :request, proper_status: true do
expect(Search::Postgres::Article).to have_received(:search_documents)
end
it "calls Search::Postgres::Article without a class_name with :search_2_homepage active" do
allow(FeatureFlag).to receive(:enabled?).with(:search_2_homepage, anything).and_return(true)
allow(Search::Postgres::Article).to receive(:search_documents)
get search_feed_content_path
expect(Search::Postgres::Article).to have_received(:search_documents)
end
it "calls Search::Postgres::Article with class_name=Article with :search_2_homepage active" do
allow(FeatureFlag).to receive(:enabled?).with(:search_2_homepage, anything).and_return(true)
it "calls Search::Postgres::Article with class_name=Article" do
allow(Search::Postgres::Article).to receive(:search_documents)
get search_feed_content_path(class_name: "Article")
@ -275,11 +187,7 @@ RSpec.describe "Search", type: :request, proper_status: true do
end
end
context "when using PostgreSQL for comments" do
before do
allow(FeatureFlag).to receive(:enabled?).with(:search_2_comments).and_return(true)
end
context "when searching for comments" do
it "returns the correct keys for comments" do
create(:comment, body_markdown: "Ruby on Rails rocks!")
get search_feed_content_path(search_fields: "rails", class_name: "Comment")
@ -299,11 +207,7 @@ RSpec.describe "Search", type: :request, proper_status: true do
end
end
context "when using PostgreSQL for users" do
before do
allow(FeatureFlag).to receive(:enabled?).with(:search_2_users).and_return(true)
end
context "when using searching for users" do
it "returns the correct keys", :aggregate_failures do
create(:user)
@ -324,11 +228,7 @@ RSpec.describe "Search", type: :request, proper_status: true do
end
end
context "when using PostgreSQL for podcasts" do
before do
allow(FeatureFlag).to receive(:enabled?).with(:search_2_podcast_episodes).and_return(true)
end
context "when using searching for podcasts" do
it "returns the correct keys for podcasts" do
create(:podcast_episode, body: "DHH talks about how Ruby on Rails rocks!")
get search_feed_content_path(search_fields: "rails", class_name: "PodcastEpisode")
@ -350,57 +250,26 @@ RSpec.describe "Search", type: :request, proper_status: true do
end
describe "GET /search/reactions" do
let(:article) { create(:article) }
before do
sign_in authorized_user
create(:reaction, category: :readinglist, reactable: article, user: authorized_user)
end
context "when using Elasticsearch" do
let(:mock_response) do
{ "reactions" => [{ id: 123 }], "total" => 100 }
end
it "returns the correct keys", :aggregate_failures do
get search_reactions_path
before do
allow(Search::ReadingList).to receive(:search_documents).and_return(mock_response)
end
it "returns json with reactions and total" do
get search_reactions_path
expect(response.parsed_body).to eq("result" => [{ "id" => 123 }], "total" => 100)
end
it "accepts array of tag names" do
get search_reactions_path(tag_names: [1, 2])
expect(Search::ReadingList).to(
have_received(:search_documents)
.with(params: { "tag_names" => %w[1 2] }, user: authorized_user),
)
end
expect(response.parsed_body["result"]).to be_present
expect(response.parsed_body["total"]).to eq(1)
end
context "when using PostgreSQL" do
let(:article) { create(:article) }
it "supports the search params" do
article.update_columns(title: "Title", cached_tag_list: "ruby, python")
before do
allow(FeatureFlag).to receive(:enabled?).with(:search_2_reading_list).and_return(true)
create(:reaction, category: :readinglist, reactable: article, user: authorized_user)
end
get search_reactions_path(page: 0, per_page: 1, status: %w[valid], tags: %w[ruby], term: "title")
it "returns the correct keys", :aggregate_failures do
get search_reactions_path
expect(response.parsed_body["result"]).to be_present
expect(response.parsed_body["total"]).to eq(1)
end
it "supports the search params" do
article.update_columns(title: "Title", cached_tag_list: "ruby, python")
get search_reactions_path(page: 0, per_page: 1, status: %w[valid], tags: %w[ruby], term: "title")
expect(response.parsed_body["result"].first["reactable"]).to include("title" => "Title")
end
expect(response.parsed_body["result"].first["reactable"]).to include("title" => "Title")
end
end
end

30
spec/system/search/display_articles_search_spec.rb
View file

@ -1,16 +1,13 @@
require "rails_helper"
RSpec.describe "Display articles search spec", type: :system, js: true, elasticsearch: "FeedContent" do
let(:found_article_one) { create(:article) }
let(:found_article_two) { create(:article) }
let(:not_found_article) { create(:article) }
it "returns correct results for a search" do
found_article_one.tags << create(:tag, name: "ruby")
allow(found_article_two).to receive(:body_text).and_return("Ruby Tuesday")
allow(not_found_article).to receive(:body_text).and_return("Python All Day Long")
articles = [found_article_one, found_article_two, not_found_article]
index_documents_for_search_class(articles, Search::FeedContent)
found_article_one = create(:article)
found_article_one.update_columns(cached_tag_list: "ruby")
found_article_two = create(:article)
found_article_two.update_columns(body_markdown: "#{found_article_two.body_markdown} Ruby Tuesday")
not_found_article = create(:article)
visit "/search?q=ruby&filters=class_name:Article"
expect(page).to have_content(found_article_one.title)
@ -19,11 +16,11 @@ RSpec.describe "Display articles search spec", type: :system, js: true, elastics
end
it "returns all expected article fields" do
allow(found_article_one).to receive(:reading_time).and_return(5)
allow(found_article_one).to receive(:comments_count).and_return(2)
allow(found_article_one).to receive(:public_reactions_count).and_return(3)
found_article_one.tags << create(:tag, name: "ruby")
index_documents_for_search_class([found_article_one], Search::FeedContent)
found_article_one = create(:article)
found_article_one.update_columns(cached_tag_list: "ruby",
reading_time: 5,
comments_count: 2,
public_reactions_count: 3)
visit "/search?q=ruby&filters=class_name:Article"
expect(page).to have_content(found_article_one.title)
@ -37,9 +34,8 @@ RSpec.describe "Display articles search spec", type: :system, js: true, elastics
end
it "does not show reaction data if article has no reactions" do
allow(found_article_one).to receive(:public_reactions_count).and_return(0)
found_article_one.tags << create(:tag, name: "ruby")
index_documents_for_search_class([found_article_one], Search::FeedContent)
found_article_one = create(:article)
found_article_one.update_columns(cached_tag_list: "ruby", public_reactions_count: 0)
visit "/search?q=ruby&filters=class_name:Article"
expect(page).not_to have_content("0 reactions")

5
spec/system/search/display_comments_search_spec.rb
View file

@ -3,13 +3,12 @@ require "rails_helper"
RSpec.describe "Display articles search spec", type: :system, js: true, elasticsearch: "FeedContent" do
it "returns correct results for a search" do
query = "<marquee='alert(document.cookie)'>XSS"
found_comment = create(:comment, body_markdown: query)
index_documents_for_search_class([found_comment], Search::FeedContent)
create(:comment, body_markdown: query)
url_encoded_query = CGI.escape(query)
visit "/search?q=#{url_encoded_query}&filters=class_name:Comment"
expect(page.find(".crayons-story__snippet")["innerHTML"])
.to eq("…&lt;<mark>marquee</mark>='<mark>alert</mark>(<mark>document.cookie</mark>)'&gt;<mark>XSS</mark>…")
.to eq("…<mark>marquee</mark>='<mark>alert</mark>(<mark>document.cookie</mark>)'&gt;<mark>XSS</mark>…")
end
end