From 21e02d696940dcd6123bf001f1b7c687098619df Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 17 Jul 2017 11:57:13 -0400 Subject: [PATCH] Include the `search_log_id` in search results --- app/models/search_log.rb | 5 ++++- .../grouped_search_result_serializer.rb | 7 ++++++- lib/search.rb | 11 ++++++++-- lib/search/grouped_search_results.rb | 18 +++++++++++++---- spec/components/search_spec.rb | 12 +++++++++++ spec/controllers/search_controller_spec.rb | 9 +++++++++ spec/models/search_log_spec.rb | 20 +++++++++++++++++++ 7 files changed, 74 insertions(+), 8 deletions(-) diff --git a/app/models/search_log.rb b/app/models/search_log.rb index cfa2b7232a8..ab873eb3c57 100644 --- a/app/models/search_log.rb +++ b/app/models/search_log.rb @@ -12,6 +12,9 @@ class SearchLog < ActiveRecord::Base def self.log(term:, search_type:, ip_address:, user_id:nil) + search_type = search_types[search_type] + return [:error] unless search_type.present? && ip_address.present? + update_sql = <<~SQL UPDATE search_logs SET term = :term, @@ -35,7 +38,7 @@ class SearchLog < ActiveRecord::Base if rows.cmd_tuples == 0 result = create( term: term, - search_type: search_types[search_type], + search_type: search_type, ip_address: ip_address, user_id: user_id ) diff --git a/app/serializers/grouped_search_result_serializer.rb b/app/serializers/grouped_search_result_serializer.rb index d50cdd565d0..44b833140c6 100644 --- a/app/serializers/grouped_search_result_serializer.rb +++ b/app/serializers/grouped_search_result_serializer.rb @@ -2,5 +2,10 @@ class GroupedSearchResultSerializer < ApplicationSerializer has_many :posts, serializer: SearchPostSerializer has_many :users, serializer: SearchResultUserSerializer has_many :categories, serializer: BasicCategorySerializer - attributes :more_posts, :more_users, :more_categories, :term + attributes :more_posts, :more_users, :more_categories, :term, :search_log_id + + def search_log_id + object.search_log_id + end + end diff --git a/lib/search.rb b/lib/search.rb index 4bb5f1092cb..2f2dfd64207 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -182,7 +182,13 @@ class Search @limit = Search.per_filter end - @results = GroupedSearchResults.new(@opts[:type_filter], clean_term, @search_context, @include_blurbs, @blurb_length) + @results = GroupedSearchResults.new( + @opts[:type_filter], + clean_term, + @search_context, + @include_blurbs, + @blurb_length + ) end def valid? @@ -197,12 +203,13 @@ class Search def execute if SiteSetting.log_search_queries? - SearchLog.log( + status, search_log_id = SearchLog.log( term: @term, search_type: @opts[:search_type], ip_address: @opts[:ip_address], user_id: @opts[:user_id] ) + @results.search_log_id = search_log_id unless status == :error end unless @filters.present? diff --git a/lib/search/grouped_search_results.rb b/lib/search/grouped_search_results.rb index 47e13254883..d6677155c28 100644 --- a/lib/search/grouped_search_results.rb +++ b/lib/search/grouped_search_results.rb @@ -9,10 +9,20 @@ class Search extend ActionView::Helpers::TextHelper end - attr_reader :type_filter, - :posts, :categories, :users, - :more_posts, :more_categories, :more_users, - :term, :search_context, :include_blurbs + attr_reader( + :type_filter, + :posts, + :categories, + :users, + :more_posts, + :more_categories, + :more_users, + :term, + :search_context, + :include_blurbs + ) + + attr_accessor :search_log_id def initialize(type_filter, term, search_context, include_blurbs, blurb_length) @type_filter = type_filter diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index fee0c6008a7..a17c6e8f609 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -790,4 +790,16 @@ describe Search do end end + context "search_log_id" do + it "returns an id when the search succeeds" do + s = Search.new( + 'indiana jones', + search_type: :header, + ip_address: '127.0.0.1' + ) + results = s.execute + expect(results.search_log_id).to be_present + end + end + end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 0aaf369e646..b14034c10fb 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -57,8 +57,17 @@ describe SearchController do it "logs the search term" do SiteSetting.log_search_queries = true xhr :get, :query, term: 'wookie' + expect(response).to be_success expect(SearchLog.where(term: 'wookie')).to be_present + + json = JSON.parse(response.body) + search_log_id = json['grouped_search_result']['search_log_id'] + expect(search_log_id).to be_present + + log = SearchLog.where(id: search_log_id).first + expect(log).to be_present + expect(log.term).to eq('wookie') end it "doesn't log when disabled" do diff --git a/spec/models/search_log_spec.rb b/spec/models/search_log_spec.rb index 0c5f2bf0b00..63f23712fb1 100644 --- a/spec/models/search_log_spec.rb +++ b/spec/models/search_log_spec.rb @@ -4,6 +4,26 @@ RSpec.describe SearchLog, type: :model do describe ".log" do + context "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' + ) + 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 + end + context "when anonymous" do it "logs and updates the search" do Timecop.freeze do