bots: Add owner_id to bot-related payloads.

For the below payloads we want `owner_id` instead
of `owner`, which we should deprecate.  (The
`owner` field is actually an email, which is
not a stable key.)

    page_params.realm_bots

    realm_bot/add

    realm_bot/update

IMPORTANT NOTE: Some of the data served in
these payloads is cached with the key
`bot_dicts_in_realm_cache_key`.

For page_params, we get the new field
via `get_owned_bot_dicts`.

For realm_bot/add, we modified
`created_bot_event`.

For realm_bot/update, we modified
`do_change_bot_owner`.

On the JS side, we no longer
look up the bot's owner directly in
`server_events_dispatch` when we get
a realm_bot/update event. Instead, we
delegate that job to `bot_data.js`.
I modified the tests accordingly.

frontend_tests/node_tests/bot_data.js

@@ -80,6 +80,15 @@ run_test('test_basics', () => {
         assert.equal('New Bot 1', bot.full_name);
         assert.equal(2, services[0].interface);
         assert.equal('http://baz.com', services[0].base_url);
+
+        const change_owner_event = {
+            owner_id: fred.user_id,
+        };
+        bot_data.update(43, change_owner_event);
+
+        bot = bot_data.get(43);
+        assert.equal(bot.owner_id, fred.user_id);
+        assert.equal(bot.owner, fred.email);
     }());
 
     (function test_embedded_bot_update() {

frontend_tests/node_tests/dispatch.js

@@ -380,17 +380,6 @@ const event_fixtures = {
         },
     },
 
-    realm_bot__update_owner: {
-        type: 'realm_bot',
-        op: 'update',
-        bot: {
-            email: 'the-bot@example.com',
-            user_id: 4321,
-            full_name: 'The Bot Has A New Name',
-            owner_id: test_user.user_id,
-        },
-    },
-
     realm_emoji: {
         type: 'realm_emoji',
         realm_emoji: {
@@ -1110,12 +1099,6 @@ with_overrides(function (override) {
             assert_same(args.update_bot_data, event.bot);
         });
     });
-
-    event = event_fixtures.realm_bot__update_owner;
-    override('bot_data.update', noop);
-    override('settings_users.update_user_data', noop);
-    dispatch(event);
-    assert_same(event.bot.owner, 'test@example.com');
 });
 
 with_overrides(function (override) {

static/js/bot_data.js

@@ -1,7 +1,20 @@
 const bots = new Map();
-const bot_fields = ['api_key', 'avatar_url', 'default_all_public_streams',
-                    'default_events_register_stream', 'default_sending_stream',
-                    'email', 'full_name', 'is_active', 'owner', 'bot_type', 'user_id'];
+
+const bot_fields = [
+    'api_key',
+    'avatar_url',
+    'bot_type',
+    'default_all_public_streams',
+    'default_events_register_stream',
+    'default_sending_stream',
+    'email',
+    'full_name',
+    'is_active',
+    'owner', // TODO: eliminate
+    'owner_id',
+    'user_id',
+];
+
 const services = new Map();
 const services_fields = ['base_url', 'interface',
                          'config_data', 'service_name', 'token'];
@@ -39,6 +52,12 @@ exports.update = function (bot_id, bot_update) {
     if (typeof bot_update.services !== 'undefined' && bot_update.services.length > 0) {
         Object.assign(service, _.pick(bot_update.services[0], services_fields));
     }
+
+    // TODO: eliminate `owner` (which is an email)
+    if (bot.owner_id) {
+        const bot_owner = people.get_by_user_id(bot.owner_id);
+        bot.owner = bot_owner.email;
+    }
     send_change_event();
 };
 

static/js/server_events_dispatch.js

@@ -205,9 +205,6 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) {
             bot_data.del(event.bot.user_id);
             settings_users.update_user_data(event.bot.user_id, event.bot);
         } else if (event.op === 'update') {
-            if (Object.prototype.hasOwnProperty.call(event.bot, 'owner_id')) {
-                event.bot.owner = people.get_by_user_id(event.bot.owner_id).email;
-            }
             bot_data.update(event.bot.user_id, event.bot);
             settings_users.update_user_data(event.bot.user_id, event.bot);
         }

templates/zerver/api/changelog.md

@@ -10,6 +10,12 @@ below features are supported.
 
 ## Changes in Zulip 2.2
 
+**Feature level 5**
+* [`GET /events`](/api/get-events-from-queue): `realm_bot` events,
+  sent when changes are made to bot users, now contain an
+  integer-format `owner_id` field, replacing the `owner` field (which
+  was an email address).
+
 **Feature level 4**
 
 * `jitsi_server_url`, `development_environment`, `server_generation`,

zerver/lib/actions.py

@@ -477,6 +477,7 @@ def created_bot_event(user_profile: UserProfile) -> Dict[str, Any]:
     # set the owner key while reactivating them.
     if user_profile.bot_owner is not None:
         bot['owner'] = user_profile.bot_owner.email
+        bot['owner_id'] = user_profile.bot_owner.id
 
     return dict(type="realm_bot", op="add", bot=bot)
 
@@ -5732,7 +5733,8 @@ def get_owned_bot_dicts(user_profile: UserProfile,
              'default_sending_stream': botdict['default_sending_stream__name'],
              'default_events_register_stream': botdict['default_events_register_stream__name'],
              'default_all_public_streams': botdict['default_all_public_streams'],
-             'owner': botdict['bot_owner__email'],
+             'owner': botdict['bot_owner__email'],  # TODO: eliminate
+             'owner_id': botdict['bot_owner__id'],
              'avatar_url': avatar_url_from_dict(botdict),
              'services': services_by_ids[botdict['id']],
              }

zerver/lib/cache.py

@@ -461,13 +461,21 @@ def active_non_guest_user_ids_cache_key(realm_id: int) -> str:
     return "active_non_guest_user_ids:%s" % (realm_id,)
 
 bot_dict_fields: List[str] = [
-    'id', 'full_name', 'short_name', 'bot_type', 'email',
-    'is_active', 'default_sending_stream__name',
-    'realm_id',
-    'default_events_register_stream__name',
-    'default_all_public_streams', 'api_key',
-    'bot_owner__email', 'avatar_source',
+    'api_key',
+    'avatar_source',
     'avatar_version',
+    'bot_owner__id',
+    'bot_owner__email',  # TODO: eliminate
+    'bot_type',
+    'default_all_public_streams',
+    'default_events_register_stream__name',
+    'default_sending_stream__name',
+    'email',
+    'full_name',
+    'id',
+    'is_active',
+    'realm_id',
+    'short_name',
 ]
 
 def bot_dicts_in_realm_cache_key(realm: 'Realm') -> str:

zerver/lib/events.py

@@ -480,7 +480,12 @@ def apply_event(state: Dict[str, Any],
             for bot in state['realm_bots']:
                 if bot['email'] == event['bot']['email']:
                     if 'owner_id' in event['bot']:
-                        bot['owner'] = get_user_profile_by_id(event['bot']['owner_id']).email
+                        bot_owner_id = event['bot']['owner_id']
+                        bot['owner_id'] = bot_owner_id
+
+                        # TODO: eliminate `owner` field.
+                        bot_owner = get_user_profile_by_id(bot_owner_id)
+                        bot['owner'] = bot_owner.email
                     else:
                         bot.update(event['bot'])
 

zerver/tests/test_bots.py

@@ -181,6 +181,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
                     default_events_register_stream=None,
                     default_all_public_streams=False,
                     services=[],
+                    owner_id=hamlet.id,
                     owner=hamlet.email,
                 ),
             ),
@@ -314,6 +315,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
                          default_events_register_stream=None,
                          default_all_public_streams=False,
                          services=[],
+                         owner_id=user.id,
                          # Important: This is the product-facing
                          # email, not the delivery email, for this
                          # user.  TODO: Migrate this to an integer ID.
@@ -417,6 +419,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
                     default_events_register_stream=None,
                     default_all_public_streams=False,
                     services=[],
+                    owner_id=user_profile.id,
                     owner=user_profile.email,
                 ),
             ),
@@ -492,6 +495,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
                     default_events_register_stream='Denmark',
                     default_all_public_streams=False,
                     services=[],
+                    owner_id=user_profile.id,
                     owner=user_profile.email,
                 ),
             ),

zerver/tests/test_events.py

@@ -2110,6 +2110,7 @@ class EventsRegisterTest(ZulipTestCase):
                     ('default_all_public_streams', check_bool),
                     ('avatar_url', check_string),
                     ('owner', check_string),
+                    ('owner_id', check_int),
                     ('services', check_services),
                 ])),
             ])
@@ -2303,6 +2304,7 @@ class EventsRegisterTest(ZulipTestCase):
                 ('default_all_public_streams', check_bool),
                 ('avatar_url', check_string),
                 ('owner', check_string),
+                ('owner_id', check_int),
                 ('services', check_services),
             ])),
         ])
@@ -2374,6 +2376,7 @@ class EventsRegisterTest(ZulipTestCase):
                 ('default_all_public_streams', check_bool),
                 ('avatar_url', check_string),
                 ('owner', check_none_or(check_string)),
+                ('owner_id', check_none_or(check_int)),
                 ('services', check_list(check_dict_only([
                     ('base_url', check_url),
                     ('interface', check_int),

zerver/tests/test_home.py

@@ -279,6 +279,7 @@ class HomeTest(ZulipTestCase):
             'full_name',
             'is_active',
             'owner',
+            'owner_id',
             'services',
             'user_id',
         ]