FIX: Show user filter hints when typing `@` in search (#13799)

Will show the last 6 seen users as filtering suggestions when typing @ in quick search. (Previously the user suggestion required a character after the @.)

This also adds a default limit of 6 to the user search query, previously the backend was returning 20 results but a maximum of 6 results was being shown anyway.
This commit is contained in:
Penar Musaraj 2021-07-21 09:14:53 -04:00 committed by GitHub
parent 519528daa2
commit 2ce2c83bc9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 116 additions and 12 deletions

View File

@ -22,6 +22,8 @@ function performSearch(
allowedUsers, allowedUsers,
groupMembersOf, groupMembersOf,
includeStagedUsers, includeStagedUsers,
lastSeenUsers,
limit,
resultsFn resultsFn
) { ) {
let cached = cache[term]; let cached = cache[term];
@ -32,7 +34,7 @@ function performSearch(
const eagerComplete = eagerCompleteSearch(term, topicId || categoryId); const eagerComplete = eagerCompleteSearch(term, topicId || categoryId);
if (term === "" && !eagerComplete) { if (term === "" && !eagerComplete && !lastSeenUsers) {
// The server returns no results in this case, so no point checking // The server returns no results in this case, so no point checking
// do not return empty list, because autocomplete will get terminated // do not return empty list, because autocomplete will get terminated
resultsFn(CANCELLED_STATUS); resultsFn(CANCELLED_STATUS);
@ -51,6 +53,8 @@ function performSearch(
groups: groupMembersOf, groups: groupMembersOf,
topic_allowed_users: allowedUsers, topic_allowed_users: allowedUsers,
include_staged_users: includeStagedUsers, include_staged_users: includeStagedUsers,
last_seen_users: lastSeenUsers,
limit: limit,
}, },
}); });
@ -93,6 +97,8 @@ let debouncedSearch = function (
allowedUsers, allowedUsers,
groupMembersOf, groupMembersOf,
includeStagedUsers, includeStagedUsers,
lastSeenUsers,
limit,
resultsFn resultsFn
) { ) {
discourseDebounce( discourseDebounce(
@ -107,6 +113,8 @@ let debouncedSearch = function (
allowedUsers, allowedUsers,
groupMembersOf, groupMembersOf,
includeStagedUsers, includeStagedUsers,
lastSeenUsers,
limit,
resultsFn, resultsFn,
300 300
); );
@ -169,7 +177,10 @@ function organizeResults(r, options) {
// we also ignore if we notice a double space or a string that is only a space // we also ignore if we notice a double space or a string that is only a space
const ignoreRegex = /([\u2000-\u206F\u2E00-\u2E7F\\'!"#$%&()*,\/:;<=>?\[\]^`{|}~])|\s\s|^\s$|^[^+]*\+[^@]*$/; const ignoreRegex = /([\u2000-\u206F\u2E00-\u2E7F\\'!"#$%&()*,\/:;<=>?\[\]^`{|}~])|\s\s|^\s$|^[^+]*\+[^@]*$/;
export function skipSearch(term, allowEmails) { export function skipSearch(term, allowEmails, lastSeenUsers = false) {
if (lastSeenUsers) {
return false;
}
if (term.indexOf("@") > -1 && !allowEmails) { if (term.indexOf("@") > -1 && !allowEmails) {
return true; return true;
} }
@ -194,7 +205,9 @@ export default function userSearch(options) {
topicId = options.topicId, topicId = options.topicId,
categoryId = options.categoryId, categoryId = options.categoryId,
groupMembersOf = options.groupMembersOf, groupMembersOf = options.groupMembersOf,
includeStagedUsers = options.includeStagedUsers; includeStagedUsers = options.includeStagedUsers,
lastSeenUsers = options.lastSeenUsers,
limit = options.limit || 6;
if (oldSearch) { if (oldSearch) {
oldSearch.abort(); oldSearch.abort();
@ -217,7 +230,7 @@ export default function userSearch(options) {
clearPromise = later(() => resolve(CANCELLED_STATUS), 5000); clearPromise = later(() => resolve(CANCELLED_STATUS), 5000);
} }
if (skipSearch(term, options.allowEmails)) { if (skipSearch(term, options.allowEmails, options.lastSeenUsers)) {
resolve([]); resolve([]);
return; return;
} }
@ -232,6 +245,8 @@ export default function userSearch(options) {
allowedUsers, allowedUsers,
groupMembersOf, groupMembersOf,
includeStagedUsers, includeStagedUsers,
lastSeenUsers,
limit,
function (r) { function (r) {
cancel(clearPromise); cancel(clearPromise);
resolve(organizeResults(r, options)); resolve(organizeResults(r, options));

View File

@ -10,9 +10,7 @@ import { popupAjaxError } from "discourse/lib/ajax-error";
import userSearch from "discourse/lib/user-search"; import userSearch from "discourse/lib/user-search";
const CATEGORY_SLUG_REGEXP = /(\#[a-zA-Z0-9\-:]*)$/gi; const CATEGORY_SLUG_REGEXP = /(\#[a-zA-Z0-9\-:]*)$/gi;
// The backend user search query returns zero results for a term-free search const USERNAME_REGEXP = /(\@[a-zA-Z0-9\-\_]*)$/gi;
// so the regexp below only matches @ followed by a valid character
const USERNAME_REGEXP = /(\@[a-zA-Z0-9\-\_]+)$/gi;
const searchData = {}; const searchData = {};
const suggestionTriggers = ["in:", "status:", "order:"]; const suggestionTriggers = ["in:", "status:", "order:"];
@ -72,11 +70,19 @@ const SearchHelper = {
return; return;
} }
if (matchSuggestions.type === "username") { if (matchSuggestions.type === "username") {
userSearch({ const userSearchTerm = matchSuggestions.usernamesMatch[0].replace(
term: matchSuggestions.usernamesMatch[0], "@",
includeGroups: true, ""
}).then((result) => { );
if (result?.users.length > 0) { const opts = { includeGroups: true, limit: 6 };
if (userSearchTerm.length > 0) {
opts.term = userSearchTerm;
} else {
opts.lastSeenUsers = true;
}
userSearch(opts).then((result) => {
if (result?.users?.length > 0) {
searchData.suggestionResults = result.users; searchData.suggestionResults = result.users;
searchData.suggestionKeyword = "@"; searchData.suggestionKeyword = "@";
} else { } else {

View File

@ -268,6 +268,33 @@ acceptance("Search - with tagging enabled", function (needs) {
acceptance("Search - assistant", function (needs) { acceptance("Search - assistant", function (needs) {
needs.user(); needs.user();
needs.pretender((server, helper) => {
server.get("/u/search/users", () => {
return helper.response({
users: [
{
username: "TeaMoe",
name: "TeaMoe",
avatar_template:
"https://avatars.discourse.org/v3/letter/t/41988e/{size}.png",
},
{
username: "TeamOneJ",
name: "J Cobb",
avatar_template:
"https://avatars.discourse.org/v3/letter/t/3d9bf3/{size}.png",
},
{
username: "kudos",
name: "Team Blogeto.com",
avatar_template:
"/user_avatar/meta.discourse.org/kudos/{size}/62185_1.png",
},
],
});
});
});
test("shows category shortcuts when typing #", async function (assert) { test("shows category shortcuts when typing #", async function (assert) {
await visit("/"); await visit("/");
@ -317,4 +344,21 @@ acceptance("Search - assistant", function (needs) {
await triggerKeyEvent("#search-term", "keyup", 51); await triggerKeyEvent("#search-term", "keyup", 51);
assert.equal(query(firstTarget).innerText, "sam in:title"); assert.equal(query(firstTarget).innerText, "sam in:title");
}); });
test("shows users when typing @", async function (assert) {
await visit("/");
await click("#search-button");
await fillIn("#search-term", "@");
await triggerKeyEvent("#search-term", "keyup", 51);
const firstUser =
".search-menu .results ul.search-menu-assistant .search-item-user";
const firstUsername = query(firstUser).innerText.trim();
assert.equal(firstUsername, "TeaMoe");
await click(query(firstUser));
assert.equal(query("#search-term").value, `@${firstUsername} `);
});
}); });

View File

@ -1079,6 +1079,8 @@ class UsersController < ApplicationController
} }
options[:include_staged_users] = !!ActiveModel::Type::Boolean.new.cast(params[:include_staged_users]) options[:include_staged_users] = !!ActiveModel::Type::Boolean.new.cast(params[:include_staged_users])
options[:last_seen_users] = !!ActiveModel::Type::Boolean.new.cast(params[:last_seen_users])
options[:limit] = params[:limit].to_i if params[:limit].present?
options[:topic_id] = topic_id if topic_id options[:topic_id] = topic_id if topic_id
options[:category_id] = category_id if category_id options[:category_id] = category_id if category_id

View File

@ -12,6 +12,7 @@ class UserSearch
@topic_allowed_users = opts[:topic_allowed_users] @topic_allowed_users = opts[:topic_allowed_users]
@searching_user = opts[:searching_user] @searching_user = opts[:searching_user]
@include_staged_users = opts[:include_staged_users] || false @include_staged_users = opts[:include_staged_users] || false
@last_seen_users = opts[:last_seen_users] || false
@limit = opts[:limit] || 20 @limit = opts[:limit] || 20
@groups = opts[:groups] @groups = opts[:groups]
@ -162,6 +163,15 @@ class UserSearch
.each { |id| users << id } .each { |id| users << id }
end end
# 5. last seen users (for search auto-suggestions)
if @last_seen_users
scoped_users
.order('last_seen_at DESC NULLS LAST')
.limit(@limit - users.size)
.pluck(:id)
.each { |id| users << id }
end
users.to_a users.to_a
end end

View File

@ -238,5 +238,14 @@ describe UserSearch do
results = search_for("", topic_id: topic.id, searching_user: mr_b) results = search_for("", topic_id: topic.id, searching_user: mr_b)
expect(results).to eq [mr_pink, mr_orange].map(&:username) expect(results).to eq [mr_pink, mr_orange].map(&:username)
end end
it "works with last_seen_users option" do
results = search_for("", last_seen_users: true)
expect(results).not_to be_blank
expect(results[0]).to eq("mrbrown")
expect(results[1]).to eq("mrpink")
expect(results[2]).to eq("mrorange")
end
end end
end end

View File

@ -4022,6 +4022,24 @@ describe UsersController do
expect(json["users"].map { |u| u["name"] }).not_to include(staged_user.name) expect(json["users"].map { |u| u["name"] }).not_to include(staged_user.name)
end end
end end
context '`last_seen_users`' do
it "returns results when the param is true" do
get "/u/search/users.json", params: { last_seen_users: true }
json = response.parsed_body
expect(json["users"]).not_to be_empty
end
it "respects limit parameter at the same time" do
limit = 3
get "/u/search/users.json", params: { last_seen_users: true, limit: limit }
json = response.parsed_body
expect(json["users"]).not_to be_empty
expect(json["users"].size).to eq(limit)
end
end
end end
describe '#email_login' do describe '#email_login' do