actions: Split out zerver.actions.reactions.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
(cherry picked from commit e5500a2226)

zerver/actions/reactions.py

@@ -0,0 +1,170 @@
+from typing import Any, Dict, Optional
+
+from django.db import transaction
+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.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
+from zerver.models import Message, Reaction, Recipient, Stream, UserMessage, UserProfile
+from zerver.tornado.django_api import send_event
+
+
+def notify_reaction_update(
+    user_profile: UserProfile, message: Message, reaction: Reaction, op: str
+) -> None:
+    user_dict = {
+        "user_id": user_profile.id,
+        "email": user_profile.email,
+        "full_name": user_profile.full_name,
+    }
+
+    event: Dict[str, Any] = {
+        "type": "reaction",
+        "op": op,
+        "user_id": user_profile.id,
+        # TODO: We plan to remove this redundant user_dict object once
+        # clients are updated to support accessing use user_id.  See
+        # https://github.com/zulip/zulip/pull/14711 for details.
+        "user": user_dict,
+        "message_id": message.id,
+        "emoji_name": reaction.emoji_name,
+        "emoji_code": reaction.emoji_code,
+        "reaction_type": reaction.reaction_type,
+    }
+
+    # Update the cached message since new reaction is added.
+    update_to_dict_cache([message])
+
+    # Recipients for message update events, including reactions, are
+    # everyone who got the original message, plus subscribers of
+    # streams with the access to stream's full history.
+    #
+    # This means reactions won't live-update in preview narrows for a
+    # stream the user isn't yet subscribed to; this is the right
+    # performance tradeoff to avoid sending every reaction to public
+    # stream messages to all users.
+    #
+    # To ensure that reactions do live-update for any user who has
+    # actually participated in reacting to a message, we add a
+    # "historical" UserMessage row for any user who reacts to message,
+    # subscribing them to future notifications, even if they are not
+    # subscribed to the stream.
+    user_ids = set(
+        UserMessage.objects.filter(message=message.id).values_list("user_profile_id", flat=True)
+    )
+    if message.recipient.type == Recipient.STREAM:
+        stream_id = message.recipient.type_id
+        stream = Stream.objects.get(id=stream_id)
+        user_ids |= subscriber_ids_with_stream_history_access(stream)
+
+    transaction.on_commit(lambda: send_event(user_profile.realm, event, list(user_ids)))
+
+
+def do_add_reaction(
+    user_profile: UserProfile,
+    message: Message,
+    emoji_name: str,
+    emoji_code: str,
+    reaction_type: str,
+) -> None:
+    """Should be called while holding a SELECT FOR UPDATE lock
+    (e.g. via access_message(..., lock_message=True)) on the
+    Message row, to prevent race conditions.
+    """
+
+    reaction = Reaction(
+        user_profile=user_profile,
+        message=message,
+        emoji_name=emoji_name,
+        emoji_code=emoji_code,
+        reaction_type=reaction_type,
+    )
+
+    reaction.save()
+
+    notify_reaction_update(user_profile, message, reaction, "add")
+
+
+def check_add_reaction(
+    user_profile: UserProfile,
+    message_id: int,
+    emoji_name: str,
+    emoji_code: Optional[str],
+    reaction_type: Optional[str],
+) -> 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 reaction_type is None:
+        reaction_type = emoji_name_to_emoji_code(message.sender.realm, emoji_name)[1]
+
+    if Reaction.objects.filter(
+        user_profile=user_profile,
+        message=message,
+        emoji_code=emoji_code,
+        reaction_type=reaction_type,
+    ).exists():
+        raise JsonableError(_("Reaction already exists."))
+
+    query = Reaction.objects.filter(
+        message=message, emoji_code=emoji_code, reaction_type=reaction_type
+    )
+    if query.exists():
+        # If another user has already reacted to this message with
+        # same emoji code, we treat the new reaction as a vote for the
+        # existing reaction.  So the emoji name used by that earlier
+        # reaction takes precedence over whatever was passed in this
+        # request.  This is necessary to avoid a message having 2
+        # "different" emoji reactions with the same emoji code (and
+        # thus same image) on the same message, which looks ugly.
+        #
+        # In this "voting for an existing reaction" case, we shouldn't
+        # check whether the emoji code and emoji name match, since
+        # it's possible that the (emoji_type, emoji_name, emoji_code)
+        # triple for this existing reaction may not pass validation
+        # now (e.g. because it is for a realm emoji that has been
+        # since deactivated).  We still want to allow users to add a
+        # vote any old reaction they see in the UI even if that is a
+        # deactivated custom emoji, so we just use the emoji name from
+        # the existing reaction with no further validation.
+        reaction = query.first()
+        assert reaction is not None
+        emoji_name = reaction.emoji_name
+    else:
+        # Otherwise, use the name provided in this request, but verify
+        # it is valid in the user's realm (e.g. not a deactivated
+        # realm emoji).
+        check_emoji_request(user_profile.realm, emoji_name, emoji_code, reaction_type)
+
+    if user_message is None:
+        # See called function for more context.
+        create_historical_user_messages(user_id=user_profile.id, message_ids=[message.id])
+
+    do_add_reaction(user_profile, message, emoji_name, emoji_code, reaction_type)
+
+
+def do_remove_reaction(
+    user_profile: UserProfile, message: Message, emoji_code: str, reaction_type: str
+) -> None:
+    """Should be called while holding a SELECT FOR UPDATE lock
+    (e.g. via access_message(..., lock_message=True)) on the
+    Message row, to prevent race conditions.
+    """
+    reaction = Reaction.objects.filter(
+        user_profile=user_profile,
+        message=message,
+        emoji_code=emoji_code,
+        reaction_type=reaction_type,
+    ).get()
+    reaction.delete()
+
+    notify_reaction_update(user_profile, message, reaction, "remove")

zerver/lib/actions.py

@@ -34,7 +34,6 @@ from zerver.lib.bulk_create import create_users
 from zerver.lib.cache import flush_user_profile
 from zerver.lib.create_user import get_display_email_address
 from zerver.lib.email_validation import email_reserved_for_system_bots_error
-from zerver.lib.emoji import check_emoji_request, emoji_name_to_emoji_code
 from zerver.lib.exceptions import JsonableError
 from zerver.lib.markdown import MessageRenderingResult, topic_links
 from zerver.lib.markdown import version as markdown_version
@@ -55,10 +54,7 @@ from zerver.lib.retention import move_messages_to_archive
 from zerver.lib.send_email import FromAddress, send_email_to_admins
 from zerver.lib.server_initialization import create_internal_realm, server_initialized
 from zerver.lib.sessions import delete_user_sessions
-from zerver.lib.stream_subscription import (
+from zerver.lib.stream_subscription import get_active_subscriptions_for_stream_id
-    get_active_subscriptions_for_stream_id,
-    subscriber_ids_with_stream_history_access,
-)
 from zerver.lib.stream_topic import StreamTopicTarget
 from zerver.lib.streams import (
     access_stream_by_id,
@@ -98,7 +94,6 @@ from zerver.models import (
     RealmAuditLog,
     RealmDomain,
     RealmUserDefault,
-    Recipient,
     ScheduledEmail,
     Stream,
     UserMessage,
@@ -515,164 +510,6 @@ def do_scrub_realm(realm: Realm, *, acting_user: Optional[UserProfile]) -> None:
     )
 
 
-def notify_reaction_update(
-    user_profile: UserProfile, message: Message, reaction: Reaction, op: str
-) -> None:
-    user_dict = {
-        "user_id": user_profile.id,
-        "email": user_profile.email,
-        "full_name": user_profile.full_name,
-    }
-
-    event: Dict[str, Any] = {
-        "type": "reaction",
-        "op": op,
-        "user_id": user_profile.id,
-        # TODO: We plan to remove this redundant user_dict object once
-        # clients are updated to support accessing use user_id.  See
-        # https://github.com/zulip/zulip/pull/14711 for details.
-        "user": user_dict,
-        "message_id": message.id,
-        "emoji_name": reaction.emoji_name,
-        "emoji_code": reaction.emoji_code,
-        "reaction_type": reaction.reaction_type,
-    }
-
-    # Update the cached message since new reaction is added.
-    update_to_dict_cache([message])
-
-    # Recipients for message update events, including reactions, are
-    # everyone who got the original message, plus subscribers of
-    # streams with the access to stream's full history.
-    #
-    # This means reactions won't live-update in preview narrows for a
-    # stream the user isn't yet subscribed to; this is the right
-    # performance tradeoff to avoid sending every reaction to public
-    # stream messages to all users.
-    #
-    # To ensure that reactions do live-update for any user who has
-    # actually participated in reacting to a message, we add a
-    # "historical" UserMessage row for any user who reacts to message,
-    # subscribing them to future notifications, even if they are not
-    # subscribed to the stream.
-    user_ids = set(
-        UserMessage.objects.filter(message=message.id).values_list("user_profile_id", flat=True)
-    )
-    if message.recipient.type == Recipient.STREAM:
-        stream_id = message.recipient.type_id
-        stream = Stream.objects.get(id=stream_id)
-        user_ids |= subscriber_ids_with_stream_history_access(stream)
-
-    transaction.on_commit(lambda: send_event(user_profile.realm, event, list(user_ids)))
-
-
-def do_add_reaction(
-    user_profile: UserProfile,
-    message: Message,
-    emoji_name: str,
-    emoji_code: str,
-    reaction_type: str,
-) -> None:
-    """Should be called while holding a SELECT FOR UPDATE lock
-    (e.g. via access_message(..., lock_message=True)) on the
-    Message row, to prevent race conditions.
-    """
-
-    reaction = Reaction(
-        user_profile=user_profile,
-        message=message,
-        emoji_name=emoji_name,
-        emoji_code=emoji_code,
-        reaction_type=reaction_type,
-    )
-
-    reaction.save()
-
-    notify_reaction_update(user_profile, message, reaction, "add")
-
-
-def check_add_reaction(
-    user_profile: UserProfile,
-    message_id: int,
-    emoji_name: str,
-    emoji_code: Optional[str],
-    reaction_type: Optional[str],
-) -> 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 reaction_type is None:
-        reaction_type = emoji_name_to_emoji_code(message.sender.realm, emoji_name)[1]
-
-    if Reaction.objects.filter(
-        user_profile=user_profile,
-        message=message,
-        emoji_code=emoji_code,
-        reaction_type=reaction_type,
-    ).exists():
-        raise JsonableError(_("Reaction already exists."))
-
-    query = Reaction.objects.filter(
-        message=message, emoji_code=emoji_code, reaction_type=reaction_type
-    )
-    if query.exists():
-        # If another user has already reacted to this message with
-        # same emoji code, we treat the new reaction as a vote for the
-        # existing reaction.  So the emoji name used by that earlier
-        # reaction takes precedence over whatever was passed in this
-        # request.  This is necessary to avoid a message having 2
-        # "different" emoji reactions with the same emoji code (and
-        # thus same image) on the same message, which looks ugly.
-        #
-        # In this "voting for an existing reaction" case, we shouldn't
-        # check whether the emoji code and emoji name match, since
-        # it's possible that the (emoji_type, emoji_name, emoji_code)
-        # triple for this existing reaction may not pass validation
-        # now (e.g. because it is for a realm emoji that has been
-        # since deactivated).  We still want to allow users to add a
-        # vote any old reaction they see in the UI even if that is a
-        # deactivated custom emoji, so we just use the emoji name from
-        # the existing reaction with no further validation.
-        reaction = query.first()
-        assert reaction is not None
-        emoji_name = reaction.emoji_name
-    else:
-        # Otherwise, use the name provided in this request, but verify
-        # it is valid in the user's realm (e.g. not a deactivated
-        # realm emoji).
-        check_emoji_request(user_profile.realm, emoji_name, emoji_code, reaction_type)
-
-    if user_message is None:
-        # See called function for more context.
-        create_historical_user_messages(user_id=user_profile.id, message_ids=[message.id])
-
-    do_add_reaction(user_profile, message, emoji_name, emoji_code, reaction_type)
-
-
-def do_remove_reaction(
-    user_profile: UserProfile, message: Message, emoji_code: str, reaction_type: str
-) -> None:
-    """Should be called while holding a SELECT FOR UPDATE lock
-    (e.g. via access_message(..., lock_message=True)) on the
-    Message row, to prevent race conditions.
-    """
-    reaction = Reaction.objects.filter(
-        user_profile=user_profile,
-        message=message,
-        emoji_code=emoji_code,
-        reaction_type=reaction_type,
-    ).get()
-    reaction.delete()
-
-    notify_reaction_update(user_profile, message, reaction, "remove")
-
-
 def validate_message_edit_payload(
     message: Message,
     stream_id: Optional[int],

zerver/lib/onboarding.py

@@ -10,7 +10,8 @@ from zerver.actions.message_send import (
     internal_prep_stream_message_by_name,
     internal_send_private_message,
 )
-from zerver.lib.actions import do_add_reaction, setup_realm_internal_bots
+from zerver.actions.reactions import do_add_reaction
+from zerver.lib.actions import setup_realm_internal_bots
 from zerver.lib.emoji import emoji_name_to_emoji_code
 from zerver.lib.message import SendMessageRequest
 from zerver.models import Message, Realm, UserProfile, get_system_bot

zerver/openapi/curl_param_value_generators.py

@@ -12,9 +12,9 @@ from django.utils.timezone import now as timezone_now
 
 from zerver.actions.create_user import do_create_user
 from zerver.actions.presence import update_user_presence
+from zerver.actions.reactions import do_add_reaction
 from zerver.actions.realm_linkifiers import do_add_linkifier
 from zerver.actions.realm_playgrounds import do_add_realm_playground
-from zerver.lib.actions import do_add_reaction
 from zerver.lib.events import do_events_register
 from zerver.lib.initial_password import initial_password
 from zerver.lib.test_classes import ZulipTestCase

zerver/tests/test_events.py

@@ -40,6 +40,7 @@ from zerver.actions.invites import (
     do_revoke_user_invite,
 )
 from zerver.actions.presence import do_update_user_presence, do_update_user_status
+from zerver.actions.reactions import do_add_reaction, do_remove_reaction
 from zerver.actions.realm_emoji import check_add_realm_emoji, do_remove_realm_emoji
 from zerver.actions.realm_icon import do_change_icon_source
 from zerver.actions.realm_linkifiers import (
@@ -86,7 +87,6 @@ from zerver.actions.users import (
 )
 from zerver.actions.video_calls import do_set_zoom_token
 from zerver.lib.actions import (
-    do_add_reaction,
     do_add_realm_domain,
     do_change_bot_owner,
     do_change_default_all_public_streams,
@@ -97,7 +97,6 @@ from zerver.lib.actions import (
     do_deactivate_realm,
     do_delete_messages,
     do_mute_user,
-    do_remove_reaction,
     do_remove_realm_domain,
     do_set_realm_authentication_methods,
     do_set_realm_message_editing,

zerver/tests/test_import_export.py

@@ -17,6 +17,7 @@ from zerver.actions.custom_profile_fields import (
     try_add_realm_custom_profile_field,
 )
 from zerver.actions.presence import do_update_user_presence, do_update_user_status
+from zerver.actions.reactions import check_add_reaction, do_add_reaction
 from zerver.actions.realm_emoji import check_add_realm_emoji
 from zerver.actions.realm_icon import do_change_icon_source
 from zerver.actions.realm_logo import do_change_logo_source
@@ -24,12 +25,7 @@ from zerver.actions.user_activity import do_update_user_activity, do_update_user
 from zerver.actions.user_topics import do_mute_topic
 from zerver.actions.users import do_deactivate_user
 from zerver.lib import upload
-from zerver.lib.actions import (
+from zerver.lib.actions import do_change_realm_plan_type, do_mute_user
-    check_add_reaction,
-    do_add_reaction,
-    do_change_realm_plan_type,
-    do_mute_user,
-)
 from zerver.lib.avatar_hash import user_avatar_path
 from zerver.lib.bot_config import set_bot_config
 from zerver.lib.bot_lib import StateHandler

zerver/tests/test_management_commands.py

@@ -14,7 +14,7 @@ from django.utils.timezone import now as timezone_now
 
 from confirmation.models import RealmCreationKey, generate_realm_creation_url
 from zerver.actions.create_user import do_create_user
-from zerver.lib.actions import do_add_reaction
+from zerver.actions.reactions import do_add_reaction
 from zerver.lib.management import ZulipBaseCommand, check_config
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import most_recent_message, stdout_suppressed

zerver/tests/test_message_edit.py

@@ -8,11 +8,11 @@ from django.db import IntegrityError
 from django.http import HttpResponse
 from django.utils.timezone import now as timezone_now
 
+from zerver.actions.reactions import do_add_reaction
 from zerver.actions.streams import do_change_stream_post_policy, do_deactivate_stream
 from zerver.actions.users import do_change_user_role
 from zerver.lib.actions import (
     check_update_message,
-    do_add_reaction,
     do_change_realm_plan_type,
     do_delete_messages,
     do_set_realm_property,

zerver/tests/test_reactions.py

@@ -4,8 +4,8 @@ from unittest import mock
 import orjson
 from django.http import HttpResponse
 
+from zerver.actions.reactions import notify_reaction_update
 from zerver.actions.streams import do_change_stream_permission
-from zerver.lib.actions import notify_reaction_update
 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.exceptions import JsonableError
@@ -1035,7 +1035,7 @@ class ReactionAPIEventTest(EmojiReactionBase):
         }
         events: List[Mapping[str, Any]] = []
         with self.tornado_redirected_to_list(events, expected_num_events=1):
-            with mock.patch("zerver.lib.actions.send_event") as m:
+            with mock.patch("zerver.actions.reactions.send_event") as m:
                 m.side_effect = AssertionError(
                     "Events should be sent only after the transaction commits!"
                 )
@@ -1119,7 +1119,7 @@ class ReactionAPIEventTest(EmojiReactionBase):
         )
 
         with self.tornado_redirected_to_list([], expected_num_events=1):
-            with mock.patch("zerver.lib.actions.send_event") as m:
+            with mock.patch("zerver.actions.reactions.send_event") as m:
                 m.side_effect = AssertionError(
                     "Events should be sent only after the transaction commits."
                 )

zerver/views/reactions.py

@@ -4,7 +4,7 @@ from django.db import transaction
 from django.http import HttpRequest, HttpResponse
 from django.utils.translation import gettext as _
 
-from zerver.lib.actions import check_add_reaction, do_remove_reaction
+from zerver.actions.reactions import check_add_reaction, do_remove_reaction
 from zerver.lib.emoji import emoji_name_to_emoji_code
 from zerver.lib.exceptions import JsonableError
 from zerver.lib.message import access_message

zilencer/management/commands/add_mock_conversation.py

@@ -4,9 +4,9 @@ from django.core.management.base import BaseCommand
 
 from zerver.actions.create_user import do_create_user
 from zerver.actions.message_send import do_send_messages, internal_prep_stream_message
+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.actions import do_add_reaction
 from zerver.lib.emoji import emoji_name_to_emoji_code
 from zerver.lib.streams import ensure_stream
 from zerver.lib.upload import upload_avatar_image