paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-18 12:54:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: Extract transmit.js from compose.js.

Browse Source 
We now isolate the code to transmit messages into transmit.js.
It is stable code that most folks doing UI work in compose.js don't
care about the details of, so it's just clutter there.  Also, we may
soon have other widgets than the compose box that send messages.

This change mostly preserves test coverage, although in some cases
we stub at a higher level for the compose path (this is a good thing).
Extracting out transmit.js allows us to lock down 100% coverage on that
file.
This commit is contained in:
Steve Howell
2018-02-20 07:08:50 -05:00
committed by Tim Abbott
parent 80ff3d8da5
commit 1f6ddf0110
8 changed files with 259 additions and 202 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.eslintrc.json
View File

@@ -65,6 +65,7 @@
"typing_data": false,
"typing_status": false,
"sent_messages": false,
"transmit": false,
"compose": false,
"compose_actions": false,
"compose_state": false,

158
frontend_tests/node_tests/compose.js
View File

@@ -2,15 +2,14 @@ set_global('$', global.make_zjquery());
set_global('i18n', global.stub_i18n);
set_global('page_params', {
use_websockets: true,
});
set_global('navigator', {});
set_global('document', {
getElementById: function () { return $('#compose-textarea'); },
execCommand: function () { return false; },
location: {},
});
set_global('transmit', {});
set_global('channel', {});
set_global('templates', {});
@@ -27,17 +26,12 @@ set_global('feature_flags', {
resize_bottom_whitespace: noop,
});
set_global('echo', {});
set_global('socket', {});
set_global('Socket', function () {
return global.socket;
});
set_global('stream_edit', {});
set_global('markdown', {});
set_global('loading', {});
set_global('sent_messages', {
start_tracking_message: noop,
report_server_ack: noop,
});
set_global('notifications', {
notify_above_composebox: noop,
@@ -58,7 +52,6 @@ zrequire('compose_state');
zrequire('people');
zrequire('compose');
zrequire('upload');
page_params.use_websockets = false;
var me = {
email: 'me@example.com',
@@ -404,7 +397,7 @@ people.add(bob);
function initialize_state_stub_dict() {
stub_state = {};
stub_state.local_id_counter = 0;
stub_state.send_msg_ajax_post_called = 0;
stub_state.send_msg_called = 0;
stub_state.get_events_running_called = 0;
stub_state.reify_message_id_checked = 0;
return stub_state;
@@ -430,7 +423,7 @@ people.add(bob);
stub_state.local_id_counter += 1;
return stub_state.local_id_counter;
};
channel.post = function (payload) {
transmit.send_message = function (payload, success) {
var single_msg = {
type: 'private',
content: '[foobar](/user_uploads/123456)',
@@ -445,12 +438,10 @@ people.add(bob);
local_id: 1,
locally_echoed: true,
};
assert.equal(payload.url, '/json/messages');
assert.equal(_.keys(payload.data).length, 12);
assert.deepEqual(payload.data, single_msg);
payload.data.id = stub_state.local_id_counter;
payload.success(payload.data);
stub_state.send_msg_ajax_post_called += 1;
assert.deepEqual(payload, single_msg);
payload.id = stub_state.local_id_counter;
success(payload);
stub_state.send_msg_called += 1;
};
echo.reify_message_id = function (local_id, message_id) {
assert.equal(typeof(local_id), 'number');
@@ -473,7 +464,7 @@ people.add(bob);
local_id_counter: 1,
get_events_running_called: 1,
reify_message_id_checked: 1,
send_msg_ajax_post_called: 1,
send_msg_called: 1,
};
assert.deepEqual(stub_state, state);
assert.equal($("#compose-textarea").val(), '');
@@ -483,52 +474,14 @@ people.add(bob);
assert(!$("#sending-indicator").visible());
}());
(function test_error_code_path_when_error_type_not_timeout() {
stub_state = initialize_state_stub_dict();
compose_state.set_message_type('stream');
var server_error_triggered = false;
channel.post = function (payload) {
payload.error('500', 'Internal Server Error');
stub_state.send_msg_ajax_post_called += 1;
server_error_triggered = true;
};
var reload_initiate_triggered = false;
global.reload = {
is_pending: function () { return true; },
initiate: function () {
reload_initiate_triggered = true;
},
};
compose.send_message();
var state = {
local_id_counter: 1,
get_events_running_called: 1,
reify_message_id_checked: 0,
send_msg_ajax_post_called: 1,
};
assert.deepEqual(stub_state, state);
assert(server_error_triggered);
assert(reload_initiate_triggered);
}());
// This is the additional setup which is common to both the tests below.
var server_error_triggered = false;
var reload_initiate_triggered = false;
channel.post = function (payload) {
payload.error('408', 'timeout');
stub_state.send_msg_ajax_post_called += 1;
server_error_triggered = true;
transmit.send_message = function (payload, success, error) {
stub_state.send_msg_called += 1;
error('Error sending message: Server says 408');
};
var xhr_error_msg_checked = false;
channel.xhr_error_message = function (error, xhr) {
assert.equal(error, 'Error sending message');
assert.equal(xhr, '408');
xhr_error_msg_checked = true;
return 'Error sending message: Server says 408';
};
var echo_error_msg_checked = false;
var echo_error_msg_checked;
echo.message_send_error = function (local_id, error_response) {
assert.equal(local_id, 1);
assert.equal(error_response, 'Error sending message: Server says 408');
@@ -545,12 +498,9 @@ people.add(bob);
local_id_counter: 1,
get_events_running_called: 1,
reify_message_id_checked: 0,
send_msg_ajax_post_called: 1,
send_msg_called: 1,
};
assert.deepEqual(stub_state, state);
assert(server_error_triggered);
assert(!reload_initiate_triggered);
assert(xhr_error_msg_checked);
assert(echo_error_msg_checked);
}());
@@ -563,9 +513,6 @@ people.add(bob);
$("#sending-indicator").show();
$("#compose-textarea").select(noop);
echo_error_msg_checked = false;
xhr_error_msg_checked = false;
server_error_triggered = false;
reload_initiate_triggered = false;
echo.try_deliver_locally = function () {
return;
};
@@ -580,12 +527,9 @@ people.add(bob);
local_id_counter: 0,
get_events_running_called: 1,
reify_message_id_checked: 0,
send_msg_ajax_post_called: 1,
send_msg_called: 1,
};
assert.deepEqual(stub_state, state);
assert(server_error_triggered);
assert(!reload_initiate_triggered);
assert(xhr_error_msg_checked);
assert(!echo_error_msg_checked);
assert.equal($("#compose-send-button").prop('disabled'), false);
assert.equal($('#compose-error-msg').html(),
@@ -600,6 +544,7 @@ people.add(bob);
(function test_enter_with_preview_open() {
// Test sending a message with content.
compose_state.set_message_type('stream');
$("#compose-textarea").val('message me');
$("#compose-textarea").hide();
$("#undo_markdown_preview").show();
@@ -824,75 +769,6 @@ function test_raw_file_drop(raw_drop_func) {
}());
}());
function test_with_mock_socket(test_params) {
var socket_send_called;
var send_args = {};
global.socket.send = function (request, success, error) {
global.socket.send = undefined;
socket_send_called = true;
// Save off args for check_send_args callback.
send_args.request = request;
send_args.success = success;
send_args.error = error;
};
// Run the actual code here.
test_params.run_code();
assert(socket_send_called);
test_params.check_send_args(send_args);
}
(function test_transmit_message() {
page_params.use_websockets = true;
global.navigator.userAgent = 'unittest_transmit_message';
// Our request is mostly unimportant, except that the
// socket_user_agent field will be added.
var request = {foo: 'bar'};
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');
error_func_checked = true;
};
test_with_mock_socket({
run_code: function () {
compose.transmit_message(request, success, error);
},
check_send_args: function (send_args) {
// The real code patches new data on the request, rather
// than making a copy, so we test both that it didn't
// clone the object and that it did add a field.
assert.equal(send_args.request, request);
assert.deepEqual(send_args.request, {
foo: 'bar',
socket_user_agent: 'unittest_transmit_message',
});
send_args.success({});
assert(success_func_checked);
// Our error function does get wrapped, so we test by
// using socket.send's error callback, which should
// invoke our test error function via a wrapper
// function in the real code.
send_args.error('response', {msg: 'simulated_error'});
assert(error_func_checked);
},
});
}());
(function test_update_fade() {
var selector = '#stream,#subject,#private_message_recipient';
var keyup_handler_func = $(selector).get_on_handler('keyup');

169
frontend_tests/node_tests/transmit.js Normal file
View File

@@ -0,0 +1,169 @@
var noop = function () {};
set_global('page_params', {
use_websockets: true,
});
set_global('channel', {});
set_global('navigator', {});
set_global('reload', {});
set_global('socket', {});
set_global('Socket', function () {
return global.socket;
});
set_global('sent_messages', {
start_tracking_message: noop,
report_server_ack: noop,
});
zrequire('transmit');
function test_with_mock_socket(test_params) {
var socket_send_called;
var send_args = {};
global.socket.send = function (request, success, error) {
global.socket.send = undefined;
socket_send_called = true;
// Save off args for check_send_args callback.
send_args.request = request;
send_args.success = success;
send_args.error = error;
};
// Run the actual code here.
test_params.run_code();
assert(socket_send_called);
test_params.check_send_args(send_args);
}
(function test_transmit_message_sockets() {
page_params.use_websockets = true;
global.navigator.userAgent = 'unittest_transmit_message';
// Our request is mostly unimportant, except that the
// socket_user_agent field will be added.
var request = {foo: 'bar'};
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');
error_func_checked = true;
};
test_with_mock_socket({
run_code: function () {
transmit.send_message(request, success, error);
},
check_send_args: function (send_args) {
// The real code patches new data on the request, rather
// than making a copy, so we test both that it didn't
// clone the object and that it did add a field.
assert.equal(send_args.request, request);
assert.deepEqual(send_args.request, {
foo: 'bar',
socket_user_agent: 'unittest_transmit_message',
});
send_args.success({});
assert(success_func_checked);
// Our error function does get wrapped, so we test by
// using socket.send's error callback, which should
// invoke our test error function via a wrapper
// function in the real code.
send_args.error('response', {msg: 'simulated_error'});
assert(error_func_checked);
},
});
}());
page_params.use_websockets = false;
(function test_transmit_message_ajax() {
var success_func_called;
var success = function () {
success_func_called = true;
};
var request = {foo: 'bar'};
channel.post = function (opts) {
assert.equal(opts.url, '/json/messages');
assert.equal(opts.data.foo, 'bar');
opts.success();
};
transmit.send_message(request, success);
assert(success_func_called);
channel.xhr_error_message = function (msg) {
assert.equal(msg, 'Error sending message');
return msg;
};
channel.post = function (opts) {
assert.equal(opts.url, '/json/messages');
assert.equal(opts.data.foo, 'bar');
var xhr = 'whatever';
opts.error(xhr, 'timeout');
};
var error_func_called;
var error = function (response) {
assert.equal(response, 'Error sending message');
error_func_called = true;
};
transmit.send_message(request, success, error);
assert(error_func_called);
}());
(function test_transmit_message_ajax_reload_pending() {
var success = function () { throw 'unexpected success'; };
reload.is_pending = function () {
return true;
};
var reload_initiated;
reload.initiate = function (opts) {
reload_initiated = true;
assert.deepEqual(opts, {
immediate: true,
save_pointer: true,
save_narrow: true,
save_compose: true,
send_after_reload: true,
});
};
var request = {foo: 'bar'};
var error_func_called;
var error = function (response) {
assert.equal(response, 'Error sending message');
error_func_called = true;
};
error_func_called = false;
channel.post = function (opts) {
assert.equal(opts.url, '/json/messages');
assert.equal(opts.data.foo, 'bar');
var xhr = 'whatever';
opts.error(xhr, 'bad request');
};
transmit.send_message(request, success, error);
assert(!error_func_called);
assert(reload_initiated);
}());

62
static/js/compose.js
View File

@@ -188,46 +188,6 @@ function nonexistent_stream_reply_error() {
exports.nonexistent_stream_reply_error = nonexistent_stream_reply_error;
function send_message_ajax(request, success, error) {
channel.post({
url: '/json/messages',
data: request,
success: success,
error: function (xhr, error_type) {
if (error_type !== 'timeout' && reload.is_pending()) {
// The error might be due to the server changing
reload.initiate({immediate: true,
save_pointer: true,
save_narrow: true,
save_compose: true,
send_after_reload: true});
return;
}
var response = channel.xhr_error_message("Error sending message", xhr);
error(response);
},
});
}
var socket;
if (page_params.use_websockets) {
socket = new Socket("/sockjs");
}
// For debugging. The socket will eventually move out of this file anyway.
exports._socket = socket;
function send_message_socket(request, success, error) {
request.socket_user_agent = navigator.userAgent;
socket.send(request, success, function (type, resp) {
var err_msg = "Error sending message";
if (type === 'response') {
err_msg += ": " + resp.msg;
}
error(err_msg);
});
}
function clear_compose_box() {
$("#compose-textarea").val('').focus();
drafts.delete_draft_after_send();
@@ -246,24 +206,6 @@ exports.send_message_success = function (local_id, message_id, locally_echoed) {
echo.reify_message_id(local_id, message_id);
};
exports.transmit_message = function (request, on_success, error) {
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);
// Once everything is done, get ready to report times to the server.
sent_messages.report_server_ack(request.local_id);
}
if (page_params.use_websockets) {
send_message_socket(request, success, error);
} else {
send_message_ajax(request, success, error);
}
};
exports.send_message = function send_message(request) {
if (request === undefined) {
request = create_message_object();
@@ -317,7 +259,7 @@ exports.send_message = function send_message(request) {
echo.message_send_error(local_id, response);
}
exports.transmit_message(request, success, error);
transmit.send_message(request, success, error);
server_events.assert_get_events_running("Restarting get_events because it was not running during send");
if (locally_echoed) {
@@ -418,7 +360,7 @@ exports.schedule_message = function schedule_message(request, success, error) {
return;
}
exports.transmit_message(request, success, error);
transmit.send_message(request, success, error);
};
exports.enter_with_preview_open = function () {

2
static/js/echo.js
View File

@@ -36,7 +36,7 @@ function resend_message(message, row) {
}
sent_messages.start_resend(local_id);
compose.transmit_message(message, on_success, on_error);
transmit.send_message(message, on_success, on_error);
}
function truncate_precision(float) {

67
static/js/transmit.js Normal file
View File

@@ -0,0 +1,67 @@
var transmit = (function () {
var exports = {};
var socket;
if (page_params.use_websockets) {
socket = new Socket("/sockjs");
}
// For debugging. The socket will eventually move out of this file anyway.
exports._socket = socket;
function send_message_socket(request, success, error) {
request.socket_user_agent = navigator.userAgent;
socket.send(request, success, function (type, resp) {
var err_msg = "Error sending message";
if (type === 'response') {
err_msg += ": " + resp.msg;
}
error(err_msg);
});
}
function send_message_ajax(request, success, error) {
channel.post({
url: '/json/messages',
data: request,
success: success,
error: function (xhr, error_type) {
if (error_type !== 'timeout' && reload.is_pending()) {
// The error might be due to the server changing
reload.initiate({immediate: true,
save_pointer: true,
save_narrow: true,
save_compose: true,
send_after_reload: true});
return;
}
var response = channel.xhr_error_message("Error sending message", xhr);
error(response);
},
});
}
exports.send_message = function (request, on_success, error) {
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);
// Once everything is done, get ready to report times to the server.
sent_messages.report_server_ack(request.local_id);
}
if (page_params.use_websockets) {
send_message_socket(request, success, error);
} else {
send_message_ajax(request, success, error);
}
};
return exports;
}());
if (typeof module !== 'undefined') {
module.exports = transmit;
}

1
tools/test-js-with-node
View File

@@ -55,6 +55,7 @@ enforce_fully_covered = {
'static/js/stream_sort.js',
'static/js/topic_data.js',
'static/js/topic_generator.js',
'static/js/transmit.js',
'static/js/typeahead_helper.js',
'static/js/typing_data.js',
'static/js/typing_status.js',

1
zproject/settings.py
View File

@@ -1033,6 +1033,7 @@ JS_SPECS = {
'js/sent_messages.js',
'js/compose_state.js',
'js/compose_actions.js',
'js/transmit.js',
'js/compose.js',
'js/upload.js',
'js/stream_color.js',