From d99deaf1ab27e24e42a88140a67aabfc907acd8c Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Thu, 25 Nov 2021 15:44:15 -0500 Subject: [PATCH] FEATURE: show recent searches in quick search panel (#15024) --- .../app/controllers/full-page-search.js | 4 + .../javascripts/discourse/app/lib/search.js | 14 +++ .../javascripts/discourse/app/models/user.js | 8 ++ .../app/widgets/search-menu-results.js | 60 +++++++++++- .../discourse/app/widgets/search-menu.js | 9 +- .../discourse/tests/acceptance/search-test.js | 22 +++++ .../discourse/tests/fixtures/user-fixtures.js | 7 ++ .../stylesheets/common/base/search-menu.scss | 22 ++++- app/controllers/users_controller.rb | 32 ++++++- app/models/search_log.rb | 3 +- app/models/user_option.rb | 1 + app/serializers/user_option_serializer.rb | 1 + config/locales/client.en.yml | 2 + config/locales/server.en.yml | 1 + config/routes.rb | 2 + .../20211123144714_add_recent_searches.rb | 9 ++ lib/search.rb | 2 +- .../api/schemas/json/user_get_response.json | 3 + spec/requests/users_controller_spec.rb | 91 +++++++++++++++++++ 19 files changed, 286 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20211123144714_add_recent_searches.rb diff --git a/app/assets/javascripts/discourse/app/controllers/full-page-search.js b/app/assets/javascripts/discourse/app/controllers/full-page-search.js index d27a5734f51..d75bc34c9b3 100644 --- a/app/assets/javascripts/discourse/app/controllers/full-page-search.js +++ b/app/assets/javascripts/discourse/app/controllers/full-page-search.js @@ -5,6 +5,7 @@ import { isValidSearchTerm, searchContextDescription, translateResults, + updateRecentSearches, } from "discourse/lib/search"; import Category from "discourse/models/category"; import Composer from "discourse/models/composer"; @@ -345,6 +346,9 @@ export default Controller.extend({ }); break; default: + if (this.currentUser) { + updateRecentSearches(this.currentUser, searchTerm); + } ajax("/search", { data: args }) .then(async (results) => { const model = (await translateResults(results)) || {}; diff --git a/app/assets/javascripts/discourse/app/lib/search.js b/app/assets/javascripts/discourse/app/lib/search.js index 09cdeaea253..6df0545edea 100644 --- a/app/assets/javascripts/discourse/app/lib/search.js +++ b/app/assets/javascripts/discourse/app/lib/search.js @@ -17,6 +17,7 @@ import { userPath } from "discourse/lib/url"; import userSearch from "discourse/lib/user-search"; const translateResultsCallbacks = []; +const MAX_RECENT_SEARCHES = 5; // should match backend constant with the same name export function addSearchResultsCallback(callback) { translateResultsCallbacks.push(callback); @@ -230,3 +231,16 @@ export function applySearchAutocomplete($input, siteSettings) { ); } } + +export function updateRecentSearches(currentUser, term) { + let recentSearches = Object.assign(currentUser.recent_searches || []); + + if (recentSearches.includes(term)) { + recentSearches = recentSearches.without(term); + } else if (recentSearches.length === MAX_RECENT_SEARCHES) { + recentSearches.popObject(); + } + + recentSearches.unshiftObject(term); + currentUser.set("recent_searches", recentSearches); +} diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index 2876dd604bb..21434572c80 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -1072,6 +1072,14 @@ User.reopenClass(Singleton, { return ajax(userPath("check_email"), { data: { email } }); }, + loadRecentSearches() { + return ajax(`/u/recent-searches`); + }, + + resetRecentSearches() { + return ajax(`/u/recent-searches`, { type: "DELETE" }); + }, + groupStats(stats) { const responses = UserActionStat.create({ count: 0, diff --git a/app/assets/javascripts/discourse/app/widgets/search-menu-results.js b/app/assets/javascripts/discourse/app/widgets/search-menu-results.js index d607ad3de51..ae3d08bbe22 100644 --- a/app/assets/javascripts/discourse/app/widgets/search-menu-results.js +++ b/app/assets/javascripts/discourse/app/widgets/search-menu-results.js @@ -8,10 +8,12 @@ import { dateNode } from "discourse/helpers/node"; import { emojiUnescape } from "discourse/lib/text"; import getURL from "discourse-common/lib/get-url"; import { h } from "virtual-dom"; +import hbs from "discourse/widgets/hbs-compiler"; import highlightSearch from "discourse/lib/highlight-search"; import { iconNode } from "discourse-common/lib/icon-library"; import renderTag from "discourse/lib/render-tag"; import { MODIFIER_REGEXP } from "discourse/widgets/search-menu"; +import User from "discourse/models/user"; const suggestionShortcuts = [ "in:title", @@ -585,6 +587,14 @@ createWidget("search-menu-initial-options", { if (content.length === 0) { content.push(this.attach("random-quick-tip")); + + if (this.currentUser && this.siteSettings.log_search_queries) { + if (this.currentUser.recent_searches?.length) { + content.push(this.attach("search-menu-recent-searches")); + } else { + this.loadRecentSearches(); + } + } } return content; @@ -602,6 +612,22 @@ createWidget("search-menu-initial-options", { ], }); }, + + refreshSearchMenuResults() { + this.scheduleRerender(); + }, + + loadRecentSearches() { + User.loadRecentSearches().then((result) => { + if (result.success && result.recent_searches?.length) { + this.currentUser.set( + "recent_searches", + Object.assign(result.recent_searches) + ); + this.scheduleRerender(); + } + }); + }, }); createWidget("search-menu-assistant-item", { @@ -612,7 +638,7 @@ createWidget("search-menu-assistant-item", { const attributes = {}; attributes.href = "#"; - let content = [iconNode("search")]; + let content = [iconNode(attrs.icon || "search")]; if (prefix) { content.push(h("span.search-item-prefix", `${prefix} `)); @@ -702,3 +728,35 @@ createWidget("random-quick-tip", { } }, }); + +createWidget("search-menu-recent-searches", { + tagName: "div.search-menu-recent", + + template: hbs` +
+

{{i18n "search.recent"}}

+ {{flat-button + className="clear-recent-searches" + title="search.clear_recent" + icon="times" + action="clearRecent" + }} +
+ + {{#each this.currentUser.recent_searches as |slug|}} + {{attach + widget="search-menu-assistant-item" + attrs=(hash slug=slug icon="history") + }} + {{/each}} + `, + + clearRecent() { + return User.resetRecentSearches().then((result) => { + if (result.success) { + this.currentUser.recent_searches.clear(); + this.sendWidgetAction("refreshSearchMenuResults"); + } + }); + }, +}); diff --git a/app/assets/javascripts/discourse/app/widgets/search-menu.js b/app/assets/javascripts/discourse/app/widgets/search-menu.js index 8a76a330147..78f16baf163 100644 --- a/app/assets/javascripts/discourse/app/widgets/search-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/search-menu.js @@ -1,4 +1,8 @@ -import { isValidSearchTerm, searchForTerm } from "discourse/lib/search"; +import { + isValidSearchTerm, + searchForTerm, + updateRecentSearches, +} from "discourse/lib/search"; import DiscourseURL from "discourse/lib/url"; import { createWidget } from "discourse/widgets/widget"; import discourseDebounce from "discourse-common/lib/debounce"; @@ -456,6 +460,9 @@ export default createWidget("search-menu", { searchData.loading = true; cancel(this.state._debouncer); SearchHelper.perform(this); + if (this.currentUser) { + updateRecentSearches(this.currentUser, searchData.term); + } } else { searchData.loading = false; if (!this.state.inTopicContext) { diff --git a/app/assets/javascripts/discourse/tests/acceptance/search-test.js b/app/assets/javascripts/discourse/tests/acceptance/search-test.js index 6be320421a7..f5008978de2 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/search-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/search-test.js @@ -332,6 +332,7 @@ acceptance("Search - Anonymous", function (needs) { acceptance("Search - Authenticated", function (needs) { needs.user(); + needs.settings({ log_search_queries: true }); needs.pretender((server, helper) => { server.get("/search/query", (request) => { @@ -506,6 +507,27 @@ acceptance("Search - Authenticated", function (needs) { await triggerKeyEvent("#search-term", "keydown", keyEnter); assert.ok(exists(query(`.search-menu`)), "search dropdown is visible"); }); + + test("Shows recent search results", async function (assert) { + await visit("/"); + await click("#search-button"); + + assert.strictEqual( + query( + ".search-menu .search-menu-recent li:nth-of-type(1) .search-link" + ).textContent.trim(), + "yellow", + "shows first recent search" + ); + + assert.strictEqual( + query( + ".search-menu .search-menu-recent li:nth-of-type(2) .search-link" + ).textContent.trim(), + "blue", + "shows second recent search" + ); + }); }); acceptance("Search - with tagging enabled", function (needs) { diff --git a/app/assets/javascripts/discourse/tests/fixtures/user-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/user-fixtures.js index 102d369ac80..0883ca4a6f9 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/user-fixtures.js +++ b/app/assets/javascripts/discourse/tests/fixtures/user-fixtures.js @@ -3474,4 +3474,11 @@ export default { timezone: "Australia/Brisbane", }, }, + "/u/recent-searches": { + success: "OK", + recent_searches: [ + "yellow", + "blue" + ] + } }; diff --git a/app/assets/stylesheets/common/base/search-menu.scss b/app/assets/stylesheets/common/base/search-menu.scss index 32935f37a87..898e5e60b58 100644 --- a/app/assets/stylesheets/common/base/search-menu.scss +++ b/app/assets/stylesheets/common/base/search-menu.scss @@ -229,6 +229,7 @@ $search-pad-horizontal: 0.5em; .search-result-tag, .search-menu-assistant { .search-link { + @include ellipsis; .d-icon { margin-right: 5px; vertical-align: middle; @@ -246,8 +247,7 @@ $search-pad-horizontal: 0.5em; .browser-search-tip, .search-random-quick-tip { - padding: $search-pad-vertical $search-pad-horizontal; - padding-bottom: 0; + padding: $search-pad-vertical 1px; font-size: var(--font-down-2); color: var(--primary-medium); .tip-label { @@ -261,6 +261,24 @@ $search-pad-horizontal: 0.5em; } } + .search-menu-recent { + @include separator; + + .heading { + display: flex; + justify-content: space-between; + h4 { + color: var(--primary-medium); + font-weight: normal; + margin-bottom: 0; + } + .clear-recent-searches { + cursor: pointer; + color: var(--primary-low-mid); + } + } + } + .browser-search-tip { padding-top: 0.5em; } diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 3e2a815949d..efc2b4984d9 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -11,7 +11,8 @@ class UsersController < ApplicationController :update_second_factor, :create_second_factor_backup, :select_avatar, :notification_level, :revoke_auth_token, :register_second_factor_security_key, :create_second_factor_security_key, :feature_topic, :clear_featured_topic, - :bookmarks, :invited, :check_sso_email, :check_sso_payload + :bookmarks, :invited, :check_sso_email, :check_sso_payload, + :recent_searches, :reset_recent_searches ] skip_before_action :check_xhr, only: [ @@ -50,6 +51,8 @@ class UsersController < ApplicationController after_action :add_noindex_header, only: [:show, :my_redirect] + MAX_RECENT_SEARCHES = 5 + def index end @@ -1302,6 +1305,33 @@ class UsersController < ApplicationController render json: success_json end + def recent_searches + if !SiteSetting.log_search_queries + return render json: failed_json.merge( + error: I18n.t("user_activity.no_log_search_queries") + ), status: 403 + end + + query = SearchLog.where(user_id: current_user.id) + + if current_user.user_option.oldest_search_log_date + query = query + .where("created_at > ?", current_user.user_option.oldest_search_log_date) + end + + results = query.group(:term) + .order("max(created_at) DESC") + .limit(MAX_RECENT_SEARCHES) + .pluck(:term) + + render json: success_json.merge(recent_searches: results) + end + + def reset_recent_searches + current_user.user_option.update!(oldest_search_log_date: 1.second.ago) + render json: success_json + end + def staff_info @user = fetch_user_from_params(include_inactive: true) guardian.ensure_can_see_staff_info!(@user) diff --git a/app/models/search_log.rb b/app/models/search_log.rb index 003331250bc..ecaa2db6e79 100644 --- a/app/models/search_log.rb +++ b/app/models/search_log.rb @@ -187,5 +187,6 @@ end # # Indexes # -# index_search_logs_on_created_at (created_at) +# index_search_logs_on_created_at (created_at) +# index_search_logs_on_user_id_and_created_at (user_id,created_at) WHERE (user_id IS NOT NULL) # diff --git a/app/models/user_option.rb b/app/models/user_option.rb index 47afb7d2d29..9a396827bf7 100644 --- a/app/models/user_option.rb +++ b/app/models/user_option.rb @@ -259,6 +259,7 @@ end # skip_new_user_tips :boolean default(FALSE), not null # color_scheme_id :integer # default_calendar :integer default("none_selected"), not null +# oldest_search_log_date :datetime # # Indexes # diff --git a/app/serializers/user_option_serializer.rb b/app/serializers/user_option_serializer.rb index a4c9826ba3c..a086a6f3ee6 100644 --- a/app/serializers/user_option_serializer.rb +++ b/app/serializers/user_option_serializer.rb @@ -34,6 +34,7 @@ class UserOptionSerializer < ApplicationSerializer :timezone, :skip_new_user_tips, :default_calendar, + :oldest_search_log_date, def auto_track_topics_after_msecs object.auto_track_topics_after_msecs || SiteSetting.default_other_auto_track_topics_after_msecs diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 30d77f99d20..556c6567652 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2384,6 +2384,8 @@ en: in_posts_by: "in posts by %{username}" browser_tip: "%{modifier} + f" browser_tip_description: "again to use native browser search" + recent: "Recent Searches" + clear_recent: "Clear Recent Searches" type: default: "Topics/posts" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c090367f9e5..6f8f2f95771 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -968,6 +968,7 @@ en: others: "No bookmarks." no_drafts: self: "You have no drafts; begin composing a reply in any topic and it will be auto-saved as a new draft." + no_log_search_queries: "Search log queries are currently disabled (an administrator can enable them in site settings)." email_settings: pop3_authentication_error: "There was an issue with the POP3 credentials provided, check the username and password and try again." diff --git a/config/routes.rb b/config/routes.rb index 943a773fff5..4444f1e92d3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -428,6 +428,8 @@ Discourse::Application.routes.draw do put "#{root_path}/admin-login" => "users#admin_login" post "#{root_path}/toggle-anon" => "users#toggle_anon" post "#{root_path}/read-faq" => "users#read_faq" + get "#{root_path}/recent-searches" => "users#recent_searches", constraints: { format: 'json' } + delete "#{root_path}/recent-searches" => "users#reset_recent_searches", constraints: { format: 'json' } get "#{root_path}/search/users" => "users#search_users" get({ "#{root_path}/account-created/" => "users#account_created" }.merge(index == 1 ? { as: :users_account_created } : { as: :old_account_created })) diff --git a/db/migrate/20211123144714_add_recent_searches.rb b/db/migrate/20211123144714_add_recent_searches.rb new file mode 100644 index 00000000000..0e586f6afb2 --- /dev/null +++ b/db/migrate/20211123144714_add_recent_searches.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddRecentSearches < ActiveRecord::Migration[6.1] + def change + add_column :user_options, :oldest_search_log_date, :datetime + + add_index :search_logs, [:user_id, :created_at], where: 'user_id IS NOT NULL' + end +end diff --git a/lib/search.rb b/lib/search.rb index 6230955b6a9..c5abbc503ea 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -254,7 +254,7 @@ class Search def execute(readonly_mode: Discourse.readonly_mode?) if log_query?(readonly_mode) status, search_log_id = SearchLog.log( - term: @term, + term: @clean_term, search_type: @opts[:search_type], ip_address: @opts[:ip_address], user_id: @opts[:user_id] diff --git a/spec/requests/api/schemas/json/user_get_response.json b/spec/requests/api/schemas/json/user_get_response.json index d0f019cb720..d6261a91bc1 100644 --- a/spec/requests/api/schemas/json/user_get_response.json +++ b/spec/requests/api/schemas/json/user_get_response.json @@ -778,6 +778,9 @@ }, "default_calendar": { "type": "string" + }, + "oldest_search_log_date": { + "type": ["string", "null"] } }, "required": [ diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index bbe67e69ad0..637cbe399dc 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -5125,6 +5125,97 @@ describe UsersController do end end + describe "#reset_recent_searches" do + fab!(:user) { Fabricate(:user) } + + it 'does nothing for anon' do + delete "/u/recent-searches.json" + expect(response.status).to eq(403) + end + + it 'works for logged in user' do + sign_in(user) + delete "/u/recent-searches.json" + + expect(response.status).to eq(200) + user.reload + expect(user.user_option.oldest_search_log_date).to be_within(5.seconds).of(1.second.ago) + end + end + + describe "#recent_searches" do + fab!(:user) { Fabricate(:user) } + + it 'does nothing for anon' do + get "/u/recent-searches.json" + expect(response.status).to eq(403) + end + + it 'works for logged in user' do + sign_in(user) + SiteSetting.log_search_queries = true + user.user_option.update!(oldest_search_log_date: nil) + + get "/u/recent-searches.json" + + expect(response.status).to eq(200) + expect(response.parsed_body["recent_searches"]).to eq([]) + + SearchLog.create!( + term: "old one", + user_id: user.id, + search_type: 1, + ip_address: '192.168.0.1', + created_at: 5.minutes.ago + ) + SearchLog.create!( + term: "also old", + user_id: user.id, + search_type: 1, + ip_address: '192.168.0.1', + created_at: 15.minutes.ago + ) + + get "/u/recent-searches.json" + expect(response.status).to eq(200) + expect(response.parsed_body["recent_searches"]).to eq(["old one", "also old"]) + + user.user_option.update!(oldest_search_log_date: 20.minutes.ago) + + get "/u/recent-searches.json" + expect(response.status).to eq(200) + expect(response.parsed_body["recent_searches"]).to eq(["old one", "also old"]) + + user.user_option.update!(oldest_search_log_date: 10.seconds.ago) + + get "/u/recent-searches.json" + expect(response.status).to eq(200) + expect(response.parsed_body["recent_searches"]).to eq([]) + + SearchLog.create!( + term: "new search", + user_id: user.id, + search_type: 1, + ip_address: '192.168.0.1', + created_at: 2.seconds.ago + ) + + get "/u/recent-searches.json" + expect(response.status).to eq(200) + expect(response.parsed_body["recent_searches"]).to eq(["new search"]) + end + + it 'shows an error message when log_search_queries are off' do + sign_in(user) + SiteSetting.log_search_queries = false + + get "/u/recent-searches.json" + + expect(response.status).to eq(403) + expect(response.parsed_body["error"]).to eq(I18n.t("user_activity.no_log_search_queries")) + end + end + def create_second_factor_security_key sign_in(user) stub_secure_session_confirmed