From 005f623c42b0e18392cc0ad29432961b3df7dccb Mon Sep 17 00:00:00 2001 From: Isaac Janzen <50783505+janzenisaac@users.noreply.github.com> Date: Fri, 5 Jul 2024 14:05:00 -0500 Subject: [PATCH] DEV: Add `user_agent` column to `search_logs` (#27742) Add a new column - `user_agent` - to the `SearchLog` table. This column can be null as we are only allowing a the user-agent string to have a max length of 2000 characters. In the case the user-agent string surpasses the max characters allowed, we simply nullify the value, and save/write the log as normal. --- app/controllers/search_controller.rb | 2 ++ app/models/search_log.rb | 13 ++++++-- ...705153533_add_user_agent_to_search_logs.rb | 7 +++++ lib/search.rb | 1 + spec/models/search_log_spec.rb | 31 +++++++++++++++++-- 5 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20240705153533_add_user_agent_to_search_logs.rb diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 8de199f6af2..c868db5f691 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -46,6 +46,7 @@ class SearchController < ApplicationController search_args[:search_type] = :full_page search_args[:ip_address] = request.remote_ip + search_args[:user_agent] = request.user_agent search_args[:user_id] = current_user.id if current_user.present? if rate_limit_search @@ -99,6 +100,7 @@ class SearchController < ApplicationController search_args[:search_type] = :header search_args[:ip_address] = request.remote_ip + search_args[:user_agent] = request.user_agent search_args[:user_id] = current_user.id if current_user.present? search_args[:restrict_to_archetype] = params[:restrict_to_archetype] if params[ :restrict_to_archetype diff --git a/app/models/search_log.rb b/app/models/search_log.rb index 46606367b2a..0cbbdee8d18 100644 --- a/app/models/search_log.rb +++ b/app/models/search_log.rb @@ -32,7 +32,7 @@ class SearchLog < ActiveRecord::Base Discourse.redis.keys("__SEARCH__LOG_*").each { |k| Discourse.redis.del(k) } end - def self.log(term:, search_type:, ip_address:, user_id: nil) + def self.log(term:, search_type:, ip_address:, user_agent: nil, user_id: nil) return [:error] if term.blank? search_type = search_types[search_type] @@ -41,6 +41,8 @@ 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 + result = nil if existing = Discourse.redis.get(key) @@ -55,7 +57,13 @@ class SearchLog < ActiveRecord::Base if !result log = - self.create!(term: term, search_type: search_type, ip_address: ip_address, user_id: user_id) + self.create!( + term: term, + search_type: search_type, + ip_address: ip_address, + user_agent: user_agent, + user_id: user_id, + ) result = [:created, log.id] end @@ -165,6 +173,7 @@ end # search_type :integer not null # created_at :datetime not null # search_result_type :integer +# user_agent :string(2000) # # Indexes # diff --git a/db/migrate/20240705153533_add_user_agent_to_search_logs.rb b/db/migrate/20240705153533_add_user_agent_to_search_logs.rb new file mode 100644 index 00000000000..705b4ccf094 --- /dev/null +++ b/db/migrate/20240705153533_add_user_agent_to_search_logs.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddUserAgentToSearchLogs < ActiveRecord::Migration[7.1] + def change + add_column :search_logs, :user_agent, :string, null: true, limit: 2000 + end +end diff --git a/lib/search.rb b/lib/search.rb index a265a6ecbd8..ca48fb33f55 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -306,6 +306,7 @@ class Search term: @clean_term, search_type: @opts[:search_type], ip_address: @opts[:ip_address], + user_agent: @opts[:user_agent], user_id: @opts[:user_id], ) @results.search_log_id = search_log_id unless status == :error diff --git a/spec/models/search_log_spec.rb b/spec/models/search_log_spec.rb index efe27dd1171..98f00bbb829 100644 --- a/spec/models/search_log_spec.rb +++ b/spec/models/search_log_spec.rb @@ -7,26 +7,49 @@ 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") + SearchLog.log( + term: "bounty hunter", + search_type: :missing, + ip_address: "127.0.0.1", + user_agent: "Mozilla", + ) expect(status).to eq(:error) end it "no IP returns error" do - status, _ = SearchLog.log(term: "bounty hunter", search_type: :header, ip_address: nil) + status, _ = + SearchLog.log( + term: "bounty hunter", + search_type: :header, + ip_address: nil, + user_agent: "Mozilla", + ) 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) + end end context "when anonymous" do it "logs and updates the search" do freeze_time action, log_id = - SearchLog.log(term: "jabba", search_type: :header, ip_address: "192.168.0.33") + SearchLog.log( + term: "jabba", + search_type: :header, + ip_address: "192.168.0.33", + user_agent: "Mozilla", + ) expect(action).to eq(:created) log = SearchLog.find(log_id) expect(log.term).to eq("jabba") expect(log.search_type).to eq(SearchLog.search_types[:header]) expect(log.ip_address).to eq("192.168.0.33") + expect(log.user_agent).to eq("Mozilla") action, updated_log_id = SearchLog.log(term: "jabba the hut", search_type: :header, ip_address: "192.168.0.33") @@ -63,6 +86,7 @@ RSpec.describe SearchLog, type: :model do term: "hello", search_type: :full_page, ip_address: "192.168.0.1", + user_agent: "Mozilla", user_id: user.id, ) expect(action).to eq(:created) @@ -70,6 +94,7 @@ RSpec.describe SearchLog, type: :model do expect(log.term).to eq("hello") expect(log.search_type).to eq(SearchLog.search_types[:full_page]) expect(log.ip_address).to eq(nil) + expect(log.user_agent).to eq("Mozilla") expect(log.user_id).to eq(user.id) action, updated_log_id =