From ff3871fc634b9f050f2e26bd2bb948bcef3edef2 Mon Sep 17 00:00:00 2001 From: Vishnu KS Date: Fri, 26 Jul 2019 22:09:50 +0530 Subject: [PATCH] slack_import: Clean up return values of channels_to_zerver_stream. This commits reduces the number of values returned by channel_to_zerver_stream function by setting the values directly in realm dict and returning it instead. --- zerver/data_import/slack.py | 83 +++++++++++------------------ zerver/tests/test_slack_importer.py | 43 ++++++++------- 2 files changed, 55 insertions(+), 71 deletions(-) diff --git a/zerver/data_import/slack.py b/zerver/data_import/slack.py index 9d0b662f82..099ea1796b 100755 --- a/zerver/data_import/slack.py +++ b/zerver/data_import/slack.py @@ -65,10 +65,10 @@ def slack_workspace_to_realm(domain_name: str, realm_id: int, user_list: List[Ze zerver_userprofile, avatars, added_users, zerver_customprofilefield, \ zerver_customprofilefield_value = users_to_zerver_userprofile(slack_data_dir, user_list, realm_id, int(NOW), domain_name) - channels_to_zerver_stream_fields = channels_to_zerver_stream(slack_data_dir, - realm_id, - added_users, - zerver_userprofile) + realm, added_channels, added_mpims, dm_members, \ + added_recipient = channels_to_zerver_stream(slack_data_dir, realm_id, realm, + added_users, zerver_userprofile) + zerver_realmemoji, emoji_url_map = build_realmemoji(custom_emoji_list, realm_id) realm['zerver_realmemoji'] = zerver_realmemoji @@ -80,16 +80,6 @@ def slack_workspace_to_realm(domain_name: str, realm_id: int, user_list: List[Ze realm['zerver_customprofilefield'] = zerver_customprofilefield realm['zerver_customprofilefieldvalue'] = zerver_customprofilefield_value - realm['zerver_defaultstream'] = channels_to_zerver_stream_fields[0] - realm['zerver_stream'] = channels_to_zerver_stream_fields[1] - realm['zerver_huddle'] = channels_to_zerver_stream_fields[2] - realm['zerver_subscription'] = channels_to_zerver_stream_fields[6] - realm['zerver_recipient'] = channels_to_zerver_stream_fields[7] - added_channels = channels_to_zerver_stream_fields[3] - added_mpims = channels_to_zerver_stream_fields[4] - dm_members = channels_to_zerver_stream_fields[5] - added_recipient = channels_to_zerver_stream_fields[8] - return realm, added_users, added_recipient, added_channels, added_mpims, \ dm_members, avatars, emoji_url_map @@ -329,27 +319,17 @@ def get_user_timezone(user: ZerverFieldsT) -> str: timezone = _default_timezone return timezone -def channels_to_zerver_stream(slack_data_dir: str, realm_id: int, added_users: AddedUsersT, - zerver_userprofile: List[ZerverFieldsT]) -> Tuple[List[ZerverFieldsT], - List[ZerverFieldsT], - List[ZerverFieldsT], - AddedChannelsT, - AddedMPIMsT, - DMMembersT, - List[ZerverFieldsT], - List[ZerverFieldsT], - AddedRecipientsT]: +def channels_to_zerver_stream(slack_data_dir: str, realm_id: int, + realm: Dict[str, Any], added_users: AddedUsersT, + zerver_userprofile: List[ZerverFieldsT]) \ + -> Tuple[Dict[str, List[ZerverFieldsT]], AddedChannelsT, AddedMPIMsT, DMMembersT, AddedRecipientsT]: """ Returns: - 1. zerver_defaultstream, which is a list of the default streams - 2. zerver_stream, while is a list of all streams - 3. zerver_huddle, while is a list of all huddles - 3. added_channels, which is a dictionary to map from channel name to channel id, zulip stream_id - 4. added_mpims, which is a dictionary to map from MPIM(multiparty IM) name to MPIM id, zulip huddle_id - 5. dm_members, which is a dictionary to map from DM id to tuple of DM participants. - 6. zerver_subscription, which is a list of the subscriptions - 7. zerver_recipient, which is a list of the recipients - 8. added_recipient, which is a dictionary to map from channel name to zulip recipient_id + 1. realm, Converted Realm data + 2. added_channels, which is a dictionary to map from channel name to channel id, zulip stream_id + 3. added_mpims, which is a dictionary to map from MPIM(multiparty IM) name to MPIM id, zulip huddle_id + 4. dm_members, which is a dictionary to map from DM id to tuple of DM participants. + 5. added_recipient, which is a dictionary to map from channel name to zulip recipient_id """ logging.info('######### IMPORTING CHANNELS STARTED #########\n') @@ -358,11 +338,11 @@ def channels_to_zerver_stream(slack_data_dir: str, realm_id: int, added_users: A dm_members = {} added_recipient = {} - zerver_stream = [] - zerver_huddle = [] - zerver_subscription = [] # type: List[ZerverFieldsT] - zerver_recipient = [] - zerver_defaultstream = [] + realm["zerver_stream"] = [] + realm["zerver_huddle"] = [] + realm["zerver_subscription"] = [] + realm["zerver_recipient"] = [] + realm["zerver_defaultstream"] = [] subscription_id_count = recipient_id_count = 0 stream_id_count = defaultstream_id = 0 @@ -384,10 +364,10 @@ def channels_to_zerver_stream(slack_data_dir: str, realm_id: int, added_users: A stream_id = stream_id_count recipient_id = recipient_id_count - # construct the stream object and append it to zerver_stream + # construct the stream object and append it to realm["zerver_stream"] stream = build_stream(float(channel["created"]), realm_id, channel["name"], description, stream_id, channel["is_archived"], invite_only) - zerver_stream.append(stream) + realm["zerver_stream"].append(stream) # construct defaultstream object # slack has the default channel 'general' and 'random' @@ -396,21 +376,21 @@ def channels_to_zerver_stream(slack_data_dir: str, realm_id: int, added_users: A if channel['name'] in default_channels: defaultstream = build_defaultstream(realm_id, stream_id, defaultstream_id) - zerver_defaultstream.append(defaultstream) + realm["zerver_defaultstream"].append(defaultstream) defaultstream_id += 1 added_channels[stream['name']] = (channel['id'], stream_id) recipient = build_recipient(stream_id, recipient_id, Recipient.STREAM) - zerver_recipient.append(recipient) + realm["zerver_recipient"].append(recipient) added_recipient[stream['name']] = recipient_id # TODO add recipients for private message and huddles - # construct the subscription object and append it to zerver_subscription - subscription_id_count = get_subscription(channel['members'], zerver_subscription, + # construct the subscription object and append it to realm["zerver_subscription"] + subscription_id_count = get_subscription(channel['members'], realm["zerver_subscription"], recipient_id, added_users, subscription_id_count) - # TODO add zerver_subscription which correspond to + # TODO add realm["zerver_subscription"] which correspond to # huddles type recipient # For huddles: # sub['recipient']=recipient['id'] where recipient['type_id']=added_users[member] @@ -449,15 +429,15 @@ def channels_to_zerver_stream(slack_data_dir: str, realm_id: int, added_users: A for mpim in mpims: huddle = build_huddle(huddle_id_count) - zerver_huddle.append(huddle) + realm["zerver_huddle"].append(huddle) added_mpims[mpim['name']] = (mpim['id'], huddle_id_count) recipient = build_recipient(huddle_id_count, recipient_id_count, Recipient.HUDDLE) - zerver_recipient.append(recipient) + realm["zerver_recipient"].append(recipient) added_recipient[mpim['name']] = recipient_id_count - subscription_id_count = get_subscription(mpim['members'], zerver_subscription, + subscription_id_count = get_subscription(mpim['members'], realm["zerver_subscription"], recipient_id_count, added_users, subscription_id_count) @@ -475,8 +455,8 @@ def channels_to_zerver_stream(slack_data_dir: str, realm_id: int, added_users: A recipient = build_recipient(added_users[username], recipient_id_count, Recipient.PERSONAL) added_recipient[username] = recipient_id_count sub = build_subscription(recipient_id_count, added_users[username], subscription_id_count) - zerver_recipient.append(recipient) - zerver_subscription.append(sub) + realm["zerver_recipient"].append(recipient) + realm["zerver_subscription"].append(sub) recipient_id_count += 1 subscription_id_count += 1 @@ -493,8 +473,7 @@ def channels_to_zerver_stream(slack_data_dir: str, realm_id: int, added_users: A process_dms(dms) logging.info('######### IMPORTING STREAMS FINISHED #########\n') - return zerver_defaultstream, zerver_stream, zerver_huddle, added_channels, added_mpims, \ - dm_members, zerver_subscription, zerver_recipient, added_recipient + return realm, added_channels, added_mpims, dm_members, added_recipient def get_subscription(channel_members: List[str], zerver_subscription: List[ZerverFieldsT], recipient_id: int, added_users: AddedUsersT, diff --git a/zerver/tests/test_slack_importer.py b/zerver/tests/test_slack_importer.py index c459b62cdc..87efdb2b7f 100644 --- a/zerver/tests/test_slack_importer.py +++ b/zerver/tests/test_slack_importer.py @@ -51,6 +51,7 @@ import logging import shutil import os import mock +from mock import ANY from typing import Any, Dict, List, Set, Tuple, Iterator def remove_folder(path: str) -> None: @@ -261,18 +262,9 @@ class SlackImporter(ZulipTestCase): zerver_userprofile = [{'id': 1}, {'id': 8}, {'id': 7}, {'id': 5}] realm_id = 3 - channel_to_zerver_stream_output = channels_to_zerver_stream(self.fixture_file_name("", "slack_fixtures"), - realm_id, added_users, zerver_userprofile) - - zerver_defaultstream = channel_to_zerver_stream_output[0] - zerver_stream = channel_to_zerver_stream_output[1] - zerver_huddle = channel_to_zerver_stream_output[2] - added_channels = channel_to_zerver_stream_output[3] - added_mpims = channel_to_zerver_stream_output[4] - dm_members = channel_to_zerver_stream_output[5] - zerver_subscription = channel_to_zerver_stream_output[6] - zerver_recipient = channel_to_zerver_stream_output[7] - added_recipient = channel_to_zerver_stream_output[8] + realm, added_channels, added_mpims, dm_members, \ + added_recipient = channels_to_zerver_stream(self.fixture_file_name("", "slack_fixtures"), realm_id, + {"zerver_userpresence": []}, added_users, zerver_userprofile) test_added_channels = {'feedback': ("C061A0HJG", 3), 'general': ("C061A0YJG", 1), 'general1': ("C061A0YJP", 2), 'random': ("C061A0WJG", 0)} @@ -285,8 +277,8 @@ class SlackImporter(ZulipTestCase): self.assertDictEqual(test_added_channels, added_channels) # zerver defaultstream already tested in helper functions - self.assertEqual(zerver_defaultstream, [{'id': 0, 'realm': 3, 'stream': 0}, - {'id': 1, 'realm': 3, 'stream': 1}]) + self.assertEqual(realm["zerver_defaultstream"], + [{'id': 0, 'realm': 3, 'stream': 0}, {'id': 1, 'realm': 3, 'stream': 1}]) self.assertDictEqual(test_added_mpims, added_mpims) self.assertDictEqual(test_dm_members, dm_members) @@ -299,6 +291,10 @@ class SlackImporter(ZulipTestCase): # functioning of zerver subscriptions are already tested in the helper functions # This is to check the concatenation of the output lists from the helper functions # subscriptions for stream + zerver_subscription = realm["zerver_subscription"] + zerver_recipient = realm["zerver_recipient"] + zerver_stream = realm["zerver_stream"] + self.assertEqual(self.get_set(zerver_subscription, "recipient"), {i for i in range(11)}) self.assertEqual(self.get_set(zerver_subscription, "user_profile"), {1, 5, 7, 8}) @@ -315,12 +311,13 @@ class SlackImporter(ZulipTestCase): self.assertEqual(zerver_stream[2]['id'], test_added_channels[zerver_stream[2]['name']][1]) - self.assertEqual(self.get_set(zerver_huddle, "id"), {0, 1, 2}) + self.assertEqual(self.get_set(realm["zerver_huddle"], "id"), {0, 1, 2}) + self.assertEqual(realm["zerver_userpresence"], []) @mock.patch("zerver.data_import.slack.users_to_zerver_userprofile", return_value=[[], [], {}, [], []]) @mock.patch("zerver.data_import.slack.channels_to_zerver_stream", - return_value=[[], [], [], {}, {}, {}, [], [], {}]) + return_value=[{"zerver_stream": []}, {}, {}, {}, {}]) def test_slack_workspace_to_realm(self, mock_channels_to_zerver_stream: mock.Mock, mock_users_to_zerver_userprofile: mock.Mock) -> None: @@ -337,12 +334,20 @@ class SlackImporter(ZulipTestCase): self.assertEqual(added_recipient, {}) self.assertEqual(avatar_list, []) - zerver_realmdomain = realm['zerver_realmdomain'] + mock_channels_to_zerver_stream.assert_called_once_with("./random_path", 1, ANY, {}, []) + passed_realm = mock_channels_to_zerver_stream.call_args_list[0][0][2] + zerver_realmdomain = passed_realm['zerver_realmdomain'] self.assertListEqual(zerver_realmdomain, test_zerver_realmdomain) - self.assertEqual(realm['zerver_userpresence'], []) + self.assertEqual(passed_realm['zerver_realm'][0]['description'], 'Organization imported from Slack!') + self.assertEqual(passed_realm['zerver_userpresence'], []) + self.assertEqual(len(passed_realm.keys()), 14) + self.assertEqual(realm['zerver_stream'], []) self.assertEqual(realm['zerver_userprofile'], []) - self.assertEqual(realm['zerver_realm'][0]['description'], 'Organization imported from Slack!') + self.assertEqual(realm['zerver_realmemoji'], []) + self.assertEqual(realm['zerver_customprofilefield'], []) + self.assertEqual(realm['zerver_customprofilefieldvalue'], []) + self.assertEqual(len(realm.keys()), 5) def test_get_message_sending_user(self) -> None: message_with_file = {'subtype': 'file', 'type': 'message',