From e8d5241c0d1dfd0bbe82fc82df173d910e764de8 Mon Sep 17 00:00:00 2001 From: evykassirer Date: Thu, 1 Feb 2024 15:23:35 -0800 Subject: [PATCH] drafts: Return updatedAt timestamp in snapshot_message. It was always being set after the fact, and it will be easier to manage types for the conversion to Typescript if we just set this when setting the rest of the data for the draft message. --- web/src/drafts.js | 19 +++++++--------- web/tests/drafts.test.js | 48 ++++++++++++++++------------------------ 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/web/src/drafts.js b/web/src/drafts.js index e78f08e4bd..a271344c88 100644 --- a/web/src/drafts.js +++ b/web/src/drafts.js @@ -26,6 +26,10 @@ export function set_count(count) { ui_util.update_unread_count_in_dom($drafts_li, count); } +function getTimestamp() { + return Date.now(); +} + export const draft_model = (function () { const exports = {}; @@ -33,10 +37,6 @@ export const draft_model = (function () { const KEY = "drafts"; const ls = localstorage(); - function getTimestamp() { - return Date.now(); - } - function get() { return ls.get(KEY) || {}; } @@ -65,14 +65,13 @@ export const draft_model = (function () { // collisions to essentially zero. const id = getTimestamp().toString(16) + "-" + Math.random().toString(16).split(/\./).pop(); - draft.updatedAt = getTimestamp(); drafts[id] = draft; save(drafts, update_count); return id; }; - exports.editDraft = function (id, draft, update_timestamp = true) { + exports.editDraft = function (id, draft) { const drafts = get(); let changed = false; @@ -82,9 +81,6 @@ export const draft_model = (function () { if (drafts[id]) { changed = !check_if_equal(drafts[id], draft); - if (update_timestamp) { - draft.updatedAt = getTimestamp(); - } drafts[id] = draft; save(drafts); } @@ -121,7 +117,7 @@ export function fix_drafts_with_undefined_topics() { if (draft.type === "stream" && draft.topic === undefined) { const draft = data[draft_id]; draft.topic = ""; - draft_model.editDraft(draft_id, draft, false); + draft_model.editDraft(draft_id, draft); } } fixed_buggy_drafts = true; @@ -162,7 +158,7 @@ export function rename_stream_recipient(old_stream_id, old_topic, new_stream_id, if (new_topic !== undefined) { draft.topic = new_topic; } - draft_model.editDraft(draft_id, draft, false); + draft_model.editDraft(draft_id, draft); } } } @@ -178,6 +174,7 @@ export function snapshot_message() { const message = { type: compose_state.get_message_type(), content: compose_state.message_content(), + updatedAt: getTimestamp(), }; if (message.type === "private") { const recipient = compose_state.private_message_recipient(); diff --git a/web/tests/drafts.test.js b/web/tests/drafts.test.js index 16c0a8137e..ce92f9563c 100644 --- a/web/tests/drafts.test.js +++ b/web/tests/drafts.test.js @@ -4,7 +4,7 @@ const {strict: assert} = require("assert"); const {mock_stream_header_colorblock} = require("./lib/compose"); const {mock_banners} = require("./lib/compose_banner"); -const {mock_esm, set_global, zrequire, with_overrides} = require("./lib/namespace"); +const {mock_esm, set_global, zrequire} = require("./lib/namespace"); const {run_test, noop} = require("./lib/test"); const $ = require("./lib/zjquery"); const {user_settings} = require("./lib/zpage_params"); @@ -65,23 +65,28 @@ const drafts_overlay_ui = zrequire("drafts_overlay_ui"); const messages_overlay_ui = zrequire("messages_overlay_ui"); const timerender = zrequire("timerender"); +const mock_current_timestamp = 1234; + const draft_1 = { stream_id: 30, topic: "topic", type: "stream", content: "Test stream message", + updatedAt: mock_current_timestamp, }; const draft_2 = { private_message_recipient: "aaron@zulip.com", reply_to: "aaron@zulip.com", type: "private", content: "Test direct message", + updatedAt: mock_current_timestamp, }; const short_msg = { stream_id: 30, topic: "topic", type: "stream", content: "a", + updatedAt: mock_current_timestamp, }; function test(label, f) { @@ -92,7 +97,7 @@ function test(label, f) { }); } -test("draft_model add", ({override}) => { +test("draft_model add", () => { const draft_model = drafts.draft_model; const ls = localstorage(); assert.equal(ls.get("draft"), undefined); @@ -100,40 +105,26 @@ test("draft_model add", ({override}) => { const $unread_count = $(""); $(".top_left_drafts").set_find_results(".unread_count", $unread_count); - override(Date, "now", () => 1); - const expected = {...draft_1}; - expected.updatedAt = 1; - const id = draft_model.addDraft({...draft_1}); - assert.deepEqual(draft_model.getDraft(id), expected); + const id = draft_model.addDraft(draft_1); + assert.deepEqual(draft_model.getDraft(id), draft_1); }); test("draft_model edit", () => { const draft_model = drafts.draft_model; const ls = localstorage(); assert.equal(ls.get("draft"), undefined); - let id; const $unread_count = $(""); $(".top_left_drafts").set_find_results(".unread_count", $unread_count); - with_overrides(({override}) => { - override(Date, "now", () => 1); - const expected = {...draft_1}; - expected.updatedAt = 1; - id = draft_model.addDraft({...draft_1}); - assert.deepEqual(draft_model.getDraft(id), expected); - }); + const id = draft_model.addDraft(draft_1); + assert.deepEqual(draft_model.getDraft(id), draft_1); - with_overrides(({override}) => { - override(Date, "now", () => 2); - const expected = {...draft_2}; - expected.updatedAt = 2; - draft_model.editDraft(id, {...draft_2}); - assert.deepEqual(draft_model.getDraft(id), expected); - }); + draft_model.editDraft(id, draft_2); + assert.deepEqual(draft_model.getDraft(id), draft_2); }); -test("draft_model delete", ({override}) => { +test("draft_model delete", () => { const draft_model = drafts.draft_model; const ls = localstorage(); assert.equal(ls.get("draft"), undefined); @@ -141,17 +132,14 @@ test("draft_model delete", ({override}) => { const $unread_count = $(""); $(".top_left_drafts").set_find_results(".unread_count", $unread_count); - override(Date, "now", () => 1); - const expected = {...draft_1}; - expected.updatedAt = 1; - const id = draft_model.addDraft({...draft_1}); - assert.deepEqual(draft_model.getDraft(id), expected); + const id = draft_model.addDraft(draft_1); + assert.deepEqual(draft_model.getDraft(id), draft_1); draft_model.deleteDraft(id); assert.deepEqual(draft_model.getDraft(id), false); }); -test("snapshot_message", ({override_rewire}) => { +test("snapshot_message", ({override, override_rewire}) => { override_rewire(user_pill, "get_user_ids", () => [aaron.user_id]); override_rewire(compose_pm_pill, "set_from_emails", noop); mock_banners(); @@ -181,6 +169,8 @@ test("snapshot_message", ({override_rewire}) => { stream_data.add_sub(stream); compose_state.set_stream_id(stream.stream_id); + override(Date, "now", () => mock_current_timestamp); + curr_draft = draft_1; set_compose_state(); assert.deepEqual(drafts.snapshot_message(), draft_1);