Skip to content

Commit

Permalink
markdown: Simulate message flags in frontend markdown processor.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
timabbott committed Mar 24, 2022
1 parent 2dd0b38 commit 00332fd
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 65 deletions.
55 changes: 31 additions & 24 deletions frontend_tests/node_tests/markdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down
19 changes: 0 additions & 19 deletions static/js/echo.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
38 changes: 30 additions & 8 deletions static/js/markdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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";
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
}

Expand All @@ -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);
}

Expand Down
14 changes: 0 additions & 14 deletions static/js/message_store.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 00332fd

Please sign in to comment.