slack_data_import: Update how Slack user avatars are processed.

Previously, the Slack export converter can only process Slack's avatar
URL from Slack's "ca.slack-edge.com" server, which looks like this:

https://ca.slack-edge.com/T0CDRA6HM3P-U06NABE26M9-1173e04f818e-512

This commit adds support for converting any public downloadable image
URLs.

This is done to support importing Slack's integration bots and their
messages, which typically have PNG type file url:

https://avatars.slack-edge.com/2024-05-01/7057208497908_a4351f6deb91094eac4c_72.png

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)

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:

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