From e9cd0ad3d64bbc9aedd163214fa0af4ebfc228c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Yago=20Gonz=C3=A1lez?= <yagogonzalezg@gmail.com>
Date: Mon, 9 Jul 2018 14:18:42 +0530
Subject: [PATCH] reactions: Allow using emoji_name for removing reactions.

Given that we allow adding emoji reactions by only using the
emoji_name, we should offer the same possibility for removing
reactions to make the experience for API clients not require looking
up emoji codes.

Since this is an additional optional parameter, this also preserves
backward compatibility.
---
 zerver/tests/test_reactions.py | 29 +++++++++++++++++++++++++++++
 zerver/views/reactions.py      | 16 +++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/zerver/tests/test_reactions.py b/zerver/tests/test_reactions.py
index 6463f7ad7b..2d1b8dff32 100644
--- a/zerver/tests/test_reactions.py
+++ b/zerver/tests/test_reactions.py
@@ -494,6 +494,12 @@ class DefaultEmojiReactionTests(EmojiReactionBase):
         result = self.delete_reaction(reaction_info)
         self.assert_json_success(result)
 
+    def test_delete_insufficient_arguments_reaction(self) -> None:
+        result = self.delete_reaction({})
+        self.assert_json_error(result, 'At least one of the following '
+                               'arguments must be present: emoji_name, '
+                               'emoji_code')
+
     def test_delete_non_existing_emoji_reaction(self) -> None:
         reaction_info = {
             'emoji_name': 'thumbs_up',
@@ -519,6 +525,29 @@ class DefaultEmojiReactionTests(EmojiReactionBase):
         result = self.delete_reaction(reaction_info)
         self.assert_json_success(result)
 
+    def test_delete_reaction_by_name(self) -> None:
+        hamlet = self.example_user('hamlet')
+        message = Message.objects.get(id=1)
+        Reaction.objects.create(user_profile=hamlet,
+                                message=message,
+                                emoji_name='+1',
+                                emoji_code='1f44d',
+                                reaction_type='unicode_emoji',
+                                )
+
+        reaction_info = {
+            'emoji_name': '+1'
+        }
+        result = self.delete_reaction(reaction_info)
+        self.assert_json_success(result)
+        self.assertFalse(
+            Reaction.objects.filter(user_profile=hamlet,
+                                    message=message,
+                                    emoji_name=reaction_info['emoji_name'],
+                                    emoji_code='1f44d',
+                                    reaction_type='unicode_emoji').exists()
+        )
+
     def test_react_historical(self) -> None:
         """
         Reacting with valid emoji on a historical message succeeds.
diff --git a/zerver/views/reactions.py b/zerver/views/reactions.py
index 934020bdbe..59c0f74500 100644
--- a/zerver/views/reactions.py
+++ b/zerver/views/reactions.py
@@ -83,10 +83,24 @@ def add_reaction(request: HttpRequest, user_profile: UserProfile, message_id: in
 
 @has_request_variables
 def remove_reaction(request: HttpRequest, user_profile: UserProfile, message_id: int,
-                    emoji_code: str=REQ(),
+                    emoji_name: Optional[str]=REQ(default=None),
+                    emoji_code: Optional[str]=REQ(default=None),
                     reaction_type: str=REQ(default="unicode_emoji")) -> HttpResponse:
     message, user_message = access_message(user_profile, message_id)
 
+    if emoji_code is None:
+        if emoji_name is None:
+            raise JsonableError(_('At least one of the following arguments '
+                                  'must be present: emoji_name, emoji_code'))
+        # A correct full Zulip client implementation should always
+        # pass an emoji_code, because of the corner cases discussed in
+        # the long block comments elsewhere in this file.  However, to
+        # make it easy for simple API clients to use the reactions API
+        # 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]
+
     if not Reaction.objects.filter(user_profile=user_profile,
                                    message=message,
                                    emoji_code=emoji_code,