From c3598847fe47812e63db4798947c48a80eabc229 Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Mon, 8 Jul 2024 13:58:20 +0800
Subject: [PATCH] DEV: Truncate user agent string when it is too long instead
 of null (#27758)

This is a follow up to 005f623c42b0e18392cc0ad29432961b3df7dccb where
we want to truncate the user agent string instead of nulling out the
column when the user agent string is too low. By truncating, we still
get to retain information that can still be useful.
---
 app/models/search_log.rb       |  7 ++++++-
 spec/models/search_log_spec.rb | 26 +++++++++++++-------------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/app/models/search_log.rb b/app/models/search_log.rb
index 0cbbdee8d18..b134dc04cdf 100644
--- a/app/models/search_log.rb
+++ b/app/models/search_log.rb
@@ -1,7 +1,10 @@
 # frozen_string_literal: true
 
 class SearchLog < ActiveRecord::Base
+  MAXIMUM_USER_AGENT_LENGTH = 2000
+
   validates_presence_of :term
+  validates :user_agent, length: { maximum: MAXIMUM_USER_AGENT_LENGTH }
 
   belongs_to :user
 
@@ -41,7 +44,9 @@ class SearchLog < ActiveRecord::Base
     ip_address = nil if user_id
     key = redis_key(user_id: user_id, ip_address: ip_address)
 
-    user_agent = nil if user_agent && user_agent.length > 2000
+    if user_agent && user_agent.length > MAXIMUM_USER_AGENT_LENGTH
+      user_agent = user_agent.truncate(MAXIMUM_USER_AGENT_LENGTH, omission: "")
+    end
 
     result = nil
 
diff --git a/spec/models/search_log_spec.rb b/spec/models/search_log_spec.rb
index 98f00bbb829..de5c8967f02 100644
--- a/spec/models/search_log_spec.rb
+++ b/spec/models/search_log_spec.rb
@@ -7,30 +7,30 @@ RSpec.describe SearchLog, type: :model do
     context "with invalid arguments" do
       it "no search type returns error" do
         status, _ =
-          SearchLog.log(
-            term: "bounty hunter",
-            search_type: :missing,
-            ip_address: "127.0.0.1",
-            user_agent: "Mozilla",
-          )
+          SearchLog.log(term: "bounty hunter", search_type: :missing, ip_address: "127.0.0.1")
+
         expect(status).to eq(:error)
       end
 
       it "no IP returns error" do
+        status, _ = SearchLog.log(term: "bounty hunter", search_type: :header, ip_address: nil)
+
+        expect(status).to eq(:error)
+      end
+
+      it "truncates the `user_agent` attribute if it exceeds #{described_class::MAXIMUM_USER_AGENT_LENGTH} characters" do
+        user_agent = "a" * (described_class::MAXIMUM_USER_AGENT_LENGTH + 1)
+
         status, _ =
           SearchLog.log(
             term: "bounty hunter",
             search_type: :header,
-            ip_address: nil,
-            user_agent: "Mozilla",
+            user_agent:,
+            ip_address: "127.0.0.1",
           )
-        expect(status).to eq(:error)
-      end
 
-      it "does not error when no user_agent" do
-        status, _ =
-          SearchLog.log(term: "bounty hunter", search_type: :header, ip_address: "127.0.0.1")
         expect(status).to eq(:created)
+        expect(SearchLog.last.user_agent).to eq("a" * described_class::MAXIMUM_USER_AGENT_LENGTH)
       end
     end