Extract compose_state.js.

This is mostly just moving methods out of compose.js.

The variable `is_composing_message`, which isn't a boolean, has
been renamed to `message_type`, and there are new functions
set_message_type() and get_message_type() that wrap it.

This commit removes some shims related to the global variable
`compose_state`; now, `compose_state` is a typical global
variable with a 1:1 relationship with the module by the same
name.

The new module has 100% line coverage, most of it coming
via the tests on compose_actions.js.  (The methods here are
super simple, so it's a good thing that the tests are somewhat
integrated with a higher layer.)

frontend_tests/node_tests/compose.js

@@ -11,6 +11,7 @@ set_global('document', {
 });
 
 add_dependencies({
+    compose_state: 'js/compose_state',
     people: 'js/people',
     stream_data: 'js/stream_data',
     util: 'js/util',
@@ -18,10 +19,6 @@ add_dependencies({
 
 var compose = require('js/compose.js');
 
-set_global('compose_state', {
-    recipient: compose.recipient,
-});
-
 var me = {
     email: 'me@example.com',
     user_id: 30,

frontend_tests/node_tests/compose_actions.js

@@ -15,6 +15,7 @@ set_global('$', function () {
 
 add_dependencies({
     compose: 'js/compose',
+    compose_state: 'js/compose_state',
     people: 'js/people',
     util: 'js/util',
 });
@@ -27,6 +28,8 @@ var get_focus_area = compose_actions._get_focus_area;
 var respond_to_message = compose_actions.respond_to_message;
 var reply_with_mention = compose_actions.reply_with_mention;
 
+var compose_state = global.compose_state;
+
 set_global('reload', {
     is_in_progress: return_false,
 });
@@ -55,12 +58,6 @@ set_global('unread_ops', {
     mark_message_as_read: noop,
 });
 
-// these are shimmed in shim.js
-set_global('compose_state', {
-    composing: global.compose.composing,
-    recipient: global.compose.recipient,
-});
-
 set_global('status_classes', 'status_classes');
 
 var fake_jquery = function () {
@@ -163,6 +160,12 @@ function assert_hidden(sel) {
     assert(!$(sel).visible());
 }
 
+(function test_initial_state() {
+    assert.equal(compose_state.composing(), false);
+    assert.equal(compose_state.get_message_type(), false);
+    assert.equal(compose_state.has_message_content(), false);
+}());
+
 (function test_start() {
     compose_actions.autosize_message_content = noop;
     compose_actions.expand_compose_box = noop;
@@ -185,6 +188,8 @@ function assert_hidden(sel) {
 
     assert.equal($('#stream').val(), 'stream1');
     assert.equal($('#subject').val(), 'topic1');
+    assert.equal(compose_state.get_message_type(), 'stream');
+    assert(compose_state.composing());
 
     // Start PM
     global.narrow_state.set_compose_defaults = function (opts) {
@@ -201,12 +206,15 @@ function assert_hidden(sel) {
 
     assert.equal($('#private_message_recipient').val(), 'foo@example.com');
     assert.equal($('#new_message_content').val(), 'hello');
+    assert.equal(compose_state.get_message_type(), 'private');
+    assert(compose_state.composing());
 
     // Cancel compose.
     assert_hidden('#compose_controls');
     cancel();
     assert_visible('#compose_controls');
     assert_hidden('#private-message');
+    assert(!compose_state.composing());
 }());
 
 (function test_respond_to_message() {
@@ -263,6 +271,7 @@ function assert_hidden(sel) {
     reply_with_mention(opts);
     assert.equal($('#stream').val(), 'devel');
     assert.equal($('#new_message_content').val(), '@**Bob Roberts** ');
+    assert(compose_state.has_message_content());
 }());
 
 (function test_get_focus_area() {

frontend_tests/node_tests/drafts.js

@@ -102,16 +102,16 @@ var draft_2 = {
         global.compose_state.composing = function () {
             return draft.type;
         };
-        global.compose.message_content = function () {
+        global.compose_state.message_content = function () {
             return draft.content;
         };
         global.compose_state.recipient = function () {
             return draft.private_message_recipient;
         };
-        global.compose.stream_name = function () {
+        global.compose_state.stream_name = function () {
             return draft.stream;
         };
-        global.compose.subject = function () {
+        global.compose_state.subject = function () {
             return draft.subject;
         };
     }

static/js/compose.js

@@ -1,11 +1,6 @@
 var compose = (function () {
 
 var exports = {};
-var is_composing_message = false;
-
-exports.set_message_type = function (msg_type) {
-    is_composing_message = msg_type;
-};
 
 /* Track the state of the @all warning. The user must acknowledge that they are spamming the entire
    stream before the warning will go away. If they try to send before explicitly dismissing the
@@ -52,7 +47,7 @@ exports.autosize_textarea = function () {
 };
 
 function show_all_everyone_warnings() {
-    var current_stream = stream_data.get_sub(compose.stream_name());
+    var current_stream = stream_data.get_sub(compose_state.stream_name());
     var stream_count = current_stream.subscribers.num_items();
 
     var all_everyone_template = templates.render("compose_all_everyone", {count: stream_count});
@@ -91,12 +86,11 @@ exports.clear_preview_area = function () {
 };
 
 function update_fade() {
-    if (!is_composing_message) {
+    if (!compose_state.composing()) {
         return;
     }
 
-    // Legacy strangeness: is_composing_message can be false, "stream", or "private"
+    var msg_type = compose_state.get_message_type();
-    var msg_type = is_composing_message;
     compose_fade.set_focused_recipient(msg_type);
     compose_fade.update_faded_messages();
 }
@@ -123,12 +117,12 @@ exports.empty_topic_placeholder = function () {
 
 function create_message_object() {
     // Subjects are optional, and we provide a placeholder if one isn't given.
-    var subject = compose.subject();
+    var subject = compose_state.subject();
     if (subject === "") {
         subject = compose.empty_topic_placeholder();
     }
 
-    var content = make_uploads_relative(compose.message_content());
+    var content = make_uploads_relative(compose_state.message_content());
 
     // Changes here must also be kept in sync with echo.try_deliver_locally
     var message = {
@@ -149,7 +143,7 @@ function create_message_object() {
         message.private_message_recipient = recipient;
         message.to_user_ids = people.email_list_to_user_ids_string(emails);
     } else {
-        var stream_name = compose.stream_name();
+        var stream_name = compose_state.stream_name();
         message.to = stream_name;
         message.stream = stream_name;
         var sub = stream_data.get_sub(stream_name);
@@ -437,37 +431,8 @@ $(function () {
     });
 });
 
-exports.composing = function () {
-    return is_composing_message;
-};
-
-function get_or_set(fieldname, keep_leading_whitespace) {
-    // We can't hoist the assignment of 'elem' out of this lambda,
-    // because the DOM element might not exist yet when get_or_set
-    // is called.
-    return function (newval) {
-        var elem = $('#'+fieldname);
-        var oldval = elem.val();
-        if (newval !== undefined) {
-            elem.val(newval);
-        }
-        return keep_leading_whitespace ? util.rtrim(oldval) : $.trim(oldval);
-    };
-}
-
-exports.stream_name     = get_or_set('stream');
-exports.subject         = get_or_set('subject');
-// We can't trim leading whitespace in `new_message_content` because
-// of the indented syntax for multi-line code blocks.
-exports.message_content = get_or_set('new_message_content', true);
-exports.recipient       = get_or_set('private_message_recipient');
-
-exports.has_message_content = function () {
-    return exports.message_content() !== "";
-};
-
 exports.update_email = function (user_id, new_email) {
-    var reply_to = exports.recipient();
+    var reply_to = compose_state.recipient();
 
     if (!reply_to) {
         return;
@@ -475,14 +440,14 @@ exports.update_email = function (user_id, new_email) {
 
     reply_to = people.update_email_in_reply_to(reply_to, user_id, new_email);
 
-    exports.recipient(reply_to);
+    compose_state.recipient(reply_to);
 };
 
 exports.get_invalid_recipient_emails = function () {
     var private_recipients = util.extract_pm_recipients(compose_state.recipient());
     var invalid_recipients = [];
     _.each(private_recipients, function (email) {
-        // This case occurs when exports.recipient() ends with ','
+        // This case occurs when compose_state.recipient() ends with ','
         if (email === "") {
             return;
         }
@@ -517,7 +482,7 @@ function validate_stream_message_mentions(stream_name) {
     var stream_count = current_stream.subscribers.num_items();
 
     // check if @all or @everyone is in the message
-    if (util.is_all_or_everyone_mentioned(exports.message_content()) &&
+    if (util.is_all_or_everyone_mentioned(compose_state.message_content()) &&
         stream_count > compose.all_everyone_warn_threshold) {
         if (user_acknowledged_all_everyone === undefined ||
             user_acknowledged_all_everyone === false) {
@@ -564,14 +529,14 @@ function validate_stream_message_address_info(stream_name) {
 }
 
 function validate_stream_message() {
-    var stream_name = exports.stream_name();
+    var stream_name = compose_state.stream_name();
     if (stream_name === "") {
         compose_error(i18n.t("Please specify a stream"), $("#stream"));
         return false;
     }
 
     if (page_params.mandatory_topics) {
-        var topic = exports.subject();
+        var topic = compose_state.subject();
         if (topic === "") {
             compose_error(i18n.t("Please specify a topic"), $("#subject"));
             return false;
@@ -589,7 +554,7 @@ function validate_stream_message() {
 // The function checks whether the recipients are users of the realm or cross realm users (bots
 // for now)
 function validate_private_message() {
-    if (exports.recipient() === "") {
+    if (compose_state.recipient() === "") {
         compose_error(i18n.t("Please specify at least one recipient"), $("#private_message_recipient"));
         return false;
     } else if (page_params.is_zephyr_mirror_realm) {
@@ -616,7 +581,7 @@ exports.validate = function () {
     $("#compose-send-button").attr('disabled', 'disabled').blur();
     $("#sending-indicator").show();
 
-    if (/^\s*$/.test(exports.message_content())) {
+    if (/^\s*$/.test(compose_state.message_content())) {
         compose_error(i18n.t("You have nothing to send!"), $("#new_message_content"));
         return false;
     }
@@ -626,7 +591,7 @@ exports.validate = function () {
         return false;
     }
 
-    if (exports.composing() === 'private') {
+    if (compose_state.composing() === 'private') {
         return validate_private_message();
     }
     return validate_stream_message();
@@ -687,9 +652,12 @@ $(function () {
 
     // Show a warning if a user @-mentions someone who will not receive this message
     $(document).on('usermention_completed.zulip', function (event, data) {
-        // Legacy strangeness: is_composing_message can be false, "stream", or "private"
+        if (compose_state.get_message_type() !== 'stream') {
+            return;
+        }
+
         // Disable for Zephyr mirroring realms, since we never have subscriber lists there
-        if (is_composing_message !== "stream" || page_params.is_zephyr_mirror_realm) {
+        if (page_params.is_zephyr_mirror_realm) {
             return;
         }
 
@@ -755,7 +723,7 @@ $(function () {
             $(event.target).attr('disabled', true);
         }
 
-        var stream_name = compose.stream_name();
+        var stream_name = compose_state.stream_name();
         var sub = stream_data.get_sub(stream_name);
         if (!sub) {
             // This should only happen if a stream rename occurs

static/js/compose_actions.js

@@ -171,8 +171,8 @@ function fill_in_opts_from_current_narrowed_view(msg_type, opts) {
 function same_recipient_as_before(msg_type, opts) {
     return (compose_state.composing() === msg_type) &&
             ((msg_type === "stream" &&
-              opts.stream === compose.stream_name() &&
+              opts.stream === compose_state.stream_name() &&
-              opts.subject === compose.subject()) ||
+              opts.subject === compose_state.subject()) ||
              (msg_type === "private" &&
               opts.private_message_recipient === compose_state.recipient()));
 }
@@ -199,8 +199,8 @@ exports.start = function (msg_type, opts) {
         clear_box();
     }
 
-    compose.stream_name(opts.stream);
+    compose_state.stream_name(opts.stream);
-    compose.subject(opts.subject);
+    compose_state.subject(opts.subject);
 
     // Set the recipients with a space after each comma, so it looks nice.
     compose_state.recipient(opts.private_message_recipient.replace(/,\s*/g, ", "));
@@ -208,10 +208,10 @@ exports.start = function (msg_type, opts) {
     // If the user opens the compose box, types some text, and then clicks on a
     // different stream/subject, we want to keep the text in the compose box
     if (opts.content !== undefined) {
-        compose.message_content(opts.content);
+        compose_state.message_content(opts.content);
     }
 
-    compose.set_message_type(msg_type);
+    compose_state.set_message_type(msg_type);
 
     // Show either stream/topic fields or "You and" field.
     show_box(msg_type, opts);
@@ -227,9 +227,9 @@ exports.cancel = function () {
         // at least clear the subject and unfade.
         compose_fade.clear_compose();
         if (page_params.narrow_topic !== undefined) {
-            compose.subject(page_params.narrow_topic);
+            compose_state.subject(page_params.narrow_topic);
         } else {
-            compose.subject("");
+            compose_state.subject("");
         }
         return;
     }
@@ -239,7 +239,7 @@ exports.cancel = function () {
     clear_box();
     notifications.clear_compose_notifications();
     compose.abort_xhr();
-    compose.set_message_type(false);
+    compose_state.set_message_type(false);
     $(document).trigger($.Event('compose_canceled.zulip'));
 };
 

static/js/compose_state.js

@@ -0,0 +1,57 @@
+var compose_state = (function () {
+
+var exports = {};
+
+var message_type = false; // 'stream', 'private', or false-y
+
+exports.set_message_type = function (msg_type) {
+    message_type = msg_type;
+};
+
+exports.get_message_type = function () {
+    return message_type;
+};
+
+exports.composing = function () {
+    // For legacy reasons, this is the same as get_message_type.
+    // Most callers use this in a boolean context, but there are
+    // some stragglers that inspect the string value.
+    //
+    // TODO: Fix callers who care about stream/private to use
+    //       get_message_type(), and then convert this to return
+    //       `!!message_type` or something like that.
+    return message_type;
+};
+
+function get_or_set(fieldname, keep_leading_whitespace) {
+    // We can't hoist the assignment of 'elem' out of this lambda,
+    // because the DOM element might not exist yet when get_or_set
+    // is called.
+    return function (newval) {
+        var elem = $('#'+fieldname);
+        var oldval = elem.val();
+        if (newval !== undefined) {
+            elem.val(newval);
+        }
+        return keep_leading_whitespace ? util.rtrim(oldval) : $.trim(oldval);
+    };
+}
+
+// TODO: Break out setters and getter into their own functions.
+exports.stream_name     = get_or_set('stream');
+exports.subject         = get_or_set('subject');
+// We can't trim leading whitespace in `new_message_content` because
+// of the indented syntax for multi-line code blocks.
+exports.message_content = get_or_set('new_message_content', true);
+exports.recipient       = get_or_set('private_message_recipient');
+
+exports.has_message_content = function () {
+    return exports.message_content() !== "";
+};
+
+return exports;
+}());
+
+if (typeof module !== 'undefined') {
+    module.exports = compose_state;
+}

static/js/composebox_typeahead.js

@@ -359,7 +359,8 @@ exports.initialize_compose_typeahead = function (selector, completions) {
             if (this.completing === 'emoji') {
                 return typeahead_helper.sort_emojis(matches, this.token);
             } else if (this.completing === 'mention') {
-                return typeahead_helper.sort_recipients(matches, this.token, compose.stream_name());
+                return typeahead_helper.sort_recipients(matches, this.token,
+                                                        compose_state.stream_name());
             } else if (this.completing === 'stream') {
                 return typeahead_helper.sort_streams(matches, this.token);
             }
@@ -468,7 +469,7 @@ exports.initialize = function () {
             return query_matches_person(current_recipient, item);
         },
         sorter: function (matches) {
-            var current_stream = compose.stream_name();
+            var current_stream = compose_state.stream_name();
             return typeahead_helper.sort_recipientbox_typeahead(
                 this.query, matches, current_stream);
         },

static/js/drafts.js

@@ -64,7 +64,7 @@ var draft_model = (function () {
 exports.draft_model = draft_model;
 
 exports.snapshot_message = function () {
-    if (!compose_state.composing() || (compose.message_content() === "")) {
+    if (!compose_state.composing() || (compose_state.message_content() === "")) {
         // If you aren't in the middle of composing the body of a
         // message, don't try to snapshot.
         return;
@@ -73,15 +73,15 @@ exports.snapshot_message = function () {
     // Save what we can.
     var message = {
         type: compose_state.composing(),
-        content: compose.message_content(),
+        content: compose_state.message_content(),
     };
     if (message.type === "private") {
         var recipient = compose_state.recipient();
         message.reply_to = recipient;
         message.private_message_recipient = recipient;
     } else {
-        message.stream = compose.stream_name();
+        message.stream = compose_state.stream_name();
-        message.subject = compose.subject();
+        message.subject = compose_state.subject();
     }
     return message;
 };

static/js/hotkey.js

@@ -11,7 +11,7 @@ function do_narrow_action(action) {
 function focus_in_empty_compose() {
     return (
         compose_state.composing() &&
-        compose.message_content() === "" &&
+        compose_state.message_content() === "" &&
         $('#new_message_content').is(':focus'));
 }
 

static/js/message_events.js

@@ -143,12 +143,12 @@ exports.update_messages = function update_messages(events) {
             var going_forward_change = _.indexOf(['change_later', 'change_all'], event.propagate_mode) >= 0;
 
             var stream_name = stream_data.get_sub_by_id(event.stream_id).name;
-            var compose_stream_name = compose.stream_name();
+            var compose_stream_name = compose_state.stream_name();
 
             if (going_forward_change && stream_name && compose_stream_name) {
                 if (stream_name.toLowerCase() === compose_stream_name.toLowerCase()) {
-                    if (event.orig_subject === compose.subject()) {
+                    if (event.orig_subject === compose_state.subject()) {
-                        compose.subject(event.subject);
+                        compose_state.subject(event.subject);
                     }
                 }
             }

static/js/reload.js

@@ -24,15 +24,15 @@ function preserve_state(send_after_reload, save_pointer, save_narrow, save_compo
     if (save_compose) {
         if (compose_state.composing() === 'stream') {
             url += "+msg_type=stream";
-            url += "+stream=" + encodeURIComponent(compose.stream_name());
+            url += "+stream=" + encodeURIComponent(compose_state.stream_name());
-            url += "+subject=" + encodeURIComponent(compose.subject());
+            url += "+subject=" + encodeURIComponent(compose_state.subject());
         } else if (compose_state.composing() === 'private') {
             url += "+msg_type=private";
             url += "+recipient=" + encodeURIComponent(compose_state.recipient());
         }
 
         if (compose_state.composing()) {
-            url += "+msg=" + encodeURIComponent(compose.message_content());
+            url += "+msg=" + encodeURIComponent(compose_state.message_content());
         }
     }
 

static/js/shim.js

@@ -9,8 +9,3 @@ that still refer to the old name.
 var narrow_state = {}; // global, should be made into module
 narrow_state.set_compose_defaults = narrow.set_compose_defaults;
 
-var compose_state = {};
-compose_state.has_message_content = compose.has_message_content;
-compose_state.recipient = compose.recipient;
-compose_state.composing = compose.composing;
-

static/js/subs.js

@@ -349,7 +349,7 @@ function show_subscription_settings(sub_row) {
                 (item.full_name.toLowerCase().indexOf(query) !== -1);
         },
         sorter: function (matches) {
-            var current_stream = compose.stream_name();
+            var current_stream = compose_state.stream_name();
             return typeahead_helper.sort_recipientbox_typeahead(
                 this.query, matches, current_stream);
         },

zproject/settings.py

@@ -852,6 +852,7 @@ JS_SPECS = {
             'js/fenced_code.js',
             'js/echo.js',
             'js/socket.js',
+            'js/compose_state.js',
             'js/compose_actions.js',
             'js/compose.js',
             'js/stream_color.js',