From eab66065d1d4256552d3ada00e243c961a9dc707 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Wed, 20 Dec 2017 08:11:31 +0530 Subject: [PATCH] FEATURE: search log term details page (#5445) --- ....js.es6 => admin-search-logs-index.js.es6} | 0 .../controllers/admin-search-logs-term.js.es6 | 13 ++++++++ .../admin/routes/admin-route-map.js.es6 | 9 +++-- ....js.es6 => admin-search-logs-index.js.es6} | 4 --- .../routes/admin-search-logs-term.js.es6 | 33 +++++++++++++++++++ .../javascripts/admin/templates/logs.hbs | 2 +- .../search-logs.hbs => search-logs-index.hbs} | 4 ++- .../admin/templates/search-logs-term.hbs | 13 ++++++++ .../admin/search_logs_controller.rb | 13 ++++++++ app/models/search_log.rb | 24 ++++++++++++++ config/locales/client.en.yml | 1 + config/locales/server.en.yml | 3 ++ config/routes.rb | 1 + spec/models/search_log_spec.rb | 20 +++++++++++ spec/requests/admin/search_logs_spec.rb | 25 ++++++++++++++ .../admin-search-log-term-test.js.es6 | 10 ++++++ .../helpers/create-pretender.js.es6 | 6 ++++ 17 files changed, 172 insertions(+), 9 deletions(-) rename app/assets/javascripts/admin/controllers/{admin-logs-search-logs.js.es6 => admin-search-logs-index.js.es6} (100%) create mode 100644 app/assets/javascripts/admin/controllers/admin-search-logs-term.js.es6 rename app/assets/javascripts/admin/routes/{admin-logs-search-logs.js.es6 => admin-search-logs-index.js.es6} (85%) create mode 100644 app/assets/javascripts/admin/routes/admin-search-logs-term.js.es6 rename app/assets/javascripts/admin/templates/{logs/search-logs.hbs => search-logs-index.hbs} (88%) create mode 100644 app/assets/javascripts/admin/templates/search-logs-term.hbs create mode 100644 test/javascripts/acceptance/admin-search-log-term-test.js.es6 diff --git a/app/assets/javascripts/admin/controllers/admin-logs-search-logs.js.es6 b/app/assets/javascripts/admin/controllers/admin-search-logs-index.js.es6 similarity index 100% rename from app/assets/javascripts/admin/controllers/admin-logs-search-logs.js.es6 rename to app/assets/javascripts/admin/controllers/admin-search-logs-index.js.es6 diff --git a/app/assets/javascripts/admin/controllers/admin-search-logs-term.js.es6 b/app/assets/javascripts/admin/controllers/admin-search-logs-term.js.es6 new file mode 100644 index 00000000000..98d061e0607 --- /dev/null +++ b/app/assets/javascripts/admin/controllers/admin-search-logs-term.js.es6 @@ -0,0 +1,13 @@ +export default Ember.Controller.extend({ + loading: false, + term: null, + period: "yearly", + 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')}, + {id: 'click_through_only', name: I18n.t('admin.logs.search_logs.types.click_through_only')} + ] +}); diff --git a/app/assets/javascripts/admin/routes/admin-route-map.js.es6 b/app/assets/javascripts/admin/routes/admin-route-map.js.es6 index b221ba639c1..a6ccabc810e 100644 --- a/app/assets/javascripts/admin/routes/admin-route-map.js.es6 +++ b/app/assets/javascripts/admin/routes/admin-route-map.js.es6 @@ -66,10 +66,13 @@ export default function() { this.route('screenedEmails', { path: '/screened_emails' }); this.route('screenedIpAddresses', { path: '/screened_ip_addresses' }); this.route('screenedUrls', { path: '/screened_urls' }); - this.route('searchLogs', { path: '/search_logs' }); + this.route('adminSearchLogs', { path: '/search_logs', resetNamespace: true}, function() { + this.route('index', { path: '/' }); + this.route('term', { path: '/term/:term' }); + }); this.route('adminWatchedWords', { path: '/watched_words', resetNamespace: true}, function() { - this.route('index', { path: '/' } ); - this.route('action', { path: '/action/:action_id' } ); + this.route('index', { path: '/' }); + this.route('action', { path: '/action/:action_id' }); }); }); diff --git a/app/assets/javascripts/admin/routes/admin-logs-search-logs.js.es6 b/app/assets/javascripts/admin/routes/admin-search-logs-index.js.es6 similarity index 85% rename from app/assets/javascripts/admin/routes/admin-logs-search-logs.js.es6 rename to app/assets/javascripts/admin/routes/admin-search-logs-index.js.es6 index 41094b54fa5..08beeb5ea65 100644 --- a/app/assets/javascripts/admin/routes/admin-logs-search-logs.js.es6 +++ b/app/assets/javascripts/admin/routes/admin-search-logs-index.js.es6 @@ -1,10 +1,6 @@ import { ajax } from 'discourse/lib/ajax'; export default Discourse.Route.extend({ - renderTemplate() { - this.render('admin/templates/logs/search-logs', {into: 'adminLogs'}); - }, - queryParams: { period: { refreshModel: true }, searchType: { refreshModel: true } diff --git a/app/assets/javascripts/admin/routes/admin-search-logs-term.js.es6 b/app/assets/javascripts/admin/routes/admin-search-logs-term.js.es6 new file mode 100644 index 00000000000..775fec62c5e --- /dev/null +++ b/app/assets/javascripts/admin/routes/admin-search-logs-term.js.es6 @@ -0,0 +1,33 @@ +import { ajax } from 'discourse/lib/ajax'; + +export default Discourse.Route.extend({ + queryParams: { + period: { refreshModel: true }, + searchType: { refreshModel: true } + }, + + model(params) { + this._params = params; + + return ajax(`/admin/logs/search_logs/term/${params.term}.json`, { + data: { + period: params.period, + search_type: params.searchType + } + }).then(json => { + const model = Ember.Object.create({ type: "search_log_term" }); + model.setProperties(json.term); + return model; + }); + }, + + setupController(controller, model) { + const params = this._params; + controller.setProperties({ + model, + term: params.term, + period: params.period, + searchType: params.searchType + }); + } +}); diff --git a/app/assets/javascripts/admin/templates/logs.hbs b/app/assets/javascripts/admin/templates/logs.hbs index 362c3767699..b936904999a 100644 --- a/app/assets/javascripts/admin/templates/logs.hbs +++ b/app/assets/javascripts/admin/templates/logs.hbs @@ -4,7 +4,7 @@ {{nav-item route='adminLogs.screenedIpAddresses' label='admin.logs.screened_ips.title'}} {{nav-item route='adminLogs.screenedUrls' label='admin.logs.screened_urls.title'}} {{nav-item route='adminWatchedWords' label='admin.watched_words.title'}} - {{nav-item route='adminLogs.searchLogs' label='admin.logs.search_logs.title'}} + {{nav-item route='adminSearchLogs' label='admin.logs.search_logs.title'}} {{#if currentUser.admin}} {{nav-item path='/logs' label='admin.logs.logster.title'}} {{/if}} diff --git a/app/assets/javascripts/admin/templates/logs/search-logs.hbs b/app/assets/javascripts/admin/templates/search-logs-index.hbs similarity index 88% rename from app/assets/javascripts/admin/templates/logs/search-logs.hbs rename to app/assets/javascripts/admin/templates/search-logs-index.hbs index bfd87144da7..257155b4b5a 100644 --- a/app/assets/javascripts/admin/templates/logs/search-logs.hbs +++ b/app/assets/javascripts/admin/templates/search-logs-index.hbs @@ -17,7 +17,9 @@ {{#each model as |item|}}
-
{{item.term}}
+
+ {{#link-to 'adminSearchLogs.term' item.term}}{{item.term}}{{/link-to}} +
{{item.searches}}
{{item.click_through}}
{{item.unique}}
diff --git a/app/assets/javascripts/admin/templates/search-logs-term.hbs b/app/assets/javascripts/admin/templates/search-logs-term.hbs new file mode 100644 index 00000000000..c405a86d358 --- /dev/null +++ b/app/assets/javascripts/admin/templates/search-logs-term.hbs @@ -0,0 +1,13 @@ +

+ {{period-chooser period=period}} + {{combo-box content=searchTypeOptions value=searchType class='search-logs-filter'}} +

+
+ +

+ {{#link-to 'full-page-search' (query-params q=term)}}{{term}}{{/link-to}} +

+ +{{#conditional-loading-spinner condition=refreshing}} + {{admin-graph model=model}} +{{/conditional-loading-spinner}} diff --git a/app/controllers/admin/search_logs_controller.rb b/app/controllers/admin/search_logs_controller.rb index 38b45ce512e..515839b1b50 100644 --- a/app/controllers/admin/search_logs_controller.rb +++ b/app/controllers/admin/search_logs_controller.rb @@ -6,4 +6,17 @@ class Admin::SearchLogsController < Admin::AdminController render_serialized(SearchLog.trending(period&.to_sym, search_type&.to_sym), SearchLogsSerializer) end + def term + params.require(:term) + + term = params[:term] + period = params[:period] || "yearly" + search_type = params[:search_type] || "all" + + details = SearchLog.term_details(term, period&.to_sym, search_type&.to_sym) + raise Discourse::NotFound if details.blank? + + render_json_dump(term: details) + end + end diff --git a/app/models/search_log.rb b/app/models/search_log.rb index 376b3e99b8d..d91b1ae7a1a 100644 --- a/app/models/search_log.rb +++ b/app/models/search_log.rb @@ -57,6 +57,30 @@ class SearchLog < ActiveRecord::Base end end + def self.term_details(term, period = :weekly, search_type = :all) + details = [] + + result = SearchLog.select("COUNT(*) AS count, created_at::date AS date") + .where('term LIKE ?', term) + .where('created_at > ?', start_of(period)) + + result = result.where('search_type = ?', search_types[search_type]) if search_type == :header || search_type == :full_page + result = result.where('search_result_id IS NOT NULL') if search_type == :click_through_only + + result.group(:term) + .order("date") + .group("date") + .each do |record| + details << { x: Date.parse(record['date'].to_s), y: record['count'] } + end + + return { + type: "search_log_term", + title: I18n.t("search_logs.graph_title"), + data: details + } + end + def self.trending(period = :all, search_type = :all) result = SearchLog.select("term, COUNT(*) AS searches, diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6d544020d85..3c8e5aa4c04 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3252,6 +3252,7 @@ en: all_search_types: "All search types" header: "Header" full_page: "Full Page" + click_through_only: "All (click through only)" logster: title: "Error Logs" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 9de917c3c9f..ada766f8ed4 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -3660,3 +3660,6 @@ en: description: |

If you ever feel like changing these settings, visit your admin section; find it next to the wrench icon in the site menu.

Have fun, and good luck building your new community!

+ + search_logs: + graph_title: "Search Count" diff --git a/config/routes.rb b/config/routes.rb index 1b2df375fd8..06962901e08 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -177,6 +177,7 @@ Discourse::Application.routes.draw do end post "watched_words/upload" => "watched_words#upload" resources :search_logs, only: [:index] + get 'search_logs/term/:term' => 'search_logs#term' end get "/logs" => "staff_action_logs#index" diff --git a/spec/models/search_log_spec.rb b/spec/models/search_log_spec.rb index 509d9517aaf..478fc8d0334 100644 --- a/spec/models/search_log_spec.rb +++ b/spec/models/search_log_spec.rb @@ -156,6 +156,26 @@ RSpec.describe SearchLog, type: :model do end end + context "term_details" do + before do + SearchLog.log(term: "ruby", search_type: :header, ip_address: "127.0.0.1") + SearchLog.log(term: 'ruby', search_type: :header, ip_address: '127.0.0.1', user_id: Fabricate(:user).id) + SearchLog.log(term: "ruby", search_type: :full_page, ip_address: "127.0.0.2") + end + + it "correctly returns term details" do + term_details = SearchLog.term_details("ruby") + expect(term_details[:data][0][:y]).to eq(3) + + term_header_details = SearchLog.term_details("ruby", :all, :header) + expect(term_header_details[:data][0][:y]).to eq(2) + + SearchLog.where(term: 'ruby', ip_address: '127.0.0.2').update_all(search_result_id: 24) + term_click_through_details = SearchLog.term_details("ruby", :all, :click_through_only) + expect(term_click_through_details[:data][0][:y]).to eq(1) + end + end + context "trending" do before do SearchLog.log(term: 'ruby', search_type: :header, ip_address: '127.0.0.1') diff --git a/spec/requests/admin/search_logs_spec.rb b/spec/requests/admin/search_logs_spec.rb index 310931b35ec..c4f852b4802 100644 --- a/spec/requests/admin/search_logs_spec.rb +++ b/spec/requests/admin/search_logs_spec.rb @@ -32,4 +32,29 @@ RSpec.describe Admin::SearchLogsController do expect(json[0]['term']).to eq('ruby') end end + + context "#term" do + it "raises an error if you aren't logged in" do + expect do + get '/admin/logs/search_logs/term/ruby.json' + end.to raise_error(ActionController::RoutingError) + end + + it "raises an error if you aren't an admin" do + sign_in(user) + expect do + get '/admin/logs/search_logs/term/ruby.json' + end.to raise_error(ActionController::RoutingError) + end + + it "should work if you are an admin" do + sign_in(admin) + get '/admin/logs/search_logs/term/ruby.json' + + expect(response).to be_success + + json = ::JSON.parse(response.body) + expect(json['term']['type']).to eq('search_log_term') + end + end end diff --git a/test/javascripts/acceptance/admin-search-log-term-test.js.es6 b/test/javascripts/acceptance/admin-search-log-term-test.js.es6 new file mode 100644 index 00000000000..dd6b1e62693 --- /dev/null +++ b/test/javascripts/acceptance/admin-search-log-term-test.js.es6 @@ -0,0 +1,10 @@ +import { acceptance } from "helpers/qunit-helpers"; +acceptance("Admin - Search Log Term", { loggedIn: true }); + +QUnit.test("show search log term details", assert => { + visit("/admin/logs/search_logs/term/ruby"); + andThen(() => { + assert.ok($('div.search-logs-filter').length, "has the search type filter"); + assert.ok(exists('iframe.chartjs-hidden-iframe'), "has graph iframe"); + }); +}); diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6 index 1c18061fa13..b0bb47e3e8d 100644 --- a/test/javascripts/helpers/create-pretender.js.es6 +++ b/test/javascripts/helpers/create-pretender.js.es6 @@ -401,6 +401,12 @@ export default function() { ]); }); + this.get('/admin/logs/search_logs/term/ruby.json', () => { + return response(200, { + "term":{"type":"search_log_term","title":"Search Count","data":[{"x":"2017-07-20","y":2}]} + }); + }); + this.get('/onebox', request => { if (request.queryParams.url === 'http://www.example.com/has-title.html' || request.queryParams.url === 'http://www.example.com/has-title-and-a-url-that-is-more-than-80-characters-because-thats-good-for-seo-i-guess.html') {