diff --git a/app/services/discovery_engine/query/filter_expression_helpers.rb b/app/services/discovery_engine/query/filter_expression_helpers.rb index b06d6e0..127a30d 100644 --- a/app/services/discovery_engine/query/filter_expression_helpers.rb +++ b/app/services/discovery_engine/query/filter_expression_helpers.rb @@ -29,7 +29,10 @@ def filter_not_string(string_or_array_field, string_value_or_values) # timestamp_value def filter_timestamp(timestamp_field, timestamp_value) match = timestamp_value.match(TIMESTAMP_VALUE_REGEX) - return nil unless match && (match[:from] || match[:to]) + unless match && (match[:from] || match[:to]) + Rails.logger.warn("#{self.class.name}: Invalid timestamp value: '#{timestamp_value}'") + return nil + end from = match[:from] ? Date.parse(match[:from]).beginning_of_day.to_i : "*" to = match[:to] ? Date.parse(match[:to]).end_of_day.to_i : "*" diff --git a/app/services/discovery_engine/query/filters.rb b/app/services/discovery_engine/query/filters.rb index f26f0f3..9838844 100644 --- a/app/services/discovery_engine/query/filters.rb +++ b/app/services/discovery_engine/query/filters.rb @@ -30,7 +30,18 @@ def parse_param(key, value) when *FILTERABLE_STRING_FIELDS string_filter_expression(filter_type, filter_field, value) when *FILTERABLE_TIMESTAMP_FIELDS + if filter_type != "filter" + Rails.logger.warn( + "#{self.class.name}: Cannot filter on timestamp field '#{filter_field}' " \ + "with filter type '#{filter_type}'", + ) + return nil + end + filter_timestamp(filter_field, value) + else + Rails.logger.info("#{self.class.name}: Ignoring unknown filter field: '#{filter_field}'") + nil end end diff --git a/spec/services/discovery_engine/query/filters_spec.rb b/spec/services/discovery_engine/query/filters_spec.rb index 57bf45c..9e0bdcc 100644 --- a/spec/services/discovery_engine/query/filters_spec.rb +++ b/spec/services/discovery_engine/query/filters_spec.rb @@ -26,6 +26,12 @@ it { is_expected.to eq('NOT link: ANY("/foo","/bar")') } end + + context "with an unknown field" do + let(:query_params) { { q: "garden centres", reject_foo: "bar" } } + + it { is_expected.to be_nil } + end end context "with an 'any' string filter" do @@ -50,6 +56,12 @@ it { is_expected.to eq('content_purpose_supergroup: ANY("services","guidance")') } end + + context "with an unknown field" do + let(:query_params) { { q: "garden centres", filter_foo: "bar" } } + + it { is_expected.to be_nil } + end end context "with an 'all' string filter" do @@ -72,6 +84,12 @@ it { is_expected.to eq('(part_of_taxonomy_tree: ANY("cafe-1234")) AND (part_of_taxonomy_tree: ANY("face-5678"))') } end + + context "with an unknown field" do + let(:query_params) { { q: "garden centres", filter_all_foo: "bar" } } + + it { is_expected.to be_nil } + end end context "with a timestamp filter" do @@ -110,6 +128,18 @@ it { is_expected.to be_nil } end + + context "with an invalid filter_all type" do + let(:query_params) { { q: "garden centres", filter_all_public_timestamp: "from:1989-12-13" } } + + it { is_expected.to be_nil } + end + + context "with an invalid reject type" do + let(:query_params) { { q: "garden centres", reject_public_timestamp: "from:1989-12-13" } } + + it { is_expected.to be_nil } + end end context "with several filters specified" do