From eb2bf77212052b5937b042c1b12a3d74295c58b1 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Thu, 27 Jan 2022 13:39:19 +0000 Subject: [PATCH] search suggestions: Find older topics. We no longer limit our list of candidate topics to 300. We continue to limit the topic results to 10, since we don't want to overwhelm users or crowd out non-topic-related suggestions. We try to handle this is an efficient manner. --- .../node_tests/search_suggestion.js | 43 +++++++++++++++++ static/js/search_suggestion.js | 46 ++++++++++++++----- 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/frontend_tests/node_tests/search_suggestion.js b/frontend_tests/node_tests/search_suggestion.js index b9f6ba6e3b..1f4c930bc7 100644 --- a/frontend_tests/node_tests/search_suggestion.js +++ b/frontend_tests/node_tests/search_suggestion.js @@ -761,6 +761,49 @@ test("topic_suggestions", ({override, override_rewire}) => { assert.deepEqual(suggestions.strings, expected); }); +test("topic_suggestions (limits)", () => { + let candidate_topics = []; + + function assert_result(guess, expected_topics) { + assert.deepEqual( + search.get_topic_suggestions_from_candidates({candidate_topics, guess}), + expected_topics, + ); + } + + assert_result("", []); + assert_result("zzz", []); + + candidate_topics = ["a", "b", "c"]; + assert_result("", ["a", "b", "c"]); + assert_result("b", ["b"]); + assert_result("z", []); + + candidate_topics = [ + "a1", + "a2", + "b1", + "b2", + "a3", + "a4", + "a5", + "c1", + "a6", + "a7", + "a8", + "c2", + "a9", + "a10", + "a11", + "a12", + ]; + // We max out at 10 topics, so as not to overwhelm the user. + assert_result("", ["a1", "a2", "b1", "b2", "a3", "a4", "a5", "c1", "a6", "a7"]); + assert_result("a", ["a1", "a2", "a3", "a4", "a5", "a6", "a7", "a8", "a9", "a10"]); + assert_result("b", ["b1", "b2"]); + assert_result("z", []); +}); + test("whitespace_glitch", ({override_rewire}) => { const query = "stream:office "; // note trailing space diff --git a/static/js/search_suggestion.js b/static/js/search_suggestion.js index 94121189ee..dee90334cd 100644 --- a/static/js/search_suggestion.js +++ b/static/js/search_suggestion.js @@ -282,6 +282,38 @@ function get_default_suggestion(operators) { return format_as_suggestion(operators); } +export function get_topic_suggestions_from_candidates({candidate_topics, guess}) { + // This function is exported for unit testing purposes. + const max_num_topics = 10; + + if (guess === "") { + // In the search UI, once you autocomplete the stream, + // we just show you the most recent topics before you even + // need to start typing any characters. + return candidate_topics.slice(0, max_num_topics); + } + + // Once the user starts typing characters for a topic name, + // it is pretty likely they want to get suggestions for + // topics that may be fairly low in our list of candidates, + // so we do an aggressive search here. + // + // The following loop can be expensive if you have lots + // of topics in a stream, so we try to exit the loop as + // soon as we find enough matches. + const topics = []; + for (const topic of candidate_topics) { + if (common.phrase_match(guess, topic)) { + topics.push(topic); + if (topics.length >= max_num_topics) { + break; + } + } + } + + return topics; +} + function get_topic_suggestions(last, operators) { const invalid = [ {operator: "pm-with"}, @@ -344,21 +376,13 @@ function get_topic_suggestions(last, operators) { // Fetch topic history from the server, in case we will need it. stream_topic_history_util.get_server_history(stream_id, () => {}); - let topics = stream_topic_history.get_recent_topic_names(stream_id); + const candidate_topics = stream_topic_history.get_recent_topic_names(stream_id); - if (!topics || !topics.length) { + if (!candidate_topics || !candidate_topics.length) { return []; } - // Be defensive here in case stream_data.get_recent_topics gets - // super huge, but still slice off enough topics to find matches. - topics = topics.slice(0, 300); - - if (guess !== "") { - topics = topics.filter((topic) => common.phrase_match(guess, topic)); - } - - topics = topics.slice(0, 10); + const topics = get_topic_suggestions_from_candidates({candidate_topics, guess}); // Just use alphabetical order. While recency and read/unreadness of // topics do matter in some contexts, you can get that from the left sidebar,