emoji.py: Refactor emoji data consistency checking functions.

Inorder to provide more explicit error messages I have merged the
`emoji_code_is_valid()` and `emoji_name_is_valid()` functions into
`check_emoji_code_consistency()` and `check_emoji_name_consistency()`
respectively.

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:
+def check_emoji_name_consistency(emoji_name: str, emoji_code: str, emoji_type: str) -> None:
-    # For a given realm and emoji type, determines whether an emoji
+    # Given a realm, emoji code and emoji type, checks whether the
-    # 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
     # 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
+        if name_to_codepoint.get(emoji_name) == emoji_code:
-    raise AssertionError("Emoji type should have been checked previously")
+            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:
+    raise JsonableError(_("Invalid emoji name."))
-    if not emoji_name_is_valid(emoji_name, emoji_code, emoji_type):
-        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):
+    # For a given realm and emoji type, checks whether an emoji
-        raise JsonableError(_("Emoji for this emoji code not found."))
+    # 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

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: