From 00332fd49d6f53ef69946abfa0840c272f35651a Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Wed, 23 Mar 2022 23:01:47 -0700 Subject: [PATCH] markdown: Simulate message flags in frontend markdown processor. This eliminates an annoying bundle of complexity that caused the frontend markdown processor's interface with the rest of Zulip's new message processing code paths being more similar to that of a new message from the server. It also cuts down on code duplication. --- frontend_tests/node_tests/markdown.js | 55 +++++++++++++++------------ static/js/echo.js | 19 --------- static/js/markdown.js | 38 ++++++++++++++---- static/js/message_store.js | 14 ------- 4 files changed, 61 insertions(+), 65 deletions(-) diff --git a/frontend_tests/node_tests/markdown.js b/frontend_tests/node_tests/markdown.js index 68599faf98..b69531100f 100644 --- a/frontend_tests/node_tests/markdown.js +++ b/frontend_tests/node_tests/markdown.js @@ -268,18 +268,15 @@ test("marked_shared", () => { test("message_flags", () => { let message = {raw_content: "@**Leo**"}; markdown.apply_markdown(message); - assert.ok(!message.mentioned); - assert.ok(!message.mentioned_me_directly); + assert.ok(!message.flags.includes("mentioned")); message = {raw_content: "@**Cordelia, Lear's daughter**"}; markdown.apply_markdown(message); - assert.ok(message.mentioned); - assert.ok(message.mentioned_me_directly); + assert.ok(message.flags.includes("mentioned")); message = {raw_content: "@**all**"}; markdown.apply_markdown(message); - assert.ok(message.mentioned); - assert.ok(!message.mentioned_me_directly); + assert.ok(message.flags.includes("wildcard_mentioned")); }); test("marked", () => { @@ -680,77 +677,87 @@ test("message_flags", () => { markdown.apply_markdown(message); assert.equal(message.is_me_message, false); - assert.equal(message.mentioned, true); - assert.equal(message.mentioned_me_directly, true); - + assert.equal(message.flags.includes("mentioned"), true); + assert.equal(message.flags.includes("wildcard_mentioned"), true); input = "test @**everyone**"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.is_me_message, false); - assert.equal(message.mentioned, true); - assert.equal(message.mentioned_me_directly, false); + assert.equal(message.flags.includes("wildcard_mentioned"), true); + assert.equal(message.flags.includes("mentioned"), false); input = "test @**stream**"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.is_me_message, false); - assert.equal(message.mentioned, true); - assert.equal(message.mentioned_me_directly, false); + assert.equal(message.flags.includes("wildcard_mentioned"), true); + assert.equal(message.flags.includes("mentioned"), false); input = "test @all"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, false); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), false); input = "test @everyone"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, false); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), false); input = "test @any"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, false); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), false); input = "test @alleycat.com"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, false); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), false); input = "test @*hamletcharacters*"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, true); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), true); input = "test @*backend*"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, false); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), false); input = "test @**invalid_user**"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, false); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), false); input = "test @_**all**"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, false); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), false); input = "> test @**all**"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, false); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), false); input = "test @_*hamletcharacters*"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, false); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), false); input = "> test @*hamletcharacters*"; message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); - assert.equal(message.mentioned, false); + assert.equal(message.flags.includes("wildcard_mentioned"), false); + assert.equal(message.flags.includes("mentioned"), false); }); test("backend_only_linkifiers", () => { diff --git a/static/js/echo.js b/static/js/echo.js index 76464fa67e..f15fe3917e 100644 --- a/static/js/echo.js +++ b/static/js/echo.js @@ -178,25 +178,6 @@ export function insert_local_message(message_request, local_id_float) { // NOTE: This will parse synchronously. We're not using the async pipeline markdown.apply_markdown(message); - // TODO: markdown.apply_markdown has a messy interface around - // message flags. It mutates the message to set all the message - // booleans to false via message_store.init_booleans, and then - // proceeds to mutate primarily the mentioned flag during - // rendering in a messy way. - // - // We should clean that up to instead just mutate the flags list - // on the message, to match how the Zulip API returns this - // information via flags. For now, we translate back to the flags - // namespace here, and then message_store.set_message_booleans - // will get us to the correct final flags. - // - // Locally delivered messages cannot be unread (since we sent - // them), nor can they alert the user. - message.flags = ["read"]; - if (message.mentioned) { - message.flags.push("mentioned"); - } - message.content_type = "text/html"; message.sender_email = people.my_current_email(); message.sender_full_name = people.my_full_name(); diff --git a/static/js/markdown.js b/static/js/markdown.js index 1289776181..b758f4497e 100644 --- a/static/js/markdown.js +++ b/static/js/markdown.js @@ -8,7 +8,6 @@ import marked from "../third/marked/lib/marked"; import * as blueslip from "./blueslip"; import * as linkifiers from "./linkifiers"; -import * as message_store from "./message_store"; // This contains zulip's frontend Markdown implementation; see // docs/subsystems/markdown.md for docs on our Markdown syntax. The other @@ -96,7 +95,9 @@ export function contains_backend_only_syntax(content) { } export function apply_markdown(message) { - message_store.init_booleans(message); + let mentioned = false; + let mentioned_group = false; + let mentioned_wildcard = false; const options = { userMentionHandler(mention, silently) { @@ -107,7 +108,8 @@ export function apply_markdown(message) { classes = "user-mention silent"; display_text = mention; } else { - message.mentioned = true; + // Wildcard mention + mentioned_wildcard = true; display_text = "@" + mention; classes = "user-mention"; } @@ -183,8 +185,8 @@ export function apply_markdown(message) { classes = "user-mention silent"; } else { if (helpers.my_user_id() === user_id) { - message.mentioned = true; - message.mentioned_me_directly = true; + // Personal mention of current user. + mentioned = true; } classes = "user-mention"; display_text = "@" + display_text; @@ -206,7 +208,8 @@ export function apply_markdown(message) { display_text = "@" + group.name; classes = "user-group-mention"; if (helpers.is_member_of_user_group(group.id, helpers.my_user_id())) { - message.mentioned = true; + // Mentioned the current user's group. + mentioned_group = true; } } @@ -233,13 +236,32 @@ export function apply_markdown(message) { // mention yourself outside of the blockquote (and, above it). If that you do that, the // following mentioned status is false; the backend rendering is authoritative and the // only side effect is the lack red flash on immediately sending the message. - message.mentioned = false; - message.mentioned_me_directly = false; + // + // A better parser would be able to just ignore mentions + // inside; we just set all flags to False and let the + // server rendering correct the message flags, to avoid a + // flash of mention styling. + mentioned = false; + mentioned_group = false; + mentioned_wildcard = false; return quote; }, }; + // Our Python-Markdown processor appends two \n\n to input message.content = marked(message.raw_content + "\n\n", options).trim(); + + // Simulate message flags for our locally rendered + // message. Messages the user themselves sent via the browser are + // always marked as read. + message.flags = ["read"]; + if (mentioned || mentioned_group) { + message.flags.push("mentioned"); + } + if (mentioned_wildcard) { + message.flags.push("wildcard_mentioned"); + } + message.is_me_message = is_status_message(message.raw_content); } diff --git a/static/js/message_store.js b/static/js/message_store.js index 5aefb64b72..0af8f9af37 100644 --- a/static/js/message_store.js +++ b/static/js/message_store.js @@ -79,20 +79,6 @@ export function set_message_booleans(message) { delete message.flags; } -export function init_booleans(message) { - // This initializes booleans for the local-echo path where - // we don't have flags from the server yet. (We want to - // explicitly set flags to false to be consistent with other - // codepaths.) - message.unread = false; - message.historical = false; - message.starred = false; - message.mentioned = false; - message.mentioned_me_directly = false; - message.collapsed = false; - message.alerted = false; -} - export function update_booleans(message, flags) { // When we get server flags for local echo or message edits, // we are vulnerable to race conditions, so only update flags