From b7e56ccbdccd0ccc409b9846cd6f6ea9c50023ca Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Sun, 14 Jan 2024 19:08:50 +0530 Subject: [PATCH] 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. --- zerver/lib/drafts.py | 8 ++++---- zerver/lib/email_mirror.py | 12 ++++++------ zerver/lib/email_notifications.py | 4 ++-- zerver/lib/events.py | 4 ++-- zerver/lib/generate_test_data.py | 10 ++++++---- zerver/lib/message.py | 30 +++++++++++++++--------------- zerver/lib/onboarding.py | 26 +++++++++++++------------- zerver/lib/string_validation.py | 6 +++--- zerver/lib/test_classes.py | 8 ++++---- zerver/lib/url_encoding.py | 8 ++++---- zerver/lib/user_topics.py | 4 ++-- 11 files changed, 61 insertions(+), 59 deletions(-) diff --git a/zerver/lib/drafts.py b/zerver/lib/drafts.py index fc28adb92d..4b75d1073f 100644 --- a/zerver/lib/drafts.py +++ b/zerver/lib/drafts.py @@ -52,12 +52,12 @@ def further_validated_draft_dict( raise JsonableError(_("Timestamp must not be negative.")) last_edit_time = timestamp_to_datetime(timestamp) - topic = "" + topic_name = "" recipient_id = None to = draft_dict.to if draft_dict.type == "stream": - topic = truncate_topic(draft_dict.topic) - if "\0" in topic: + topic_name = truncate_topic(draft_dict.topic) + if "\0" in topic_name: raise JsonableError(_("Topic must not contain null bytes")) if len(to) != 1: raise JsonableError(_("Must specify exactly 1 stream ID for stream messages")) @@ -72,7 +72,7 @@ def further_validated_draft_dict( return { "recipient_id": recipient_id, - "topic": topic, + "topic": topic_name, "content": content, "last_edit_time": last_edit_time, } diff --git a/zerver/lib/email_mirror.py b/zerver/lib/email_mirror.py index b156487d35..d29a65943e 100644 --- a/zerver/lib/email_mirror.py +++ b/zerver/lib/email_mirror.py @@ -182,18 +182,18 @@ def construct_zulip_body( ## 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( sender, stream, - truncate_topic(topic), + truncate_topic(topic_name), normalize_body(content), email_gateway=True, ) 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: try: check_send_message( @@ -201,7 +201,7 @@ def send_mm_reply_to_stream( client=get_client("Internal"), recipient_type_name="stream", message_to=[stream.id], - topic_name=topic, + topic_name=topic_name, message_content=body, ) except JsonableError as error: @@ -428,7 +428,7 @@ def process_missed_message(to: str, message: EmailMessage) -> None: mm_address.increment_times_used() 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: # 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 if recipient.type == Recipient.STREAM: 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 elif recipient.type == Recipient.PERSONAL: recipient_user_id = recipient.type_id diff --git a/zerver/lib/email_notifications.py b/zerver/lib/email_notifications.py index 56c4bb85b9..421732bf84 100644 --- a/zerver/lib/email_notifications.py +++ b/zerver/lib/email_notifications.py @@ -285,7 +285,7 @@ def build_message_list( narrow_link = topic_narrow_url( realm=user.realm, stream=stream, - topic=message.topic_name(), + topic_name=message.topic_name(), ) header = f"{stream.name} > {message.topic_name()}" 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( realm=user_profile.realm, stream=stream, - topic=message.topic_name(), + topic_name=message.topic_name(), ) context.update(narrow_url=narrow_url) topic_resolved, topic_name = get_topic_resolution_and_bare_name(message.topic_name()) diff --git a/zerver/lib/events.py b/zerver/lib/events.py index d2cd1eaa16..a4833e905d 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -1309,10 +1309,10 @@ def apply_event( if "raw_unread_msgs" in state and TOPIC_NAME in event: stream_dict = state["raw_unread_msgs"]["stream_dict"] - topic = event[TOPIC_NAME] + topic_name = event[TOPIC_NAME] for message_id in event["message_ids"]: if message_id in stream_dict: - stream_dict[message_id]["topic"] = topic + stream_dict[message_id]["topic"] = topic_name elif event["type"] == "delete_message": if "message_id" in event: message_ids = [event["message_id"]] diff --git a/zerver/lib/generate_test_data.py b/zerver/lib/generate_test_data.py index d6a27a4c78..057bdcd96c 100644 --- a/zerver/lib/generate_test_data.py +++ b/zerver/lib/generate_test_data.py @@ -23,14 +23,14 @@ def generate_topics(num_topics: int) -> List[str]: # Single word topics are most common, thus # it is important we test on it. 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"] for pos in sentence: # Add an empty string so that we can generate variable length topics. config[pos].append("") - topics.extend( + topic_names.extend( " ".join(word for pos in sentence if (word := random.choice(config[pos])) != "") 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 return [ - RESOLVED_TOPIC_PREFIX + topic if random.random() < resolved_topic_probability else topic - for topic in topics + RESOLVED_TOPIC_PREFIX + topic_name + if random.random() < resolved_topic_probability + else topic_name + for topic_name in topic_names ] diff --git a/zerver/lib/message.py b/zerver/lib/message.py index 8de57e4dab..292839a6d7 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -238,8 +238,8 @@ def normalize_body(body: str) -> str: return truncate_content(body, settings.MAX_MESSAGE_LENGTH, "\n[message truncated]") -def truncate_topic(topic: str) -> str: - return truncate_content(topic, MAX_TOPIC_NAME_LENGTH, "...") +def truncate_topic(topic_name: str) -> str: + return truncate_content(topic_name, MAX_TOPIC_NAME_LENGTH, "...") 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) - 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 - 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 [ UserTopic.VisibilityPolicy.UNMUTED, @@ -1216,12 +1216,12 @@ def extract_unread_data_from_um_rows( if msg_type == Recipient.STREAM: stream_id = row["message__recipient__type_id"] - topic = row[MESSAGE__TOPIC] + topic_name = row[MESSAGE__TOPIC] stream_dict[message_id] = dict( 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) 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 msg_type == Recipient.STREAM: stream_id = row["message__recipient__type_id"] - topic = row[MESSAGE__TOPIC] - if not is_row_muted(stream_id, recipient_id, topic): + topic_name = row[MESSAGE__TOPIC] + if not is_row_muted(stream_id, recipient_id, topic_name): mentions.add(message_id) else: # nocoverage # TODO: Test wildcard mentions in direct messages. 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] = {} for message_id, attribute_dict in input_dict.items(): stream_id = attribute_dict["stream_id"] - topic = attribute_dict["topic"] - lookup_key = (stream_id, topic) + topic_name = attribute_dict["topic"] + lookup_key = (stream_id, topic_name) if lookup_key not in lookup_dict: obj = UnreadStreamInfo( stream_id=stream_id, - topic=topic, + topic=topic_name, unread_message_ids=[], ) lookup_dict[lookup_key] = obj @@ -1387,14 +1387,14 @@ def apply_unread_message_event( if recipient_type == "stream": stream_id = message["stream_id"] - topic = message[TOPIC_NAME] + topic_name = message[TOPIC_NAME] state["stream_dict"][message_id] = RawUnreadStreamDict( stream_id=stream_id, - topic=topic, + topic=topic_name, ) 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 not muted topic in a normal stream # * an unmuted or followed topic in a muted stream diff --git a/zerver/lib/onboarding.py b/zerver/lib/onboarding.py index 4877c8f92f..858d62fe46 100644 --- a/zerver/lib/onboarding.py +++ b/zerver/lib/onboarding.py @@ -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 # view slightly less overwhelming 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.") + " " + _("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, ) - content1_of_topic_demonstration_topic = ( + content1_of_topic_demonstration_topic_name = ( _( "This is a message on stream #**{default_notification_stream_name}** with the " "topic `topic demonstration`." ) ).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.") + " " + _("You can learn more about topics at [Streams and topics]({about_topics_help_url}).") ).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 " "topic `swimming turtles`." @@ -317,23 +317,23 @@ def send_initial_realm_messages(realm: Realm) -> None: welcome_messages: List[Dict[str, str]] = [ { "stream": Realm.INITIAL_PRIVATE_STREAM_NAME, - "topic": "private streams", - "content": content_of_private_streams_topic, + "topic_name": "private streams", + "content": content_of_private_streams_topic_name, }, { "stream": Realm.DEFAULT_NOTIFICATION_STREAM_NAME, - "topic": "topic demonstration", - "content": content1_of_topic_demonstration_topic, + "topic_name": "topic demonstration", + "content": content1_of_topic_demonstration_topic_name, }, { "stream": Realm.DEFAULT_NOTIFICATION_STREAM_NAME, - "topic": "topic demonstration", - "content": content2_of_topic_demonstration_topic, + "topic_name": "topic demonstration", + "content": content2_of_topic_demonstration_topic_name, }, { "stream": realm.DEFAULT_NOTIFICATION_STREAM_NAME, - "topic": "swimming turtles", - "content": content_of_swimming_turtles_topic, + "topic_name": "swimming turtles", + "content": content_of_swimming_turtles_topic_name, }, ] @@ -342,7 +342,7 @@ def send_initial_realm_messages(realm: Realm) -> None: realm, welcome_bot, message["stream"], - message["topic"], + message["topic_name"], message["content"], ) for message in welcome_messages diff --git a/zerver/lib/string_validation.py b/zerver/lib/string_validation.py index 09df29a835..b7c3f329a1 100644 --- a/zerver/lib/string_validation.py +++ b/zerver/lib/string_validation.py @@ -56,11 +56,11 @@ def check_stream_name(stream_name: str) -> None: ) -def check_stream_topic(topic: str) -> None: - if topic.strip() == "": +def check_stream_topic(topic_name: str) -> None: + if topic_name.strip() == "": 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: raise JsonableError( _("Invalid character in topic, at position {position}!").format( diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 9548fef8b6..4388f763f0 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -2178,7 +2178,7 @@ You can fix this by adding "{complete_event_type}" to ALL_EVENT_TYPES for this w def check_webhook( self, fixture_name: str, - expected_topic: Optional[str] = None, + expected_topic_name: Optional[str] = None, expected_message: Optional[str] = None, content_type: Optional[str] = "application/json", 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: self.STREAM_NAME: stream name - expected_topic: topic + expected_topic_name: topic name expected_message: content 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. """.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( message=msg, stream_name=self.STREAM_NAME, - topic_name=expected_topic, + topic_name=expected_topic_name, content=expected_message, ) diff --git a/zerver/lib/url_encoding.py b/zerver/lib/url_encoding.py index dd419081ae..12986b502b 100644 --- a/zerver/lib/url_encoding.py +++ b/zerver/lib/url_encoding.py @@ -41,9 +41,9 @@ def stream_narrow_url(realm: Realm, stream: Stream) -> str: 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/" - 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: @@ -66,7 +66,7 @@ def near_stream_message_url(realm: Realm, message: Dict[str, Any]) -> str: stream_id = message["stream_id"] stream_name = message["display_recipient"] 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) parts = [ @@ -75,7 +75,7 @@ def near_stream_message_url(realm: Realm, message: Dict[str, Any]) -> str: "stream", encoded_stream, "topic", - encoded_topic, + encoded_topic_name, "near", message_id, ] diff --git a/zerver/lib/user_topics.py b/zerver/lib/user_topics.py index 8f3e6e499c..6daa693310 100644 --- a/zerver/lib/user_topics.py +++ b/zerver/lib/user_topics.py @@ -272,8 +272,8 @@ def build_get_topic_visibility_policy( visibility_policy = row["visibility_policy"] topic_to_visibility_policy[(recipient_id, topic_name)] = visibility_policy - def get_topic_visibility_policy(recipient_id: int, topic: str) -> int: - return topic_to_visibility_policy[(recipient_id, topic.lower())] + def get_topic_visibility_policy(recipient_id: int, topic_name: str) -> int: + return topic_to_visibility_policy[(recipient_id, topic_name.lower())] return get_topic_visibility_policy