From cd6ff4e3ef314b787924d787f3b3a4b0fed51f79 Mon Sep 17 00:00:00 2001
From: Christian Sutter <git@doublesignal.com>
Date: Fri, 8 Dec 2023 12:26:32 +0000
Subject: [PATCH] Allow sorting of search results

This adds handling of the `order` parameter from the API. Specifically,
it allows for handling the ascending and descending `public_timestamp`
options, whereas the other options should default to no explicit search
ordering ("relevance").
---
 app/services/discovery_engine/search.rb       | 39 +++++++++++++++----
 spec/services/discovery_engine/search_spec.rb | 30 ++++++++++++++
 2 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/app/services/discovery_engine/search.rb b/app/services/discovery_engine/search.rb
index 138a72a..68262ff 100644
--- a/app/services/discovery_engine/search.rb
+++ b/app/services/discovery_engine/search.rb
@@ -2,6 +2,7 @@ module DiscoveryEngine
   class Search
     DEFAULT_PAGE_SIZE = 10
     DEFAULT_OFFSET = 0
+    DEFAULT_ORDER_BY = nil # not specifying an order_by means the results are ordered by relevance
 
     def initialize(
       query_params,
@@ -12,13 +13,7 @@ def initialize(
     end
 
     def result_set
-      response = client.search(
-        query:,
-        serving_config:,
-        page_size:,
-        offset:,
-        boost_spec:,
-      ).response
+      response = client.search(discovery_engine_params).response
 
       ResultSet.new(
         results: response.results.map { Result.from_stored_document(_1.document.struct_data.to_h) },
@@ -31,6 +26,17 @@ def result_set
 
     attr_reader :query_params, :client
 
+    def discovery_engine_params
+      {
+        query:,
+        serving_config:,
+        page_size:,
+        offset:,
+        order_by:,
+        boost_spec:,
+      }.compact
+    end
+
     def query
       query_params[:q].presence || ""
     end
@@ -43,6 +49,25 @@ def offset
       query_params[:start].presence&.to_i || DEFAULT_OFFSET
     end
 
+    def order_by
+      case query_params[:order].presence
+      when "public_timestamp"
+        "public_timestamp"
+      when "-public_timestamp"
+        "public_timestamp desc"
+      when nil, "relevance", "popularity"
+        # "relevance" and "popularity" behave differently on the previous search-api, but we can
+        # treat them the same with Discovery Engine (as empty searches will default to a
+        # popularity-ish order anyway and we don't have an explicit popularity option available).
+        DEFAULT_ORDER_BY
+      else
+        # This helps us spot clients that are sending unexpected values and probably should continue
+        # to use the previoius search-api instead of this API.
+        Rails.logger.warn("Unexpected order_by value: #{query_params[:order].inspect}")
+        DEFAULT_ORDER_BY
+      end
+    end
+
     def serving_config
       Rails.configuration.discovery_engine_serving_config
     end
diff --git a/spec/services/discovery_engine/search_spec.rb b/spec/services/discovery_engine/search_spec.rb
index 1fa3bfa..fdb6fb6 100644
--- a/spec/services/discovery_engine/search_spec.rb
+++ b/spec/services/discovery_engine/search_spec.rb
@@ -77,6 +77,36 @@
         end
       end
 
+      context "when sorting by ascending public timestamp" do
+        let(:query_params) { { q: "garden centres", order: "public_timestamp" } }
+
+        it "calls the client with the expected parameters" do
+          expect(client).to have_received(:search).with(
+            hash_including(order_by: "public_timestamp"),
+          )
+        end
+      end
+
+      context "when sorting by descending public timestamp" do
+        let(:query_params) { { q: "garden centres", order: "-public_timestamp" } }
+
+        it "calls the client with the expected parameters" do
+          expect(client).to have_received(:search).with(
+            hash_including(order_by: "public_timestamp desc"),
+          )
+        end
+      end
+
+      context "when attempting to sort by an unexpected value" do
+        let(:query_params) { { q: "garden centres", order: "foobarbaz" } }
+
+        it "calls the client with the expected parameters" do
+          expect(client).to have_received(:search).with(
+            hash_not_including(:order_by),
+          )
+        end
+      end
+
       context "when searching for a query that has a single best bets defined" do
         # see test section in YAML config
         let(:query_params) { { q: "i want to test a single best bet" } }