Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

emojis: Use get_emoji_data. 

The previous function was poorly named, asked for a
Realm object when realm_id sufficed, and returned a
tuple of strings that had different semantics.

I also avoid calling it duplicate times in a couple
places, although it was probably rarely the case that
both invocations actually happened if upstream
validations were working.

Note that there is a TypedDict called EmojiInfo, so I
chose EmojiData here.  Perhaps a better name would be
TinyEmojiData or something.

I also simplify the reaction tests with a verify
helper.
This commit is contained in:
Steve Howell 2023-07-14 12:25:57 +00:00 committed by Tim Abbott
parent b742f1241f
commit 67cdf1a7b4
9 changed files with 77 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
zerver/actions/reactions.py
View File

@ -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,

2
zerver/data_import/mattermost.py
View File

@ -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"]

2
zerver/data_import/slack.py
View File

@ -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:

28
zerver/lib/emoji.py
View File

@ -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))

8
zerver/lib/onboarding.py
View File

@ -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
)

47
zerver/tests/test_reactions.py
View File

@ -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)

20
zerver/views/presence.py
View File

@ -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(

4
zerver/views/reactions.py
View File

@ -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,

12
zilencer/management/commands/add_mock_conversation.py
View File

@ -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()