topic -> subject: Extract get_topic_from_message_info().

This changes files where it's safe to just assume caller
may use either "topic" or "subject", and we prefer "topic"
but support "subject".

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".

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'],
             ))

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():

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

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.

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)
 

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":[