diff --git a/zerver/lib/emoji.py b/zerver/lib/emoji.py index 415ce87501..8a708336a8 100644 --- a/zerver/lib/emoji.py +++ b/zerver/lib/emoji.py @@ -35,44 +35,43 @@ def check_valid_emoji(realm, emoji_name): # type: (Realm, Text) -> None emoji_name_to_emoji_code(realm, emoji_name) -def emoji_code_is_valid(realm: Realm, emoji_code: str, emoji_type: str) -> bool: - # For a given realm and emoji type, determines whether an emoji - # code is valid for new reactions, or not. - if emoji_type == "realm_emoji": - realm_emojis = realm.get_emoji() - if emoji_code not in realm_emojis: - return False - if realm_emojis[emoji_code]["deactivated"]: - return False - return True - elif emoji_type == "zulip_extra_emoji": - return emoji_code == "zulip" - elif emoji_type == "unicode_emoji": - return emoji_code in codepoint_to_name - - # The above are the only valid emoji types - return False - -def emoji_name_is_valid(emoji_name: str, emoji_code: str, emoji_type: str) -> bool: - # Given a realm, emoji code and emoji type, determines whether the +def check_emoji_name_consistency(emoji_name: str, emoji_code: str, emoji_type: str) -> None: + # Given a realm, emoji code and emoji type, checks whether the # passed emoji name is a valid name for that emoji. It is assumed # here that the emoji code has been checked for validity before # calling this function. if emoji_type == "realm_emoji": - return emoji_code == emoji_name + if emoji_code == emoji_name: + return elif emoji_type == "zulip_extra_emoji": - return emoji_name == "zulip" + if emoji_name in ["zulip"]: + return elif emoji_type == "unicode_emoji": - return name_to_codepoint.get(emoji_name) == emoji_code - raise AssertionError("Emoji type should have been checked previously") + if name_to_codepoint.get(emoji_name) == emoji_code: + return + else: + raise AssertionError("Emoji type should have been checked previously.") -def check_emoji_name_consistency(emoji_name: str, emoji_code: str, emoji_type: str) -> None: - if not emoji_name_is_valid(emoji_name, emoji_code, emoji_type): - raise JsonableError(_("Invalid emoji name.")) + raise JsonableError(_("Invalid emoji name.")) def check_emoji_code_consistency(realm: Realm, emoji_code: str, emoji_type: str) -> None: - if not emoji_code_is_valid(realm, emoji_code, emoji_type): - raise JsonableError(_("Emoji for this emoji code not found.")) + # For a given realm and emoji type, checks whether an emoji + # code is valid for new reactions, or not. + if emoji_type == "realm_emoji": + realm_emojis = realm.get_emoji() + if emoji_code not in realm_emojis: + raise JsonableError(_("No such realm emoji found.")) + if realm_emojis[emoji_code]["deactivated"]: + raise JsonableError(_("This realm emoji has been deactivated.")) + elif emoji_type == "zulip_extra_emoji": + if emoji_code not in ["zulip"]: + raise JsonableError(_("No such extra emoji found.")) + elif emoji_type == "unicode_emoji": + if emoji_code not in codepoint_to_name: + raise JsonableError(_("No unicode emoji with this emoji code found.")) + else: + # The above are the only valid emoji types + raise JsonableError(_("Invalid emoji type.")) def check_emoji_admin(user_profile, emoji_name=None): # type: (UserProfile, Optional[Text]) -> None diff --git a/zerver/tests/test_reactions.py b/zerver/tests/test_reactions.py index cbe9f0c21e..1207271a3e 100644 --- a/zerver/tests/test_reactions.py +++ b/zerver/tests/test_reactions.py @@ -428,7 +428,7 @@ class DefaultEmojiReactionTests(EmojiReactionBase): 'emoji_code': 'TBD', } result = self.post_reaction(reaction_info) - self.assert_json_error(result, 'Emoji for this emoji code not found.') + self.assert_json_error(result, 'No unicode emoji with this emoji code found.') def test_add_default_emoji_invalid_name(self) -> None: reaction_info = { @@ -567,6 +567,15 @@ class ZulipExtraEmojiReactionTest(EmojiReactionBase): result = self.post_zulip_reaction() self.assert_json_error(result, 'Reaction already exists.') + def test_add_invalid_extra_emoji(self) -> None: + reaction_info = { + 'emoji_name': 'extra_emoji', + 'emoji_code': 'extra_emoji', + 'reaction_type': 'zulip_extra_emoji', + } + result = self.post_reaction(reaction_info) + self.assert_json_error(result, 'No such extra emoji found.') + def test_delete_zulip_emoji(self) -> None: result = self.post_zulip_reaction() self.assert_json_success(result) @@ -593,7 +602,7 @@ class RealmEmojiReactionTests(EmojiReactionBase): 'emoji_code': 'non_existent', } result = self.post_reaction(reaction_info) - self.assert_json_error(result, 'Emoji for this emoji code not found.') + self.assert_json_error(result, 'No such realm emoji found.') def test_add_deactivated_realm_emoji(self) -> None: emoji = RealmEmoji.objects.get(name="green_tick") @@ -605,7 +614,7 @@ class RealmEmojiReactionTests(EmojiReactionBase): 'emoji_code': 'green_tick', } result = self.post_reaction(reaction_info) - self.assert_json_error(result, 'Emoji for this emoji code not found.') + self.assert_json_error(result, 'This realm emoji has been deactivated.') def test_add_to_existing_deactivated_realm_emoji_reaction(self) -> None: reaction_info = { @@ -670,7 +679,7 @@ class RealmEmojiReactionTests(EmojiReactionBase): result = self.client_post('/api/v1/messages/%s/reactions' % (message_id,), reaction_info, **self.api_auth(sender)) - self.assert_json_error(result, "Emoji for this emoji code not found.") + self.assert_json_error(result, "Invalid emoji type.") class ReactionAPIEventTest(EmojiReactionBase): def test_add_event(self) -> None: