Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make filters more generic #148

Merged
merged 1 commit into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 32 additions & 23 deletions app/services/discovery_engine/query/filters.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
module DiscoveryEngine::Query
class Filters
FILTERABLE_FIELDS = %i[content_purpose_supergroup link part_of_taxonomy_tree].freeze

def initialize(query_params)
@query_params = query_params
end

def filter_expression
expressions = [
reject_links_filter,
content_purpose_supergroup_filter,
]
*query_params_of_type(:reject).map { reject_filter(_1, _2) },
*query_params_of_type(:filter).map { any_filter(_1, _2) },
*query_params_of_type(:filter_all).map { all_filter(_1, _2) },
].compact

expressions
.compact
.map { surround(_1, delimiter: "(", delimiter_end: ")") }
.join(" AND ")
.presence
Expand All @@ -21,31 +23,38 @@ def filter_expression

attr_reader :query_params

def reject_links_filter
return nil if query_params[:reject_link].blank?

values = Array(query_params[:reject_link])
.map { filter_string_value(_1) }
.join(",")

"NOT link: ANY(#{values})"
def query_params_of_type(type)
FILTERABLE_FIELDS
.filter_map { [_1, query_params["#{type}_#{_1}".to_sym]] }
.to_h
.compact_blank
end

def content_purpose_supergroup_filter
return nil if query_params[:filter_content_purpose_supergroup].blank?
def reject_filter(field, value_or_values)
string_filter_expression(field, value_or_values, negate: true)
end

values = Array(query_params[:filter_content_purpose_supergroup])
.map { filter_string_value(_1) }
.join(",")
def all_filter(field, value_or_values)
Array(value_or_values)
.map { string_filter_expression(field, _1) }
.join(" AND ")
.presence
end

"content_purpose_supergroup: ANY(#{values})"
def any_filter(field, value_or_values)
string_filter_expression(field, value_or_values)
end

# Input strings need to be wrapped in double quotes and have double quotes or backslashes
# escaped for Discovery Engine's filter syntax
def filter_string_value(str)
escaped_str = str.gsub(/(["\\])/, '\\\\\1')
surround(escaped_str, delimiter: '"')
def string_filter_expression(field, value_or_values, negate: false)
values = Array(value_or_values).map do |value|
# Input strings need to be wrapped in double quotes and have double quotes or backslashes
# escaped for Discovery Engine's filter syntax
escaped_value = value.gsub(/(["\\])/, '\\\\\1')
surround(escaped_value, delimiter: '"')
end
return if values.blank?

"#{negate ? 'NOT ' : ''}#{field}: ANY(#{values.join(',')})"
end

def surround(str, delimiter:, delimiter_end: delimiter)
Expand Down
45 changes: 39 additions & 6 deletions spec/services/discovery_engine/query/filters_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
it { is_expected.to be_nil }
end

context "with reject_link" do
context "with a reject filter" do
context "with an empty parameter" do
let(:query_params) { { q: "garden centres", reject_link: "" } }

Expand All @@ -28,30 +28,63 @@
end
end

context "with filter_content_purpose_supergroup" do
context "with an 'any' filter" do
context "with an empty parameter" do
let(:query_params) { { q: "garden centres", filter_content_purpose_supergroup: "" } }

it { is_expected.to be_nil }
end

context "with a single parameter" do
let(:query_params) { { q: "garden centres", filter_content_purpose_supergroup: "services" } }
let(:query_params) do
{ q: "garden centres", filter_content_purpose_supergroup: "services" }
end

it { is_expected.to eq('(content_purpose_supergroup: ANY("services"))') }
end

context "with several parameters" do
let(:query_params) { { q: "garden centres", filter_content_purpose_supergroup: %w[services guidance] } }
let(:query_params) do
{ q: "garden centres", filter_content_purpose_supergroup: %w[services guidance] }
end

it { is_expected.to eq('(content_purpose_supergroup: ANY("services","guidance"))') }
end
end

context "with an 'all' filter" do
context "with an empty parameter" do
let(:query_params) { { q: "garden centres", filter_all_part_of_taxonomy_tree: "" } }

it { is_expected.to be_nil }
end

context "with a single parameter" do
let(:query_params) { { q: "garden centres", filter_all_part_of_taxonomy_tree: "cafe-1234" } }

it { is_expected.to eq('(part_of_taxonomy_tree: ANY("cafe-1234"))') }
end

context "with several parameters" do
let(:query_params) do
{ q: "garden centres", filter_all_part_of_taxonomy_tree: %w[cafe-1234 face-5678] }
end

it { is_expected.to eq('(part_of_taxonomy_tree: ANY("cafe-1234") AND part_of_taxonomy_tree: ANY("face-5678"))') }
end
end

context "with several filters specified" do
let(:query_params) { { q: "garden centres", reject_link: "/foo", filter_content_purpose_supergroup: "services" } }
let(:query_params) do
{
q: "garden centres",
reject_link: "/foo",
filter_content_purpose_supergroup: "services",
filter_all_part_of_taxonomy_tree: %w[cafe-1234 face-5678],
}
end

it { is_expected.to eq('(NOT link: ANY("/foo")) AND (content_purpose_supergroup: ANY("services"))') }
it { is_expected.to eq('(NOT link: ANY("/foo")) AND (content_purpose_supergroup: ANY("services")) AND (part_of_taxonomy_tree: ANY("cafe-1234") AND part_of_taxonomy_tree: ANY("face-5678"))') }
end

context "with filters containing escapable characters" do
Expand Down