diff --git a/zerver/actions/reactions.py b/zerver/actions/reactions.py index 98b8e0fd30..e8b378966c 100644 --- a/zerver/actions/reactions.py +++ b/zerver/actions/reactions.py @@ -3,7 +3,7 @@ from typing import Any, Dict, Optional from django.utils.translation import gettext as _ from zerver.actions.create_user import create_historical_user_messages -from zerver.lib.emoji import check_emoji_request, emoji_name_to_emoji_code +from zerver.lib.emoji import check_emoji_request, get_emoji_data from zerver.lib.exceptions import JsonableError from zerver.lib.message import access_message, update_to_dict_cache from zerver.lib.stream_subscription import subscriber_ids_with_stream_history_access @@ -96,15 +96,18 @@ def check_add_reaction( ) -> None: message, user_message = access_message(user_profile, message_id, lock_message=True) - if emoji_code is None: - # The emoji_code argument is only required for rare corner - # cases discussed in the long block comment below. For simple - # API clients, we allow specifying just the name, and just - # look up the code using the current name->code mapping. - emoji_code = emoji_name_to_emoji_code(message.sender.realm, emoji_name)[0] + if emoji_code is None or reaction_type is None: + emoji_data = get_emoji_data(message.sender.realm_id, emoji_name) - if reaction_type is None: - reaction_type = emoji_name_to_emoji_code(message.sender.realm, emoji_name)[1] + if emoji_code is None: + # The emoji_code argument is only required for rare corner + # cases discussed in the long block comment below. For simple + # API clients, we allow specifying just the name, and just + # look up the code using the current name->code mapping. + emoji_code = emoji_data.emoji_code + + if reaction_type is None: + reaction_type = emoji_data.reaction_type if Reaction.objects.filter( user_profile=user_profile, diff --git a/zerver/data_import/mattermost.py b/zerver/data_import/mattermost.py index 6eb16e4b99..6fc03aa2f2 100644 --- a/zerver/data_import/mattermost.py +++ b/zerver/data_import/mattermost.py @@ -268,7 +268,7 @@ def build_reactions( realmemoji[realm_emoji["name"]] = realm_emoji["id"] # For the Unicode emoji codes, we use equivalent of - # function 'emoji_name_to_emoji_code' in 'zerver/lib/emoji' here + # function 'get_emoji_data' in 'zerver/lib/emoji' here for mattermost_reaction in reactions: emoji_name = mattermost_reaction["emoji_name"] username = mattermost_reaction["user"] diff --git a/zerver/data_import/slack.py b/zerver/data_import/slack.py index 83bb14b315..03dfeb2c94 100644 --- a/zerver/data_import/slack.py +++ b/zerver/data_import/slack.py @@ -1163,7 +1163,7 @@ def build_reactions( reactions = [{"name": k, "users": v, "count": len(v)} for k, v in merged_reactions.items()] # For the Unicode emoji codes, we use equivalent of - # function 'emoji_name_to_emoji_code' in 'zerver/lib/emoji' here + # function 'get_emoji_data' in 'zerver/lib/emoji' here for slack_reaction in reactions: emoji_name = slack_reaction["name"] if emoji_name in slack_emoji_name_to_codepoint: diff --git a/zerver/lib/emoji.py b/zerver/lib/emoji.py index 56dd59c48f..43be22036c 100644 --- a/zerver/lib/emoji.py +++ b/zerver/lib/emoji.py @@ -1,6 +1,6 @@ import os import re -from typing import Tuple +from dataclasses import dataclass import orjson from django.contrib.staticfiles.storage import staticfiles_storage @@ -62,17 +62,29 @@ def translate_emoticons(text: str) -> str: return translated -def emoji_name_to_emoji_code(realm: Realm, emoji_name: str) -> Tuple[str, str]: - # TODO: Just ask callers for realm_id. Also consider returning a - # tiny dataclass instead of a tuple. - realm_emoji_dict = get_name_keyed_dict_for_active_realm_emoji(realm.id) +@dataclass +class EmojiData: + emoji_code: str + reaction_type: str + + +def get_emoji_data(realm_id: int, emoji_name: str) -> EmojiData: + # Even if emoji_name is either in name_to_codepoint or named "zulip", + # we still need to call get_realm_active_emoji. + realm_emoji_dict = get_name_keyed_dict_for_active_realm_emoji(realm_id) realm_emoji = realm_emoji_dict.get(emoji_name) + if realm_emoji is not None: - return str(realm_emoji["id"]), Reaction.REALM_EMOJI + emoji_code = str(realm_emoji["id"]) + return EmojiData(emoji_code=emoji_code, reaction_type=Reaction.REALM_EMOJI) + if emoji_name == "zulip": - return emoji_name, Reaction.ZULIP_EXTRA_EMOJI + return EmojiData(emoji_code=emoji_name, reaction_type=Reaction.ZULIP_EXTRA_EMOJI) + if emoji_name in name_to_codepoint: - return name_to_codepoint[emoji_name], Reaction.UNICODE_EMOJI + emoji_code = name_to_codepoint[emoji_name] + return EmojiData(emoji_code=emoji_code, reaction_type=Reaction.UNICODE_EMOJI) + raise JsonableError(_("Emoji '{}' does not exist").format(emoji_name)) diff --git a/zerver/lib/onboarding.py b/zerver/lib/onboarding.py index 3bd5c519fc..bd6d807760 100644 --- a/zerver/lib/onboarding.py +++ b/zerver/lib/onboarding.py @@ -13,7 +13,7 @@ from zerver.actions.message_send import ( internal_send_private_message, ) from zerver.actions.reactions import do_add_reaction -from zerver.lib.emoji import emoji_name_to_emoji_code +from zerver.lib.emoji import get_emoji_data from zerver.lib.message import SendMessageRequest from zerver.models import Message, Realm, UserProfile, get_system_bot @@ -355,5 +355,7 @@ def send_initial_realm_messages(realm: Realm) -> None: turtle_message = Message.objects.select_for_update().get( id__in=message_ids, content__icontains="cute/turtle.png" ) - (emoji_code, reaction_type) = emoji_name_to_emoji_code(realm, "turtle") - do_add_reaction(welcome_bot, turtle_message, "turtle", emoji_code, reaction_type) + emoji_data = get_emoji_data(realm.id, "turtle") + do_add_reaction( + welcome_bot, turtle_message, "turtle", emoji_data.emoji_code, emoji_data.reaction_type + ) diff --git a/zerver/tests/test_reactions.py b/zerver/tests/test_reactions.py index 42ff022837..3c3c635540 100644 --- a/zerver/tests/test_reactions.py +++ b/zerver/tests/test_reactions.py @@ -6,7 +6,7 @@ import orjson from zerver.actions.reactions import notify_reaction_update from zerver.actions.streams import do_change_stream_permission from zerver.lib.cache import cache_get, to_dict_cache_key_id -from zerver.lib.emoji import emoji_name_to_emoji_code +from zerver.lib.emoji import get_emoji_data from zerver.lib.exceptions import JsonableError from zerver.lib.message import extract_message_dict from zerver.lib.test_classes import ZulipTestCase @@ -189,59 +189,49 @@ class ReactionEmojiTest(ZulipTestCase): result = self.api_post(sender, "/api/v1/messages/1/reactions", reaction_info) self.assert_json_success(result) - def test_emoji_name_to_emoji_code(self) -> None: - """ - An emoji name is mapped canonically to emoji code. - """ + def test_get_emoji_data(self) -> None: realm = get_realm("zulip") realm_emoji = RealmEmoji.objects.get(name="green_tick") + def verify(emoji_name: str, emoji_code: str, reaction_type: str) -> None: + emoji_data = get_emoji_data(realm.id, emoji_name) + self.assertEqual(emoji_data.emoji_code, emoji_code) + self.assertEqual(emoji_data.reaction_type, reaction_type) + # Test active realm emoji. - emoji_code, reaction_type = emoji_name_to_emoji_code(realm, "green_tick") - self.assertEqual(emoji_code, str(realm_emoji.id)) - self.assertEqual(reaction_type, "realm_emoji") + verify("green_tick", str(realm_emoji.id), "realm_emoji") # Test deactivated realm emoji. realm_emoji.deactivated = True realm_emoji.save(update_fields=["deactivated"]) with self.assertRaises(JsonableError) as exc: - emoji_name_to_emoji_code(realm, "green_tick") + get_emoji_data(realm.id, "green_tick") self.assertEqual(str(exc.exception), "Emoji 'green_tick' does not exist") # Test ':zulip:' emoji. - emoji_code, reaction_type = emoji_name_to_emoji_code(realm, "zulip") - self.assertEqual(emoji_code, "zulip") - self.assertEqual(reaction_type, "zulip_extra_emoji") + verify("zulip", "zulip", "zulip_extra_emoji") # Test Unicode emoji. - emoji_code, reaction_type = emoji_name_to_emoji_code(realm, "astonished") - self.assertEqual(emoji_code, "1f632") - self.assertEqual(reaction_type, "unicode_emoji") + verify("astonished", "1f632", "unicode_emoji") # Test override Unicode emoji. overriding_emoji = RealmEmoji.objects.create( name="astonished", realm=realm, file_name="astonished" ) - emoji_code, reaction_type = emoji_name_to_emoji_code(realm, "astonished") - self.assertEqual(emoji_code, str(overriding_emoji.id)) - self.assertEqual(reaction_type, "realm_emoji") + verify("astonished", str(overriding_emoji.id), "realm_emoji") # Test deactivate over-ridding realm emoji. overriding_emoji.deactivated = True overriding_emoji.save(update_fields=["deactivated"]) - emoji_code, reaction_type = emoji_name_to_emoji_code(realm, "astonished") - self.assertEqual(emoji_code, "1f632") - self.assertEqual(reaction_type, "unicode_emoji") + verify("astonished", "1f632", "unicode_emoji") # Test override `:zulip:` emoji. overriding_emoji = RealmEmoji.objects.create(name="zulip", realm=realm, file_name="zulip") - emoji_code, reaction_type = emoji_name_to_emoji_code(realm, "zulip") - self.assertEqual(emoji_code, str(overriding_emoji.id)) - self.assertEqual(reaction_type, "realm_emoji") + verify("zulip", str(overriding_emoji.id), "realm_emoji") # Test non-existent emoji. with self.assertRaises(JsonableError) as exc: - emoji_name_to_emoji_code(realm, "invalid_emoji") + get_emoji_data(realm.id, "invalid_emoji") self.assertEqual(str(exc.exception), "Emoji 'invalid_emoji' does not exist") @@ -376,13 +366,12 @@ class ReactionTest(ZulipTestCase): Removes an old existing reaction but the name of emoji got changed during various emoji infra changes. """ - realm = get_realm("zulip") sender = self.example_user("hamlet") - emoji_code, reaction_type = emoji_name_to_emoji_code(realm, "smile") + emoji_data = get_emoji_data(sender.realm_id, "smile") reaction_info = { "emoji_name": "smile", - "emoji_code": emoji_code, - "reaction_type": reaction_type, + "emoji_code": emoji_data.emoji_code, + "reaction_type": emoji_data.reaction_type, } result = self.api_post(sender, "/api/v1/messages/1/reactions", reaction_info) diff --git a/zerver/views/presence.py b/zerver/views/presence.py index ec2cd8e218..0773ba0c8b 100644 --- a/zerver/views/presence.py +++ b/zerver/views/presence.py @@ -9,7 +9,7 @@ from django.utils.translation import gettext as _ from zerver.actions.presence import update_user_presence from zerver.actions.user_status import do_update_user_status from zerver.decorator import human_users_only -from zerver.lib.emoji import check_emoji_request, emoji_name_to_emoji_code +from zerver.lib.emoji import check_emoji_request, get_emoji_data from zerver.lib.exceptions import JsonableError from zerver.lib.presence import get_presence_for_user, get_presence_response from zerver.lib.request import REQ, RequestNotes, has_request_variables @@ -91,15 +91,17 @@ def update_user_status_backend( emoji_type = UserStatus.UNICODE_EMOJI elif emoji_name is not None: - if emoji_code is None: - # The emoji_code argument is only required for rare corner - # cases discussed in the long block comment below. For simple - # API clients, we allow specifying just the name, and just - # look up the code using the current name->code mapping. - emoji_code = emoji_name_to_emoji_code(user_profile.realm, emoji_name)[0] + if emoji_code is None or emoji_type is None: + emoji_data = get_emoji_data(user_profile.realm_id, emoji_name) + if emoji_code is None: + # The emoji_code argument is only required for rare corner + # cases discussed in the long block comment below. For simple + # API clients, we allow specifying just the name, and just + # look up the code using the current name->code mapping. + emoji_code = emoji_data.emoji_code - if emoji_type is None: - emoji_type = emoji_name_to_emoji_code(user_profile.realm, emoji_name)[1] + if emoji_type is None: + emoji_type = emoji_data.reaction_type elif emoji_type or emoji_code: raise JsonableError( diff --git a/zerver/views/reactions.py b/zerver/views/reactions.py index 6d93e2ffa5..1df333c8bd 100644 --- a/zerver/views/reactions.py +++ b/zerver/views/reactions.py @@ -5,7 +5,7 @@ from django.http import HttpRequest, HttpResponse from django.utils.translation import gettext as _ from zerver.actions.reactions import check_add_reaction, do_remove_reaction -from zerver.lib.emoji import emoji_name_to_emoji_code +from zerver.lib.emoji import get_emoji_data from zerver.lib.exceptions import JsonableError from zerver.lib.message import access_message from zerver.lib.request import REQ, has_request_variables @@ -57,7 +57,7 @@ def remove_reaction( # without needing the mapping between emoji names and codes, # we allow instead passing the emoji_name and looking up the # corresponding code using the current data. - emoji_code = emoji_name_to_emoji_code(message.sender.realm, emoji_name)[0] + emoji_code = get_emoji_data(message.sender.realm_id, emoji_name).emoji_code if not Reaction.objects.filter( user_profile=user_profile, diff --git a/zilencer/management/commands/add_mock_conversation.py b/zilencer/management/commands/add_mock_conversation.py index 6d5492c37c..5f681912b3 100644 --- a/zilencer/management/commands/add_mock_conversation.py +++ b/zilencer/management/commands/add_mock_conversation.py @@ -7,7 +7,7 @@ from zerver.actions.message_send import do_send_messages, internal_prep_stream_m from zerver.actions.reactions import do_add_reaction from zerver.actions.streams import bulk_add_subscriptions from zerver.actions.user_settings import do_change_avatar_fields -from zerver.lib.emoji import emoji_name_to_emoji_code +from zerver.lib.emoji import get_emoji_data from zerver.lib.streams import ensure_stream from zerver.lib.upload import upload_avatar_image from zerver.models import Message, UserProfile, get_realm @@ -122,8 +122,8 @@ From image editing program: preview_message = Message.objects.get( id__in=message_ids, content__icontains="image previews" ) - (emoji_code, reaction_type) = emoji_name_to_emoji_code(realm, "whale") - do_add_reaction(starr, preview_message, "whale", emoji_code, reaction_type) + whale = get_emoji_data(realm.id, "whale") + do_add_reaction(starr, preview_message, "whale", whale.emoji_code, whale.reaction_type) twitter_message = Message.objects.get(id__in=message_ids, content__icontains="gvanrossum") # Setting up a twitter integration in dev is a decent amount of work. If you need @@ -142,8 +142,10 @@ From image editing program: # Put a short pause between the whale reaction and this, so that the # thumbs_up shows up second - (emoji_code, reaction_type) = emoji_name_to_emoji_code(realm, "thumbs_up") - do_add_reaction(starr, preview_message, "thumbs_up", emoji_code, reaction_type) + thumbs_up = get_emoji_data(realm.id, "thumbs_up") + do_add_reaction( + starr, preview_message, "thumbs_up", thumbs_up.emoji_code, thumbs_up.reaction_type + ) def handle(self, *args: Any, **options: str) -> None: self.add_message_formatting_conversation()