reactions: Migrate emoji_code to store id for realm emoji.

Till now, we had been storing realm emoji's name in emoji code field
in reactions' model. This commit migrates it to store realm emoji's id.
It is a part of effort to migrate realm emojis to be referenced by their
id and not by name.

frontend_tests/node_tests/reactions.js

@@ -11,16 +11,19 @@ set_global('emoji', {
     },
     all_realm_emojis: {
         realm_emoji: {
+            id: '991',
             emoji_name: 'realm_emoji',
             emoji_url: 'TBD',
             deactivated: false,
         },
         inactive_realm_emoji: {
+            id: '992',
             emoji_name: 'inactive_realm_emoji',
             emoji_url: 'TBD',
             deactivated: true,
         },
         zulip: {
+            id: 'zulip',
             emoji_name: 'zulip',
             emoji_url: 'TBD',
             deactivated: false,
@@ -28,10 +31,12 @@ set_global('emoji', {
     },
     active_realm_emojis: {
         realm_emoji: {
+            id: '991',
             emoji_name: 'realm_emoji',
             emoji_url: 'TBD',
         },
         zulip: {
+            id: 'zulip',
             emoji_name: 'zulip',
             emoji_url: 'TBD',
         },
@@ -90,7 +95,7 @@ var message = {
         {emoji_name: 'smile', user: {id: 6}, reaction_type: 'unicode_emoji', emoji_code: '1f604'},
         {emoji_name: 'frown', user: {id: 7}, reaction_type: 'unicode_emoji', emoji_code: '1f626'},
         {emoji_name: 'inactive_realm_emoji', user: {id: 5}, reaction_type: 'realm_emoji',
-         emoji_code: 'inactive_realm_emoji'},
+         emoji_code: '992'},
 
         // add some bogus user_ids
         {emoji_name: 'octopus', user: {id: 8888}, reaction_type: 'unicode_emoji', emoji_code: '1f419'},
@@ -167,8 +172,8 @@ set_global('current_msg_list', {
       {
          emoji_name: 'inactive_realm_emoji',
          reaction_type: 'realm_emoji',
-         emoji_code: 'inactive_realm_emoji',
-         local_id: 'realm_emoji,inactive_realm_emoji,inactive_realm_emoji',
+         emoji_code: '992',
+         local_id: 'realm_emoji,inactive_realm_emoji,992',
          count: 1,
          user_ids: [5],
          title: 'You (click to remove) reacted with :inactive_realm_emoji:',
@@ -232,16 +237,20 @@ set_global('current_msg_list', {
         });
     });
 
-    emoji_name = 'inactive_realm_emoji'; // Test removing a deactivated realm emoji.
+    emoji_name = 'inactive_realm_emoji';
     global.with_stub(function (stub) {
+        // Test removing a deactivated realm emoji. An user can interact with a
+        // deactivated realm emoji only by clicking on a reaction, hence, only
+        // `process_reaction_click()` codepath supports deleting/adding a deactivated
+        // realm emoji.
         global.channel.del = stub.f;
-        reactions.toggle_emoji_reaction(message_id, emoji_name);
+        reactions.process_reaction_click(message_id, 'realm_emoji,inactive_realm_emoji,992');
         var args = stub.get_args('args').args;
         assert.equal(args.url, '/json/messages/1001/reactions');
         assert.deepEqual(args.data, {
             reaction_type: 'realm_emoji',
             emoji_name: 'inactive_realm_emoji',
-            emoji_code: 'inactive_realm_emoji',
+            emoji_code: '992',
         });
     });
 
@@ -422,7 +431,7 @@ set_global('current_msg_list', {
         message_id: 1001,
         reaction_type: 'realm_emoji',
         emoji_name: 'realm_emoji',
-        emoji_code: 'realm_emoji',
+        emoji_code: '991',
         user: {
             user_id: cali.user_id,
         },
@@ -450,14 +459,14 @@ set_global('current_msg_list', {
         message_id: 1001,
         reaction_type: 'realm_emoji',
         emoji_name: 'realm_emoji',
-        emoji_code: 'realm_emoji',
+        emoji_code: '991',
         user: {
             user_id: alice.user_id,
         },
     };
 
     message_reactions.find = function (selector) {
-        assert.equal(selector, "[data-reaction-id='realm_emoji,realm_emoji,realm_emoji']");
+        assert.equal(selector, "[data-reaction-id='realm_emoji,realm_emoji,991']");
         return reaction_element;
     };
     reaction_element.prop = function () {};
@@ -652,7 +661,9 @@ set_global('current_msg_list', {
 
     var bogus_event  = {
         message_id: 55,
+        reaction_type: 'realm_emoji',
         emoji_name: 'realm_emoji',
+        emoji_code: '991',
         user: {
             user_id: 99,
         },

static/js/emoji.js

@@ -11,6 +11,7 @@ exports.default_emoji_aliases = {};
 var default_emojis = [];
 
 var zulip_emoji = {
+    id: 'zulip',
     emoji_name: 'zulip',
     emoji_url: '/static/generated/emoji/images/emoji/unicode/zulip.png',
     is_realm_emoji: true,
@@ -39,7 +40,8 @@ exports.update_emojis = function update_emojis(realm_emojis) {
     // Copy the default emoji list and add realm-specific emoji to it
     exports.emojis = default_emojis.slice(0);
     _.each(realm_emojis, function (data, name) {
-        exports.all_realm_emojis[name] = {emoji_name: name,
+        exports.all_realm_emojis[name] = {id: data.id,
+                                          emoji_name: name,
                                           emoji_url: data.source_url,
                                           deactivated: data.deactivated};
         if (data.deactivated !== true) {
@@ -48,7 +50,9 @@ exports.update_emojis = function update_emojis(realm_emojis) {
             exports.emojis.push({emoji_name: name,
                                  emoji_url: data.source_url,
                                  is_realm_emoji: true});
-            exports.active_realm_emojis[name] = {emoji_name: name, emoji_url: data.source_url};
+            exports.active_realm_emojis[name] = {id: data.id,
+                                                 emoji_name: name,
+                                                 emoji_url: data.source_url};
         }
     });
     // Add the Zulip emoji to the realm emojis list

static/js/reactions.js

@@ -119,7 +119,7 @@ exports.toggle_emoji_reaction = function (message_id, emoji_name) {
         } else {
             reaction_info.reaction_type = 'realm_emoji';
         }
-        reaction_info.emoji_code = emoji_name;
+        reaction_info.emoji_code = emoji.active_realm_emojis[emoji_name].id;
     } else if (emoji_codes.name_to_codepoint.hasOwnProperty(emoji_name)) {
         reaction_info.reaction_type = 'unicode_emoji';
         reaction_info.emoji_code = emoji_codes.name_to_codepoint[emoji_name];
@@ -270,7 +270,7 @@ exports.view.insert_new_reaction = function (opts) {
 
     if (opts.reaction_type !== 'unicode_emoji') {
         context.is_realm_emoji = true;
-        context.url = emoji.all_realm_emojis[emoji_code].emoji_url;
+        context.url = emoji.all_realm_emojis[emoji_name].emoji_url;
     }
 
     context.count = 1;

zerver/lib/emoji.py

@@ -49,7 +49,7 @@ def emoji_name_to_emoji_code(realm: Realm, emoji_name: Text) -> Tuple[Text, Text
     realm_emojis = realm.get_emoji()
     realm_emoji = realm_emojis.get(emoji_name)
     if realm_emoji is not None and not realm_emoji['deactivated']:
-        return emoji_name, Reaction.REALM_EMOJI
+        return str(realm_emojis[emoji_name]['id']), Reaction.REALM_EMOJI
     if emoji_name == 'zulip':
         return emoji_name, Reaction.ZULIP_EXTRA_EMOJI
     if emoji_name in name_to_codepoint:
@@ -65,13 +65,13 @@ def check_emoji_request(realm: Realm, emoji_name: str, emoji_code: str,
     # code is valid for new reactions, or not.
     if emoji_type == "realm_emoji":
         realm_emojis = realm.get_emoji()
-        realm_emoji = realm_emojis.get(emoji_code)
+        realm_emoji = realm_emojis.get(emoji_name)
         if realm_emoji is None:
             raise JsonableError(_("Invalid custom emoji."))
+        if realm_emoji["id"] != emoji_code:
+            raise JsonableError(_("Invalid custom emoji id."))
         if realm_emoji["deactivated"]:
             raise JsonableError(_("This custom emoji has been deactivated."))
-        if emoji_name != emoji_code:
-            raise JsonableError(_("Invalid emoji name."))
     elif emoji_type == "zulip_extra_emoji":
         if emoji_code not in ["zulip"]:
             raise JsonableError(_("Invalid emoji code."))

zerver/migrations/0145_reactions_realm_emoji_name_to_id.py

@@ -0,0 +1,51 @@
+# -*- coding: utf-8 -*-
+
+import ujson
+
+from collections import defaultdict
+from django.conf import settings
+from django.db import migrations, models
+from django.db.backends.postgresql_psycopg2.schema import DatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+from typing import Any, Dict
+
+def realm_emoji_name_to_id(apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None:
+    Reaction = apps.get_model('zerver', 'Reaction')
+    RealmEmoji = apps.get_model('zerver', 'RealmEmoji')
+    realm_emoji_by_realm_id = defaultdict(dict)   # type: Dict[int, Dict[str, Any]]
+    for realm_emoji in RealmEmoji.objects.all():
+        realm_emoji_by_realm_id[realm_emoji.realm_id] = {
+            'id': str(realm_emoji.id),
+            'name': realm_emoji.name,
+            'deactivated': realm_emoji.deactivated,
+        }
+    for reaction in Reaction.objects.filter(reaction_type='realm_emoji'):
+        realm_id = reaction.user_profile.realm_id
+        emoji_name = reaction.emoji_name
+        if realm_id in realm_emoji_by_realm_id:
+            realm_emoji = realm_emoji_by_realm_id[realm_id].get(emoji_name)
+            if realm_emoji is None:
+                # Realm emoji used in this reaction has been deleted so this
+                # reaction should also be deleted. We don't need to reverse
+                # this step in migration reversal code.
+                reaction.delete()
+            else:
+                reaction.emoji_code = realm_emoji["id"]
+                reaction.save()
+
+def reversal(apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None:
+    Reaction = apps.get_model('zerver', 'Reaction')
+    for reaction in Reaction.objects.filter(reaction_type='realm_emoji'):
+        reaction.emoji_code = reaction.emoji_name
+        reaction.save()
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('zerver', '0144_remove_realm_create_generic_bot_by_admins_only'),
+    ]
+
+    operations = [
+        migrations.RunPython(realm_emoji_name_to_id,
+                             reverse_code=reversal),
+    ]

zerver/models.py

@@ -411,7 +411,8 @@ def get_realm_emoji_uncached(realm: Realm) -> Dict[Text, Dict[str, Any]]:
                 'id': row.author.id,
                 'email': row.author.email,
                 'full_name': row.author.full_name}
-        d[row.name] = dict(source_url=get_emoji_url(row.file_name, row.realm_id),
+        d[row.name] = dict(id=str(row.id),
+                           source_url=get_emoji_url(row.file_name, row.realm_id),
                            deactivated=row.deactivated,
                            author=author)
     return d

zerver/tests/test_reactions.py

@@ -98,16 +98,16 @@ class ReactionEmojiTest(ZulipTestCase):
         An emoji name is mapped canonically to emoji code.
         """
         realm = get_realm('zulip')
+        realm_emoji = RealmEmoji.objects.get(name="green_tick")
 
         # Test active realm emoji.
         emoji_code, reaction_type = emoji_name_to_emoji_code(realm, 'green_tick')
-        self.assertEqual(emoji_code, 'green_tick')
+        self.assertEqual(emoji_code, str(realm_emoji.id))
         self.assertEqual(reaction_type, 'realm_emoji')
 
         # Test deactivated realm emoji.
-        emoji = RealmEmoji.objects.get(name="green_tick")
-        emoji.deactivated = True
-        emoji.save(update_fields=['deactivated'])
+        realm_emoji.deactivated = True
+        realm_emoji.save(update_fields=['deactivated'])
         with self.assertRaises(JsonableError) as exc:
             emoji_name_to_emoji_code(realm, 'green_tick')
         self.assertEqual(str(exc.exception), "Emoji 'green_tick' does not exist")
@@ -126,7 +126,7 @@ class ReactionEmojiTest(ZulipTestCase):
         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, 'astonished')
+        self.assertEqual(emoji_code, str(overriding_emoji.id))
         self.assertEqual(reaction_type, 'realm_emoji')
 
         # Test deactivate over-ridding realm emoji.
@@ -140,7 +140,7 @@ class ReactionEmojiTest(ZulipTestCase):
         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, 'zulip')
+        self.assertEqual(emoji_code, str(overriding_emoji.id))
         self.assertEqual(reaction_type, 'realm_emoji')
 
         # Test non-existent emoji.
@@ -573,84 +573,73 @@ class ZulipExtraEmojiReactionTest(EmojiReactionBase):
         self.assert_json_error(result, "Reaction doesn't exist.")
 
 class RealmEmojiReactionTests(EmojiReactionBase):
-    def test_add_realm_emoji(self) -> None:
-        reaction_info = {
+    def setUp(self) -> None:
+        green_tick_emoji = RealmEmoji.objects.get(name="green_tick")
+        self.default_reaction_info = {
             'emoji_name': 'green_tick',
-            'emoji_code': 'green_tick',
+            'emoji_code': str(green_tick_emoji.id),
         }
-        result = self.post_reaction(reaction_info)
+
+    def test_add_realm_emoji(self) -> None:
+        result = self.post_reaction(self.default_reaction_info)
         self.assert_json_success(result)
 
     def test_add_realm_emoji_invalid_code(self) -> None:
         reaction_info = {
             'emoji_name': 'green_tick',
-            'emoji_code': 'non_existent',
+            'emoji_code': '9999',
         }
         result = self.post_reaction(reaction_info)
-        self.assert_json_error(result, 'Invalid custom emoji.')
+        self.assert_json_error(result, 'Invalid custom emoji id.')
 
     def test_add_realm_emoji_invalid_name(self) -> None:
         reaction_info = {
             'emoji_name': 'bogus_name',
-            'emoji_code': 'green_tick',
+            'emoji_code': '1',
         }
         result = self.post_reaction(reaction_info)
-        self.assert_json_error(result, 'Invalid emoji name.')
+        self.assert_json_error(result, 'Invalid custom emoji.')
 
     def test_add_deactivated_realm_emoji(self) -> None:
         emoji = RealmEmoji.objects.get(name="green_tick")
         emoji.deactivated = True
         emoji.save(update_fields=['deactivated'])
 
-        reaction_info = {
-            'emoji_name': 'green_tick',
-            'emoji_code': 'green_tick',
-        }
-        result = self.post_reaction(reaction_info)
+        result = self.post_reaction(self.default_reaction_info)
         self.assert_json_error(result, 'This custom emoji has been deactivated.')
 
     def test_add_to_existing_deactivated_realm_emoji_reaction(self) -> None:
-        reaction_info = {
-            'emoji_name': 'green_tick',
-            'emoji_code': 'green_tick',
-        }
-        result = self.post_reaction(reaction_info)
+        result = self.post_reaction(self.default_reaction_info)
         self.assert_json_success(result)
 
         emoji = RealmEmoji.objects.get(name="green_tick")
         emoji.deactivated = True
         emoji.save(update_fields=['deactivated'])
 
-        result = self.post_reaction(reaction_info, sender='AARON')
+        result = self.post_reaction(self.default_reaction_info, sender='AARON')
         self.assert_json_success(result)
 
-        reactions = self.get_message_reactions(1, 'green_tick', 'realm_emoji')
+        reactions = self.get_message_reactions(1,
+                                               self.default_reaction_info['emoji_code'],
+                                               'realm_emoji')
         self.assertEqual(len(reactions), 2)
 
     def test_remove_realm_emoji_reaction(self) -> None:
-        reaction_info = {
-            'emoji_name': 'green_tick',
-            'emoji_code': 'green_tick',
-        }
-        result = self.post_reaction(reaction_info)
+        result = self.post_reaction(self.default_reaction_info)
         self.assert_json_success(result)
 
-        result = self.delete_reaction(reaction_info)
+        result = self.delete_reaction(self.default_reaction_info)
         self.assert_json_success(result)
 
     def test_remove_deactivated_realm_emoji_reaction(self) -> None:
-        reaction_info = {
-            'emoji_name': 'green_tick',
-            'emoji_code': 'green_tick',
-        }
-        result = self.post_reaction(reaction_info)
+        result = self.post_reaction(self.default_reaction_info)
         self.assert_json_success(result)
 
         emoji = RealmEmoji.objects.get(name="green_tick")
         emoji.deactivated = True
         emoji.save(update_fields=['deactivated'])
 
-        result = self.delete_reaction(reaction_info)
+        result = self.delete_reaction(self.default_reaction_info)
         self.assert_json_success(result)
 
     def test_remove_non_existent_realm_emoji_reaction(self) -> None: