Skip to content

Commit

Permalink
refactor: Plumb rendered_message to actions.py for safer typecheck.
Browse files Browse the repository at this point in the history
This is a preparation for adding django stubs.

Related: zulip#18777
  • Loading branch information
PIG208 committed Jun 15, 2021
1 parent cb19856 commit 4073a81
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 34 deletions.
75 changes: 46 additions & 29 deletions zerver/lib/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
from zerver.lib.mention import MentionData
from zerver.lib.message import (
MessageDict,
RenderedMessage,
SendMessageRequest,
access_message,
get_last_message_id,
Expand Down Expand Up @@ -1413,10 +1414,10 @@ def render_incoming_message(
realm: Realm,
mention_data: Optional[MentionData] = None,
email_gateway: bool = False,
) -> str:
) -> Tuple[str, RenderedMessage]:
realm_alert_words_automaton = get_alert_word_automaton(realm)
try:
rendered_content, __ = render_markdown(
rendered_content, rendered_message = render_markdown(
message=message,
content=content,
realm=realm,
Expand All @@ -1426,7 +1427,7 @@ def render_incoming_message(
)
except MarkdownRenderingException:
raise JsonableError(_("Unable to render message"))
return rendered_content
return rendered_content, rendered_message


class RecipientInfoResult(TypedDict):
Expand Down Expand Up @@ -1780,7 +1781,7 @@ def build_message_send_dict(
# Render our message_dicts.
assert message.rendered_content is None

rendered_content = render_incoming_message(
rendered_content, rendered_message = render_incoming_message(
message,
message.content,
info["active_user_ids"],
Expand All @@ -1790,18 +1791,18 @@ def build_message_send_dict(
)
message.rendered_content = rendered_content
message.rendered_content_version = markdown_version
links_for_embed = message.links_for_preview
links_for_embed = rendered_message.links_for_preview

# Add members of the mentioned user groups into `mentions_user_ids`.
for group_id in message.mentions_user_group_ids:
for group_id in rendered_message.mentions_user_group_ids:
members = mention_data.get_group_members(group_id)
message.mentions_user_ids.update(members)
rendered_message.mentions_user_ids.update(members)

# Only send data to Tornado about wildcard mentions if message
# rendering determined the message had an actual wildcard
# mention in it (and not e.g. wildcard mention syntax inside a
# code block).
if message.mentions_wildcard:
if rendered_message.mentions_wildcard:
wildcard_mention_user_ids = info["wildcard_mention_user_ids"]
else:
wildcard_mention_user_ids = set()
Expand All @@ -1812,7 +1813,7 @@ def build_message_send_dict(
who were directly mentioned in this message as eligible to
get UserMessage rows.
"""
mentioned_user_ids = message.mentions_user_ids
mentioned_user_ids = rendered_message.mentions_user_ids
default_bot_user_ids = info["default_bot_user_ids"]
mentioned_bot_user_ids = default_bot_user_ids & mentioned_user_ids
info["um_eligible_user_ids"] |= mentioned_bot_user_ids
Expand All @@ -1824,6 +1825,7 @@ def build_message_send_dict(
realm=realm,
mention_data=mention_data,
message=message,
rendered_message=rendered_message,
active_user_ids=info["active_user_ids"],
online_push_user_ids=info["online_push_user_ids"],
stream_push_user_ids=info["stream_push_user_ids"],
Expand Down Expand Up @@ -1875,14 +1877,15 @@ def do_send_messages(
for send_request in send_message_requests:
# Service bots (outgoing webhook bots and embedded bots) don't store UserMessage rows;
# they will be processed later.
mentioned_user_ids = send_request.message.mentions_user_ids
mentioned_user_ids = send_request.rendered_message.mentions_user_ids

# Extend the set with users who have muted the sender.
mark_as_read_for_users = send_request.muted_sender_user_ids
mark_as_read_for_users.update(mark_as_read)

user_messages = create_user_messages(
message=send_request.message,
rendered_message=send_request.rendered_message,
um_eligible_user_ids=send_request.um_eligible_user_ids,
long_term_idle_user_ids=send_request.long_term_idle_user_ids,
stream_push_user_ids=send_request.stream_push_user_ids,
Expand Down Expand Up @@ -2059,6 +2062,7 @@ def flags_list(self) -> List[str]:

def create_user_messages(
message: Message,
rendered_message: RenderedMessage,
um_eligible_user_ids: AbstractSet[int],
long_term_idle_user_ids: AbstractSet[int],
stream_push_user_ids: AbstractSet[int],
Expand All @@ -2068,12 +2072,12 @@ def create_user_messages(
) -> List[UserMessageLite]:
# These properties on the Message are set via
# render_markdown by code in the Markdown inline patterns
ids_with_alert_words = message.user_ids_with_alert_words
ids_with_alert_words = rendered_message.user_ids_with_alert_words
sender_id = message.sender.id
is_stream_message = message.is_stream_message()

base_flags = 0
if message.mentions_wildcard:
if rendered_message.mentions_wildcard:
base_flags |= UserMessage.flags.wildcard_mentioned
if message.recipient.type in [Recipient.HUDDLE, Recipient.PERSONAL]:
base_flags |= UserMessage.flags.is_private
Expand Down Expand Up @@ -2884,6 +2888,7 @@ def check_update_message(
raise JsonableError(_("The time limit for editing this message's topic has passed"))

rendered_content = None
rendered_message = None
links_for_embed: Set[str] = set()
prior_mention_user_ids: Set[int] = set()
mention_user_ids: Set[int] = set()
Expand All @@ -2904,16 +2909,16 @@ def check_update_message(
# the cross-realm bots never edit messages, this should be
# always correct.
# Note: If rendering fails, the called code will raise a JsonableError.
rendered_content = render_incoming_message(
rendered_content, rendered_message = render_incoming_message(
message,
content,
user_info["message_user_ids"],
user_profile.realm,
mention_data=mention_data,
)
links_for_embed |= message.links_for_preview
links_for_embed |= rendered_message.links_for_preview

mention_user_ids = message.mentions_user_ids
mention_user_ids = rendered_message.mentions_user_ids

new_stream = None
number_changed = 0
Expand Down Expand Up @@ -2944,6 +2949,7 @@ def check_update_message(
send_notification_to_new_thread,
content,
rendered_content,
rendered_message,
prior_mention_user_ids,
mention_user_ids,
mention_data,
Expand Down Expand Up @@ -3250,7 +3256,7 @@ def check_message(
email_gateway=email_gateway,
)

if stream is not None and message_send_dict.message.mentions_wildcard:
if stream is not None and message_send_dict.rendered_message.mentions_wildcard:
if not wildcard_mention_allowed(sender, stream):
raise JsonableError(
_("You do not have permission to use wildcard mentions in this stream.")
Expand Down Expand Up @@ -5497,7 +5503,10 @@ def do_mark_muted_user_messages_as_read(


def do_update_mobile_push_notification(
message: Message, prior_mention_user_ids: Set[int], stream_push_user_ids: Set[int]
message: Message,
current_mention_user_ids: Set[int],
prior_mention_user_ids: Set[int],
stream_push_user_ids: Set[int],
) -> None:
# Called during the message edit code path to remove mobile push
# notifications for users who are no longer mentioned following
Expand All @@ -5510,7 +5519,7 @@ def do_update_mobile_push_notification(
if not message.is_stream_message():
return

remove_notify_users = prior_mention_user_ids - message.mentions_user_ids - stream_push_user_ids
remove_notify_users = prior_mention_user_ids - current_mention_user_ids - stream_push_user_ids
do_clear_mobile_push_notifications_for_ids(list(remove_notify_users), [message.id])


Expand Down Expand Up @@ -5698,10 +5707,12 @@ def get_user_info_for_message_updates(message_id: int) -> MessageUpdateUserInfoR
)


def update_user_message_flags(message: Message, ums: Iterable[UserMessage]) -> None:
wildcard = message.mentions_wildcard
mentioned_ids = message.mentions_user_ids
ids_with_alert_words = message.user_ids_with_alert_words
def update_user_message_flags(
rendered_message: RenderedMessage, ums: Iterable[UserMessage]
) -> None:
wildcard = rendered_message.mentions_wildcard
mentioned_ids = rendered_message.mentions_user_ids
ids_with_alert_words = rendered_message.user_ids_with_alert_words
changed_ums: Set[UserMessage] = set()

def update_flag(um: UserMessage, should_set: bool, flag: int) -> None:
Expand Down Expand Up @@ -5751,14 +5762,16 @@ def do_update_embedded_data(
message: Message,
content: Optional[str],
rendered_content: Optional[str],
rendered_message: Optional[RenderedMessage],
) -> None:
event: Dict[str, Any] = {"type": "update_message", "message_id": message.id}
changed_messages = [message]

ums = UserMessage.objects.filter(message=message.id)

if content is not None:
update_user_message_flags(message, ums)
if rendered_message is not None:
update_user_message_flags(rendered_message, ums)
message.content = content
message.rendered_content = rendered_content
message.rendered_content_version = markdown_version
Expand Down Expand Up @@ -5800,6 +5813,7 @@ def do_update_message(
send_notification_to_new_thread: bool,
content: Optional[str],
rendered_content: Optional[str],
rendered_message: Optional[RenderedMessage],
prior_mention_user_ids: Set[int],
mention_user_ids: Set[int],
mention_data: Optional[MentionData] = None,
Expand Down Expand Up @@ -5843,17 +5857,17 @@ def do_update_message(
ums = UserMessage.objects.filter(message=target_message.id)

if content is not None:
assert rendered_content is not None
assert rendered_content is not None and rendered_message is not None

# mention_data is required if there's a content edit.
assert mention_data is not None

# add data from group mentions to mentions_user_ids.
for group_id in target_message.mentions_user_group_ids:
for group_id in rendered_message.mentions_user_group_ids:
members = mention_data.get_group_members(group_id)
target_message.mentions_user_ids.update(members)
rendered_message.mentions_user_ids.update(members)

update_user_message_flags(target_message, ums)
update_user_message_flags(rendered_message, ums)

# One could imagine checking realm.allow_edit_history here and
# modifying the events based on that setting, but doing so
Expand Down Expand Up @@ -5910,13 +5924,16 @@ def do_update_message(
event["prior_mention_user_ids"] = list(prior_mention_user_ids)
event["mention_user_ids"] = list(mention_user_ids)
event["presence_idle_user_ids"] = filter_presence_idle_user_ids(info["active_user_ids"])
if target_message.mentions_wildcard:
if rendered_message.mentions_wildcard:
event["wildcard_mention_user_ids"] = list(info["wildcard_mention_user_ids"])
else:
event["wildcard_mention_user_ids"] = []

do_update_mobile_push_notification(
target_message, prior_mention_user_ids, info["stream_push_user_ids"]
target_message,
rendered_message.mentions_user_ids,
prior_mention_user_ids,
info["stream_push_user_ids"],
)

if topic_name is not None or new_stream is not None:
Expand Down
1 change: 1 addition & 0 deletions zerver/lib/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class RenderedMessage:
@dataclass
class SendMessageRequest:
message: Message
rendered_message: RenderedMessage
stream: Optional[Stream]
local_id: Optional[str]
sender_queue_id: Optional[str]
Expand Down
6 changes: 4 additions & 2 deletions zerver/tests/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ def test_stream_send_message_events(self) -> None:
topic = "new_topic"
propagate_mode = "change_all"
content = "new content"
rendered_content, _ = render_markdown(message, content)
rendered_content, rendered_message = render_markdown(message, content)
prior_mention_user_ids: Set[int] = set()
mentioned_user_ids: Set[int] = set()
mention_data = MentionData(
Expand All @@ -470,6 +470,7 @@ def test_stream_send_message_events(self) -> None:
False,
content,
rendered_content,
rendered_message,
prior_mention_user_ids,
mentioned_user_ids,
mention_data,
Expand All @@ -486,7 +487,7 @@ def test_stream_send_message_events(self) -> None:

events = self.verify_action(
lambda: do_update_embedded_data(
self.user_profile, message, "embed_content", "<p>embed_content</p>"
self.user_profile, message, "embed_content", "<p>embed_content</p>", None
),
state_change_expected=False,
)
Expand Down Expand Up @@ -514,6 +515,7 @@ def test_stream_send_message_events(self) -> None:
True,
None,
None,
None,
set(),
set(),
None,
Expand Down
1 change: 1 addition & 0 deletions zerver/tests/test_message_edit.py
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,7 @@ def do_update_message_topic_success(
send_notification_to_new_thread=False,
content=None,
rendered_content=None,
rendered_message=None,
prior_mention_user_ids=set(),
mention_user_ids=set(),
mention_data=None,
Expand Down
3 changes: 2 additions & 1 deletion zerver/tests/test_message_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -3711,7 +3711,7 @@ def test_finds_only_links(self) -> None:
def update_message(self, msg: Message, content: str) -> None:
hamlet = self.example_user("hamlet")
realm_id = hamlet.realm.id
rendered_content, _ = render_markdown(msg, content)
rendered_content, rendered_message = render_markdown(msg, content)
mention_data = MentionData(realm_id, content)
do_update_message(
hamlet,
Expand All @@ -3723,6 +3723,7 @@ def update_message(self, msg: Message, content: str) -> None:
False,
content,
rendered_content,
rendered_message,
set(),
set(),
mention_data=mention_data,
Expand Down
6 changes: 4 additions & 2 deletions zerver/worker/queue_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -753,10 +753,12 @@ def consume(self, event: Mapping[str, Any]) -> None:
realm = Realm.objects.get(id=event["message_realm_id"])

# If rendering fails, the called code will raise a JsonableError.
rendered_content = render_incoming_message(
rendered_content, rendered_message = render_incoming_message(
message, message.content, message_user_ids, realm
)
do_update_embedded_data(message.sender, message, message.content, rendered_content)
do_update_embedded_data(
message.sender, message, message.content, rendered_content, rendered_message
)


@assign_queue("outgoing_webhooks")
Expand Down

0 comments on commit 4073a81

Please sign in to comment.