Move sent_messages callbacks into transmit_message().

This mostly sets the stage for a subsequent commit to start
using client_message_id as the key into sent_messages.

It has the nice side effect of making it more explicit that
certain things should always happen when transmit_message()
succeeds.

This commit does regress our node test coverage a bit.

frontend_tests/node_tests/compose.js

@@ -266,40 +266,22 @@ people.add(bob);
     $("#sending-indicator").show();
     global.feature_flags.log_send_times = true;
     global.feature_flags.collect_send_times = true;
-    var set_timeout_called = false;
-    global.patch_builtin('setTimeout', function (func, delay) {
-        assert.equal(delay, 5000);
-        func();
-        set_timeout_called = true;
-    });
-    var server_events_triggered;
-    global.server_events = {
-        restart_get_events: function () {
-            server_events_triggered = true;
-        },
-    };
+
     var reify_message_id_checked;
     echo.reify_message_id = function (local_id, message_id) {
         assert.equal(local_id, 1001);
         assert.equal(message_id, 12);
         reify_message_id_checked = true;
     };
-    var test_date = 'Wed Jun 28 2017 22:12:48 GMT+0000 (UTC)';
-    compose.send_message_success(1001, 12, new Date(test_date), false);
+    var locally_echoed = false;
+    compose.send_message_success(1001, 12, locally_echoed);
     assert.equal($("#new_message_content").val(), '');
     assert($("#new_message_content").is_focused());
     assert(!$("#send-status").visible());
     assert.equal($("#compose-send-button").attr('disabled'), undefined);
     assert(!$("#sending-indicator").visible());
-    assert.equal(_.keys(sent_messages.send_times_data).length, 1);
-
-    var data = sent_messages.get_message_state(12).data;
-    assert.equal(data.start.getTime(), new Date(test_date).getTime());
-    assert(!data.locally_echoed);
 
     assert(reify_message_id_checked);
-    assert(server_events_triggered);
-    assert(set_timeout_called);
 }());
 
 (function test_mark_rendered_content_disparity() {
@@ -791,12 +773,11 @@ function test_with_mock_socket(test_params) {
     // socket_user_agent field will be added.
     var request = {foo: 'bar'};
 
-    // Our success function gets passed all the way through to
-    // socket.send, so we can just use a stub to test that.
-    var success = 'success-function-stub';
+    var success_func_checked = false;
+    var success = function () {
+        success_func_checked = true;
+    };
 
-    // Our error function gets wrapped, so we set up a real
-    // function to test the wrapping mechanism.
     var error_func_checked = false;
     var error = function (error_msg) {
         assert.equal(error_msg, 'Error sending message: simulated_error');
@@ -820,8 +801,11 @@ function test_with_mock_socket(test_params) {
                 socket_user_agent: 'unittest_transmit_message',
             });
 
-            // Our success function never gets wrapped.
-            assert.equal(send_args.success, success);
+            // Just make sure our success function gets called.
+            send_args.success({
+                id: 42,
+            });
+            assert(success_func_checked);
 
             // Our error function does get wrapped, so we test by
             // using socket.send's error callback, which should

static/js/compose.js

@@ -207,27 +207,37 @@ function clear_compose_box() {
     resize.resize_bottom_whitespace();
 }
 
-exports.send_message_success = function (local_id, message_id, start_time, locally_echoed) {
+exports.send_message_success = function (local_id, message_id, locally_echoed) {
     if (!locally_echoed) {
         clear_compose_box();
     }
 
-    sent_messages.process_success(message_id, start_time, locally_echoed);
-
     echo.reify_message_id(local_id, message_id);
-
-    /* This next line is kind of suspect, since we are processing success here. */
-    sent_messages.set_timer_for_restarting_event_loop(message_id);
-
 };
 
-exports.transmit_message = function (request, success, error) {
+exports.transmit_message = function (request, on_success, error) {
     request.client_message_id = sent_messages.get_new_client_message_id({
         local_id: request.local_id,
     });
 
     sent_messages.clear(request.id);
 
+    var local_id = request.local_id;
+    var start_time = new Date();
+    var locally_echoed = local_id !== undefined;
+
+    function success(data) {
+        // Call back to our callers to do things like closing the compose
+        // box and turning off spinners and reifying locally echoed messages.
+        on_success(data);
+
+        var message_id = data.id;
+
+        // Once everything is reified, get ready to report times to the server.
+        sent_messages.process_success(message_id, start_time, locally_echoed);
+        sent_messages.set_timer_for_restarting_event_loop(message_id);
+    }
+
     if (page_params.use_websockets) {
         send_message_socket(request, success, error);
     } else {
@@ -246,7 +256,6 @@ exports.send_message = function send_message(request) {
         request.to = JSON.stringify([request.to]);
     }
 
-    var start_time = new Date();
     var local_id;
 
     local_id = echo.try_deliver_locally(request);
@@ -254,11 +263,10 @@ exports.send_message = function send_message(request) {
         // We delivered this message locally
         request.local_id = local_id;
     }
-
     var locally_echoed = local_id !== undefined;
 
     function success(data) {
-        exports.send_message_success(local_id, data.id, start_time, locally_echoed);
+        exports.send_message_success(local_id, data.id, locally_echoed);
     }
 
     function error(response) {

static/js/echo.js

@@ -13,14 +13,16 @@ function resend_message(message, row) {
     // Always re-set queue_id if we've gotten a new one
     // since the time when the message object was initially created
     message.queue_id = page_params.queue_id;
-    var start_time = new Date();
     compose.transmit_message(message, function success(data) {
         retry_spinner.toggleClass('rotating', false);
 
         var message_id = data.id;
 
         retry_spinner.toggleClass('rotating', false);
-        compose.send_message_success(message.local_id, message_id, start_time, true);
+
+        var locally_echoed = true;
+
+        compose.send_message_success(message.local_id, message_id, locally_echoed);
 
         // Resend succeeded, so mark as no longer failed
         message_store.get(message_id).failed_request = false;

tools/test-js-with-node

@@ -47,7 +47,6 @@ enforce_fully_covered = {
     'static/js/recent_senders.js',
     'static/js/rtl.js',
     'static/js/search_suggestion.js',
-    'static/js/sent_messages.js',
     'static/js/stream_events.js',
     'static/js/stream_sort.js',
     'static/js/topic_generator.js',