From 32e76c2c60366df0530ea442c797314cb22c94bb Mon Sep 17 00:00:00 2001
From: Sampriti Panda <sampdingo@hotmail.com>
Date: Wed, 29 Mar 2017 12:24:26 +0530
Subject: [PATCH] drafts: Move snapshot_message from compose to drafts

Previously drafts called compose.snapshot_message which would then
get the message object from compose.create_message_object. This method often
checked for the validity of stream/user recipients which would often cause tracebacks.

The new method in drafts.snapshot message just gets the data from the fields and
stores them in the draft model without any additional checking.
---
 frontend_tests/node_tests/compose.js |  4 ++--
 frontend_tests/node_tests/drafts.js  | 32 ++++++++++++++++++++++++++++
 static/js/compose.js                 | 13 ++---------
 static/js/drafts.js                  | 29 ++++++++++++++++++++++++-
 4 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/frontend_tests/node_tests/compose.js b/frontend_tests/node_tests/compose.js
index 0dba20978a..031ea4b0bc 100644
--- a/frontend_tests/node_tests/compose.js
+++ b/frontend_tests/node_tests/compose.js
@@ -79,7 +79,7 @@ people.add(bob);
     };
 
 
-    var message = compose.snapshot_message();
+    var message = compose.create_message_object();
     assert.equal(message.to, 'social');
     assert.equal(message.subject, 'lunch');
     assert.equal(message.content, 'burrito');
@@ -87,7 +87,7 @@ people.add(bob);
     global.compose_state.composing = function () {
         return 'private';
     };
-    message = compose.snapshot_message();
+    message = compose.create_message_object();
     assert.deepEqual(message.to, ['alice@example.com', 'bob@example.com']);
     assert.equal(message.to_user_ids, '31,32');
     assert.equal(message.content, 'burrito');
diff --git a/frontend_tests/node_tests/drafts.js b/frontend_tests/node_tests/drafts.js
index a249b232e3..b4b053aafb 100644
--- a/frontend_tests/node_tests/drafts.js
+++ b/frontend_tests/node_tests/drafts.js
@@ -20,6 +20,8 @@ set_global('localStorage', {
         ls_container = {};
     },
 });
+set_global('compose', {});
+set_global('compose_state', {});
 
 function stub_timestamp(timestamp, func) {
     var original_func = Date.prototype.getTime;
@@ -38,6 +40,7 @@ var draft_1 = {
 };
 var draft_2 = {
     private_message_recipient: "aaron@zulip.com",
+    reply_to: "aaron@zulip.com",
     type: "private",
     content: "Test Private Message",
 };
@@ -93,3 +96,32 @@ var draft_2 = {
          assert.deepEqual(ls.get("drafts"), {});
     }());
 }());
+
+(function test_snapshot_message() {
+    function stub_draft(draft) {
+        global.compose_state.composing = function () {
+            return draft.type;
+        };
+        global.compose.message_content = function () {
+            return draft.content;
+        };
+        global.compose_state.recipient = function () {
+            return draft.private_message_recipient;
+        };
+        global.compose.stream_name = function () {
+            return draft.stream;
+        };
+        global.compose.subject = function () {
+            return draft.subject;
+        };
+    }
+
+    stub_draft(draft_1);
+    assert.deepEqual(drafts.snapshot_message(), draft_1);
+
+    stub_draft(draft_2);
+    assert.deepEqual(drafts.snapshot_message(), draft_2);
+
+    stub_draft({});
+    assert.equal(drafts.snapshot_message(), undefined);
+}());
diff --git a/static/js/compose.js b/static/js/compose.js
index fa72d42e91..f08a72b447 100644
--- a/static/js/compose.js
+++ b/static/js/compose.js
@@ -356,17 +356,8 @@ function create_message_object() {
     }
     return message;
 }
-
-exports.snapshot_message = function () {
-    if (!compose_state.composing() || (exports.message_content() === "")) {
-        // If you aren't in the middle of composing the body of a
-        // message, don't try to snapshot.
-        return;
-    }
-
-    // Save what we can.
-    return create_message_object();
-};
+// Export for testing
+exports.create_message_object = create_message_object;
 
 function compose_error(error_text, bad_input) {
     $('#send-status').removeClass(status_classes)
diff --git a/static/js/drafts.js b/static/js/drafts.js
index 97631dc4b6..d10a6d8627 100644
--- a/static/js/drafts.js
+++ b/static/js/drafts.js
@@ -63,8 +63,35 @@ var draft_model = (function () {
 
 exports.draft_model = draft_model;
 
+exports.snapshot_message = function () {
+    if (!compose_state.composing() || (compose.message_content() === "")) {
+        // If you aren't in the middle of composing the body of a
+        // message, don't try to snapshot.
+        return;
+    }
+
+    // Save what we can.
+    var message = {
+        type: compose_state.composing(),
+        content: compose.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();
+        var subject = compose.subject();
+        if (subject === "") {
+          subject = compose.empty_topic_placeholder();
+        }
+        message.subject = subject;
+    }
+    return message;
+};
+
 exports.update_draft = function () {
-    var draft = compose.snapshot_message();
+    var draft = drafts.snapshot_message();
     var draft_id = $("#new_message_content").data("draft-id");
 
     if (draft_id !== undefined) {