drafts: Reimplement handlers for buggy drafts using zod.

web/src/drafts.js

@@ -3,6 +3,7 @@ import Handlebars from "handlebars/runtime";
 import $ from "jquery";
 import _ from "lodash";
 import tippy from "tippy.js";
+import {z} from "zod";
 
 import render_confirm_delete_all_drafts from "../templates/confirm_dialog/confirm_delete_all_drafts.hbs";
 
@@ -30,18 +31,94 @@ function getTimestamp() {
     return Date.now();
 }
 
+const possibly_buggy_draft_schema = z.intersection(
+    z.object({
+        content: z.string(),
+        updatedAt: z.number(),
+    }),
+    z.union([
+        z.object({
+            type: z.literal("stream"),
+            topic: z.string().optional(),
+            stream_id: z.number().optional(),
+            stream: z.string().optional(),
+        }),
+        z.object({
+            type: z.literal("private"),
+            reply_to: z.string(),
+            private_message_recipient: z.string(),
+        }),
+    ]),
+);
+
+const possibly_buggy_drafts_schema = z.record(z.string(), possibly_buggy_draft_schema);
+
 export const draft_model = (function () {
     const exports = {};
 
     // the key that the drafts are stored under.
     const KEY = "drafts";
     const ls = localstorage();
+    let fixed_buggy_drafts = false;
 
     function get() {
-        return ls.get(KEY) || {};
+        const drafts = ls.get(KEY);
+        if (ls.get(KEY) === undefined) {
+            return {};
+        }
+        if (!fixed_buggy_drafts) {
+            fix_buggy_drafts();
+            return ls.get(KEY);
+        }
+        return drafts;
     }
     exports.get = get;
 
+    function fix_buggy_drafts() {
+        const drafts = ls.get(KEY);
+        const parsed_drafts = possibly_buggy_drafts_schema.parse(drafts);
+        const valid_drafts = {};
+        for (const [draft_id, draft] of Object.entries(parsed_drafts)) {
+            if (draft.type !== "stream") {
+                valid_drafts[draft_id] = draft;
+                continue;
+            }
+
+            // draft.stream is deprecated but might still exist on old drafts
+            if (draft.stream !== undefined) {
+                const sub = stream_data.get_sub(draft.stream);
+                if (sub) {
+                    draft.stream_id = sub.stream_id;
+                }
+                delete draft.stream;
+            }
+
+            // A one-time fix for buggy drafts that had their topics renamed to
+            // `undefined` when the topic was moved to another stream without
+            // changing the topic. The bug was introduced in
+            // 4c8079c49a81b08b29871f9f1625c6149f48b579 and fixed in
+            // aebdf6af8c6675fbd2792888d701d582c4a1110a; but servers running
+            // intermediate versions may have generated some bugged drafts with
+            // this invalid topic value.
+            //
+            // TODO/compatibility: This can be deleted once servers can no longer
+            // directly upgrade from Zulip 6.0beta1 and earlier development branch where the bug was present,
+            // since we expect bugged drafts will have either been run through
+            // this code or else been deleted after 30 (DRAFT_LIFETIME) days.
+            if (draft.topic === undefined) {
+                draft.topic = "";
+            }
+
+            valid_drafts[draft_id] = {
+                ...draft,
+                topic: draft.topic,
+            };
+        }
+        ls.set(KEY, valid_drafts);
+        set_count(Object.keys(valid_drafts).length);
+        fixed_buggy_drafts = true;
+    }
+
     exports.getDraft = function (id) {
         return get()[id] || false;
     };
@@ -97,32 +174,6 @@ export const draft_model = (function () {
     return exports;
 })();
 
-// A one-time fix for buggy drafts that had their topics renamed to
-// `undefined` when the topic was moved to another stream without
-// changing the topic. The bug was introduced in
-// 4c8079c49a81b08b29871f9f1625c6149f48b579 and fixed in
-// aebdf6af8c6675fbd2792888d701d582c4a1110a; but servers running
-// intermediate versions may have generated some bugged drafts with
-// this invalid topic value.
-//
-// TODO/compatibility: This can be deleted once servers can no longer
-// directly upgrade from Zulip 6.0beta1 and earlier development branch where the bug was present,
-// since we expect bugged drafts will have either been run through
-// this code or else been deleted after 30 (DRAFT_LIFETIME) days.
-let fixed_buggy_drafts = false;
-export function fix_drafts_with_undefined_topics() {
-    const data = draft_model.get();
-    for (const draft_id of Object.keys(data)) {
-        const draft = data[draft_id];
-        if (draft.type === "stream" && draft.topic === undefined) {
-            const draft = data[draft_id];
-            draft.topic = "";
-            draft_model.editDraft(draft_id, draft);
-        }
-    }
-    fixed_buggy_drafts = true;
-}
-
 export function sync_count() {
     const drafts = draft_model.get();
     set_count(Object.keys(drafts).length);
@@ -391,13 +442,6 @@ export function format_draft(draft) {
         let sub;
         if (draft.stream_id) {
             sub = sub_store.get(draft.stream_id);
-        } else if (draft.stream && draft.stream.length > 0) {
-            // draft.stream is deprecated but might still exist on old drafts
-            stream_name = draft.stream;
-            const sub = stream_data.get_sub(stream_name);
-            if (sub) {
-                draft.stream_id = sub.stream_id;
-            }
         }
         if (sub) {
             stream_name = sub.name;
@@ -439,10 +483,6 @@ export function format_draft(draft) {
 export function initialize() {
     remove_old_drafts();
 
-    if (!fixed_buggy_drafts) {
-        fix_drafts_with_undefined_topics();
-    }
-
     window.addEventListener("beforeunload", () => {
         update_draft();
     });

web/tests/compose.test.js

@@ -219,6 +219,7 @@ test_ui("send_message_success", ({override, override_rewire}) => {
 test_ui("send_message", ({override, override_rewire, mock_template}) => {
     mock_banners();
     MockDate.set(new Date(fake_now * 1000));
+    override_rewire(drafts, "sync_count", noop);
 
     // This is the common setup stuff for all of the four tests.
     let stub_state;

web/tests/drafts.test.js

@@ -9,7 +9,6 @@ const {run_test, noop} = require("./lib/test");
 const $ = require("./lib/zjquery");
 const {user_settings} = require("./lib/zpage_params");
 
-const blueslip = zrequire("blueslip");
 const compose_pm_pill = zrequire("compose_pm_pill");
 const user_pill = zrequire("user_pill");
 const people = zrequire("people");
@@ -97,6 +96,54 @@ function test(label, f) {
     });
 }
 
+// There were some buggy drafts that had their topics
+// renamed to `undefined` in #23238.
+// TODO/compatibility: The next two tests can be deleted
+// when we get to delete drafts.fix_drafts_with_undefined_topics.
+//
+// This test must run before others, so that
+// fixed_buggy_drafts is false.
+test("fix buggy drafts", ({override_rewire}) => {
+    override_rewire(drafts, "set_count", noop);
+
+    const stream_A = {
+        subscribed: false,
+        name: "A",
+        stream_id: 1,
+    };
+    stream_data.add_sub(stream_A);
+    const stream_B = {
+        subscribed: false,
+        name: "B",
+        stream_id: 2,
+    };
+    stream_data.add_sub(stream_B);
+
+    const buggy_draft = {
+        stream_id: stream_B.stream_id,
+        topic: undefined,
+        type: "stream",
+        content: "Test stream message",
+        updatedAt: Date.now(),
+    };
+    const data = {id1: buggy_draft};
+    const ls = localstorage();
+    ls.set("drafts", data);
+    const draft_model = drafts.draft_model;
+
+    // The draft is fixed in this codepath.
+    drafts.rename_stream_recipient(
+        stream_B.stream_id,
+        "old_topic",
+        stream_A.stream_id,
+        "new_topic",
+    );
+
+    const draft = draft_model.getDraft("id1");
+    assert.equal(draft.stream_id, stream_B.stream_id);
+    assert.equal(draft.topic, "");
+});
+
 test("draft_model add", () => {
     const draft_model = drafts.draft_model;
     const ls = localstorage();
@@ -378,72 +425,6 @@ test("rename_stream_recipient", ({override_rewire}) => {
     assert_draft("id4", stream_B.stream_id, "e");
 });
 
-// There were some buggy drafts that had their topics
-// renamed to `undefined` in #23238.
-// TODO/compatibility: The next two tests can be deleted
-// when we get to delete drafts.fix_drafts_with_undefined_topics.
-test("catch_buggy_draft_error", () => {
-    const stream_A = {
-        subscribed: false,
-        name: "A",
-        stream_id: 1,
-    };
-    stream_data.add_sub(stream_A);
-    const stream_B = {
-        subscribed: false,
-        name: "B",
-        stream_id: 2,
-    };
-    stream_data.add_sub(stream_B);
-
-    const buggy_draft = {
-        stream_id: stream_B.stream_id,
-        topic: undefined,
-        type: "stream",
-        content: "Test stream message",
-        updatedAt: Date.now(),
-    };
-    const data = {id1: buggy_draft};
-    const ls = localstorage();
-    ls.set("drafts", data);
-    const draft_model = drafts.draft_model;
-
-    // An error is logged but the draft isn't fixed in this codepath.
-    blueslip.expect("error", "Cannot compare strings; at least one value is undefined");
-    drafts.rename_stream_recipient(
-        stream_B.stream_id,
-        "old_topic",
-        stream_A.stream_id,
-        "new_topic",
-    );
-    const draft = draft_model.getDraft("id1");
-    assert.equal(draft.stream_id, stream_B.stream_id);
-    assert.equal(draft.topic, undefined);
-});
-
-test("fix_buggy_draft", ({override_rewire}) => {
-    override_rewire(drafts, "set_count", noop);
-
-    const buggy_draft = {
-        stream_id: 1,
-        // This is the bug: topic never be undefined for a stream
-        // message draft.
-        topic: undefined,
-        type: "stream",
-        content: "Test stream message",
-        updatedAt: Date.now(),
-    };
-    const data = {id1: buggy_draft};
-    const ls = localstorage();
-    ls.set("drafts", data);
-    const draft_model = drafts.draft_model;
-
-    drafts.fix_drafts_with_undefined_topics();
-    const draft = draft_model.getDraft("id1");
-    assert.equal(draft.stream_id, buggy_draft.stream_id);
-    assert.equal(draft.topic, "");
-});
-
 test("delete_all_drafts", () => {
     const draft_model = drafts.draft_model;
     const ls = localstorage();