From cc56f226b8aa6791ac2a3e4eaf064df2628f7109 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 21 Oct 2019 14:52:54 +0100 Subject: [PATCH] FIX: Correct mention autocomplete in new topics in unsecured categories When autocompleting mentions in secure categories, we immediately populate the list with users which have permission to view the category. This logic is applied to unsecured categories as well, but the server returns an empty list of users. This commit teaches the autocomplete to understand empty lists of users without terminating the autocomplete dropdown. --- .../discourse/lib/user-search.js.es6 | 24 +++++++++++---- test/javascripts/lib/user-search-test.js.es6 | 30 +++++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/user-search.js.es6 b/app/assets/javascripts/discourse/lib/user-search.js.es6 index 1cb70232e98..90b3a3d28e1 100644 --- a/app/assets/javascripts/discourse/lib/user-search.js.es6 +++ b/app/assets/javascripts/discourse/lib/user-search.js.es6 @@ -26,11 +26,13 @@ function performSearch( return; } - // I am not strongly against unconditionally returning - // however this allows us to return a list of probable - // users we want to mention, early on a topic - if (term === "" && !topicId && !categoryId) { - return []; + const eagerComplete = term === "" && !!(topicId || categoryId); + + if (term === "" && !eagerComplete) { + // The server returns no results in this case, so no point checking + // do not return empty list, because autocomplete will get terminated + resultsFn(CANCELLED_STATUS); + return; } // need to be able to cancel this @@ -51,6 +53,18 @@ function performSearch( oldSearch .then(function(r) { + const hasResults = !!( + r.users.length || + r.groups.length || + r.emails.length + ); + + if (eagerComplete && !hasResults) { + // we are trying to eager load, but received no results + // do not return empty list, because autocomplete will get terminated + r = CANCELLED_STATUS; + } + cache[term] = r; cacheTime = new Date(); // If there is a newer search term, return null diff --git a/test/javascripts/lib/user-search-test.js.es6 b/test/javascripts/lib/user-search-test.js.es6 index f061c2ec614..5d11b479037 100644 --- a/test/javascripts/lib/user-search-test.js.es6 +++ b/test/javascripts/lib/user-search-test.js.es6 @@ -1,4 +1,5 @@ import userSearch from "discourse/lib/user-search"; +import { CANCELLED_STATUS } from "discourse/lib/autocomplete"; QUnit.module("lib:user-search", { beforeEach() { @@ -12,6 +13,9 @@ QUnit.module("lib:user-search", { // special responder for per category search const categoryMatch = request.url.match(/category_id=([0-9]+)/); if (categoryMatch) { + if(categoryMatch[1] === "3"){ + return response({}); + } return response({ users: [ { @@ -92,6 +96,32 @@ QUnit.test("it flushes cache when switching categories", async assert => { assert.equal(results.length, 1); }); +QUnit.test( + "it returns cancel when eager completing with no results", + async assert => { + // Do everything twice, to check the cache works correctly + + for (let i = 0; i < 2; i++) { + // No topic or category, will always cancel + let result = await userSearch({ term: "" }); + assert.equal(result, CANCELLED_STATUS); + } + + for (let i = 0; i < 2; i++) { + // Unsecured category, so has no recommendations + let result = await userSearch({ term: "", categoryId: 3 }); + assert.equal(result, CANCELLED_STATUS); + } + + for (let i = 0; i < 2; i++) { + // Secured category, will have 1 recommendation + let results = await userSearch({ term: "", categoryId: 1 }); + assert.equal(results[0].username, "category_1"); + assert.equal(results.length, 1); + } + } +); + QUnit.test("it places groups unconditionally for exact match", async assert => { let results = await userSearch({ term: "Team" }); assert.equal(results[results.length - 1]["name"], "team");