diff --git a/app/assets/javascripts/admin/controllers/admin-logs-search-logs.js.es6 b/app/assets/javascripts/admin/controllers/admin-logs-search-logs.js.es6
index 1de32762a6a..83436849fe7 100644
--- a/app/assets/javascripts/admin/controllers/admin-logs-search-logs.js.es6
+++ b/app/assets/javascripts/admin/controllers/admin-logs-search-logs.js.es6
@@ -1,4 +1,11 @@
export default Ember.Controller.extend({
loading: false,
- period: "all"
+ period: "all",
+ searchType: "all",
+
+ searchTypeOptions: [
+ {id: 'all', name: I18n.t('admin.logs.search_logs.types.all_search_types')},
+ {id: 'header', name: I18n.t('admin.logs.search_logs.types.header')},
+ {id: 'full_page', name: I18n.t('admin.logs.search_logs.types.full_page')}
+ ]
});
diff --git a/app/assets/javascripts/admin/routes/admin-logs-search-logs.js.es6 b/app/assets/javascripts/admin/routes/admin-logs-search-logs.js.es6
index 013bdb4655a..41094b54fa5 100644
--- a/app/assets/javascripts/admin/routes/admin-logs-search-logs.js.es6
+++ b/app/assets/javascripts/admin/routes/admin-logs-search-logs.js.es6
@@ -6,20 +6,19 @@ export default Discourse.Route.extend({
},
queryParams: {
- period: {
- refreshModel: true
- }
+ period: { refreshModel: true },
+ searchType: { refreshModel: true }
},
model(params) {
this._params = params;
- return ajax('/admin/logs/search_logs.json', { data: { period: params.period } }).then(search_logs => {
+ return ajax('/admin/logs/search_logs.json', { data: { period: params.period, search_type: params.searchType } }).then(search_logs => {
return search_logs.map(sl => Ember.Object.create(sl));
});
},
setupController(controller, model) {
const params = this._params;
- controller.setProperties({ model, period: params.period });
+ controller.setProperties({ model, period: params.period, searchType: params.searchType });
}
});
diff --git a/app/assets/javascripts/admin/templates/logs/search-logs.hbs b/app/assets/javascripts/admin/templates/logs/search-logs.hbs
index 5b606de9d6d..bfd87144da7 100644
--- a/app/assets/javascripts/admin/templates/logs/search-logs.hbs
+++ b/app/assets/javascripts/admin/templates/logs/search-logs.hbs
@@ -1,5 +1,6 @@
{{period-chooser period=period}}
+ {{combo-box content=searchTypeOptions value=searchType class='search-logs-filter'}}
@@ -11,7 +12,6 @@
{{i18n 'admin.logs.search_logs.term'}}
{{i18n 'admin.logs.search_logs.searches'}}
{{i18n 'admin.logs.search_logs.click_through'}}
- {{i18n 'admin.logs.search_logs.most_viewed_topic'}}
{{i18n 'admin.logs.search_logs.unique'}}
@@ -20,11 +20,6 @@
{{item.term}}
{{item.searches}}
{{item.click_through}}
-
{{item.unique}}
{{/each}}
diff --git a/app/assets/javascripts/discourse/widgets/search-menu-results.js.es6 b/app/assets/javascripts/discourse/widgets/search-menu-results.js.es6
index 705f9e685b0..f00fcf8277f 100644
--- a/app/assets/javascripts/discourse/widgets/search-menu-results.js.es6
+++ b/app/assets/javascripts/discourse/widgets/search-menu-results.js.es6
@@ -24,9 +24,13 @@ function createSearchResult({ type, linkField, builder }) {
return attrs.results.map(r => {
let searchResultId;
+
if (type === "topic") {
searchResultId = r.get('topic_id');
+ } else {
+ searchResultId = r.get('id');
}
+
return h('li', this.attach('link', {
href: r.get(linkField),
contents: () => builder.call(this, r, attrs.term),
diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss
index 9926a73a996..513a076947d 100644
--- a/app/assets/stylesheets/common/admin/admin_base.scss
+++ b/app/assets/stylesheets/common/admin/admin_base.scss
@@ -227,6 +227,11 @@ $mobile-breakpoint: 700px;
.select-box-kit.dropdown-select-box {
width: auto;
}
+
+ .search-logs-filter {
+ width: 200px;
+ float: right;
+ }
}
.admin-container .controls {
@@ -1267,20 +1272,13 @@ table.api-keys {
.search-logs-list{
.col {
text-align: center;
- width: 10%;
+ width: 15%;
}
.col.term {
- width: 30%;
+ width: 45%;
text-align: left;
}
-
- .col.topic {
- width: 35%;
- text-align: left;
- text-overflow: ellipsis;
- white-space: nowrap;
- }
}
.log-details-modal {
diff --git a/app/controllers/admin/search_logs_controller.rb b/app/controllers/admin/search_logs_controller.rb
index a7981eb7383..38b45ce512e 100644
--- a/app/controllers/admin/search_logs_controller.rb
+++ b/app/controllers/admin/search_logs_controller.rb
@@ -2,7 +2,8 @@ class Admin::SearchLogsController < Admin::AdminController
def index
period = params[:period] || "all"
- render_serialized(SearchLog.trending(period.to_sym), SearchLogsSerializer)
+ search_type = params[:search_type] || "all"
+ render_serialized(SearchLog.trending(period&.to_sym, search_type&.to_sym), SearchLogsSerializer)
end
end
diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb
index b0d44da59dc..ad74f595745 100644
--- a/app/controllers/search_controller.rb
+++ b/app/controllers/search_controller.rb
@@ -77,7 +77,8 @@ class SearchController < ApplicationController
params.require(:search_result_type)
params.require(:search_result_id)
- if params[:search_result_type] == 'topic'
+ search_result_type = params[:search_result_type].downcase.to_sym
+ if SearchLog.search_result_types.has_key?(search_result_type)
attributes = { id: params[:search_log_id] }
if current_user.present?
attributes[:user_id] = current_user.id
@@ -85,8 +86,15 @@ class SearchController < ApplicationController
attributes[:ip_address] = request.remote_ip
end
+ if search_result_type == :tag
+ search_result_id = Tag.find_by_name(params[:search_result_id])&.id
+ else
+ search_result_id = params[:search_result_id]
+ end
+
SearchLog.where(attributes).update_all(
- clicked_topic_id: params[:search_result_id]
+ search_result_type: SearchLog.search_result_types[search_result_type],
+ search_result_id: search_result_id
)
end
diff --git a/app/models/search_log.rb b/app/models/search_log.rb
index 8308a244437..4ab7672ca41 100644
--- a/app/models/search_log.rb
+++ b/app/models/search_log.rb
@@ -1,7 +1,6 @@
require_dependency 'enum'
class SearchLog < ActiveRecord::Base
- belongs_to :topic, foreign_key: :clicked_topic_id
validates_presence_of :term, :ip_address
def self.search_types
@@ -11,6 +10,15 @@ class SearchLog < ActiveRecord::Base
)
end
+ def self.search_result_types
+ @search_result_types ||= Enum.new(
+ topic: 1,
+ user: 2,
+ category: 3,
+ tag: 4
+ )
+ end
+
def self.log(term:, search_type:, ip_address:, user_id: nil)
search_type = search_types[search_type]
@@ -49,19 +57,19 @@ class SearchLog < ActiveRecord::Base
end
end
- def self.trending(period = :all)
- SearchLog.select("term,
- COUNT(*) AS searches,
- SUM(CASE
- WHEN clicked_topic_id IS NOT NULL THEN 1
- ELSE 0
- END) AS click_through,
- MODE() WITHIN GROUP (ORDER BY clicked_topic_id) AS clicked_topic_id,
- COUNT(DISTINCT ip_address) AS unique")
- .includes(:topic)
+ def self.trending(period = :all, search_type = :all)
+ result = SearchLog.select("term,
+ COUNT(*) AS searches,
+ SUM(CASE
+ WHEN search_result_id IS NOT NULL THEN 1
+ ELSE 0
+ END) AS click_through,
+ COUNT(DISTINCT ip_address) AS unique")
.where('created_at > ?', start_of(period))
- .group(:term)
- .order('COUNT(DISTINCT ip_address) DESC')
+
+ result = result.where('search_type = ?', search_types[search_type]) unless search_type == :all
+ result = result.group(:term)
+ .order('COUNT(DISTINCT ip_address) DESC, COUNT(*) DESC')
.limit(100).to_a
end
diff --git a/app/serializers/search_logs_serializer.rb b/app/serializers/search_logs_serializer.rb
index 434ef5dec9c..f059c33cb61 100644
--- a/app/serializers/search_logs_serializer.rb
+++ b/app/serializers/search_logs_serializer.rb
@@ -2,16 +2,5 @@ class SearchLogsSerializer < ApplicationSerializer
attributes :term,
:searches,
:click_through,
- :clicked_topic_id,
- :topic_title,
- :topic_url,
:unique
-
- def topic_title
- object&.topic&.title
- end
-
- def topic_url
- object&.topic&.url
- end
end
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 0721af6522c..e491bc1f762 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -3209,9 +3209,12 @@ en:
term: "Term"
searches: "Searches"
click_through: "Click Through"
- most_viewed_topic: "Most Viewed Topic"
unique: "Unique"
unique_title: "unique users performing the search"
+ types:
+ all_search_types: "All search types"
+ header: "Header"
+ full_page: "Full Page"
logster:
title: "Error Logs"
diff --git a/db/migrate/20171128172835_rename_clicked_topic_id_to_search_result_id.rb b/db/migrate/20171128172835_rename_clicked_topic_id_to_search_result_id.rb
new file mode 100644
index 00000000000..998dc7b95e5
--- /dev/null
+++ b/db/migrate/20171128172835_rename_clicked_topic_id_to_search_result_id.rb
@@ -0,0 +1,13 @@
+class RenameClickedTopicIdToSearchResultId < ActiveRecord::Migration[5.1]
+ def up
+ rename_column :search_logs, :clicked_topic_id, :search_result_id
+ add_column :search_logs, :search_result_type, :integer, null: true
+
+ execute "UPDATE search_logs SET search_result_type = 1 WHERE search_result_id is NOT NULL"
+ end
+
+ def down
+ rename_column :search_logs, :search_result_id, :clicked_topic_id
+ remove_column :search_logs, :search_result_type
+ end
+end
diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb
index 1cc46050392..d6cec92d0b3 100644
--- a/spec/controllers/search_controller_spec.rb
+++ b/spec/controllers/search_controller_spec.rb
@@ -187,14 +187,14 @@ describe SearchController do
}
expect(response).to be_success
- expect(SearchLog.find(search_log_id).clicked_topic_id).to be_blank
+ expect(SearchLog.find(search_log_id).search_result_id).to be_blank
end
it "records the click for a logged in user" do
user = log_in(:user)
_, search_log_id = SearchLog.log(
- term: 'kitty',
+ term: 'foobar',
search_type: :header,
user_id: user.id,
ip_address: '127.0.0.1'
@@ -203,11 +203,12 @@ describe SearchController do
post :click, params: {
search_log_id: search_log_id,
search_result_id: 12345,
- search_result_type: 'topic'
+ search_result_type: 'user'
}, format: :json
expect(response).to be_success
- expect(SearchLog.find(search_log_id).clicked_topic_id).to eq(12345)
+ expect(SearchLog.find(search_log_id).search_result_id).to eq(12345)
+ expect(SearchLog.find(search_log_id).search_result_type).to eq(SearchLog.search_result_types[:user])
end
it "records the click for an anonymous user" do
@@ -226,7 +227,8 @@ describe SearchController do
}, format: :json
expect(response).to be_success
- expect(SearchLog.find(search_log_id).clicked_topic_id).to eq(22222)
+ expect(SearchLog.find(search_log_id).search_result_id).to eq(22222)
+ expect(SearchLog.find(search_log_id).search_result_type).to eq(SearchLog.search_result_types[:topic])
end
it "doesn't record the click for a different IP" do
@@ -245,7 +247,48 @@ describe SearchController do
}
expect(response).to be_success
- expect(SearchLog.find(search_log_id).clicked_topic_id).to be_blank
+ expect(SearchLog.find(search_log_id).search_result_id).to be_blank
+ end
+
+ it "records the click for search result type category" do
+ request.remote_addr = '192.168.0.1';
+
+ _, search_log_id = SearchLog.log(
+ term: 'dev',
+ search_type: :header,
+ ip_address: '192.168.0.1'
+ )
+
+ post :click, params: {
+ search_log_id: search_log_id,
+ search_result_id: 23456,
+ search_result_type: 'category'
+ }, format: :json
+
+ expect(response).to be_success
+ expect(SearchLog.find(search_log_id).search_result_id).to eq(23456)
+ expect(SearchLog.find(search_log_id).search_result_type).to eq(SearchLog.search_result_types[:category])
+ end
+
+ it "records the click for search result type tag" do
+ request.remote_addr = '192.168.0.1';
+ tag = Fabricate(:tag, name: 'test')
+
+ _, search_log_id = SearchLog.log(
+ term: 'test',
+ search_type: :header,
+ ip_address: '192.168.0.1'
+ )
+
+ post :click, params: {
+ search_log_id: search_log_id,
+ search_result_id: tag.name,
+ search_result_type: 'tag'
+ }, format: :json
+
+ expect(response).to be_success
+ expect(SearchLog.find(search_log_id).search_result_id).to eq(tag.id)
+ expect(SearchLog.find(search_log_id).search_result_type).to eq(SearchLog.search_result_types[:tag])
end
end
end
diff --git a/spec/models/search_log_spec.rb b/spec/models/search_log_spec.rb
index 3bc2f39106b..509d9517aaf 100644
--- a/spec/models/search_log_spec.rb
+++ b/spec/models/search_log_spec.rb
@@ -179,15 +179,11 @@ RSpec.describe SearchLog, type: :model do
expect(top_trending.searches).to eq(3)
expect(top_trending.unique).to eq(2)
expect(top_trending.click_through).to eq(0)
- expect(top_trending.clicked_topic_id).to eq(nil)
- popular_topic = Fabricate(:topic)
- not_so_popular_topic = Fabricate(:topic)
- SearchLog.where(term: 'ruby', ip_address: '127.0.0.1').update_all(clicked_topic_id: popular_topic.id)
- SearchLog.where(term: 'ruby', ip_address: '127.0.0.2').update_all(clicked_topic_id: not_so_popular_topic.id)
+ SearchLog.where(term: 'ruby', ip_address: '127.0.0.1').update_all(search_result_id: 12)
+ SearchLog.where(term: 'ruby', ip_address: '127.0.0.2').update_all(search_result_id: 24)
top_trending = SearchLog.trending.first
expect(top_trending.click_through).to eq(3)
- expect(top_trending.clicked_topic_id).to eq(popular_topic.id)
end
end
diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6
index 81539e88dfb..56c119ad85c 100644
--- a/test/javascripts/helpers/create-pretender.js.es6
+++ b/test/javascripts/helpers/create-pretender.js.es6
@@ -393,7 +393,7 @@ export default function() {
this.get('/admin/logs/search_logs.json', () => {
return response(200, [
- {"term":"foobar","searches":35,"click_through":6,"clicked_topic_id":1550,"topic_title":"Foo Bar Topic Title","topic_url":"http://discourse.example.com/t/foo-bar-topic-title/1550","unique":16}
+ {"term":"foobar","searches":35,"click_through":6,"unique":16}
]);
});