From cd682f3dee5f2ddc17394923c8fc62b04e9bf804 Mon Sep 17 00:00:00 2001 From: Pratik Chanda Date: Thu, 28 Aug 2025 23:27:24 +0530 Subject: [PATCH] search_suggestion: Remove subset suggestions from search suggestion. Earlier, we would show subset suggestions for current search pills in the suggestion list. Like if `is:dm`, `is:mentioned` and `has:link` are already selected in the searchbox, we would show `is:dm, is:mentioned` and `is:dm` again in the suggestion. This commit removes subset suggestions for current search pills from the suggestion line. (cherry picked from commit 85e7d2f5f79f330424d1d08ce7b0a479e884cd82) --- web/src/search_suggestion.ts | 23 -------- web/tests/search_suggestion.test.cjs | 80 ++++++---------------------- 2 files changed, 16 insertions(+), 87 deletions(-) diff --git a/web/src/search_suggestion.ts b/web/src/search_suggestion.ts index becfc4dd65..c783525d93 100644 --- a/web/src/search_suggestion.ts +++ b/web/src/search_suggestion.ts @@ -557,24 +557,6 @@ function get_topic_suggestions(last: NarrowTerm, terms: NarrowTerm[]): Suggestio }); } -function get_term_subset_suggestions(terms: NarrowTerm[]): Suggestion[] { - // For channel:a topic:b search:c, suggest: - // channel:a topic:b - // channel:a - if (terms.length === 0) { - return []; - } - - const suggestions: Suggestion[] = []; - - for (let i = terms.length - 1; i >= 1; i -= 1) { - const subset = terms.slice(0, i); - suggestions.push(format_as_suggestion(subset)); - } - - return suggestions; -} - type SuggestionAndIncompatiblePatterns = Suggestion & {incompatible_patterns: TermPattern[]}; function get_special_filter_suggestions( @@ -1110,11 +1092,6 @@ export function get_search_result( } } - if (attacher.result.length < max_items) { - const subset_suggestions = get_term_subset_suggestions(all_search_terms); - const subset_suggestion_lines = subset_suggestions.map((suggestion) => [suggestion]); - attacher.push_many(subset_suggestion_lines); - } return attacher.get_result().slice(0, max_items); } diff --git a/web/tests/search_suggestion.test.cjs b/web/tests/search_suggestion.test.cjs index a7effc8718..c76900ff72 100644 --- a/web/tests/search_suggestion.test.cjs +++ b/web/tests/search_suggestion.test.cjs @@ -149,25 +149,6 @@ test("get_is_suggestions_for_spectator", () => { page_params.is_spectator = false; }); -test("subset_suggestions", ({mock_template}) => { - mock_template("search_description.hbs", true, (_data, html) => html); - - const denmark_id = new_stream_id(); - const sub = {name: "Denmark", stream_id: denmark_id}; - stream_data.add_sub(sub); - const query = `channel:${denmark_id} topic:Hamlet shakespeare`; - - const suggestions = get_suggestions(query); - - const expected = [ - `channel:${denmark_id} topic:Hamlet shakespeare`, - `channel:${denmark_id} topic:Hamlet`, - `channel:${denmark_id}`, - ]; - - assert.deepEqual(suggestions.strings, expected); -}); - test("dm_suggestions", ({override, mock_template}) => { mock_template("search_description.hbs", true, (_data, html) => html); @@ -191,7 +172,6 @@ test("dm_suggestions", ({override, mock_template}) => { "is:dm dm:alice@zulip.com", "is:dm sender:alice@zulip.com", "is:dm dm-including:alice@zulip.com", - "is:dm", ]; assert.deepEqual(suggestions.strings, expected); @@ -253,25 +233,25 @@ test("dm_suggestions", ({override, mock_template}) => { query = "is:unread from:ted"; suggestions = get_suggestions(query); - expected = ["is:unread sender:ted@zulip.com", "is:unread"]; + expected = ["is:unread sender:ted@zulip.com"]; assert.deepEqual(suggestions.strings, expected); // Users can enter bizarre queries, and if they do, we want to // be conservative with suggestions. query = "is:dm near:3"; suggestions = get_suggestions(query); - expected = ["is:dm near:3", "is:dm"]; + expected = ["is:dm near:3"]; assert.deepEqual(suggestions.strings, expected); query = "dm:ted@zulip.com near:3"; suggestions = get_suggestions(query); - expected = ["dm:ted@zulip.com near:3", "dm:ted@zulip.com"]; + expected = ["dm:ted@zulip.com near:3"]; assert.deepEqual(suggestions.strings, expected); // Make sure suggestions still work if preceding tokens query = "is:alerted sender:ted@zulip.com"; suggestions = get_suggestions(query); - expected = ["is:alerted sender:ted@zulip.com", "is:alerted"]; + expected = ["is:alerted sender:ted@zulip.com"]; assert.deepEqual(suggestions.strings, expected); query = "is:starred has:link is:dm al"; @@ -282,9 +262,6 @@ test("dm_suggestions", ({override, mock_template}) => { "is:starred has:link is:dm dm:alice@zulip.com", "is:starred has:link is:dm sender:alice@zulip.com", "is:starred has:link is:dm dm-including:alice@zulip.com", - "is:starred has:link is:dm", - "is:starred has:link", - "is:starred", ]; assert.deepEqual(suggestions.strings, expected); @@ -321,7 +298,6 @@ test("group_suggestions", ({mock_template}) => { "dm:bob@zulip.com,alice@zulip.com", "dm:bob@zulip.com sender:alice@zulip.com", "dm:bob@zulip.com dm-including:alice@zulip.com", - "dm:bob@zulip.com", ]; assert.deepEqual(suggestions.strings, expected); @@ -333,7 +309,6 @@ test("group_suggestions", ({mock_template}) => { "dm:ted@zulip.com my", "dm:ted@zulip.com sender:myself@zulip.com", "dm:ted@zulip.com dm-including:myself@zulip.com", - "dm:ted@zulip.com", ]; assert.deepEqual(suggestions.strings, expected); @@ -355,7 +330,6 @@ test("group_suggestions", ({mock_template}) => { "dm:bob@zulip.com,alice@zulip.com", "dm:bob@zulip.com sender:alice@zulip.com", "dm:bob@zulip.com dm-including:alice@zulip.com", - "dm:bob@zulip.com", ]; assert.deepEqual(suggestions.strings, expected); @@ -368,9 +342,6 @@ test("group_suggestions", ({mock_template}) => { "is:starred has:link dm:bob@zulip.com,ted@zulip.com", "is:starred has:link dm:bob@zulip.com sender:ted@zulip.com", "is:starred has:link dm:bob@zulip.com dm-including:ted@zulip.com", - "is:starred has:link dm:bob@zulip.com", - "is:starred has:link", - "is:starred", ]; assert.deepEqual(suggestions.strings, expected); @@ -378,13 +349,13 @@ test("group_suggestions", ({mock_template}) => { // with a channel. (Random channel id.) query = "channel:66 has:link dm:bob@zulip.com,Smit"; suggestions = get_suggestions(query); - expected = ["channel:66 has:link", "channel:66"]; + expected = []; assert.deepEqual(suggestions.strings, expected); // Invalid emails don't give suggestions - query = "has:link dm:invalid@zulip.com,Smit"; + query = "dm:invalid@zulip.com,Smit"; suggestions = get_suggestions(query); - expected = ["has:link"]; + expected = []; assert.deepEqual(suggestions.strings, expected); }); @@ -497,7 +468,7 @@ test("has_suggestions", ({override, mock_template}) => { // 66 is misc channel id. query = "channel:66 is:alerted has:lin"; suggestions = get_suggestions(query); - expected = ["channel:66 is:alerted has:link", "channel:66 is:alerted", "channel:66"]; + expected = ["channel:66 is:alerted has:link"]; assert.deepEqual(suggestions.strings, expected); }); @@ -597,7 +568,7 @@ test("check_is_suggestions", ({override, mock_template}) => { query = "channel:66 has:link is:sta"; suggestions = get_suggestions(query); - expected = ["channel:66 has:link is:starred", "channel:66 has:link", "channel:66"]; + expected = ["channel:66 has:link is:starred"]; assert.deepEqual(suggestions.strings, expected); }); @@ -662,14 +633,12 @@ test("sent_by_me_suggestions", ({override, mock_template}) => { expected = [ `channel:${denmark_id} topic:Denmark1 sent`, `channel:${denmark_id} topic:Denmark1 sender:myself@zulip.com`, - `channel:${denmark_id} topic:Denmark1`, - `channel:${denmark_id}`, ]; assert.deepEqual(suggestions.strings, expected); query = "is:starred sender:m"; suggestions = get_suggestions(query); - expected = ["is:starred sender:myself@zulip.com", "is:starred"]; + expected = ["is:starred sender:myself@zulip.com"]; assert.deepEqual(suggestions.strings, expected); }); @@ -721,23 +690,15 @@ test("topic_suggestions", ({override, mock_template}) => { assert.equal(describe(`channel:${office_id} topic:team`), "Messages in #office > team"); suggestions = get_suggestions(`topic:staplers channel:${office_id}`); - expected = [`topic:staplers channel:${office_id}`, "topic:staplers"]; + expected = [`topic:staplers channel:${office_id}`]; assert.deepEqual(suggestions.strings, expected); suggestions = get_suggestions(`channel:${devel_id} topic:`); - expected = [ - `channel:${devel_id} topic:`, - `channel:${devel_id} topic:REXX`, - `channel:${devel_id}`, - ]; + expected = [`channel:${devel_id} topic:`, `channel:${devel_id} topic:REXX`]; assert.deepEqual(suggestions.strings, expected); suggestions = get_suggestions(`channel:${devel_id} -topic:`); - expected = [ - `channel:${devel_id} -topic:`, - `channel:${devel_id} -topic:REXX`, - `channel:${devel_id}`, - ]; + expected = [`channel:${devel_id} -topic:`, `channel:${devel_id} -topic:REXX`]; assert.deepEqual(suggestions.strings, expected); suggestions = get_suggestions("-topic:te"); @@ -752,22 +713,15 @@ test("topic_suggestions", ({override, mock_template}) => { expected = [ `is:alerted channel:${devel_id} is:starred topic:`, `is:alerted channel:${devel_id} is:starred topic:REXX`, - `is:alerted channel:${devel_id} is:starred`, - `is:alerted channel:${devel_id}`, - "is:alerted", ]; assert.deepEqual(suggestions.strings, expected); suggestions = get_suggestions(`is:dm channel:${devel_id} topic:`); - expected = [`is:dm channel:${devel_id} topic:`, `is:dm channel:${devel_id}`, `is:dm`]; + expected = [`is:dm channel:${devel_id} topic:`]; assert.deepEqual(suggestions.strings, expected); suggestions = get_suggestions(`topic:REXX channel:${devel_id} topic:`); - expected = [ - `topic:REXX channel:${devel_id} topic:`, - `topic:REXX channel:${devel_id}`, - "topic:REXX", - ]; + expected = [`topic:REXX channel:${devel_id} topic:`]; assert.deepEqual(suggestions.strings, expected); }); @@ -983,7 +937,7 @@ test("people_suggestions", ({override, mock_template}) => { assert.deepEqual(suggestions.strings, expected); query = "sender:ted@zulip.com new"; - expected = ["sender:ted@zulip.com new", "sender:ted@zulip.com"]; + expected = ["sender:ted@zulip.com new"]; suggestions = get_suggestions(query); assert.deepEqual(suggestions.strings, expected); @@ -1021,8 +975,6 @@ test("operator_suggestions", ({override, mock_template}) => { "channel:66 is:alerted -f", "channel:66 is:alerted -sender:", "channel:66 is:alerted -sender:myself@zulip.com", - "channel:66 is:alerted", - "channel:66", ]; assert.deepEqual(suggestions.strings, expected); });