lib: Rename *topic local variables to *topic_name.

This is preparatory work towards adding a Topic model.
We plan to use the local variable name as 'topic' for
the Topic model objects.

Currently, we use *topic as the local variable name for
topic names.

We rename local variables of the form *topic to *topic_name
so that we don't need to think about type collisions in
individual code paths where we might want to talk about both
Topic objects and strings for the topic name.
This commit is contained in:
Prakhar Pratyush 2024-01-14 19:08:50 +05:30 committed by Tim Abbott
parent bc66eaee7d
commit b7e56ccbdc
11 changed files with 61 additions and 59 deletions

View File

@ -52,12 +52,12 @@ def further_validated_draft_dict(
raise JsonableError(_("Timestamp must not be negative.")) raise JsonableError(_("Timestamp must not be negative."))
last_edit_time = timestamp_to_datetime(timestamp) last_edit_time = timestamp_to_datetime(timestamp)
topic = "" topic_name = ""
recipient_id = None recipient_id = None
to = draft_dict.to to = draft_dict.to
if draft_dict.type == "stream": if draft_dict.type == "stream":
topic = truncate_topic(draft_dict.topic) topic_name = truncate_topic(draft_dict.topic)
if "\0" in topic: if "\0" in topic_name:
raise JsonableError(_("Topic must not contain null bytes")) raise JsonableError(_("Topic must not contain null bytes"))
if len(to) != 1: if len(to) != 1:
raise JsonableError(_("Must specify exactly 1 stream ID for stream messages")) raise JsonableError(_("Must specify exactly 1 stream ID for stream messages"))
@ -72,7 +72,7 @@ def further_validated_draft_dict(
return { return {
"recipient_id": recipient_id, "recipient_id": recipient_id,
"topic": topic, "topic": topic_name,
"content": content, "content": content,
"last_edit_time": last_edit_time, "last_edit_time": last_edit_time,
} }

View File

@ -182,18 +182,18 @@ def construct_zulip_body(
## Sending the Zulip ## ## Sending the Zulip ##
def send_zulip(sender: UserProfile, stream: Stream, topic: str, content: str) -> None: def send_zulip(sender: UserProfile, stream: Stream, topic_name: str, content: str) -> None:
internal_send_stream_message( internal_send_stream_message(
sender, sender,
stream, stream,
truncate_topic(topic), truncate_topic(topic_name),
normalize_body(content), normalize_body(content),
email_gateway=True, email_gateway=True,
) )
def send_mm_reply_to_stream( def send_mm_reply_to_stream(
user_profile: UserProfile, stream: Stream, topic: str, body: str user_profile: UserProfile, stream: Stream, topic_name: str, body: str
) -> None: ) -> None:
try: try:
check_send_message( check_send_message(
@ -201,7 +201,7 @@ def send_mm_reply_to_stream(
client=get_client("Internal"), client=get_client("Internal"),
recipient_type_name="stream", recipient_type_name="stream",
message_to=[stream.id], message_to=[stream.id],
topic_name=topic, topic_name=topic_name,
message_content=body, message_content=body,
) )
except JsonableError as error: except JsonableError as error:
@ -428,7 +428,7 @@ def process_missed_message(to: str, message: EmailMessage) -> None:
mm_address.increment_times_used() mm_address.increment_times_used()
user_profile = mm_address.user_profile user_profile = mm_address.user_profile
topic = mm_address.message.topic_name() topic_name = mm_address.message.topic_name()
if mm_address.message.recipient.type == Recipient.PERSONAL: if mm_address.message.recipient.type == Recipient.PERSONAL:
# We need to reply to the sender so look up their personal recipient_id # We need to reply to the sender so look up their personal recipient_id
@ -445,7 +445,7 @@ def process_missed_message(to: str, message: EmailMessage) -> None:
assert recipient is not None assert recipient is not None
if recipient.type == Recipient.STREAM: if recipient.type == Recipient.STREAM:
stream = get_stream_by_id_in_realm(recipient.type_id, user_profile.realm) stream = get_stream_by_id_in_realm(recipient.type_id, user_profile.realm)
send_mm_reply_to_stream(user_profile, stream, topic, body) send_mm_reply_to_stream(user_profile, stream, topic_name, body)
recipient_str = stream.name recipient_str = stream.name
elif recipient.type == Recipient.PERSONAL: elif recipient.type == Recipient.PERSONAL:
recipient_user_id = recipient.type_id recipient_user_id = recipient.type_id

View File

@ -285,7 +285,7 @@ def build_message_list(
narrow_link = topic_narrow_url( narrow_link = topic_narrow_url(
realm=user.realm, realm=user.realm,
stream=stream, stream=stream,
topic=message.topic_name(), topic_name=message.topic_name(),
) )
header = f"{stream.name} > {message.topic_name()}" header = f"{stream.name} > {message.topic_name()}"
stream_link = stream_narrow_url(user.realm, stream) stream_link = stream_narrow_url(user.realm, stream)
@ -525,7 +525,7 @@ def do_send_missedmessage_events_reply_in_zulip(
narrow_url = topic_narrow_url( narrow_url = topic_narrow_url(
realm=user_profile.realm, realm=user_profile.realm,
stream=stream, stream=stream,
topic=message.topic_name(), topic_name=message.topic_name(),
) )
context.update(narrow_url=narrow_url) context.update(narrow_url=narrow_url)
topic_resolved, topic_name = get_topic_resolution_and_bare_name(message.topic_name()) topic_resolved, topic_name = get_topic_resolution_and_bare_name(message.topic_name())

View File

@ -1309,10 +1309,10 @@ def apply_event(
if "raw_unread_msgs" in state and TOPIC_NAME in event: if "raw_unread_msgs" in state and TOPIC_NAME in event:
stream_dict = state["raw_unread_msgs"]["stream_dict"] stream_dict = state["raw_unread_msgs"]["stream_dict"]
topic = event[TOPIC_NAME] topic_name = event[TOPIC_NAME]
for message_id in event["message_ids"]: for message_id in event["message_ids"]:
if message_id in stream_dict: if message_id in stream_dict:
stream_dict[message_id]["topic"] = topic stream_dict[message_id]["topic"] = topic_name
elif event["type"] == "delete_message": elif event["type"] == "delete_message":
if "message_id" in event: if "message_id" in event:
message_ids = [event["message_id"]] message_ids = [event["message_id"]]

View File

@ -23,14 +23,14 @@ def generate_topics(num_topics: int) -> List[str]:
# Single word topics are most common, thus # Single word topics are most common, thus
# it is important we test on it. # it is important we test on it.
num_single_word_topics = num_topics // 3 num_single_word_topics = num_topics // 3
topics = random.choices(config["nouns"], k=num_single_word_topics) topic_names = random.choices(config["nouns"], k=num_single_word_topics)
sentence = ["adjectives", "nouns", "connectors", "verbs", "adverbs"] sentence = ["adjectives", "nouns", "connectors", "verbs", "adverbs"]
for pos in sentence: for pos in sentence:
# Add an empty string so that we can generate variable length topics. # Add an empty string so that we can generate variable length topics.
config[pos].append("") config[pos].append("")
topics.extend( topic_names.extend(
" ".join(word for pos in sentence if (word := random.choice(config[pos])) != "") " ".join(word for pos in sentence if (word := random.choice(config[pos])) != "")
for _ in range(num_topics - num_single_word_topics) for _ in range(num_topics - num_single_word_topics)
) )
@ -45,8 +45,10 @@ def generate_topics(num_topics: int) -> List[str]:
resolved_topic_probability = 0.05 resolved_topic_probability = 0.05
return [ return [
RESOLVED_TOPIC_PREFIX + topic if random.random() < resolved_topic_probability else topic RESOLVED_TOPIC_PREFIX + topic_name
for topic in topics if random.random() < resolved_topic_probability
else topic_name
for topic_name in topic_names
] ]

View File

@ -238,8 +238,8 @@ def normalize_body(body: str) -> str:
return truncate_content(body, settings.MAX_MESSAGE_LENGTH, "\n[message truncated]") return truncate_content(body, settings.MAX_MESSAGE_LENGTH, "\n[message truncated]")
def truncate_topic(topic: str) -> str: def truncate_topic(topic_name: str) -> str:
return truncate_content(topic, MAX_TOPIC_NAME_LENGTH, "...") return truncate_content(topic_name, MAX_TOPIC_NAME_LENGTH, "...")
def messages_for_ids( def messages_for_ids(
@ -1175,9 +1175,9 @@ def extract_unread_data_from_um_rows(
get_topic_visibility_policy = build_get_topic_visibility_policy(user_profile) get_topic_visibility_policy = build_get_topic_visibility_policy(user_profile)
def is_row_muted(stream_id: int, recipient_id: int, topic: str) -> bool: def is_row_muted(stream_id: int, recipient_id: int, topic_name: str) -> bool:
stream_muted = stream_id in muted_stream_ids stream_muted = stream_id in muted_stream_ids
visibility_policy = get_topic_visibility_policy(recipient_id, topic) visibility_policy = get_topic_visibility_policy(recipient_id, topic_name)
if stream_muted and visibility_policy in [ if stream_muted and visibility_policy in [
UserTopic.VisibilityPolicy.UNMUTED, UserTopic.VisibilityPolicy.UNMUTED,
@ -1216,12 +1216,12 @@ def extract_unread_data_from_um_rows(
if msg_type == Recipient.STREAM: if msg_type == Recipient.STREAM:
stream_id = row["message__recipient__type_id"] stream_id = row["message__recipient__type_id"]
topic = row[MESSAGE__TOPIC] topic_name = row[MESSAGE__TOPIC]
stream_dict[message_id] = dict( stream_dict[message_id] = dict(
stream_id=stream_id, stream_id=stream_id,
topic=topic, topic=topic_name,
) )
if not is_row_muted(stream_id, recipient_id, topic): if not is_row_muted(stream_id, recipient_id, topic_name):
unmuted_stream_msgs.add(message_id) unmuted_stream_msgs.add(message_id)
elif msg_type == Recipient.PERSONAL: elif msg_type == Recipient.PERSONAL:
@ -1253,8 +1253,8 @@ def extract_unread_data_from_um_rows(
if is_stream_wildcard_mentioned or is_topic_wildcard_mentioned: if is_stream_wildcard_mentioned or is_topic_wildcard_mentioned:
if msg_type == Recipient.STREAM: if msg_type == Recipient.STREAM:
stream_id = row["message__recipient__type_id"] stream_id = row["message__recipient__type_id"]
topic = row[MESSAGE__TOPIC] topic_name = row[MESSAGE__TOPIC]
if not is_row_muted(stream_id, recipient_id, topic): if not is_row_muted(stream_id, recipient_id, topic_name):
mentions.add(message_id) mentions.add(message_id)
else: # nocoverage # TODO: Test wildcard mentions in direct messages. else: # nocoverage # TODO: Test wildcard mentions in direct messages.
mentions.add(message_id) mentions.add(message_id)
@ -1271,12 +1271,12 @@ def aggregate_streams(*, input_dict: Dict[int, RawUnreadStreamDict]) -> List[Unr
lookup_dict: Dict[Tuple[int, str], UnreadStreamInfo] = {} lookup_dict: Dict[Tuple[int, str], UnreadStreamInfo] = {}
for message_id, attribute_dict in input_dict.items(): for message_id, attribute_dict in input_dict.items():
stream_id = attribute_dict["stream_id"] stream_id = attribute_dict["stream_id"]
topic = attribute_dict["topic"] topic_name = attribute_dict["topic"]
lookup_key = (stream_id, topic) lookup_key = (stream_id, topic_name)
if lookup_key not in lookup_dict: if lookup_key not in lookup_dict:
obj = UnreadStreamInfo( obj = UnreadStreamInfo(
stream_id=stream_id, stream_id=stream_id,
topic=topic, topic=topic_name,
unread_message_ids=[], unread_message_ids=[],
) )
lookup_dict[lookup_key] = obj lookup_dict[lookup_key] = obj
@ -1387,14 +1387,14 @@ def apply_unread_message_event(
if recipient_type == "stream": if recipient_type == "stream":
stream_id = message["stream_id"] stream_id = message["stream_id"]
topic = message[TOPIC_NAME] topic_name = message[TOPIC_NAME]
state["stream_dict"][message_id] = RawUnreadStreamDict( state["stream_dict"][message_id] = RawUnreadStreamDict(
stream_id=stream_id, stream_id=stream_id,
topic=topic, topic=topic_name,
) )
stream_muted = stream_id in state["muted_stream_ids"] stream_muted = stream_id in state["muted_stream_ids"]
visibility_policy = get_topic_visibility_policy(user_profile, stream_id, topic) visibility_policy = get_topic_visibility_policy(user_profile, stream_id, topic_name)
# A stream message is unmuted if it belongs to: # A stream message is unmuted if it belongs to:
# * a not muted topic in a normal stream # * a not muted topic in a normal stream
# * an unmuted or followed topic in a muted stream # * an unmuted or followed topic in a muted stream

View File

@ -267,7 +267,7 @@ def send_initial_realm_messages(realm: Realm) -> None:
# Order corresponds to the ordering of the streams on the left sidebar, to make the initial Home # Order corresponds to the ordering of the streams on the left sidebar, to make the initial Home
# view slightly less overwhelming # view slightly less overwhelming
with override_language(realm.default_language): with override_language(realm.default_language):
content_of_private_streams_topic = ( content_of_private_streams_topic_name = (
_("This is a private stream, as indicated by the lock icon next to the stream name.") _("This is a private stream, as indicated by the lock icon next to the stream name.")
+ " " + " "
+ _("Private streams are only visible to stream members.") + _("Private streams are only visible to stream members.")
@ -282,20 +282,20 @@ def send_initial_realm_messages(realm: Realm) -> None:
initial_private_stream_name=Realm.INITIAL_PRIVATE_STREAM_NAME, initial_private_stream_name=Realm.INITIAL_PRIVATE_STREAM_NAME,
) )
content1_of_topic_demonstration_topic = ( content1_of_topic_demonstration_topic_name = (
_( _(
"This is a message on stream #**{default_notification_stream_name}** with the " "This is a message on stream #**{default_notification_stream_name}** with the "
"topic `topic demonstration`." "topic `topic demonstration`."
) )
).format(default_notification_stream_name=Realm.DEFAULT_NOTIFICATION_STREAM_NAME) ).format(default_notification_stream_name=Realm.DEFAULT_NOTIFICATION_STREAM_NAME)
content2_of_topic_demonstration_topic = ( content2_of_topic_demonstration_topic_name = (
_("Topics are a lightweight tool to keep conversations organized.") _("Topics are a lightweight tool to keep conversations organized.")
+ " " + " "
+ _("You can learn more about topics at [Streams and topics]({about_topics_help_url}).") + _("You can learn more about topics at [Streams and topics]({about_topics_help_url}).")
).format(about_topics_help_url="/help/streams-and-topics") ).format(about_topics_help_url="/help/streams-and-topics")
content_of_swimming_turtles_topic = ( content_of_swimming_turtles_topic_name = (
_( _(
"This is a message on stream #**{default_notification_stream_name}** with the " "This is a message on stream #**{default_notification_stream_name}** with the "
"topic `swimming turtles`." "topic `swimming turtles`."
@ -317,23 +317,23 @@ def send_initial_realm_messages(realm: Realm) -> None:
welcome_messages: List[Dict[str, str]] = [ welcome_messages: List[Dict[str, str]] = [
{ {
"stream": Realm.INITIAL_PRIVATE_STREAM_NAME, "stream": Realm.INITIAL_PRIVATE_STREAM_NAME,
"topic": "private streams", "topic_name": "private streams",
"content": content_of_private_streams_topic, "content": content_of_private_streams_topic_name,
}, },
{ {
"stream": Realm.DEFAULT_NOTIFICATION_STREAM_NAME, "stream": Realm.DEFAULT_NOTIFICATION_STREAM_NAME,
"topic": "topic demonstration", "topic_name": "topic demonstration",
"content": content1_of_topic_demonstration_topic, "content": content1_of_topic_demonstration_topic_name,
}, },
{ {
"stream": Realm.DEFAULT_NOTIFICATION_STREAM_NAME, "stream": Realm.DEFAULT_NOTIFICATION_STREAM_NAME,
"topic": "topic demonstration", "topic_name": "topic demonstration",
"content": content2_of_topic_demonstration_topic, "content": content2_of_topic_demonstration_topic_name,
}, },
{ {
"stream": realm.DEFAULT_NOTIFICATION_STREAM_NAME, "stream": realm.DEFAULT_NOTIFICATION_STREAM_NAME,
"topic": "swimming turtles", "topic_name": "swimming turtles",
"content": content_of_swimming_turtles_topic, "content": content_of_swimming_turtles_topic_name,
}, },
] ]
@ -342,7 +342,7 @@ def send_initial_realm_messages(realm: Realm) -> None:
realm, realm,
welcome_bot, welcome_bot,
message["stream"], message["stream"],
message["topic"], message["topic_name"],
message["content"], message["content"],
) )
for message in welcome_messages for message in welcome_messages

View File

@ -56,11 +56,11 @@ def check_stream_name(stream_name: str) -> None:
) )
def check_stream_topic(topic: str) -> None: def check_stream_topic(topic_name: str) -> None:
if topic.strip() == "": if topic_name.strip() == "":
raise JsonableError(_("Topic can't be empty!")) raise JsonableError(_("Topic can't be empty!"))
invalid_character_pos = check_string_is_printable(topic) invalid_character_pos = check_string_is_printable(topic_name)
if invalid_character_pos is not None: if invalid_character_pos is not None:
raise JsonableError( raise JsonableError(
_("Invalid character in topic, at position {position}!").format( _("Invalid character in topic, at position {position}!").format(

View File

@ -2178,7 +2178,7 @@ You can fix this by adding "{complete_event_type}" to ALL_EVENT_TYPES for this w
def check_webhook( def check_webhook(
self, self,
fixture_name: str, fixture_name: str,
expected_topic: Optional[str] = None, expected_topic_name: Optional[str] = None,
expected_message: Optional[str] = None, expected_message: Optional[str] = None,
content_type: Optional[str] = "application/json", content_type: Optional[str] = "application/json",
expect_noop: bool = False, expect_noop: bool = False,
@ -2193,7 +2193,7 @@ You can fix this by adding "{complete_event_type}" to ALL_EVENT_TYPES for this w
fixtures. Then we verify that a message gets sent to a stream: fixtures. Then we verify that a message gets sent to a stream:
self.STREAM_NAME: stream name self.STREAM_NAME: stream name
expected_topic: topic expected_topic_name: topic name
expected_message: content expected_message: content
We simulate the delivery of the payload with `content_type`, We simulate the delivery of the payload with `content_type`,
@ -2237,12 +2237,12 @@ your test code triggered an endpoint that did write
one or more new messages. one or more new messages.
""".strip() """.strip()
) )
assert expected_message is not None and expected_topic is not None assert expected_message is not None and expected_topic_name is not None
self.assert_stream_message( self.assert_stream_message(
message=msg, message=msg,
stream_name=self.STREAM_NAME, stream_name=self.STREAM_NAME,
topic_name=expected_topic, topic_name=expected_topic_name,
content=expected_message, content=expected_message,
) )

View File

@ -41,9 +41,9 @@ def stream_narrow_url(realm: Realm, stream: Stream) -> str:
return base_url + encode_stream(stream.id, stream.name) return base_url + encode_stream(stream.id, stream.name)
def topic_narrow_url(*, realm: Realm, stream: Stream, topic: str) -> str: def topic_narrow_url(*, realm: Realm, stream: Stream, topic_name: str) -> str:
base_url = f"{realm.uri}/#narrow/stream/" base_url = f"{realm.uri}/#narrow/stream/"
return f"{base_url}{encode_stream(stream.id, stream.name)}/topic/{hash_util_encode(topic)}" return f"{base_url}{encode_stream(stream.id, stream.name)}/topic/{hash_util_encode(topic_name)}"
def near_message_url(realm: Realm, message: Dict[str, Any]) -> str: def near_message_url(realm: Realm, message: Dict[str, Any]) -> str:
@ -66,7 +66,7 @@ def near_stream_message_url(realm: Realm, message: Dict[str, Any]) -> str:
stream_id = message["stream_id"] stream_id = message["stream_id"]
stream_name = message["display_recipient"] stream_name = message["display_recipient"]
topic_name = get_topic_from_message_info(message) topic_name = get_topic_from_message_info(message)
encoded_topic = hash_util_encode(topic_name) encoded_topic_name = hash_util_encode(topic_name)
encoded_stream = encode_stream(stream_id=stream_id, stream_name=stream_name) encoded_stream = encode_stream(stream_id=stream_id, stream_name=stream_name)
parts = [ parts = [
@ -75,7 +75,7 @@ def near_stream_message_url(realm: Realm, message: Dict[str, Any]) -> str:
"stream", "stream",
encoded_stream, encoded_stream,
"topic", "topic",
encoded_topic, encoded_topic_name,
"near", "near",
message_id, message_id,
] ]

View File

@ -272,8 +272,8 @@ def build_get_topic_visibility_policy(
visibility_policy = row["visibility_policy"] visibility_policy = row["visibility_policy"]
topic_to_visibility_policy[(recipient_id, topic_name)] = visibility_policy topic_to_visibility_policy[(recipient_id, topic_name)] = visibility_policy
def get_topic_visibility_policy(recipient_id: int, topic: str) -> int: def get_topic_visibility_policy(recipient_id: int, topic_name: str) -> int:
return topic_to_visibility_policy[(recipient_id, topic.lower())] return topic_to_visibility_policy[(recipient_id, topic_name.lower())]
return get_topic_visibility_policy return get_topic_visibility_policy