compose: Make the route of message sending through drafts.

Before this commit, the message or any draft is deleted as soon
as the compose box is closed. So, it removes that by removing
delete_active_drafts and instead this commit will add the deletion
process of drafts in reify_message_id that is called when a
message is successfully sent and received.

Now, see there are two types of messages, one that are locally
echoed and the second ones are that aren't locally echoed but
sent directly to server. This commit only saves the message in
draft if it is locally echoed as they are the only messages
that show message failed in message list. The non locally echoed
ones aren't remove from the compose box until they are
successfully sent. Now as the draft-id is stored in the message
data for locally echoed messages, as they are echoed from the
server, they are deleted using that draft-id.

This also adds node tests for echo reify_message_id for testing
this feature that this commit is adding.

Fixes #17697

frontend_tests/node_tests/compose.js

@@ -27,7 +27,6 @@ const fake_now = 555;
 
 const channel = mock_esm("../../static/js/channel");
 const compose_actions = mock_esm("../../static/js/compose_actions");
-const drafts = mock_esm("../../static/js/drafts");
 const giphy = mock_esm("../../static/js/giphy");
 const loading = mock_esm("../../static/js/loading");
 const markdown = mock_esm("../../static/js/markdown");
@@ -110,8 +109,6 @@ function initialize_handlers({override}) {
 }
 
 test_ui("send_message_success", ({override}) => {
-    override(drafts, "delete_active_draft", () => {});
-
     $("#compose-textarea").val("foobarfoobar");
     $("#compose-textarea").trigger("blur");
     $("#compose-send-status").show();
@@ -139,7 +136,6 @@ test_ui("send_message_success", ({override}) => {
 test_ui("send_message", ({override}) => {
     MockDate.set(new Date(fake_now * 1000));
 
-    override(drafts, "delete_active_draft", () => {});
     override(sent_messages, "start_tracking_message", () => {});
 
     // This is the common setup stuff for all of the four tests.

frontend_tests/node_tests/echo.js

@@ -11,6 +11,7 @@ const {page_params} = require("../zjsunit/zpage_params");
 
 const markdown = mock_esm("../../static/js/markdown");
 const message_lists = mock_esm("../../static/js/message_lists");
+const notifications = mock_esm("../../static/js/notifications");
 
 let disparities = [];
 
@@ -24,7 +25,7 @@ mock_esm("../../static/js/sent_messages", {
     },
 });
 
-mock_esm("../../static/js/message_store", {
+const message_store = mock_esm("../../static/js/message_store", {
     get: () => ({failed_request: true}),
 
     update_booleans: () => {},
@@ -34,6 +35,7 @@ mock_esm("../../static/js/message_list");
 message_lists.current = "";
 message_lists.home = {view: {}};
 
+const drafts = zrequire("drafts");
 const echo = zrequire("echo");
 const people = zrequire("people");
 
@@ -194,7 +196,7 @@ run_test("insert_local_message streams", ({override}) => {
     const fake_now = 555;
     MockDate.set(new Date(fake_now * 1000));
 
-    const local_id_float = 101;
+    const local_id_float = 101.01;
 
     let apply_markdown_called = false;
     let add_topic_links_called = false;
@@ -232,7 +234,7 @@ run_test("insert_local_message streams", ({override}) => {
 });
 
 run_test("insert_local_message PM", ({override}) => {
-    const local_id_float = 102;
+    const local_id_float = 102.01;
 
     page_params.user_id = 123;
 
@@ -278,4 +280,46 @@ run_test("insert_local_message PM", ({override}) => {
     assert.ok(insert_message_called);
 });
 
+run_test("test reify_message_id", ({override}) => {
+    const local_id_float = 103.01;
+
+    override(markdown, "apply_markdown", () => {});
+    override(markdown, "add_topic_links", () => {});
+    override(echo, "insert_message", () => {});
+
+    const message_request = {
+        type: "stream",
+        stream: "general",
+        sender_email: "iago@zulip.com",
+        sender_full_name: "Iago",
+        sender_id: 123,
+        draft_id: 100,
+    };
+    echo.insert_local_message(message_request, local_id_float);
+
+    let message_store_reify_called = false;
+    let notifications_reify_called = false;
+    let draft_deleted = false;
+
+    override(message_store, "reify_message_id", () => {
+        message_store_reify_called = true;
+    });
+
+    override(notifications, "reify_message_id", () => {
+        notifications_reify_called = true;
+    });
+
+    const draft_model = drafts.draft_model;
+    override(draft_model, "deleteDraft", (draft_id) => {
+        assert.ok(draft_id, 100);
+        draft_deleted = true;
+    });
+
+    echo.reify_message_id(local_id_float.toString(), 110);
+
+    assert.ok(message_store_reify_called);
+    assert.ok(notifications_reify_called);
+    assert.ok(draft_deleted);
+});
+
 MockDate.reset();

frontend_tests/puppeteer_tests/drafts.ts

@@ -241,7 +241,7 @@ async function test_save_draft_by_reloading(page: Page): Promise<void> {
     );
 }
 
-async function test_delete_draft_on_sending(page: Page): Promise<void> {
+async function test_not_delete_draft_on_sending(page: Page): Promise<void> {
     await page.click("#drafts_table .message_row.private-message .restore-draft");
     await wait_for_drafts_to_dissapear(page);
     await page.waitForSelector("#private-message", {visible: true});
@@ -253,8 +253,7 @@ async function test_delete_draft_on_sending(page: Page): Promise<void> {
     await page.click(drafts_button_in_compose);
     await wait_for_drafts_to_appear(page);
     const drafts_count = await get_drafts_count(page);
-    assert.strictEqual(drafts_count, 1, "Draft wasn't cleared on sending.");
-    await page.waitForSelector("#drafts_table .message_row.private-message", {hidden: true});
+    assert.strictEqual(drafts_count, 2, "Draft got cleared on sending.");
 }
 
 async function drafts_test(page: Page): Promise<void> {
@@ -276,7 +275,7 @@ async function drafts_test(page: Page): Promise<void> {
     await test_restore_private_message_draft(page);
     await test_delete_draft(page);
     await test_save_draft_by_reloading(page);
-    await test_delete_draft_on_sending(page);
+    await test_not_delete_draft_on_sending(page);
 }
 
 common.run_test(drafts_test);

static/js/compose.js

@@ -11,7 +11,6 @@ import * as compose_fade from "./compose_fade";
 import * as compose_state from "./compose_state";
 import * as compose_ui from "./compose_ui";
 import * as compose_validate from "./compose_validate";
-import * as drafts from "./drafts";
 import * as echo from "./echo";
 import * as giphy from "./giphy";
 import {$t, $t_html} from "./i18n";
@@ -185,7 +184,7 @@ export function create_message_object() {
 export function clear_compose_box() {
     $("#compose-textarea").val("").trigger("focus");
     compose_validate.check_overflow_text();
-    drafts.delete_active_draft();
+    $("#compose-textarea").removeData("draft-id");
     compose_ui.autosize_textarea($("#compose-textarea"));
     $("#compose-send-status").hide(0);
     $("#compose-send-button").prop("disabled", false);

static/js/drafts.js

@@ -160,7 +160,7 @@ export function update_draft() {
         // the existing draft yet--the user may have mistakenly
         // hit delete after select-all or something.
         // Just do nothing.
-        return;
+        return undefined;
     }
 
     const draft_id = $("#compose-textarea").data("draft-id");
@@ -170,7 +170,7 @@ export function update_draft() {
         // just update the existing draft.
         draft_model.editDraft(draft_id, draft);
         draft_notify();
-        return;
+        return draft_id;
     }
 
     // We have never saved a draft for this message, so add
@@ -178,14 +178,8 @@ export function update_draft() {
     const new_draft_id = draft_model.addDraft(draft);
     $("#compose-textarea").data("draft-id", new_draft_id);
     draft_notify();
-}
 
-export function delete_active_draft() {
-    const draft_id = $("#compose-textarea").data("draft-id");
-    if (draft_id) {
-        draft_model.deleteDraft(draft_id);
-    }
-    $("#compose-textarea").removeData("draft-id");
+    return new_draft_id;
 }
 
 export function restore_draft(draft_id) {

static/js/echo.js

@@ -4,6 +4,7 @@ import * as alert_words from "./alert_words";
 import {all_messages_data} from "./all_messages_data";
 import * as blueslip from "./blueslip";
 import * as compose from "./compose";
+import * as drafts from "./drafts";
 import * as local_message from "./local_message";
 import * as markdown from "./markdown";
 import * as message_events from "./message_events";
@@ -235,6 +236,11 @@ export function try_deliver_locally(message_request) {
         return undefined;
     }
 
+    // Only saving in draft for locally echoed message
+    // This draft will be cleared after successfull message
+    const draft_id = drafts.update_draft();
+    message_request.draft_id = draft_id;
+
     const message = insert_local_message(message_request, local_id_float);
     return message;
 }
@@ -331,6 +337,11 @@ export function reify_message_id(local_id, server_id) {
     message.id = server_id;
     message.locally_echoed = false;
 
+    if (message.draft_id) {
+        // Delete the draft if message was locally echoed
+        drafts.draft_model.deleteDraft(message.draft_id);
+    }
+
     const opts = {old_id: Number.parseFloat(local_id), new_id: server_id};
 
     message_store.reify_message_id(opts);