typing_status: Combine two parameters into one, with a maybe-type.

The main motivation for this change is to simplify this interface
and make it easier to reason about.

The case where it affects the behavior is when
is_valid_conversation() returns false, while current_recipient
and get_recipient() agree on some truthy value.

This means the message-content textarea is empty -- in fact the
user just cleared it, because we got here from an input event on
it -- but the compose box is still open to some PM thread that we
have a typing notification still outstanding for.

The old behavior is that in this situation we would ignore the
fact that the content was empty, and go ahead and prolong the
typing notification, by updating our timer and possibly sending a
"still typing" notice.

This contrasts with the behavior (both old and new) in the case
where the content is empty and we *don't* already have an
outstanding typing notification, or we have one to some other
thread.  In that case, we cancel any existing notification and
don't start a new one, exactly as if `stop` were called
(e.g. because the user closed the compose box.)

The new behavior is that we always treat clearing the input as
"stopped typing": not only in those cases where we already did,
but also in the case where we still have the same recipients.
(Which seems like probably the common case.)

That seems like the preferable behavior; indeed it's hard to see
the point of the "compose_empty" logic if restricted to the other
cases.  It also makes the interface simpler.

Those two properties don't seem like a coincidence, either: the
complicated interface made it difficult to unpack exactly what
logic we actually had, which made it easy for surprising wrinkles
to hang out indefinitely.

frontend_tests/node_tests/typing_status.js

@@ -16,7 +16,7 @@ run_test('basics', () => {
 
     // invalid conversation basically does nothing
     var worker = {};
-    typing_status.handle_text_input(worker, "alice", false);
+    typing_status.handle_text_input(worker, undefined);
 
     // Start setting up more testing state.
     typing_status.initialize_state();
@@ -53,9 +53,9 @@ run_test('basics', () => {
         events.timer_cleared = false;
     }
 
-    function call_handler(new_recipient, conversation_is_valid) {
+    function call_handler(new_recipient) {
         clear_events();
-        typing_status.handle_text_input(worker, new_recipient, conversation_is_valid);
+        typing_status.handle_text_input(worker, new_recipient);
     }
 
     function call_stop() {
@@ -70,7 +70,7 @@ run_test('basics', () => {
     };
 
     // Start talking to alice.
-    call_handler("alice", true);
+    call_handler("alice");
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(5 + 10),
         idle_timer: 'idle_timer_stub',
@@ -86,7 +86,7 @@ run_test('basics', () => {
 
     // type again 3 seconds later
     worker.get_current_time = returns_time(8);
-    call_handler("alice", true);
+    call_handler("alice");
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(5 + 10),
         idle_timer: 'idle_timer_stub',
@@ -103,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("alice", true);
+    call_handler("alice");
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(18 + 10),
         idle_timer: 'idle_timer_stub',
@@ -148,7 +148,7 @@ run_test('basics', () => {
 
     // Start talking to alice again.
     worker.get_current_time = returns_time(50);
-    call_handler("alice", true);
+    call_handler("alice");
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(50 + 10),
         idle_timer: 'idle_timer_stub',
@@ -178,7 +178,7 @@ run_test('basics', () => {
 
     // Start talking to alice again.
     worker.get_current_time = returns_time(80);
-    call_handler("alice", true);
+    call_handler("alice");
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(80 + 10),
         idle_timer: 'idle_timer_stub',
@@ -193,7 +193,7 @@ run_test('basics', () => {
     assert(events.idle_callback);
 
     // Switch to an invalid conversation.
-    call_handler('not-alice', false);
+    call_handler(undefined);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: undefined,
         idle_timer: undefined,
@@ -207,7 +207,7 @@ run_test('basics', () => {
     });
 
     // Switch to another invalid conversation.
-    call_handler('another-bogus-one', false);
+    call_handler(undefined);
     assert.deepEqual(typing_status.state, {
         next_send_start_time: undefined,
         idle_timer: undefined,
@@ -222,7 +222,7 @@ run_test('basics', () => {
 
     // Start talking to alice again.
     worker.get_current_time = returns_time(170);
-    call_handler("alice", true);
+    call_handler("alice");
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(170 + 10),
         idle_timer: 'idle_timer_stub',
@@ -244,7 +244,7 @@ run_test('basics', () => {
         events.started = true;
     };
 
-    call_handler("bob", true);
+    call_handler("bob");
     assert.deepEqual(typing_status.state, {
         next_send_start_time: make_time(171 + 10),
         idle_timer: 'idle_timer_stub',
@@ -281,12 +281,12 @@ 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.get_recipient(), true);
+    typing_status.handle_text_input(worker, typing.get_recipient());
     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.get_recipient(), true);
+    typing_status.handle_text_input(worker, typing.get_recipient());
     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);
@@ -294,7 +294,7 @@ run_test('basics', () => {
     // 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.get_recipient(), true);
+    typing_status.handle_text_input(worker, typing.get_recipient());
     assert.deepEqual(call_count.maybe_ping_server, 2);
     assert.deepEqual(call_count.start_or_extend_idle_timer, 3);
     assert.deepEqual(call_count.stop_last_notification, 1);

static/js/typing.js

@@ -32,13 +32,7 @@ function get_user_ids_array() {
     return people.user_ids_string_to_ids_array(user_ids_string);
 }
 
-function is_valid_conversation(user_ids_array) {
-    // TODO: Check to make sure we're in a PM conversation
-    //       with valid emails.
-    if (!user_ids_array) {
-        return false;
-    }
-
+function is_valid_conversation() {
     var compose_empty = !compose_state.has_message_content();
     if (compose_empty) {
         return false;
@@ -70,9 +64,9 @@ exports.initialize = function () {
     $(document).on('input', '#compose-textarea', function () {
         // If our previous state was no typing notification, send a
         // start-typing notice immediately.
-        var new_recipient = exports.get_recipient();
-        typing_status.handle_text_input(
-            worker, new_recipient, is_valid_conversation(new_recipient));
+        var new_recipient =
+          is_valid_conversation() ? exports.get_recipient() : undefined;
+        typing_status.handle_text_input(worker, new_recipient);
     });
 
     // We send a stop-typing notification immediately when compose is

static/shared/js/typing_status.js

@@ -85,7 +85,7 @@ export function maybe_ping_server(worker, recipient) {
     }
 }
 
-export function handle_text_input(worker, new_recipient, conversation_is_valid) {
+export function handle_text_input(worker, new_recipient) {
     var current_recipient = state.current_recipient;
     if (current_recipient) {
         // We need to use _.isEqual for comparisons; === doesn't work
@@ -107,7 +107,7 @@ export function handle_text_input(worker, new_recipient, conversation_is_valid)
         stop_last_notification(worker);
     }
 
-    if (!conversation_is_valid) {
+    if (!new_recipient) {
         // If we are not talking to somebody we care about,
         // then there is no more action to take.
         return;

static/shared/js/typing_status.js.flow

@@ -15,7 +15,6 @@ type Worker = {|
 declare export function handle_text_input(
   worker: Worker,
   new_recipient: RecipientUserIds | void,
-  conversation_is_valid: boolean
 ): void;
 
 declare export function stop(worker: Worker): void;