diff --git a/tools/linter_lib/custom_check.py b/tools/linter_lib/custom_check.py index 6427a7ae59..dd799d3918 100644 --- a/tools/linter_lib/custom_check.py +++ b/tools/linter_lib/custom_check.py @@ -37,17 +37,13 @@ FILES_WITH_LEGACY_SUBJECT = { # These are tied more to our API than our DB model. 'zerver/lib/api_test_helpers.py', - 'zerver/lib/bot_lib.py', - 'zerver/lib/outgoing_webhook.py', # TRY TO FIX THESE! If you can't fix them, try to # add comments here and/or in the file itself about # why sweeping subject is tricky. 'zerver/lib/digest.py', - 'zerver/lib/narrow.py', 'zerver/lib/onboarding.py', 'zerver/lib/stream_topic.py', - 'zerver/lib/url_encoding.py', # This has lots of query data embedded, so it's hard # to fix everything until we migrate the DB to "topic". diff --git a/zerver/lib/bot_lib.py b/zerver/lib/bot_lib.py index 1610129925..e14ed859a1 100644 --- a/zerver/lib/bot_lib.py +++ b/zerver/lib/bot_lib.py @@ -13,6 +13,7 @@ from zerver.lib.bot_storage import get_bot_storage, set_bot_storage, \ is_key_in_bot_storage, get_bot_storage_size, remove_bot_storage from zerver.lib.bot_config import get_bot_config, ConfigError from zerver.lib.integrations import EMBEDDED_BOTS +from zerver.lib.topic import get_topic_from_message_info import configparser @@ -75,7 +76,7 @@ class EmbeddedBotHandler: if message['type'] == 'stream': internal_send_stream_message(self.user_profile.realm, self.user_profile, message['to'], - message['subject'], message['content']) + message['topic'], message['content']) return assert message['type'] == 'private' @@ -103,7 +104,7 @@ class EmbeddedBotHandler: self.send_message(dict( type='stream', to=message['display_recipient'], - subject=message['subject'], + topic=get_topic_from_message_info(message), content=response, sender_email=message['sender_email'], )) diff --git a/zerver/lib/narrow.py b/zerver/lib/narrow.py index bb05bf216d..759dda8b0c 100644 --- a/zerver/lib/narrow.py +++ b/zerver/lib/narrow.py @@ -1,4 +1,7 @@ from zerver.lib.request import JsonableError +from zerver.lib.topic import ( + get_topic_from_message_info, +) from django.utils.translation import ugettext as _ from typing import Any, Callable, Dict, Iterable, Mapping, Sequence @@ -38,7 +41,8 @@ def build_narrow_filter(narrow: Iterable[Sequence[str]]) -> Callable[[Mapping[st elif operator == "topic": if message["type"] != "stream": return False - if operand.lower() != message["subject"].lower(): + topic_name = get_topic_from_message_info(message) + if operand.lower() != topic_name.lower(): return False elif operator == "sender": if operand.lower() != message["sender_email"].lower(): diff --git a/zerver/lib/outgoing_webhook.py b/zerver/lib/outgoing_webhook.py index 6949681e7c..307705fd20 100644 --- a/zerver/lib/outgoing_webhook.py +++ b/zerver/lib/outgoing_webhook.py @@ -16,6 +16,7 @@ from zerver.models import Realm, UserProfile, get_user_profile_by_id, get_client GENERIC_INTERFACE, Service, SLACK_INTERFACE, email_to_domain, get_service_profile from zerver.lib.actions import check_send_message from zerver.lib.queue import retry_event +from zerver.lib.topic import get_topic_from_message_info from zerver.lib.url_encoding import near_message_url from zerver.lib.validator import check_dict, check_string from zerver.decorator import JsonableError @@ -128,7 +129,7 @@ def send_response_message(bot_id: str, message_info: Dict[str, Any], response_da message_info is used to address the message and should have these fields: type - "stream" or "private" display_recipient - like we have in other message events - subject - the topic name (if relevant) + topic - see get_topic_from_message_info response_data is what the bot wants to send back and has these fields: content - raw markdown content for Zulip to render @@ -136,7 +137,10 @@ def send_response_message(bot_id: str, message_info: Dict[str, Any], response_da message_type = message_info['type'] display_recipient = message_info['display_recipient'] - topic_name = message_info.get('subject') + try: + topic_name = get_topic_from_message_info(message_info) + except KeyError: + topic_name = None bot_user = get_user_profile_by_id(bot_id) realm = bot_user.realm diff --git a/zerver/lib/topic.py b/zerver/lib/topic.py index fdc752b7ec..52fe75dd3b 100644 --- a/zerver/lib/topic.py +++ b/zerver/lib/topic.py @@ -36,6 +36,25 @@ LEGACY_PREV_TOPIC = "prev_subject" # database, but it's the JSON field. EXPORT_TOPIC_NAME = "subject" +''' +The following functions are for user-facing APIs +where we'll want to support "subject" for a while. +''' + +def get_topic_from_message_info(message_info: Dict[str, Any]) -> str: + ''' + Use this where you are getting dicts that are based off of messages + that may come from the outside world, especially from third party + APIs and bots. + + We prefer 'topic' to 'subject' here. We expect at least one field + to be present (or the caller must know how to handle KeyError). + ''' + if 'topic' in message_info: + return message_info['topic'] + + return message_info['subject'] + def REQ_topic() -> Optional[str]: # REQ handlers really return a REQ, but we # lie to make the rest of the type matching work. diff --git a/zerver/lib/url_encoding.py b/zerver/lib/url_encoding.py index 20778e63d9..c28287835c 100644 --- a/zerver/lib/url_encoding.py +++ b/zerver/lib/url_encoding.py @@ -1,6 +1,7 @@ import urllib from typing import Any, Dict, List +from zerver.lib.topic import get_topic_from_message_info from zerver.models import Realm, Stream, UserProfile def hash_util_encode(string: str) -> str: @@ -57,7 +58,7 @@ def near_stream_message_url(realm: Realm, message_id = str(message['id']) stream_id = message['stream_id'] stream_name = message['display_recipient'] - topic_name = message['subject'] + topic_name = get_topic_from_message_info(message) encoded_topic = hash_util_encode(topic_name) encoded_stream = encode_stream(stream_id=stream_id, stream_name=stream_name) diff --git a/zerver/tests/fixtures/narrow.json b/zerver/tests/fixtures/narrow.json index af38c77317..b4a8cffea8 100644 --- a/zerver/tests/fixtures/narrow.json +++ b/zerver/tests/fixtures/narrow.json @@ -36,7 +36,14 @@ { "message":{ "type":"stream", - "subject":"bark" + "subject":"BarK" + }, + "flags":null + }, + { + "message":{ + "type":"stream", + "topic":"bark" }, "flags":null } @@ -54,6 +61,13 @@ "subject":"play with tail" }, "flags":null + }, + { + "message":{ + "type":"stream", + "topic":"play with tail" + }, + "flags":null } ], "narrow":[