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.
This commit is contained in:
Isaac Janzen 2024-07-05 14:05:00 -05:00 committed by GitHub
parent b36cbc7d21
commit 005f623c42
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 49 additions and 5 deletions

View File

@ -46,6 +46,7 @@ class SearchController < ApplicationController
search_args[:search_type] = :full_page search_args[:search_type] = :full_page
search_args[:ip_address] = request.remote_ip 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[:user_id] = current_user.id if current_user.present?
if rate_limit_search if rate_limit_search
@ -99,6 +100,7 @@ class SearchController < ApplicationController
search_args[:search_type] = :header search_args[:search_type] = :header
search_args[:ip_address] = request.remote_ip 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[:user_id] = current_user.id if current_user.present?
search_args[:restrict_to_archetype] = params[:restrict_to_archetype] if params[ search_args[:restrict_to_archetype] = params[:restrict_to_archetype] if params[
:restrict_to_archetype :restrict_to_archetype

View File

@ -32,7 +32,7 @@ class SearchLog < ActiveRecord::Base
Discourse.redis.keys("__SEARCH__LOG_*").each { |k| Discourse.redis.del(k) } Discourse.redis.keys("__SEARCH__LOG_*").each { |k| Discourse.redis.del(k) }
end 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? return [:error] if term.blank?
search_type = search_types[search_type] search_type = search_types[search_type]
@ -41,6 +41,8 @@ class SearchLog < ActiveRecord::Base
ip_address = nil if user_id ip_address = nil if user_id
key = redis_key(user_id: user_id, ip_address: ip_address) key = redis_key(user_id: user_id, ip_address: ip_address)
user_agent = nil if user_agent && user_agent.length > 2000
result = nil result = nil
if existing = Discourse.redis.get(key) if existing = Discourse.redis.get(key)
@ -55,7 +57,13 @@ class SearchLog < ActiveRecord::Base
if !result if !result
log = 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] result = [:created, log.id]
end end
@ -165,6 +173,7 @@ end
# search_type :integer not null # search_type :integer not null
# created_at :datetime not null # created_at :datetime not null
# search_result_type :integer # search_result_type :integer
# user_agent :string(2000)
# #
# Indexes # Indexes
# #

View File

@ -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

View File

@ -306,6 +306,7 @@ class Search
term: @clean_term, term: @clean_term,
search_type: @opts[:search_type], search_type: @opts[:search_type],
ip_address: @opts[:ip_address], ip_address: @opts[:ip_address],
user_agent: @opts[:user_agent],
user_id: @opts[:user_id], user_id: @opts[:user_id],
) )
@results.search_log_id = search_log_id unless status == :error @results.search_log_id = search_log_id unless status == :error

View File

@ -7,26 +7,49 @@ RSpec.describe SearchLog, type: :model do
context "with invalid arguments" do context "with invalid arguments" do
it "no search type returns error" do it "no search type returns error" do
status, _ = 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) expect(status).to eq(:error)
end end
it "no IP returns error" do 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) expect(status).to eq(:error)
end 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 end
context "when anonymous" do context "when anonymous" do
it "logs and updates the search" do it "logs and updates the search" do
freeze_time freeze_time
action, log_id = 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) expect(action).to eq(:created)
log = SearchLog.find(log_id) log = SearchLog.find(log_id)
expect(log.term).to eq("jabba") expect(log.term).to eq("jabba")
expect(log.search_type).to eq(SearchLog.search_types[:header]) expect(log.search_type).to eq(SearchLog.search_types[:header])
expect(log.ip_address).to eq("192.168.0.33") expect(log.ip_address).to eq("192.168.0.33")
expect(log.user_agent).to eq("Mozilla")
action, updated_log_id = action, updated_log_id =
SearchLog.log(term: "jabba the hut", search_type: :header, ip_address: "192.168.0.33") 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", term: "hello",
search_type: :full_page, search_type: :full_page,
ip_address: "192.168.0.1", ip_address: "192.168.0.1",
user_agent: "Mozilla",
user_id: user.id, user_id: user.id,
) )
expect(action).to eq(:created) expect(action).to eq(:created)
@ -70,6 +94,7 @@ RSpec.describe SearchLog, type: :model do
expect(log.term).to eq("hello") expect(log.term).to eq("hello")
expect(log.search_type).to eq(SearchLog.search_types[:full_page]) expect(log.search_type).to eq(SearchLog.search_types[:full_page])
expect(log.ip_address).to eq(nil) expect(log.ip_address).to eq(nil)
expect(log.user_agent).to eq("Mozilla")
expect(log.user_id).to eq(user.id) expect(log.user_id).to eq(user.id)
action, updated_log_id = action, updated_log_id =