From be76d7592a609990caefdddcb3343bade34994fa Mon Sep 17 00:00:00 2001 From: YashRE42 <33805964+YashRE42@users.noreply.github.com> Date: Tue, 30 Nov 2021 22:27:42 +0530 Subject: [PATCH] reload: Manually save draft and preserve id when triggering reload. We received a complaint about the generation of multiple duplicate drafts for a single message. It was discovered that the likely cause of this was how we were handling clients that were frequently suspending/unsuspending, we would initiate a reload when we discovered this, and expect the `beforeunload` handler to save the draft. This behaved correctly, however, we would also save the compose state and fill it in via `preserve_state` in reload.js. The important detail here is that `preserve_state` would not encode and preserve the `draft_id` for the current message, partly because it had no way of knowing the `draft_id` of the draft... since we have not saved it yet, the `beforeunload` event happens after `preserve_state`. As such, performing any action that would trigger a draft to be saved, eg pressing Esc to close the compose box, would save a duplicate draft of the same message. To resolve the above bug, we (1) ensure that we call `drafts.update_draft()` in `preserve_state`, this returns a draft_id to us, which we (2) ensure that we encode as part of the url and (3) set on the `#composebox-textarea` as a `draft-id` data attribute, which we check the next time we try to save the draft, post reload. Note that this causes us to save the draft twice, once from preserve_state and then again from the `beforeunload` handler, but we do not add two drafts since the second update_draft call just edits the timestamp because it finds the `draft-id` data attribute on the `#composebox-textarea` set by the first call. --- static/js/compose_actions.js | 4 ++++ static/js/reload.js | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/static/js/compose_actions.js b/static/js/compose_actions.js index 3863aa4b0a..93f7295231 100644 --- a/static/js/compose_actions.js +++ b/static/js/compose_actions.js @@ -276,6 +276,10 @@ export function start(msg_type, opts) { compose_state.message_content(opts.content); } + if (opts.draft_id) { + $("#compose-textarea").data("draft-id", opts.draft_id); + } + compose_state.set_message_type(msg_type); // Show either stream/topic fields or "You and" field. diff --git a/static/js/reload.js b/static/js/reload.js index 01d969bfeb..c0a6744835 100644 --- a/static/js/reload.js +++ b/static/js/reload.js @@ -6,6 +6,7 @@ import * as compose from "./compose"; import * as compose_actions from "./compose_actions"; import * as compose_state from "./compose_state"; import {csrf_token} from "./csrf"; +import * as drafts from "./drafts"; import * as hash_util from "./hash_util"; import * as hashchange from "./hashchange"; import {localstorage} from "./localstorage"; @@ -49,6 +50,10 @@ function preserve_state(send_after_reload, save_pointer, save_narrow, save_compo if (msg_type) { url += "+msg=" + encodeURIComponent(compose_state.message_content()); + const draft_id = drafts.update_draft(); + if (draft_id) { + url += "+draft_id=" + encodeURIComponent(draft_id); + } } } @@ -147,6 +152,7 @@ export function initialize() { topic: topic || "", private_message_recipient: vars.recipient || "", content: vars.msg || "", + draft_id: vars.draft_id || "", }); if (send_now) { compose.finish();