From 2991c19fea2f9fc94c7fe6537bc173b31c8d9dbe Mon Sep 17 00:00:00 2001
From: Steve Howell <showell30@yahoo.com>
Date: Wed, 22 Mar 2017 08:20:16 -0700
Subject: [PATCH] Extract typing indicator inbound timing logic.

We now track our inbound timing events using code in
typing_data.js.

This code may be a little more robust with variations on how
recipients are represented in events, although there are no known
bugs here.
---
 frontend_tests/node_tests/typing_data.js | 75 ++++++++++++++++++++++++
 static/js/typing_data.js                 | 20 +++++++
 static/js/typing_events.js               | 26 +++-----
 3 files changed, 104 insertions(+), 17 deletions(-)

diff --git a/frontend_tests/node_tests/typing_data.js b/frontend_tests/node_tests/typing_data.js
index bee4541b7c..b66a51d4f4 100644
--- a/frontend_tests/node_tests/typing_data.js
+++ b/frontend_tests/node_tests/typing_data.js
@@ -45,3 +45,78 @@ var typing_data = require("js/typing_data.js");
     typing_data.add_typist([20, 40, 20], 20);
     assert.deepEqual(typing_data.get_group_typists([20, 40]), [20]);
 }());
+
+
+(function test_timers() {
+    var events = {};
+
+    var stub_timer_id = 'timer_id_stub';
+    var stub_group = [5, 10, 15];
+    var stub_delay = 99;
+    var stub_f = 'function';
+
+    function set_timeout(f, delay) {
+        assert.equal(delay, stub_delay);
+        events.f = f;
+        events.timer_set = true;
+        return stub_timer_id;
+    }
+
+    function clear_timeout(timer) {
+        assert.equal(timer, stub_timer_id);
+        events.timer_cleared = true;
+    }
+
+    function reset_events() {
+        events.f = undefined;
+        events.timer_cleared = false;
+        events.timer_set = false;
+    }
+
+    function kickstart() {
+        reset_events();
+        typing_data.kickstart_inbound_timer(stub_group, stub_delay, stub_f);
+    }
+
+    function clear() {
+        reset_events();
+        typing_data.clear_inbound_timer(stub_group);
+    }
+
+    global.patch_builtin('setTimeout', set_timeout);
+    global.patch_builtin('clearTimeout', clear_timeout);
+
+    // first time, we set
+    kickstart();
+    assert.deepEqual(events, {
+        f: stub_f,
+        timer_cleared: false,
+        timer_set: true,
+    });
+
+    // second time we clear and set
+    kickstart();
+    assert.deepEqual(events, {
+        f: stub_f,
+        timer_cleared: true,
+        timer_set: true,
+    });
+
+    // first time clearing, we clear
+    clear();
+    assert.deepEqual(events, {
+        f: undefined,
+        timer_cleared: true,
+        timer_set: false,
+    });
+
+    // second time clearing, we noop
+    clear();
+    assert.deepEqual(events, {
+        f: undefined,
+        timer_cleared: false,
+        timer_set: false,
+    });
+
+}());
+
diff --git a/static/js/typing_data.js b/static/js/typing_data.js
index d02cb1f216..747f13076c 100644
--- a/static/js/typing_data.js
+++ b/static/js/typing_data.js
@@ -2,6 +2,7 @@ var typing_data = (function () {
 var exports = {};
 
 var typist_dct = new Dict();
+var inbound_timer_dict = new Dict();
 
 function to_int(s) {
     return parseInt(s, 10);
@@ -63,6 +64,25 @@ exports.get_all_typists = function () {
     return typists;
 };
 
+// The next functions aren't pure data, but it is easy
+// enough to mock the setTimeout/clearTimeout functions.
+exports.clear_inbound_timer = function (group) {
+    var key = get_key(group);
+    var timer = inbound_timer_dict.get(key);
+    if (timer) {
+        clearTimeout(timer);
+        inbound_timer_dict.set(key, undefined);
+    }
+};
+
+exports.kickstart_inbound_timer = function (group, delay, callback) {
+    var key = get_key(group);
+    exports.clear_inbound_timer(group);
+    var timer = setTimeout(callback, delay);
+    inbound_timer_dict.set(key, timer);
+};
+
+
 return exports;
 }());
 
diff --git a/static/js/typing_events.js b/static/js/typing_events.js
index fdbca816b6..bd051c12ca 100644
--- a/static/js/typing_events.js
+++ b/static/js/typing_events.js
@@ -14,8 +14,6 @@ var TYPING_STARTED_EXPIRY_PERIOD = 15000; // 15s
 // Note!: There are also timing constants in typing_status.js
 // that make typing indicators work.
 
-var stop_typing_timers = new Dict();
-
 function get_users_typing_for_narrow() {
     if (!narrow.narrowed_to_pms()) {
         // Narrow is neither pm-with nor is: private
@@ -53,12 +51,7 @@ exports.hide_notification = function (event) {
     });
     recipients.sort();
 
-    // If there's an existing timer for this typing notifications
-    // thread, clear it.
-    if (stop_typing_timers[recipients] !== undefined) {
-        clearTimeout(stop_typing_timers[recipients]);
-        stop_typing_timers[recipients] = undefined;
-    }
+    typing_data.clear_inbound_timer(recipients);
 
     var removed = typing_data.remove_typist(recipients, event.sender.user_id);
 
@@ -79,15 +72,14 @@ exports.display_notification = function (event) {
     typing_data.add_typist(recipients, sender_id);
 
     render_notifications_for_narrow();
-    // If there's an existing timeout for this typing notifications
-    // thread, clear it.
-    if (stop_typing_timers[recipients] !== undefined) {
-        clearTimeout(stop_typing_timers[recipients]);
-    }
-    // Set a time to expire the data if the sender stops transmitting
-    stop_typing_timers[recipients] = setTimeout(function () {
-        exports.hide_notification(event);
-    }, TYPING_STARTED_EXPIRY_PERIOD);
+
+    typing_data.kickstart_inbound_timer(
+        recipients,
+        TYPING_STARTED_EXPIRY_PERIOD,
+        function () {
+            exports.hide_notification(event);
+        }
+    );
 };
 
 $(document).on('narrow_activated.zulip', render_notifications_for_narrow);