diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 62e6e24593..9e4d12ee0d 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -7908,7 +7908,7 @@ def notify_realm_emoji(realm: Realm) -> None: def check_add_realm_emoji( realm: Realm, name: str, author: UserProfile, image_file: IO[bytes] -) -> Optional[RealmEmoji]: +) -> RealmEmoji: try: realm_emoji = RealmEmoji(realm=realm, name=name, author=author) realm_emoji.full_clean() @@ -7931,12 +7931,10 @@ def check_add_realm_emoji( finally: if not emoji_uploaded_successfully: realm_emoji.delete() - return None - else: - realm_emoji.file_name = emoji_file_name - realm_emoji.is_animated = is_animated - realm_emoji.save(update_fields=["file_name", "is_animated"]) - notify_realm_emoji(realm_emoji.realm) + realm_emoji.file_name = emoji_file_name + realm_emoji.is_animated = is_animated + realm_emoji.save(update_fields=["file_name", "is_animated"]) + notify_realm_emoji(realm_emoji.realm) return realm_emoji diff --git a/zerver/tests/test_realm_emoji.py b/zerver/tests/test_realm_emoji.py index a56a5e8c6a..2f9e5b4681 100644 --- a/zerver/tests/test_realm_emoji.py +++ b/zerver/tests/test_realm_emoji.py @@ -10,6 +10,7 @@ from zerver.lib.actions import ( from zerver.lib.exceptions import JsonableError from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import get_test_image_file +from zerver.lib.upload import BadImageError from zerver.models import Realm, RealmEmoji, UserProfile, get_realm @@ -320,11 +321,13 @@ class RealmEmojiTest(ZulipTestCase): def test_failed_file_upload(self) -> None: self.login("iago") - with mock.patch("zerver.lib.upload.write_local_file", side_effect=Exception()): + with mock.patch( + "zerver.lib.upload.write_local_file", side_effect=BadImageError(msg="Broken") + ): with get_test_image_file("img.png") as fp1: emoji_data = {"f1": fp1} result = self.client_post("/json/realm/emoji/my_emoji", info=emoji_data) - self.assert_json_error(result, "Image file upload failed.") + self.assert_json_error(result, "Broken") def test_check_admin_realm_emoji(self) -> None: # Test that an user A is able to remove a realm emoji uploaded by him diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py index 1ee21ee14f..ce5fa77101 100644 --- a/zerver/tests/test_upload.py +++ b/zerver/tests/test_upload.py @@ -2288,17 +2288,17 @@ class UploadSpaceTests(UploadSerializeMixin, ZulipTestCase): class DecompressionBombTests(ZulipTestCase): def setUp(self) -> None: super().setUp() - self.test_urls = { - "/json/users/me/avatar": "Image size exceeds limit.", - "/json/realm/logo": "Image size exceeds limit.", - "/json/realm/icon": "Image size exceeds limit.", - "/json/realm/emoji/bomb_emoji": "Image file upload failed.", - } + self.test_urls = [ + "/json/users/me/avatar", + "/json/realm/logo", + "/json/realm/icon", + "/json/realm/emoji/bomb_emoji", + ] def test_decompression_bomb(self) -> None: self.login("iago") with get_test_image_file("bomb.png") as fp: - for url, error_string in self.test_urls.items(): + for url in self.test_urls: fp.seek(0, 0) if url == "/json/realm/logo": result = self.client_post( @@ -2306,4 +2306,4 @@ class DecompressionBombTests(ZulipTestCase): ) else: result = self.client_post(url, {"f1": fp}) - self.assert_json_error(result, error_string) + self.assert_json_error(result, "Image size exceeds limit.") diff --git a/zerver/views/realm_emoji.py b/zerver/views/realm_emoji.py index 29e03c2d69..edde4d70a4 100644 --- a/zerver/views/realm_emoji.py +++ b/zerver/views/realm_emoji.py @@ -47,9 +47,7 @@ def upload_emoji( ) ) - realm_emoji = check_add_realm_emoji(user_profile.realm, emoji_name, user_profile, emoji_file) - if realm_emoji is None: - raise JsonableError(_("Image file upload failed.")) + check_add_realm_emoji(user_profile.realm, emoji_name, user_profile, emoji_file) return json_success(request)