avatar: Ensure system bots' avatar URLs follow convention.

Previously, requesting system bots URLs did not return any -medium.png
variants and SVG file was also used for notification bots' avatar, which
was problematic.

In this commit, the -medium.png variants is added for the avatars of
system bots and zulip-icon-square.svg is also converted into
notification-bot.png for the notification bot. The get_avatar_url method
has been updated to return the "medium" file variants for the system
bots.

Additionally, the system bots' avatar files is moved to a dedicated
directory to simplify the hashing logic for these files. Now, all files
in the "images/static_avatars/" directory will be hashed.

zerver/lib/avatar.py

@@ -12,14 +12,11 @@ from zerver.lib.thumbnail import MEDIUM_AVATAR_SIZE
 from zerver.lib.upload import get_avatar_url
 from zerver.lib.url_encoding import append_url_query_string
 from zerver.models import UserProfile
+from zerver.models.users import is_cross_realm_bot_email
 
-SYSTEM_BOTS_AVATAR_FILES = {
-    # This is also used in zerver/lib/storage.py to ensure
-    # these files are hashed when served as static files.
-    settings.WELCOME_BOT: "images/welcome-bot.png",
-    settings.NOTIFICATION_BOT: "images/logo/zulip-icon-square.svg",
-    settings.EMAIL_GATEWAY_BOT: "images/email-gateway-bot.png",
-}
+STATIC_AVATARS_DIR = "images/static_avatars/"
+
+DEFAULT_AVATAR_FILE = "images/default-avatar.png"
 
 
 def avatar_url(
@@ -36,6 +33,22 @@ def avatar_url(
     )
 
 
+def get_system_bots_avatar_file_name(email: str) -> str:
+    system_bot_avatar_name_map = {
+        settings.WELCOME_BOT: "welcome-bot",
+        settings.NOTIFICATION_BOT: "notification-bot",
+        settings.EMAIL_GATEWAY_BOT: "emailgateway",
+    }
+    return urljoin(STATIC_AVATARS_DIR, system_bot_avatar_name_map.get(email, "unknown"))
+
+
+def get_static_avatar_url(email: str, medium: bool) -> str:
+    avatar_file_name = get_system_bots_avatar_file_name(email)
+    avatar_file_name += "-medium.png" if medium else ".png"
+
+    return staticfiles_storage.url(avatar_file_name)
+
+
 def get_avatar_field(
     user_id: int,
     realm_id: int,
@@ -63,9 +76,8 @@ def get_avatar_field(
     """
 
     # System bots have hardcoded avatars
-    system_bot_avatar = SYSTEM_BOTS_AVATAR_FILES.get(email)
-    if system_bot_avatar:
-        return staticfiles_storage.url(system_bot_avatar)
+    if is_cross_realm_bot_email(email):
+        return get_static_avatar_url(email, medium)
 
     """
     If our client knows how to calculate gravatar hashes, we

zerver/lib/storage.py

@@ -10,7 +10,7 @@ from django.core.files.base import File
 from django.core.files.storage import FileSystemStorage
 from typing_extensions import override
 
-from zerver.lib.avatar import SYSTEM_BOTS_AVATAR_FILES
+from zerver.lib.avatar import STATIC_AVATARS_DIR
 
 if settings.DEBUG:
     from django.contrib.staticfiles.finders import find
@@ -40,11 +40,11 @@ class IgnoreBundlesManifestStaticFilesStorage(ManifestStaticFilesStorage):
             # use a no-op hash function for these already-hashed
             # assets.
             return name
-        if name in SYSTEM_BOTS_AVATAR_FILES.values():
+        if name.startswith(STATIC_AVATARS_DIR):
             # For these avatar files, we want to make sure they are
             # so they can hit our Nginx caching block for static files.
-            # We don't need to worry about stale caches since system bot
-            # avatars rarely change.
+            # We don't need to worry about stale caches since these are
+            # only used by the system bots.
             return super().hashed_name(name, content, filename)
         if name == "generated/emoji/emoji_api.json":
             # Unlike most .json files, we do want to hash this file;

zerver/tests/test_upload.py

@@ -1532,6 +1532,29 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase):
             result = self.client_post("/json/users/me/avatar", {"file": fp})
         self.assert_json_error(result, "Uploaded file is larger than the allowed limit of 0 MiB")
 
+    def test_system_bot_avatars_url(self) -> None:
+        self.login("hamlet")
+        system_bot_emails = [
+            settings.NOTIFICATION_BOT,
+            settings.WELCOME_BOT,
+            settings.EMAIL_GATEWAY_BOT,
+        ]
+        internal_realm = get_realm(settings.SYSTEM_BOT_REALM)
+
+        for email in system_bot_emails:
+            system_bot = get_system_bot(email, internal_realm.id)
+            response = self.client_get(f"/avatar/{email}")
+            redirect_url = response["Location"]
+            self.assertEqual(redirect_url, str(avatar_url(system_bot)))
+            self.assertTrue(str(redirect_url).endswith(".png"))
+            self.assertFalse(str(redirect_url).endswith("unknown.png"))
+
+            response = self.client_get(f"/avatar/{email}/medium")
+            redirect_url = response["Location"]
+            self.assertEqual(redirect_url, str(avatar_url(system_bot, medium=True)))
+            self.assertTrue(str(redirect_url).endswith("-medium.png"))
+            self.assertFalse(str(redirect_url).endswith("unknown-medium.png"))
+
 
 class RealmIconTest(UploadSerializeMixin, ZulipTestCase):
     def test_multiple_upload_failure(self) -> None: