[Search 2.0] Add query filters for profile, organization and tag index pages (#13510)
* Re-order test cases to increase tests readability * Add user_id filter for articles query * Add organization_id and tags filters * Add new params to SearchController#feed_content * Add current_user param so that it can be tested for a small pool of users
This commit is contained in:
rhymes
GitHub
parent
333c41a962
commit
8a55c3e888
5 changed files with 246 additions and 95 deletions
|
|
@ -126,31 +126,40 @@ class SearchController < ApplicationController
|
|||
def feed_content
|
||||
class_name = feed_params[:class_name].to_s.inquiry
|
||||
|
||||
enable_search_2_homepage = (
|
||||
class_name.Article? &&
|
||||
feed_params[:search_fields].blank? &&
|
||||
FeatureFlag.enabled?(:search_2_homepage, current_user)
|
||||
)
|
||||
|
||||
result =
|
||||
if class_name.blank?
|
||||
# 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)
|
||||
elsif class_name.Article? && feed_params[:search_fields].blank?
|
||||
# homepage
|
||||
if FeatureFlag.enabled?(:search_2_homepage)
|
||||
# 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 = params.dig(:published_at, :gte)
|
||||
published_at_gte = Time.zone.parse(published_at_gte) if published_at_gte
|
||||
published_at = published_at_gte ? published_at_gte.. : nil
|
||||
elsif enable_search_2_homepage
|
||||
# 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)
|
||||
published_at_gte = Time.zone.parse(published_at_gte) if published_at_gte
|
||||
published_at = published_at_gte ? published_at_gte.. : nil
|
||||
|
||||
Homepage::FetchArticles.call(
|
||||
approved: params[:approved],
|
||||
published_at: published_at,
|
||||
sort_by: params[:sort_by],
|
||||
sort_direction: params[:sort_direction],
|
||||
page: params[:page],
|
||||
per_page: params[:per_page],
|
||||
)
|
||||
else
|
||||
feed_content_search
|
||||
end
|
||||
# Despite the name "Homepage", this is used by the following index pages:
|
||||
# => homepage (default, top week/month/year/infinity, latest)
|
||||
# => profile page
|
||||
# => organization page
|
||||
# => tag index page
|
||||
Homepage::FetchArticles.call(
|
||||
approved: feed_params[:approved],
|
||||
published_at: published_at,
|
||||
user_id: feed_params[:user_id],
|
||||
organization_id: feed_params[:organization_id],
|
||||
tags: feed_params[:tag_names],
|
||||
sort_by: params[:sort_by],
|
||||
sort_direction: params[:sort_direction],
|
||||
page: params[:page],
|
||||
per_page: params[:per_page],
|
||||
)
|
||||
elsif class_name.Comment? && FeatureFlag.enabled?(:search_2_comments)
|
||||
Search::Postgres::Comment.search_documents(
|
||||
page: feed_params[:page],
|
||||
|
|
|
|||
|
|
@ -25,12 +25,24 @@ module Homepage
|
|||
|
||||
# TODO: [@rhymes] change frontend to start from page 1
|
||||
def initialize(
|
||||
approved: nil, published_at: nil, sort_by: nil, sort_direction: nil, page: 0, per_page: DEFAULT_PER_PAGE
|
||||
approved: nil,
|
||||
published_at: nil,
|
||||
user_id: nil,
|
||||
organization_id: nil,
|
||||
tags: [],
|
||||
sort_by: nil,
|
||||
sort_direction: nil,
|
||||
page: 0,
|
||||
per_page: DEFAULT_PER_PAGE
|
||||
)
|
||||
@relation = Article.published.select(*ATTRIBUTES)
|
||||
|
||||
@approved = approved
|
||||
@published_at = published_at
|
||||
@user_id = user_id
|
||||
@organization_id = organization_id
|
||||
@tags = tags.presence || []
|
||||
|
||||
@sort_by = sort_by
|
||||
@sort_direction = sort_direction
|
||||
|
||||
|
|
@ -44,13 +56,22 @@ module Homepage
|
|||
|
||||
private
|
||||
|
||||
attr_reader :relation, :approved, :published_at, :sort_by, :sort_direction, :page, :per_page
|
||||
attr_reader :relation, :approved, :published_at, :user_id, :organization_id, :tags, :sort_by, :sort_direction,
|
||||
:page, :per_page
|
||||
|
||||
def filter
|
||||
return relation if approved.nil? && published_at.blank?
|
||||
|
||||
@relation = @relation.where(approved: approved) unless approved.nil?
|
||||
@relation = @relation.where(published_at: published_at) if published_at
|
||||
@relation = @relation.where(published_at: published_at) if published_at.present?
|
||||
@relation = @relation.where(user_id: user_id) if user_id.present?
|
||||
@relation = @relation.where(organization_id: organization_id) if organization_id.present?
|
||||
|
||||
# as tags are in `OR` mode we can't use ActiveRecord's `.or()` because it
|
||||
# would put all the previous filters in `OR` mode with tags, but what we need
|
||||
# is to only consider tags as a `OR` sub-condition
|
||||
if tags.present?
|
||||
conditions = tags.map { |tag| relation.sanitize_sql_array(["cached_tag_list LIKE ?", "%#{tag}%"]) }
|
||||
@relation = @relation.where(conditions.join(" OR "))
|
||||
end
|
||||
|
||||
relation
|
||||
end
|
||||
|
|
|
|||
|
|
@ -1,13 +1,30 @@
|
|||
# This is used to populate the following pages:
|
||||
# => homepage
|
||||
# => profile page
|
||||
# => organization page
|
||||
# => tag index page
|
||||
# TODO: rename `Homepage::FetchArticles` to something more generic
|
||||
module Homepage
|
||||
class FetchArticles
|
||||
DEFAULT_PER_PAGE = 60
|
||||
|
||||
def self.call(
|
||||
approved: nil, published_at: nil, sort_by: nil, sort_direction: nil, page: 0, per_page: DEFAULT_PER_PAGE
|
||||
approved: nil,
|
||||
published_at: nil,
|
||||
user_id: nil,
|
||||
organization_id: nil,
|
||||
tags: [],
|
||||
sort_by: nil,
|
||||
sort_direction: nil,
|
||||
page: 0,
|
||||
per_page: DEFAULT_PER_PAGE
|
||||
)
|
||||
articles = Homepage::ArticlesQuery.call(
|
||||
approved: approved,
|
||||
published_at: published_at,
|
||||
user_id: user_id,
|
||||
organization_id: organization_id,
|
||||
tags: tags,
|
||||
sort_by: sort_by,
|
||||
sort_direction: sort_direction,
|
||||
page: page,
|
||||
|
|
|
|||
|
|
@ -6,100 +6,180 @@ RSpec.describe Homepage::ArticlesQuery, type: :query do
|
|||
expect(described_class.call).to be_a(ActiveRecord::Relation)
|
||||
end
|
||||
|
||||
it "does not return draft articles" do
|
||||
article = create(:article, published: false, published_at: nil)
|
||||
|
||||
expect(described_class.call.ids).not_to include(article.id)
|
||||
end
|
||||
|
||||
it "returns both approved and unapproved articles by default" do
|
||||
approved_article = create(:article, approved: true)
|
||||
unapproved_article = create(:article, approved: false)
|
||||
|
||||
expected_result = [approved_article.id, unapproved_article.id]
|
||||
expect(described_class.call.ids).to match_array(expected_result)
|
||||
end
|
||||
|
||||
it "returns approved articles", :aggregate_failures do
|
||||
approved_article = create(:article, approved: true)
|
||||
unapproved_article = create(:article, approved: false)
|
||||
|
||||
result = described_class.call(approved: true).ids
|
||||
expect(result).to include(approved_article.id)
|
||||
expect(result).not_to include(unapproved_article.id)
|
||||
end
|
||||
|
||||
it "returns unapproved articles" do
|
||||
approved_article = create(:article, approved: true)
|
||||
unapproved_article = create(:article, approved: false)
|
||||
|
||||
result = described_class.call(approved: false).ids
|
||||
expect(result).not_to include(approved_article.id)
|
||||
expect(result).to include(unapproved_article.id)
|
||||
end
|
||||
|
||||
it "returns only published articles" do
|
||||
article = create(:article)
|
||||
|
||||
expect(described_class.call.ids).to eq([article.id])
|
||||
end
|
||||
|
||||
it "paginates by default" do
|
||||
stub_const("Homepage::ArticlesQuery::DEFAULT_PER_PAGE", 1)
|
||||
it "does not return draft articles" do
|
||||
article = create(:article, published: false, published_at: nil)
|
||||
|
||||
create_list(:article, 2)
|
||||
|
||||
expect(described_class.call.size).to eq(1)
|
||||
expect(described_class.call.ids).not_to include(article.id)
|
||||
end
|
||||
|
||||
it "supports pagination params" do
|
||||
create_list(:article, 2)
|
||||
describe "approved" do
|
||||
it "returns both approved and unapproved articles by default" do
|
||||
approved_article = create(:article, approved: true)
|
||||
unapproved_article = create(:article, approved: false)
|
||||
|
||||
expect(described_class.call(page: 1, per_page: 1).size).to eq(1)
|
||||
expected_result = [approved_article.id, unapproved_article.id]
|
||||
expect(described_class.call.ids).to match_array(expected_result)
|
||||
end
|
||||
|
||||
it "returns approved articles", :aggregate_failures do
|
||||
approved_article = create(:article, approved: true)
|
||||
unapproved_article = create(:article, approved: false)
|
||||
|
||||
result = described_class.call(approved: true).ids
|
||||
expect(result).to include(approved_article.id)
|
||||
expect(result).not_to include(unapproved_article.id)
|
||||
end
|
||||
|
||||
it "returns unapproved articles" do
|
||||
approved_article = create(:article, approved: true)
|
||||
unapproved_article = create(:article, approved: false)
|
||||
|
||||
result = described_class.call(approved: false).ids
|
||||
expect(result).not_to include(approved_article.id)
|
||||
expect(result).to include(unapproved_article.id)
|
||||
end
|
||||
end
|
||||
|
||||
it "filters by publication date", :aggregate_failures do
|
||||
article = create(:article)
|
||||
describe "published_at" do
|
||||
it "filters by publication date", :aggregate_failures do
|
||||
article = create(:article)
|
||||
|
||||
expect(described_class.call(published_at: article.published_at).size).to eq(1)
|
||||
expect(described_class.call(published_at: 1.month.ago..).size).to eq(1)
|
||||
expect(described_class.call(published_at: 1.month.from_now)).to be_empty
|
||||
expect(described_class.call(published_at: nil).size).to eq(1)
|
||||
expect(described_class.call(published_at: article.published_at).size).to eq(1)
|
||||
expect(described_class.call(published_at: 1.month.ago..).size).to eq(1)
|
||||
expect(described_class.call(published_at: 1.month.from_now)).to be_empty
|
||||
end
|
||||
end
|
||||
|
||||
it "sorts by the hotness_score", :aggregate_failures do
|
||||
article1, article2 = create_list(:article, 2)
|
||||
describe "user_id" do
|
||||
it "returns no articles if the user id does not exist" do
|
||||
expect(described_class.call(user_id: 9999)).to be_empty
|
||||
end
|
||||
|
||||
article1.update_columns(hotness_score: 1)
|
||||
article2.update_columns(hotness_score: 2)
|
||||
it "filters articles belonging to the given user id", :aggregate_failures do
|
||||
article_user1 = create(:article)
|
||||
article_user2 = create(:article, user: create(:user))
|
||||
|
||||
result = described_class.call(sort_by: :hotness_score, sort_direction: :desc).ids
|
||||
expect(result).to eq([article2.id, article1.id])
|
||||
|
||||
result = described_class.call(sort_by: :hotness_score, sort_direction: :asc).ids
|
||||
expect(result).to eq([article1.id, article2.id])
|
||||
expect(described_class.call(user_id: article_user1.user_id).ids).to include(article_user1.id)
|
||||
expect(described_class.call(user_id: article_user1.user_id).ids).not_to include(article_user2.id)
|
||||
end
|
||||
end
|
||||
|
||||
it "sorts by the public_reactions_count" do
|
||||
article1, article2 = create_list(:article, 2)
|
||||
describe "organization_id" do
|
||||
it "returns no articles if the organization id does not exist" do
|
||||
expect(described_class.call(organization_id: 9999)).to be_empty
|
||||
end
|
||||
|
||||
article1.update_columns(public_reactions_count: 1)
|
||||
article2.update_columns(public_reactions_count: 2)
|
||||
it "filters articles belonging to the given organization id", :aggregate_failures do
|
||||
org1 = create(:organization)
|
||||
org2 = create(:organization)
|
||||
article_org1 = create(:article, organization: org1)
|
||||
article_org2 = create(:article, organization: org2)
|
||||
article_no_org = create(:article)
|
||||
|
||||
result = described_class.call(sort_by: :public_reactions_count, sort_direction: :desc).ids
|
||||
expect(result).to eq([article2.id, article1.id])
|
||||
|
||||
result = described_class.call(sort_by: :public_reactions_count, sort_direction: :asc).ids
|
||||
expect(result).to eq([article1.id, article2.id])
|
||||
expect(described_class.call(organization_id: org1.id).ids).to include(article_org1.id)
|
||||
expect(described_class.call(organization_id: org1.id).ids).not_to include(article_org2.id)
|
||||
expect(described_class.call(organization_id: org1.id).ids).not_to include(article_no_org.id)
|
||||
end
|
||||
end
|
||||
|
||||
it "does not sort by unknown parameters" do
|
||||
article1, article2 = create_list(:article, 2)
|
||||
describe "tags" do
|
||||
let(:article1) { create(:article, with_tags: false) }
|
||||
let(:article2) { create(:article, with_tags: false) }
|
||||
|
||||
article1.update_columns(comments_count: 1)
|
||||
article2.update_columns(comments_count: 2)
|
||||
it "returns no articles if none of the tags match" do
|
||||
article1.tag_list.add(:beginners)
|
||||
article1.save
|
||||
article2.tag_list.add(:beginners)
|
||||
article2.save
|
||||
|
||||
result = described_class.call(sort_by: :comments_count, sort_direction: :desc).ids
|
||||
expect(result).not_to eq([article2.id, article1.id])
|
||||
expect(described_class.call(tags: [:ruby])).to be_empty
|
||||
end
|
||||
|
||||
it "filters articles matching the tag" do
|
||||
article1.tag_list.add(:beginners)
|
||||
article1.save
|
||||
|
||||
expect(described_class.call(tags: [:beginners]).ids).to eq([article1.id])
|
||||
end
|
||||
|
||||
it "filters any article matching any of the tags in the params", :aggregate_failures do
|
||||
article1.tag_list.add(:beginners)
|
||||
article1.save
|
||||
article2.tag_list.add(:ruby)
|
||||
article2.save
|
||||
|
||||
expect(described_class.call(tags: %i[beginners python]).ids).to eq([article1.id])
|
||||
end
|
||||
|
||||
it "filters all articles match any of the tags in the params" do
|
||||
article1.tag_list.add(:beginners)
|
||||
article1.save
|
||||
article2.tag_list.add(:ruby)
|
||||
article2.save
|
||||
|
||||
expect(described_class.call(tags: %i[beginners ruby]).ids).to match_array([article1.id, article2.id])
|
||||
end
|
||||
end
|
||||
|
||||
describe "pagination" do
|
||||
it "paginates by default" do
|
||||
stub_const("Homepage::ArticlesQuery::DEFAULT_PER_PAGE", 1)
|
||||
|
||||
create_list(:article, 2)
|
||||
|
||||
expect(described_class.call.size).to eq(1)
|
||||
end
|
||||
|
||||
it "supports pagination params" do
|
||||
create_list(:article, 2)
|
||||
|
||||
expect(described_class.call(page: 1, per_page: 1).size).to eq(1)
|
||||
end
|
||||
end
|
||||
|
||||
describe "sorting" do
|
||||
it "sorts by the hotness_score", :aggregate_failures do
|
||||
article1, article2 = create_list(:article, 2)
|
||||
|
||||
article1.update_columns(hotness_score: 1)
|
||||
article2.update_columns(hotness_score: 2)
|
||||
|
||||
result = described_class.call(sort_by: :hotness_score, sort_direction: :desc).ids
|
||||
expect(result).to eq([article2.id, article1.id])
|
||||
|
||||
result = described_class.call(sort_by: :hotness_score, sort_direction: :asc).ids
|
||||
expect(result).to eq([article1.id, article2.id])
|
||||
end
|
||||
|
||||
it "sorts by the public_reactions_count" do
|
||||
article1, article2 = create_list(:article, 2)
|
||||
|
||||
article1.update_columns(public_reactions_count: 1)
|
||||
article2.update_columns(public_reactions_count: 2)
|
||||
|
||||
result = described_class.call(sort_by: :public_reactions_count, sort_direction: :desc).ids
|
||||
expect(result).to eq([article2.id, article1.id])
|
||||
|
||||
result = described_class.call(sort_by: :public_reactions_count, sort_direction: :asc).ids
|
||||
expect(result).to eq([article1.id, article2.id])
|
||||
end
|
||||
|
||||
it "does not sort by unknown parameters" do
|
||||
article1, article2 = create_list(:article, 2)
|
||||
|
||||
article1.update_columns(comments_count: 1)
|
||||
article2.update_columns(comments_count: 2)
|
||||
|
||||
result = described_class.call(sort_by: :comments_count, sort_direction: :desc).ids
|
||||
expect(result).not_to eq([article2.id, article1.id])
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -235,7 +235,7 @@ RSpec.describe "Search", type: :request, proper_status: true do
|
|||
|
||||
context "when using PostgreSQL for the homepage" do
|
||||
before do
|
||||
allow(FeatureFlag).to receive(:enabled?).with(:search_2_homepage).and_return(true)
|
||||
allow(FeatureFlag).to receive(:enabled?).with(:search_2_homepage, anything).and_return(true)
|
||||
end
|
||||
|
||||
it "does not call Homepage::FetchArticles when class_name is Article with a search term", :aggregate_failures do
|
||||
|
|
@ -274,6 +274,30 @@ RSpec.describe "Search", type: :request, proper_status: true do
|
|||
get search_feed_content_path(class_name: "Article", 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(class_name: "Article", 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(class_name: "Article", 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(class_name: "Article", tag_names: %i[ruby])
|
||||
|
||||
expect(Homepage::FetchArticles).to have_received(:call).with(hash_including(tags: %w[ruby]))
|
||||
end
|
||||
end
|
||||
|
||||
context "when using PostgreSQL for comments" do
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue