typing_status: Use parameters for data rather than callbacks.

The real purpose these two callbacks serve is exactly what an ordinary
parameter is perfect for:
 * Each has just one call site, at the top of the function.
 * They're not done for side effects; the point is what they return.
 * The function doesn't pass them any arguments of its own, or
   otherwise express any internal knowledge that doesn't just as
   properly belong to its caller.

So, push the calls to these callbacks up into the function's caller,
and pass in the data they return instead.

This greatly simplifies the interface of `handle_text_input` and of
`typing_status` in general.

frontend_tests/node_tests/typing_status.js

@@ -3,11 +3,6 @@ zrequire('people');
 zrequire('compose_pm_pill');
 const typing_status = zrequire('typing_status', 'shared/js/typing_status');
 
-function return_false() { return false; }
-function return_true() { return true; }
-function return_alice() { return "alice"; }
-function return_bob() { return "bob"; }
-
 function make_time(secs) {
     // make times semi-realistic
     return 1000000 + 1000 * secs;
@@ -20,11 +15,8 @@ function returns_time(secs) {
 run_test('basics', () => {
 
     // invalid conversation basically does nothing
-    var worker = {
-        get_recipient: return_alice,
-        is_valid_conversation: return_false,
-    };
-    typing_status.handle_text_input(worker);
+    var worker = {};
+    typing_status.handle_text_input(worker, "alice", false);
 
     // Start setting up more testing state.
     typing_status.initialize_state();
@@ -61,9 +53,9 @@ run_test('basics', () => {
         events.timer_cleared = false;
     }
 
-    function call_handler() {
+    function call_handler(new_recipient, conversation_is_valid) {
         clear_events();
-        typing_status.handle_text_input(worker);
+        typing_status.handle_text_input(worker, new_recipient, conversation_is_valid);
     }
 
     function call_stop() {
@@ -72,15 +64,13 @@ run_test('basics', () => {
     }
 
     worker = {
-        get_recipient: return_alice,
-        is_valid_conversation: return_true,
         get_current_time: returns_time(5),
         notify_server_start: notify_server_start,
         notify_server_stop: notify_server_stop,
     };
 
     // Start talking to alice.
-    call_handler();
+    call_handler("alice", true);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(5 + 10),
         idle_timer: 'idle_timer_stub',
@@ -96,7 +86,7 @@ run_test('basics', () => {
 
     // type again 3 seconds later
     worker.get_current_time = returns_time(8);
-    call_handler();
+    call_handler("alice", true);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(5 + 10),
         idle_timer: 'idle_timer_stub',
@@ -113,7 +103,7 @@ run_test('basics', () => {
     // type after 15 secs, so that we can notify the server
     // again
     worker.get_current_time = returns_time(18);
-    call_handler();
+    call_handler("alice", true);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(18 + 10),
         idle_timer: 'idle_timer_stub',
@@ -158,7 +148,7 @@ run_test('basics', () => {
 
     // Start talking to alice again.
     worker.get_current_time = returns_time(50);
-    call_handler();
+    call_handler("alice", true);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(50 + 10),
         idle_timer: 'idle_timer_stub',
@@ -188,7 +178,7 @@ run_test('basics', () => {
 
     // Start talking to alice again.
     worker.get_current_time = returns_time(80);
-    call_handler();
+    call_handler("alice", true);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(80 + 10),
         idle_timer: 'idle_timer_stub',
@@ -203,11 +193,7 @@ run_test('basics', () => {
     assert(events.idle_callback);
 
     // Switch to an invalid conversation.
-    worker.get_recipient = function () {
-        return 'not-alice';
-    };
-    worker.is_valid_conversation = return_false;
-    call_handler();
+    call_handler('not-alice', false);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: undefined,
         idle_timer: undefined,
@@ -221,11 +207,7 @@ run_test('basics', () => {
     });
 
     // Switch to another invalid conversation.
-    worker.get_recipient = function () {
-        return 'another-bogus-one';
-    };
-    worker.is_valid_conversation = return_false;
-    call_handler();
+    call_handler('another-bogus-one', false);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: undefined,
         idle_timer: undefined,
@@ -239,10 +221,8 @@ run_test('basics', () => {
     });
 
     // Start talking to alice again.
-    worker.get_recipient = return_alice;
-    worker.is_valid_conversation = return_true;
     worker.get_current_time = returns_time(170);
-    call_handler();
+    call_handler("alice", true);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(170 + 10),
         idle_timer: 'idle_timer_stub',
@@ -257,16 +237,14 @@ run_test('basics', () => {
     assert(events.idle_callback);
 
     // Switch to bob now.
-    worker.get_recipient = return_bob;
-    worker.is_valid_conversation = return_true;
     worker.get_current_time = returns_time(171);
 
     worker.notify_server_start = function (recipient) {
         assert.equal(recipient, "bob");
         events.started = true;
     };
 
-    call_handler();
+    call_handler("bob", true);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(171 + 10),
         idle_timer: 'idle_timer_stub',
@@ -282,8 +260,7 @@ run_test('basics', () => {
 
     // test that we correctly detect if worker.get_recipient
     // and typing_status.state.current_recipient are the same
-    worker.get_recipient = typing.get_recipient;
-    worker.is_valid_conversation = () => false;
+
     compose_pm_pill.get_user_ids_string = () => '1,2,3';
     typing_status.state.current_recipient = typing.get_recipient();
 
@@ -303,20 +280,20 @@ run_test('basics', () => {
 
     // User ids of poeple in compose narrow doesn't change and is same as stat.current_recipent
     // so counts of function should increase except stop_last_notification
-    typing_status.handle_text_input(worker);
+    typing_status.handle_text_input(worker, typing.get_recipient(), false);
     assert.deepEqual(call_count.maybe_ping_server, 1);
     assert.deepEqual(call_count.start_or_extend_idle_timer, 1);
     assert.deepEqual(call_count.stop_last_notification, 0);
 
-    typing_status.handle_text_input(worker);
+    typing_status.handle_text_input(worker, typing.get_recipient(), false);
     assert.deepEqual(call_count.maybe_ping_server, 2);
     assert.deepEqual(call_count.start_or_extend_idle_timer, 2);
     assert.deepEqual(call_count.stop_last_notification, 0);
 
     // change in recipient and new_recipient should make us
     // call typing_status.stop_last_notification
     compose_pm_pill.get_user_ids_string = () => '2,3,4';
-    typing_status.handle_text_input(worker);
+    typing_status.handle_text_input(worker, typing.get_recipient(), false);
     assert.deepEqual(call_count.maybe_ping_server, 2);
     assert.deepEqual(call_count.start_or_extend_idle_timer, 2);
     assert.deepEqual(call_count.stop_last_notification, 1);

static/js/typing.js

@@ -75,8 +75,6 @@ function notify_server_stop(user_ids_array) {
 exports.get_recipient = get_user_ids_array;
 exports.initialize = function () {
     var worker = {
-        get_recipient: exports.get_recipient,
-        is_valid_conversation: is_valid_conversation,
         get_current_time: get_current_time,
         notify_server_start: notify_server_start,
         notify_server_stop: notify_server_stop,
@@ -85,7 +83,9 @@ exports.initialize = function () {
     $(document).on('input', '#compose-textarea', function () {
         // If our previous state was no typing notification, send a
         // start-typing notice immediately.
-        typing_status.handle_text_input(worker);
+        var new_recipient = exports.get_recipient();
+        typing_status.handle_text_input(
+            worker, new_recipient, is_valid_conversation(new_recipient));
     });
 
     // We send a stop-typing notification immediately when compose is

static/shared/js/typing_status.js

@@ -21,9 +21,7 @@ var TYPING_STOPPED_WAIT_PERIOD = 5000; // 5s
 
         notify_server_start
         notify_server_stop
-        get_recipient
         get_current_time
-        is_valid_conversation
 
     See typing.js for the implementations of the above. (Our
     node tests also act as workers and will stub those functions
@@ -87,10 +85,7 @@ export function maybe_ping_server(worker, recipient) {
     }
 }
 
-export function handle_text_input(worker) {
-    var new_recipient = worker.get_recipient();
-    var conversation_is_valid = worker.is_valid_conversation(new_recipient);
-
+export function handle_text_input(worker, new_recipient, conversation_is_valid) {
     var current_recipient = state.current_recipient;
     if (current_recipient) {
         // We need to use _.isEqual for comparisons; === doesn't work

static/shared/js/typing_status.js.flow

@@ -7,13 +7,15 @@
 type RecipientUserIds = number[];
 
 type Worker = {|
-  get_recipient: () => RecipientUserIds | void,
-  is_valid_conversation: (RecipientUserIds | void) => boolean,
   get_current_time: () => number, // as ms since epoch
   notify_server_start: RecipientUserIds => void,
   notify_server_stop: RecipientUserIds => void
 |};
 
-declare export function handle_text_input(Worker): void;
+declare export function handle_text_input(
+  worker: Worker,
+  new_recipient: RecipientUserIds | void,
+  conversation_is_valid: boolean
+): void;
 
-declare export function stop(Worker): void;
+declare export function stop(worker: Worker): void;