typeahead: Use recent_senders/pm_conversations to sort suggestions.

In typeahead_helper.js, added a compare function to first sort by
subscription, then by pm partners and lastly based on recency in the
current topic. Altered function sort_for_at_mention to take topic data
and sort using the above function. Also altered node tests for
typeahead_helper.js to test for the above added functionality.

Fixes: #4249

frontend_tests/node_tests/composebox_typeahead.js

@@ -42,6 +42,8 @@ add_dependencies({
     people: 'js/people.js',
     stream_data: 'js/stream_data',
     templates: 'js/templates',
+    pm_conversations: 'js/pm_conversations.js',
+    recent_senders: 'js/recent_senders.js',
     typeahead_helper: 'js/typeahead_helper.js',
     ui_util: 'js/ui_util.js',
     util: 'js/util.js',

frontend_tests/node_tests/typeahead_helper.js

@@ -8,6 +8,10 @@ add_dependencies({
     people: 'js/people.js',
     util: 'js/util.js',
     Handlebars: 'handlebars',
+    recent_senders: 'js/recent_senders.js',
+    pm_conversations: 'js/pm_conversations.js',
+    message_store: 'js/message_store.js',
+    typeahead_helper: 'js/typeahead_helper.js',
 });
 
 stream_data.create_streams([
@@ -103,18 +107,24 @@ var matches = [
         is_admin: true,
         is_bot: false,
         user_id: 4,
+    }, {
+        email: "b_user_3@zulip.net",
+        full_name: "Bob 3",
+        is_admin: false,
+        is_bot: false,
+        user_id: 5,
     }, {
         email: "b_bot@example.com",
         full_name: "B bot",
         is_admin: false,
         is_bot: true,
-        user_id: 5,
+        user_id: 6,
     }, {
         email: "zman@test.net",
         full_name: "Zman",
         is_admin: false,
         is_bot: false,
-        user_id: 6,
+        user_id: 7,
     },
 ];
 
@@ -123,47 +133,108 @@ _.each(matches, function (person) {
 });
 
 (function test_sort_recipients() {
-    function get_typeahead_result(query, current_stream) {
+    function get_typeahead_result(query, current_stream, current_topic) {
         var result = th.sort_recipients(
             global.people.get_realm_persons(),
             query,
-            current_stream
+            current_stream,
+            current_topic
         );
         return _.map(result, function (person) {
             return person.email;
         });
     }
 
+    // Typeahead for recipientbox [query, "", undefined]
     assert.deepEqual(get_typeahead_result("b", ""), [
         'b_user_1@zulip.net',
         'b_user_2@zulip.net',
+        'b_user_3@zulip.net',
         'b_bot@example.com',
         'a_user@zulip.org',
         'zman@test.net',
         'a_bot@zulip.com',
      ]);
 
-    var subscriber_email = "b_user_2@zulip.net";
+    // Typeahead for private message [query, "", ""]
-    stream_data.add_subscriber("Dev", people.get_user_id(subscriber_email));
+    assert.deepEqual(get_typeahead_result("a", "", ""), [
-    assert.deepEqual(get_typeahead_result("b", "Dev"), [
-        subscriber_email,
-        'b_user_1@zulip.net',
-        'b_bot@example.com',
         'a_user@zulip.org',
-        'zman@test.net',
         'a_bot@zulip.com',
-    ]);
-
-    // No match
-    assert.deepEqual(get_typeahead_result("h", "Linux"), [
-        'a_user@zulip.org',
         'b_user_1@zulip.net',
         'b_user_2@zulip.net',
+        'b_user_3@zulip.net',
         'zman@test.net',
-        'a_bot@zulip.com',
         'b_bot@example.com',
      ]);
 
+    var subscriber_email_1 = "b_user_2@zulip.net";
+    var subscriber_email_2 = "b_user_3@zulip.net";
+    var subscriber_email_3 = "b_bot@example.com";
+    stream_data.add_subscriber("Dev", people.get_user_id(subscriber_email_1));
+    stream_data.add_subscriber("Dev", people.get_user_id(subscriber_email_2));
+    stream_data.add_subscriber("Dev", people.get_user_id(subscriber_email_3));
+
+    // For spliting based on whether a PM was sent
+    global.pm_conversations.set_partner(5);
+    global.pm_conversations.set_partner(6);
+    global.pm_conversations.set_partner(2);
+    global.pm_conversations.set_partner(7);
+
+    // For splitting based on recency
+    global.recent_senders.process_message_for_senders({
+        sender_id : 7,
+        stream_id : 1,
+        subject : "Dev Topic",
+        timestamp : _.uniqueId(),
+    });
+    global.recent_senders.process_message_for_senders({
+        sender_id : 5,
+        stream_id : 1,
+        subject : "Dev Topic",
+        timestamp : _.uniqueId(),
+    });
+    global.recent_senders.process_message_for_senders({
+        sender_id : 6,
+        stream_id : 1,
+        subject : "Dev Topic",
+        timestamp : _.uniqueId(),
+    });
+
+    // Typeahead for stream message [query, stream-name, topic-name]
+    assert.deepEqual(get_typeahead_result("b", "Dev", "Dev Topic"), [
+        subscriber_email_3,
+        subscriber_email_2,
+        subscriber_email_1,
+        'b_user_1@zulip.net',
+        'zman@test.net',
+        'a_user@zulip.org',
+        'a_bot@zulip.com',
+    ]);
+
+    global.recent_senders.process_message_for_senders({
+        sender_id : 5,
+        stream_id : 2,
+        subject : "Linux Topic",
+        timestamp : _.uniqueId(),
+    });
+    global.recent_senders.process_message_for_senders({
+        sender_id : 7,
+        stream_id : 2,
+        subject : "Linux Topic",
+        timestamp : _.uniqueId(),
+    });
+
+    // No match
+    assert.deepEqual(get_typeahead_result("h", "Linux", "Linux Topic"), [
+        'zman@test.net',
+        'b_user_3@zulip.net',
+        'a_user@zulip.org',
+        'b_bot@example.com',
+        'a_bot@zulip.com',
+        'b_user_1@zulip.net',
+        'b_user_2@zulip.net',
+    ]);
+
     // Test person email is "all" or "everyone"
     var person = {
         email: "all",
@@ -174,14 +245,15 @@ _.each(matches, function (person) {
     };
     people.add_in_realm(person);
 
-    assert.deepEqual(get_typeahead_result("a", "Linux"), [
+    assert.deepEqual(get_typeahead_result("a", "Linux", "Linux Topic"), [
         'all',
         'a_user@zulip.org',
         'a_bot@zulip.com',
+        'zman@test.net',
+        'b_user_3@zulip.net',
+        'b_bot@example.com',
         'b_user_1@zulip.net',
         'b_user_2@zulip.net',
-        'zman@test.net',
-        'b_bot@example.com',
     ]);
 
     people.deactivate(person);
@@ -194,35 +266,89 @@ _.each(matches, function (person) {
     matches[4].pm_recipient_count = 0;
     matches[5].pm_recipient_count = 1;
 
-    assert.deepEqual(get_typeahead_result("b", "Linux"), [
+    assert.deepEqual(get_typeahead_result("b", "Linux", "Linux Topic"), [
-        'b_user_2@zulip.net',
+        'b_user_3@zulip.net',
-        'b_user_1@zulip.net',
         'b_bot@example.com',
-        'a_bot@zulip.com',
+        'b_user_1@zulip.net',
-        'a_user@zulip.org',
+        'b_user_2@zulip.net',
         'zman@test.net',
+        'a_user@zulip.org',
+        'a_bot@zulip.com',
     ]);
 
     // Test sort_recipients with duplicate people
     matches.push(matches[0]);
 
-    var recipients = th.sort_recipients(matches, "b", "Linux");
+    var recipients = th.sort_recipients(matches, "b", "", "");
     var recipients_email = _.map(recipients, function (person) {
         return person.email;
     });
     var expected = [
+        'b_bot@example.com',
+        'b_user_3@zulip.net',
         'b_user_2@zulip.net',
         'b_user_1@zulip.net',
-        'b_bot@example.com',
-        'a_bot@zulip.com',
-        'a_bot@zulip.com',
         'a_user@zulip.org',
         'zman@test.net',
+        'a_bot@zulip.com',
+        'a_bot@zulip.com',
     ];
     assert.deepEqual(recipients_email, expected);
 
     // Reset matches
     matches.splice(matches.length-1, 1);
+
+    // full_name starts with same character but emails are 'all'
+    var small_matches = [
+        {
+            email: "all",
+            full_name: "All 1",
+            is_admin: false,
+            is_bot: false,
+            user_id: 43,
+        }, {
+            email: "all",
+            full_name: "All 2",
+            is_admin: false,
+            is_bot: false,
+            user_id: 44,
+        },
+    ];
+
+    recipients = th.sort_recipients(small_matches, "a", "Linux", "Linux Topic");
+    recipients_email = _.map(recipients, function (person) {
+        return person.email;
+    });
+    expected = [
+        'all',
+        'all',
+    ];
+    assert.deepEqual(recipients_email, expected);
+
+    // matches[3] is a subscriber and matches[2] is not.
+    small_matches = [matches[3], matches[2]];
+    recipients = th.sort_recipients(small_matches, "b", "Dev", "Dev Topic");
+    recipients_email = _.map(recipients, function (person) {
+        return person.email;
+    });
+    expected = [
+        'b_user_2@zulip.net',
+        'b_user_1@zulip.net',
+    ];
+    assert.deepEqual(recipients_email, expected);
+
+    // matches[4] is a pm partner and matches[3] is not and
+    // both are not subscribered to the stream Linux.
+    small_matches = [matches[4], matches[3]];
+    recipients = th.sort_recipients(small_matches, "b", "Linux", "Linux Topic");
+    recipients_email = _.map(recipients, function (person) {
+        return person.email;
+    });
+    expected = [
+        'b_user_3@zulip.net',
+        'b_user_2@zulip.net',
+    ];
+    assert.deepEqual(recipients_email, expected);
 }());
 
 (function test_highlight_with_escaping() {
@@ -331,29 +457,31 @@ _.each(matches, function (person) {
 }());
 
 (function test_sort_recipientbox_typeahead() {
-    var recipients = th.sort_recipientbox_typeahead("b, a", matches, "Dev"); // search "a"
+    var recipients = th.sort_recipientbox_typeahead("b, a", matches, ""); // search "a"
     var recipients_email = _.map(recipients, function (person) {
         return person.email;
     });
     assert.deepEqual(recipients_email, [
-        'a_bot@zulip.com', // matches "a"
         'a_user@zulip.org', // matches "a"
+        'a_bot@zulip.com', // matches "a"
+        'b_bot@example.com',
+        'b_user_3@zulip.net',
+        'zman@test.net',
         'b_user_2@zulip.net',
         'b_user_1@zulip.net',
-        'zman@test.net',
-        'b_bot@example.com',
     ]);
 
-    recipients = th.sort_recipientbox_typeahead("b, a, b", matches, "Dev"); // search "b"
+    recipients = th.sort_recipientbox_typeahead("b, a, b", matches, ""); // search "b"
     recipients_email = _.map(recipients, function (person) {
         return person.email;
     });
     assert.deepEqual(recipients_email, [
+        'b_bot@example.com',
+        'b_user_3@zulip.net',
         'b_user_2@zulip.net',
         'b_user_1@zulip.net',
-        'b_bot@example.com',
-        'a_bot@zulip.com',
         'a_user@zulip.org',
         'zman@test.net',
+        'a_bot@zulip.com',
     ]);
 }());

static/js/composebox_typeahead.js

@@ -401,7 +401,8 @@ exports.initialize_compose_typeahead = function (selector, completions) {
                 return typeahead_helper.sort_emojis(matches, this.token);
             } else if (this.completing === 'mention') {
                 return typeahead_helper.sort_recipients(matches, this.token,
-                                                        compose_state.stream_name());
+                                                        compose_state.stream_name(),
+                                                        compose_state.subject());
             } else if (this.completing === 'stream') {
                 return typeahead_helper.sort_streams(matches, this.token);
             } else if (this.completing === 'syntax') {
@@ -506,9 +507,9 @@ exports.initialize = function () {
             return query_matches_person(current_recipient, item);
         },
         sorter: function (matches) {
-            var current_stream = compose_state.stream_name();
+            // var current_stream = compose_state.stream_name();
             return typeahead_helper.sort_recipientbox_typeahead(
-                this.query, matches, current_stream);
+                this.query, matches, "");
         },
         updater: function (item, event) {
             var previous_recipients = typeahead_helper.get_cleaned_pm_recipients(this.query);

static/js/typeahead_helper.js

@@ -114,28 +114,6 @@ exports.render_stream = function (stream) {
     return html;
 };
 
-function split_by_subscribers(people, current_stream) {
-    var subscribers = [];
-    var non_subscribers = [];
-
-    if (!stream_data.get_sub(current_stream)) {
-        // If there is no stream specified, everyone is considered as a subscriber.
-        return {subscribers: people, non_subscribers: []};
-    }
-
-    _.each(people, function (person) {
-        if (person.email === "all" || person.email === "everyone") {
-            subscribers.push(person);
-        } else if (stream_data.user_is_subscribed(current_stream, person.email)) {
-            subscribers.push(person);
-        } else {
-            non_subscribers.push(person);
-        }
-    });
-
-    return {subscribers: subscribers, non_subscribers: non_subscribers};
-}
-
 exports.sorter = function (query, objs, get_item) {
    var results = util.prefix_sort(query, objs, get_item);
    return results.matches.concat(results.rest);
@@ -167,12 +145,74 @@ exports.compare_by_pms = function (user_a, user_b) {
     return 1;
 };
 
-exports.sort_for_at_mentioning = function (objs, current_stream) {
+function compare_for_at_mentioning(person_a, person_b, tertiary_compare, current_stream) {
-    var objs_split = split_by_subscribers(objs, current_stream);
+    // give preference to "all" or "everyone"
+    if (person_a.email === "all" || person_a.email === "everyone") {
+        return -1;
+    } else if (person_b.email === "all" || person_b.email === "everyone") {
+        return 1;
+    }
 
-    var subs_sorted = objs_split.subscribers.sort(exports.compare_by_pms);
+    // give preference to subscribed users first
-    var non_subs_sorted = objs_split.non_subscribers.sort(exports.compare_by_pms);
+    if (current_stream !== undefined) {
-    return subs_sorted.concat(non_subs_sorted);
+        var a_is_sub = stream_data.user_is_subscribed(current_stream, person_a.email);
+        var b_is_sub = stream_data.user_is_subscribed(current_stream, person_b.email);
+
+        if (a_is_sub && !b_is_sub) {
+            return -1;
+        } else if (!a_is_sub && b_is_sub) {
+            return 1;
+        }
+    }
+
+    // give preference to pm partners if both (are)/(are not) subscribers
+    var a_is_partner = pm_conversations.is_partner(person_a.user_id);
+    var b_is_partner = pm_conversations.is_partner(person_b.user_id);
+
+    if (a_is_partner && !b_is_partner) {
+        return -1;
+    } else if (!a_is_partner && b_is_partner) {
+        return 1;
+    }
+
+    return tertiary_compare(person_a, person_b);
+}
+
+exports.sort_for_at_mentioning = function (objs, current_stream_name, current_subject) {
+    // If sorting for recipientbox typeahead or compose state is private, then current_stream = ""
+    var current_stream = false;
+    if (current_stream_name) {
+        current_stream = stream_data.get_sub(current_stream_name);
+    }
+    if (!current_stream) {
+        objs.sort(function (person_a, person_b) {
+            return compare_for_at_mentioning(
+                person_a,
+                person_b,
+                exports.compare_by_pms
+            );
+        });
+    } else {
+        var stream_id = current_stream.stream_id;
+
+        objs.sort(function (person_a, person_b) {
+            return compare_for_at_mentioning(
+                person_a,
+                person_b,
+                function (user_a, user_b) {
+                    return recent_senders.compare_by_recency(
+                        user_a,
+                        user_b,
+                        stream_id,
+                        current_subject
+                    );
+                },
+                current_stream.name
+            );
+        });
+    }
+
+    return objs;
 };
 
 exports.compare_by_popularity = function (lang_a, lang_b) {
@@ -193,18 +233,20 @@ exports.sort_languages = function (matches, query) {
     return results.matches.concat(results.rest);
 };
 
-exports.sort_recipients = function (matches, query, current_stream) {
+exports.sort_recipients = function (matches, query, current_stream, current_subject) {
     var name_results =  util.prefix_sort(query, matches, function (x) { return x.full_name; });
     var email_results = util.prefix_sort(query, name_results.rest,
         function (x) { return x.email; });
 
     var matches_sorted = exports.sort_for_at_mentioning(
         name_results.matches.concat(email_results.matches),
-        current_stream
+        current_stream,
+        current_subject
     );
     var rest_sorted = exports.sort_for_at_mentioning(
         email_results.rest,
-        current_stream
+        current_stream,
+        current_subject
     );
     return matches_sorted.concat(rest_sorted);
 };