From d86dd165da8d4927c3782d39e13cc51f8e558842 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sat, 10 Nov 2018 16:10:45 +0000 Subject: [PATCH] gitter/slack/hipchat: Remove "subject" from conversions. We (lexically) remove "subject" from the conversion code. The `build_message` helper calls `set_topic_name` under the hood, so things still have "subject" in the JSON. There was good code coverage on `build_message`. --- tools/linter_lib/custom_check.py | 1 + zerver/data_import/gitter.py | 4 ++-- zerver/data_import/hipchat.py | 6 +++--- zerver/data_import/import_util.py | 4 ++-- zerver/data_import/slack.py | 4 ++-- zerver/lib/topic.py | 4 ++++ zerver/tests/test_slack_importer.py | 7 +++++-- 7 files changed, 19 insertions(+), 11 deletions(-) diff --git a/tools/linter_lib/custom_check.py b/tools/linter_lib/custom_check.py index 58ce7ea51c..c3535e5f12 100644 --- a/tools/linter_lib/custom_check.py +++ b/tools/linter_lib/custom_check.py @@ -314,6 +314,7 @@ def build_custom_checkers(by_lang): 'bad_lines': ['subject="foo"', ' MAX_SUBJECT_LEN'], 'exclude': FILES_WITH_LEGACY_SUBJECT, 'include_only': set([ + 'zerver/data_import/', 'zerver/lib/', 'zerver/views/'])}, {'pattern': '^(?!#)@login_required', diff --git a/zerver/data_import/gitter.py b/zerver/data_import/gitter.py index 096ca286c5..99252aaf77 100644 --- a/zerver/data_import/gitter.py +++ b/zerver/data_import/gitter.py @@ -181,10 +181,10 @@ def convert_gitter_workspace_messages(gitter_data: GitterDataT, output_dir: str, mentioned_user_ids = get_usermentions(message, user_map, user_short_name_to_full_name) rendered_content = None - subject = 'imported from gitter' + topic_name = 'imported from gitter' user_id = user_map[message['fromUser']['id']] - zulip_message = build_message(subject, float(message_time), message_id, message['text'], + zulip_message = build_message(topic_name, float(message_time), message_id, message['text'], rendered_content, user_id, recipient_id) zerver_message.append(zulip_message) diff --git a/zerver/data_import/hipchat.py b/zerver/data_import/hipchat.py index 3bb477dc07..9789e5f4c6 100755 --- a/zerver/data_import/hipchat.py +++ b/zerver/data_import/hipchat.py @@ -594,9 +594,9 @@ def process_raw_message_batch(realm_id: int, rendered_content = None if is_pm_data: - subject = '' + topic_name = '' else: - subject = 'imported from hipchat' + topic_name = 'imported from hipchat' user_id = raw_message['sender_id'] # Another side effect: @@ -620,7 +620,7 @@ def process_raw_message_batch(realm_id: int, pub_date=pub_date, recipient_id=recipient_id, rendered_content=rendered_content, - subject=subject, + topic_name=topic_name, user_id=user_id, has_attachment=has_attachment, ) diff --git a/zerver/data_import/import_util.py b/zerver/data_import/import_util.py index de700ec602..3c58190baf 100644 --- a/zerver/data_import/import_util.py +++ b/zerver/data_import/import_util.py @@ -326,13 +326,12 @@ def build_stream(date_created: Any, realm_id: int, name: str, stream_dict['realm'] = realm_id return stream_dict -def build_message(subject: str, pub_date: float, message_id: int, content: str, +def build_message(topic_name: str, pub_date: float, message_id: int, content: str, rendered_content: Optional[str], user_id: int, recipient_id: int, has_image: bool=False, has_link: bool=False, has_attachment: bool=True) -> ZerverFieldsT: zulip_message = Message( rendered_content_version=1, # this is Zulip specific - subject=subject, pub_date=pub_date, id=message_id, content=content, @@ -340,6 +339,7 @@ def build_message(subject: str, pub_date: float, message_id: int, content: str, has_image=has_image, has_attachment=has_attachment, has_link=has_link) + zulip_message.set_topic_name(topic_name) zulip_message_dict = model_to_dict(zulip_message, exclude=['recipient', 'sender', 'sending_client']) zulip_message_dict['sender'] = user_id diff --git a/zerver/data_import/slack.py b/zerver/data_import/slack.py index 462862d6f5..5c636f7c64 100755 --- a/zerver/data_import/slack.py +++ b/zerver/data_import/slack.py @@ -612,9 +612,9 @@ def channel_message_to_zerver_message(realm_id: int, has_image = file_info['has_image'] # construct message - subject = 'imported from slack' + topic_name = 'imported from slack' - zulip_message = build_message(subject, float(message['ts']), message_id, content, + zulip_message = build_message(topic_name, float(message['ts']), message_id, content, rendered_content, added_users[user], recipient_id, has_image, has_link, has_attachment) zerver_message.append(zulip_message) diff --git a/zerver/lib/topic.py b/zerver/lib/topic.py index 1f971d6bb4..c62d3f9c63 100644 --- a/zerver/lib/topic.py +++ b/zerver/lib/topic.py @@ -32,6 +32,10 @@ MATCH_TOPIC = "match_subject" # unless we do a pretty tricky migration. LEGACY_PREV_TOPIC = "prev_subject" +# This constant is pretty closely coupled to the +# database, but it's the JSON field. +EXPORT_TOPIC_NAME = "subject" + # This is used in low-level message functions in # zerver/lib/message.py, and it's not user facing. DB_TOPIC_NAME = "subject" diff --git a/zerver/tests/test_slack_importer.py b/zerver/tests/test_slack_importer.py index 0440a00ff3..6b371cce48 100644 --- a/zerver/tests/test_slack_importer.py +++ b/zerver/tests/test_slack_importer.py @@ -40,6 +40,9 @@ from zerver.lib.avatar_hash import ( from zerver.lib.test_classes import ( ZulipTestCase, ) +from zerver.lib.topic import ( + EXPORT_TOPIC_NAME, +) from zerver.models import ( Realm, get_realm, @@ -465,10 +468,10 @@ class SlackImporter(ZulipTestCase): self.assertEqual(zerver_message[2]['content'], 'http://journals.plos.org/plosone/article') self.assertEqual(zerver_message[2]['has_link'], True) - self.assertEqual(zerver_message[3]['subject'], 'imported from slack') + self.assertEqual(zerver_message[3][EXPORT_TOPIC_NAME], 'imported from slack') self.assertEqual(zerver_message[3]['content'], '/me added bot') self.assertEqual(zerver_message[4]['recipient'], added_recipient['general']) - self.assertEqual(zerver_message[2]['subject'], 'imported from slack') + self.assertEqual(zerver_message[2][EXPORT_TOPIC_NAME], 'imported from slack') self.assertEqual(zerver_message[1]['recipient'], added_recipient['random']) self.assertEqual(zerver_message[3]['id'], zerver_message[0]['id'] + 3)