typeahead: Always show exact matches first for streams and users.

We refactor the triage function to optionally take in a comparator
function, and use this to sort the results, except any exact match,
which is placed highest. Now we don't need to sort the results of triage
for streams, languages and slash commands since we just pass in the
comparator function. The overall effect is same as before, except that
exact matches are always shown first.

For users, we can't use the new triage feature to achieve this goal
without sorting `rest` and breaking a key optimization, so we just add
a bit of manual code for the job.

Fixes: #25123.

web/shared/src/typeahead.ts

@@ -113,6 +113,7 @@ export function triage<T>(
     query: string,
     objs: T[],
     get_item: (x: T) => string,
+    sorting_comparator?: () => number,
 ): {matches: T[]; rest: T[]} {
     /*
         We split objs into four groups:
@@ -146,6 +147,17 @@ export function triage<T>(
             noMatch.push(obj);
         }
     }
+
+    if (sorting_comparator) {
+        const non_exact_sorted_matches = [
+            ...beginswithCaseSensitive,
+            ...beginswithCaseInsensitive,
+        ].sort(sorting_comparator);
+        return {
+            matches: [...exactMatch, ...non_exact_sorted_matches],
+            rest: noMatch.sort(sorting_comparator),
+        };
+    }
     return {
         matches: [...exactMatch, ...beginswithCaseSensitive, ...beginswithCaseInsensitive],
         rest: noMatch,

web/src/typeahead_helper.js

@@ -340,20 +340,8 @@ function retain_unique_language_aliases(matches) {
 }
 
 export function sort_languages(matches, query) {
-    const results = typeahead.triage(query, matches, (x) => x);
+    const results = typeahead.triage(query, matches, (x) => x, compare_language);
 
-    // Languages that start with the query
-    results.matches = results.matches.sort(compare_language);
-
-    // Push exact matches to top.
-    const match_index = results.matches.indexOf(query);
-    if (match_index > -1) {
-        results.matches.splice(match_index, 1);
-        results.matches.unshift(query);
-    }
-
-    // Languages that have the query somewhere in their name
-    results.rest = results.rest.sort(compare_language);
     return retain_unique_language_aliases([...results.matches, ...results.rest]);
 }
 
@@ -399,7 +387,21 @@ export function sort_recipients({
         }
     }
 
-    return items.slice(0, max_num_items);
+    // Push exact matches to top. We could do by passing the
+    // comparator parameter to triage, but that would break the
+    // optimization of only doing expensive sorting operations when
+    // necessary.
+    const exact_matches = [];
+    const rest = [];
+    for (const item of items) {
+        if (item.full_name?.toLowerCase() === query.toLowerCase()) {
+            exact_matches.push(item);
+        } else {
+            rest.push(item);
+        }
+    }
+
+    return [...exact_matches, ...rest].slice(0, max_num_items);
 }
 
 function slash_command_comparator(slash_command_a, slash_command_b) {
@@ -415,10 +417,8 @@ function slash_command_comparator(slash_command_a, slash_command_b) {
 export function sort_slash_commands(matches, query) {
     // We will likely want to in the future make this sort the
     // just-`/` commands by something approximating usefulness.
-    const results = typeahead.triage(query, matches, (x) => x.name);
+    const results = typeahead.triage(query, matches, (x) => x.name, slash_command_comparator);
 
-    results.matches = results.matches.sort(slash_command_comparator);
-    results.rest = results.rest.sort(slash_command_comparator);
     return [...results.matches, ...results.rest];
 }
 
@@ -455,16 +455,13 @@ export function compare_by_activity(stream_a, stream_b) {
 }
 
 export function sort_streams(matches, query) {
-    const name_results = typeahead.triage(query, matches, (x) => x.name);
-
-    const desc_results = typeahead.triage(query, name_results.rest, (x) => x.description);
-
-    // Streams that start with the query.
-    name_results.matches = name_results.matches.sort(compare_by_activity);
-    // Streams with descriptions that start with the query.
-    desc_results.matches = desc_results.matches.sort(compare_by_activity);
-    // Streams with names and descriptions that don't start with the query.
-    desc_results.rest = desc_results.rest.sort(compare_by_activity);
+    const name_results = typeahead.triage(query, matches, (x) => x.name, compare_by_activity);
+    const desc_results = typeahead.triage(
+        query,
+        name_results.rest,
+        (x) => x.description,
+        compare_by_activity,
+    );
 
     return [...name_results.matches, ...desc_results.matches, ...desc_results.rest];
 }

web/tests/composebox_typeahead.test.js

@@ -221,10 +221,24 @@ const netherland_stream = {
     stream_id: 3,
     subscribed: false,
 };
+const mobile_stream = {
+    name: "Mobile",
+    description: "Mobile development",
+    stream_id: 4,
+    subscribed: false,
+};
+const mobile_team_stream = {
+    name: "Mobile team",
+    description: "Mobile development team",
+    stream_id: 5,
+    subscribed: true,
+};
 
 stream_data.add_sub(sweden_stream);
 stream_data.add_sub(denmark_stream);
 stream_data.add_sub(netherland_stream);
+stream_data.add_sub(mobile_stream);
+stream_data.add_sub(mobile_team_stream);
 
 const name_to_codepoint = {};
 for (const [key, val] of emojis_by_name.entries()) {
@@ -246,6 +260,12 @@ emoji.initialize({
 emoji.active_realm_emojis = new Map();
 emoji.emojis_by_name = emojis_by_name;
 
+const ali = {
+    email: "ali@zulip.com",
+    user_id: 98,
+    full_name: "Ali",
+};
+
 const alice = {
     email: "alice@zulip.com",
     user_id: 99,
@@ -347,6 +367,7 @@ function test(label, f) {
         people.init();
         user_groups.init();
 
+        people.add_active_user(ali);
         people.add_active_user(alice);
         people.add_active_user(hamlet);
         people.add_active_user(othello);
@@ -787,6 +808,7 @@ test("initialize", ({override, override_rewire, mock_template}) => {
         // This should match the users added at the beginning of this test file.
         let actual_value = options.source("");
         let expected_value = [
+            ali,
             alice,
             cordelia,
             hal,
@@ -869,6 +891,11 @@ test("initialize", ({override, override_rewire, mock_template}) => {
         expected_value = [othello];
         assert.deepEqual(actual_value, expected_value);
 
+        query = "Ali";
+        actual_value = sorter(query, [alice, ali]);
+        expected_value = [ali, alice];
+        assert.deepEqual(actual_value, expected_value);
+
         // A literal match at the beginning of an element puts it at the top.
         query = "co"; // Matches everything ("x@zulip.COm")
         actual_value = sorter(query, [othello, deactivated_user, cordelia]);
@@ -1513,7 +1540,7 @@ test("filter_and_sort_mentions (normal)", () => {
     const suggestions = ct.filter_and_sort_mentions(is_silent, "al");
 
     const mention_all = ct.broadcast_mentions()[0];
-    assert.deepEqual(suggestions, [mention_all, alice, hal, call_center]);
+    assert.deepEqual(suggestions, [mention_all, ali, alice, hal, call_center]);
 });
 
 test("filter_and_sort_mentions (silent)", () => {
@@ -1521,11 +1548,17 @@ test("filter_and_sort_mentions (silent)", () => {
 
     const suggestions = ct.filter_and_sort_mentions(is_silent, "al");
 
-    assert.deepEqual(suggestions, [alice, hal, call_center]);
+    assert.deepEqual(suggestions, [ali, alice, hal, call_center]);
 });
 
 test("typeahead_results", () => {
-    const stream_list = [denmark_stream, sweden_stream, netherland_stream];
+    const stream_list = [
+        denmark_stream,
+        sweden_stream,
+        netherland_stream,
+        mobile_team_stream,
+        mobile_stream,
+    ];
 
     function compose_typeahead_results(completing, items, token) {
         return ct.filter_and_sort_candidates(completing, items, token);
@@ -1647,6 +1680,8 @@ test("typeahead_results", () => {
     // Do not match stream descriptions
     assert_stream_matches("cold", []);
     assert_stream_matches("city", []);
+    // Always prioritise exact matches, irrespective of activity
+    assert_stream_matches("Mobile", [mobile_stream, mobile_team_stream]);
 });
 
 test("message people", ({override, override_rewire}) => {
@@ -1736,10 +1771,22 @@ test("PM recipients sorted according to stream / topic being viewed", ({override
     // When viewing no stream, sorting is alphabetical
     compose_state.set_stream_name("");
     results = ct.get_pm_people("li");
-    assert.deepEqual(results, [alice, cordelia]);
+    assert.deepEqual(results, [ali, alice, cordelia]);
 
-    // When viewing denmark stream, subscriber twin2 is placed higher
+    // When viewing denmark stream, subscriber cordelia is placed higher
     compose_state.set_stream_name("Denmark");
     results = ct.get_pm_people("li");
-    assert.deepEqual(results, [cordelia, alice]);
+    assert.deepEqual(results, [cordelia, ali, alice]);
+
+    // Simulating just alice being subscribed to denmark.
+    override_rewire(
+        stream_data,
+        "is_user_subscribed",
+        (stream_id, user_id) => stream_id === denmark_stream.stream_id && user_id === alice.user_id,
+    );
+
+    // When viewing denmark stream to which alice is subscribed, ali is still
+    // 1st by virtue of the name being an exact match with the query.
+    results = ct.get_pm_people("ali");
+    assert.deepEqual(results, [ali, alice]);
 });