Merge pull request #144 from alphagov/filter-class

Extract filtering into a class of its own

app/services/discovery_engine/filters.rb

@@ -0,0 +1,22 @@
+module DiscoveryEngine
+  class Filters
+    def initialize(query_params)
+      @query_params = query_params
+    end
+
+    def filter_expression
+      reject_links_filter
+    end
+
+  private
+
+    attr_reader :query_params
+
+    def reject_links_filter
+      return nil if query_params[:reject_link].blank?
+
+      reject_links = Array(query_params[:reject_link]).map { "\"#{_1}\"" }.join(",")
+      "NOT link: ANY(#{reject_links})"
+    end
+  end
+end

app/services/discovery_engine/search.rb

@@ -70,14 +70,7 @@ def order_by
     end
 
     def filter
-      reject_links_filter
-    end
-
-    def reject_links_filter
-      return nil if query_params[:reject_link].blank?
-
-      reject_links = Array(query_params[:reject_link]).map { "\"#{_1}\"" }.join(",")
-      "NOT link: ANY(#{reject_links})"
+      Filters.new(query_params).filter_expression
     end
 
     def serving_config

spec/services/discovery_engine/filters_spec.rb

@@ -0,0 +1,27 @@
+RSpec.describe DiscoveryEngine::Filters do
+  describe "#filter_expression" do
+    subject(:filter_expression) { described_class.new(query_params).filter_expression }
+
+    context "when no relevant query params are present" do
+      let(:query_params) { {} }
+
+      it { is_expected.to be_nil }
+    end
+
+    context "with a single reject_link parameter" do
+      let(:query_params) { { q: "garden centres", reject_link: "/foo" } }
+
+      it "calls the client with the expected parameters" do
+        expect(filter_expression).to eq('NOT link: ANY("/foo")')
+      end
+    end
+
+    context "with several reject_link parameter" do
+      let(:query_params) { { q: "garden centres", reject_link: ["/foo", "/bar"] } }
+
+      it "calls the client with the expected parameters" do
+        expect(filter_expression).to eq('NOT link: ANY("/foo","/bar")')
+      end
+    end
+  end
+end

spec/services/discovery_engine/search_spec.rb

@@ -2,6 +2,7 @@
   subject(:search) { described_class.new(query_params, client:) }
 
   let(:client) { double("SearchService::Client", search: search_return_value) }
+  let(:filters) { double(filter_expression: "filter-expression") }
 
   let(:expected_boost_specs) do
     [{ boost: 0.2,
@@ -17,6 +18,7 @@
   before do
     allow(Rails.configuration).to receive(:discovery_engine_serving_config)
       .and_return("serving-config-path")
+    allow(DiscoveryEngine::Filters).to receive(:new).and_return(filters)
   end
 
   around do |example|
@@ -46,6 +48,7 @@
           query: "garden centres",
           offset: 0,
           page_size: 10,
+          filter: "filter-expression",
           boost_spec: { condition_boost_specs: expected_boost_specs },
         )
       end
@@ -68,6 +71,7 @@
             query: "garden centres",
             offset: 11,
             page_size: 22,
+            filter: "filter-expression",
             boost_spec: { condition_boost_specs: expected_boost_specs },
           )
         end
@@ -107,22 +111,12 @@
         end
       end
 
-      context "with a single reject_link parameter" do
-        let(:query_params) { { q: "garden centres", reject_link: "/foo" } }
+      context "when no filter expression is returned" do
+        let(:filters) { double(filter_expression: nil) }
 
-        it "calls the client with the expected parameters" do
-          expect(client).to have_received(:search).with(
-            hash_including(filter: 'NOT link: ANY("/foo")'),
-          )
-        end
-      end
-
-      context "with several reject_link parameter" do
-        let(:query_params) { { q: "garden centres", reject_link: ["/foo", "/bar"] } }
-
-        it "calls the client with the expected parameters" do
+        it "calls the client without a filter parameter" do
           expect(client).to have_received(:search).with(
-            hash_including(filter: 'NOT link: ANY("/foo","/bar")'),
+            hash_not_including(:filter),
           )
         end
       end
@@ -140,11 +134,7 @@
 
         it "calls the client with the expected parameters" do
           expect(client).to have_received(:search).with(
-            serving_config: "serving-config-path",
-            query: "i want to test a single best bet",
-            offset: 0,
-            page_size: 10,
-            boost_spec: { condition_boost_specs: expected_boost_specs },
+            hash_including(boost_spec: { condition_boost_specs: expected_boost_specs }),
           )
         end
       end
@@ -162,11 +152,7 @@
 
         it "calls the client with the expected parameters" do
           expect(client).to have_received(:search).with(
-            serving_config: "serving-config-path",
-            query: "i want to test multiple best bets",
-            offset: 0,
-            page_size: 10,
-            boost_spec: { condition_boost_specs: expected_boost_specs },
+            hash_including(boost_spec: { condition_boost_specs: expected_boost_specs }),
           )
         end
       end