diff --git a/zerver/data_import/import_util.py b/zerver/data_import/import_util.py index 630ba4fdc2..d73116e8eb 100644 --- a/zerver/data_import/import_util.py +++ b/zerver/data_import/import_util.py @@ -560,7 +560,11 @@ def get_avatar(avatar_dir: str, size_url_suffix: str, avatar_upload_item: list[s image_path = os.path.join(avatar_dir, avatar_upload_item[1]) original_image_path = os.path.join(avatar_dir, avatar_upload_item[2]) - response = requests.get(avatar_url + size_url_suffix, stream=True) + if avatar_url.startswith("https://ca.slack-edge.com/"): + # Adjust the avatar size for a typical Slack user. + avatar_url += size_url_suffix + + response = requests.get(avatar_url, stream=True) with open(image_path, "wb") as image_file: shutil.copyfileobj(response.raw, image_file) shutil.copy(image_path, original_image_path) diff --git a/zerver/data_import/slack.py b/zerver/data_import/slack.py index 27e0a56f21..b2d4c01b59 100644 --- a/zerver/data_import/slack.py +++ b/zerver/data_import/slack.py @@ -301,10 +301,9 @@ def users_to_zerver_userprofile( found_emails[email.lower()] = user_id # ref: https://zulip.com/help/change-your-profile-picture - avatar_url = build_avatar_url( - slack_user_id, user["team_id"], user["profile"]["avatar_hash"] - ) - build_avatar(user_id, realm_id, email, avatar_url, timestamp, avatar_list) + avatar_source, avatar_url = build_avatar_url(slack_user_id, user) + if avatar_source == UserProfile.AVATAR_FROM_USER: + build_avatar(user_id, realm_id, email, avatar_url, timestamp, avatar_list) role = UserProfile.ROLE_MEMBER if get_owner(user): role = UserProfile.ROLE_REALM_OWNER @@ -340,7 +339,7 @@ def users_to_zerver_userprofile( id=user_id, email=email, delivery_email=email, - avatar_source="U", + avatar_source=avatar_source, is_bot=user.get("is_bot", False), role=role, bot_type=1 if user.get("is_bot", False) else None, @@ -464,9 +463,18 @@ def get_user_email(user: ZerverFieldsT, domain_name: str) -> str: raise AssertionError(f"Could not find email address for Slack user {user}") -def build_avatar_url(slack_user_id: str, team_id: str, avatar_hash: str) -> str: - avatar_url = f"https://ca.slack-edge.com/{team_id}-{slack_user_id}-{avatar_hash}" - return avatar_url +def build_avatar_url(slack_user_id: str, user: ZerverFieldsT) -> tuple[str, str]: + avatar_url: str = "" + avatar_source = UserProfile.AVATAR_FROM_GRAVATAR + if user["profile"].get("avatar_hash"): + # Process avatar image for a typical Slack user. + team_id = user["team_id"] + avatar_hash = user["profile"]["avatar_hash"] + avatar_url = f"https://ca.slack-edge.com/{team_id}-{slack_user_id}-{avatar_hash}" + avatar_source = UserProfile.AVATAR_FROM_USER + else: + logging.info("Failed to process avatar for user -> %s\n", user.get("name")) + return avatar_source, avatar_url def get_owner(user: ZerverFieldsT) -> bool: diff --git a/zerver/tests/test_slack_importer.py b/zerver/tests/test_slack_importer.py index 21c5c454f1..072da474b3 100644 --- a/zerver/tests/test_slack_importer.py +++ b/zerver/tests/test_slack_importer.py @@ -514,6 +514,24 @@ class SlackImporter(ZulipTestCase): "image_32": "https://secure.gravatar.com/avatar/random7.png", }, }, + # Unknown user data format. + { + "id": "U1ZYFEC91", + "team_id": "T5YFFM2QZ", + "name": "daniel.who", + "real_name": "Daniel Who", + "is_admin": True, + "is_owner": False, + "is_primary_owner": False, + "is_restricted": False, + "is_ultra_restricted": False, + "is_bot": False, + "is_mirror_dummy": False, + "profile": { + "email": "daniel.who@gmail.com", + }, + # Missing the `avatar_hash` value. + }, ] mock_get_data_file.return_value = user_data @@ -528,6 +546,7 @@ class SlackImporter(ZulipTestCase): "U8X25EBAB": 5, "U015J7JSE": 6, "U1RDFEC80": 7, + "U1ZYFEC91": 8, } slack_data_dir = "./random_path" timestamp = int(timezone_now().timestamp()) @@ -563,7 +582,7 @@ class SlackImporter(ZulipTestCase): self.assertDictEqual(slack_user_id_to_zulip_user_id, test_slack_user_id_to_zulip_user_id) self.assert_length(avatar_list, 8) - self.assert_length(zerver_userprofile, 8) + self.assert_length(zerver_userprofile, 9) self.assertEqual(zerver_userprofile[0]["is_staff"], False) self.assertEqual(zerver_userprofile[0]["is_bot"], False) @@ -645,6 +664,14 @@ class SlackImporter(ZulipTestCase): expected_error_message = f"['Invalid email format, please fix the following email(s) and try again: {bad_email1}, {bad_email2}']" self.assertEqual(error_message, expected_error_message) + # Test converting unknown user data format that doesn't have + # an `avatar_hash` in its user profile. + self.assertEqual( + zerver_userprofile[8]["id"], test_slack_user_id_to_zulip_user_id["U1ZYFEC91"] + ) + self.assertEqual(zerver_userprofile[8]["is_active"], True) + self.assertEqual(zerver_userprofile[8]["avatar_source"], "G") + def test_build_defaultstream(self) -> None: realm_id = 1 stream_id = 1 @@ -1289,7 +1316,7 @@ class SlackImporter(ZulipTestCase): @mock.patch("zerver.data_import.slack.requests.get") @mock.patch("zerver.data_import.slack.process_uploads", return_value=[]) @mock.patch("zerver.data_import.slack.build_attachment", return_value=[]) - @mock.patch("zerver.data_import.slack.build_avatar_url") + @mock.patch("zerver.data_import.slack.build_avatar_url", return_value=("", "")) @mock.patch("zerver.data_import.slack.build_avatar") @mock.patch("zerver.data_import.slack.get_slack_api_data") @mock.patch("zerver.data_import.slack.check_token_access") @@ -1497,7 +1524,7 @@ class SlackImporter(ZulipTestCase): @mock.patch("zerver.data_import.slack.requests.get") @mock.patch("zerver.data_import.slack.process_uploads", return_value=[]) @mock.patch("zerver.data_import.slack.build_attachment", return_value=[]) - @mock.patch("zerver.data_import.slack.build_avatar_url") + @mock.patch("zerver.data_import.slack.build_avatar_url", return_value=("", "")) @mock.patch("zerver.data_import.slack.build_avatar") @mock.patch("zerver.data_import.slack.get_slack_api_data") @mock.patch("zerver.data_import.slack.check_token_access")