api: Remove email field from realm_user and realm_bot events.

The `email` field for identifying the user being modified in these
events was not used by either the webapp or other official Zulip
clients.  Instead, it was legacy data from before we switched years
ago to sending user_id fields as the correct way to uniquely identify
a user.

templates/zerver/api/changelog.md

@@ -10,6 +10,13 @@ below features are supported.
 
 ## Changes in Zulip 2.2
 
+**Feature level 7**
+* [`GET /events`](/api/get-events-from-queue): `realm_user` and
+  `realm_bot` events no longer contain an `email` field to identify
+  the user; use the `user_id` field instead.  Previously, some (but
+  not all) events of these types contained an `email` key in addition to
+  to `user_id`) for identifying the modified user.
+
 **Feature level 6**
 * [`GET /events`](/api/get-events-from-queue): `realm_user` events to
   update a user's avatar now include the `avatar_version` field, which

zerver/lib/actions.py

@@ -799,15 +799,13 @@ def do_deactivate_user(user_profile: UserProfile,
         update_license_ledger_if_needed(user_profile.realm, event_time)
 
     event = dict(type="realm_user", op="remove",
-                 person=dict(email=user_profile.email,
-                             user_id=user_profile.id,
+                 person=dict(user_id=user_profile.id,
                              full_name=user_profile.full_name))
     send_event(user_profile.realm, event, active_user_ids(user_profile.realm_id))
 
     if user_profile.is_bot:
         event = dict(type="realm_bot", op="remove",
-                     bot=dict(email=user_profile.email,
-                              user_id=user_profile.id,
+                     bot=dict(user_id=user_profile.id,
                               full_name=user_profile.full_name))
         send_event(user_profile.realm, event, bot_owner_user_ids(user_profile))
 
@@ -3117,8 +3115,7 @@ def do_change_full_name(user_profile: UserProfile, full_name: str,
     RealmAuditLog.objects.create(realm=user_profile.realm, acting_user=acting_user,
                                  modified_user=user_profile, event_type=RealmAuditLog.USER_FULL_NAME_CHANGED,
                                  event_time=event_time, extra_data=old_name)
-    payload = dict(email=user_profile.email,
-                   user_id=user_profile.id,
+    payload = dict(user_id=user_profile.id,
                    full_name=user_profile.full_name)
     send_event(user_profile.realm,
                dict(type='realm_user', op='update', person=payload),
@@ -3175,9 +3172,9 @@ def do_change_bot_owner(user_profile: UserProfile, bot_owner: UserProfile,
         send_event(user_profile.realm,
                    dict(type='realm_bot',
                         op="delete",
-                        bot=dict(email=user_profile.email,
-                                 user_id=user_profile.id,
-                                 )),
+                        bot=dict(
+                            user_id=user_profile.id,
+                        )),
                    {previous_owner.id, })
         # Do not send update event for previous bot owner.
         update_users = update_users - {previous_owner.id, }
@@ -3192,8 +3189,7 @@ def do_change_bot_owner(user_profile: UserProfile, bot_owner: UserProfile,
     send_event(user_profile.realm,
                dict(type='realm_bot',
                     op='update',
-                    bot=dict(email=user_profile.email,
-                             user_id=user_profile.id,
+                    bot=dict(user_id=user_profile.id,
                              owner_id=user_profile.bot_owner.id,
                              )),
                update_users)
@@ -3239,8 +3235,7 @@ def do_regenerate_api_key(user_profile: UserProfile, acting_user: UserProfile) -
         send_event(user_profile.realm,
                    dict(type='realm_bot',
                         op='update',
-                        bot=dict(email=user_profile.email,
-                                 user_id=user_profile.id,
+                        bot=dict(user_id=user_profile.id,
                                  api_key=new_api_key,
                                  )),
                    bot_owner_user_ids(user_profile))
@@ -3256,8 +3251,7 @@ def notify_avatar_url_change(user_profile: UserProfile) -> None:
         send_event(user_profile.realm,
                    dict(type='realm_bot',
                         op='update',
-                        bot=dict(email=user_profile.email,
-                                 user_id=user_profile.id,
+                        bot=dict(user_id=user_profile.id,
                                  avatar_url=avatar_url(user_profile),
                                  )),
                    bot_owner_user_ids(user_profile))
@@ -3267,7 +3261,8 @@ def notify_avatar_url_change(user_profile: UserProfile) -> None:
         avatar_url=avatar_url(user_profile),
         avatar_url_medium=avatar_url(user_profile, medium=True),
         avatar_version=user_profile.avatar_version,
-        email=user_profile.email,
+        # Even clients using client_gravatar don't need the email,
+        # since we're sending the URL anyway.
         user_id=user_profile.id
     )
 
@@ -3382,8 +3377,7 @@ def do_change_default_sending_stream(user_profile: UserProfile, stream: Optional
         send_event(user_profile.realm,
                    dict(type='realm_bot',
                         op='update',
-                        bot=dict(email=user_profile.email,
-                                 user_id=user_profile.id,
+                        bot=dict(user_id=user_profile.id,
                                  default_sending_stream=stream_name,
                                  )),
                    bot_owner_user_ids(user_profile))
@@ -3405,8 +3399,7 @@ def do_change_default_events_register_stream(user_profile: UserProfile,
         send_event(user_profile.realm,
                    dict(type='realm_bot',
                         op='update',
-                        bot=dict(email=user_profile.email,
-                                 user_id=user_profile.id,
+                        bot=dict(user_id=user_profile.id,
                                  default_events_register_stream=stream_name,
                                  )),
                    bot_owner_user_ids(user_profile))
@@ -3423,8 +3416,7 @@ def do_change_default_all_public_streams(user_profile: UserProfile, value: bool,
         send_event(user_profile.realm,
                    dict(type='realm_bot',
                         op='update',
-                        bot=dict(email=user_profile.email,
-                                 user_id=user_profile.id,
+                        bot=dict(user_id=user_profile.id,
                                  default_all_public_streams=user_profile.default_all_public_streams,
                                  )),
                    bot_owner_user_ids(user_profile))
@@ -3456,8 +3448,7 @@ def do_change_is_admin(user_profile: UserProfile, value: bool,
                 RealmAuditLog.ROLE_COUNT: realm_user_count_by_role(user_profile.realm),
             }))
         event = dict(type="realm_user", op="update",
-                     person=dict(email=user_profile.email,
-                                 user_id=user_profile.id,
+                     person=dict(user_id=user_profile.id,
                                  is_admin=value))
         send_event(user_profile.realm, event, active_user_ids(user_profile.realm_id))
 
@@ -3480,8 +3471,7 @@ def do_change_is_guest(user_profile: UserProfile, value: bool) -> None:
             RealmAuditLog.ROLE_COUNT: realm_user_count_by_role(user_profile.realm),
         }))
     event = dict(type="realm_user", op="update",
-                 person=dict(email=user_profile.email,
-                             user_id=user_profile.id,
+                 person=dict(user_id=user_profile.id,
                              is_guest=value))
     send_event(user_profile.realm, event, active_user_ids(user_profile.realm_id))
 
@@ -5640,8 +5630,7 @@ def do_update_outgoing_webhook_service(bot_profile: UserProfile,
     send_event(bot_profile.realm,
                dict(type='realm_bot',
                     op='update',
-                    bot=dict(email=bot_profile.email,
-                             user_id=bot_profile.id,
+                    bot=dict(user_id=bot_profile.id,
                              services = [dict(base_url=service.base_url,
                                               interface=service.interface,
                                               token=service.token,)],
@@ -5657,8 +5646,7 @@ def do_update_bot_config_data(bot_profile: UserProfile,
     send_event(bot_profile.realm,
                dict(type='realm_bot',
                     op='update',
-                    bot=dict(email=bot_profile.email,
-                             user_id=bot_profile.id,
+                    bot=dict(user_id=bot_profile.id,
                              services = [dict(config_data=updated_config_data)],
                              ),
                     ),

zerver/lib/events.py

@@ -467,18 +467,18 @@ def apply_event(state: Dict[str, Any],
             state['realm_bots'].append(event['bot'])
 
         if event['op'] == 'remove':
-            email = event['bot']['email']
+            user_id = event['bot']['user_id']
             for bot in state['realm_bots']:
-                if bot['email'] == email:
+                if bot['user_id'] == user_id:
                     bot['is_active'] = False
 
         if event['op'] == 'delete':
             state['realm_bots'] = [item for item
-                                   in state['realm_bots'] if item['email'] != event['bot']['email']]
+                                   in state['realm_bots'] if item['user_id'] != event['bot']['user_id']]
 
         if event['op'] == 'update':
             for bot in state['realm_bots']:
-                if bot['email'] == event['bot']['email']:
+                if bot['user_id'] == event['bot']['user_id']:
                     if 'owner_id' in event['bot']:
                         bot_owner_id = event['bot']['owner_id']
                         bot['owner_id'] = bot_owner_id

zerver/tests/test_events.py

@@ -474,7 +474,6 @@ class EventsRegisterTest(ZulipTestCase):
             ('type', equals('realm_bot')),
             ('op', equals('update')),
             ('bot', check_dict_only([
-                ('email', check_string),
                 ('user_id', check_int),
                 (field_name, check),
             ])),
@@ -1532,12 +1531,11 @@ class EventsRegisterTest(ZulipTestCase):
             ('type', equals('realm_user')),
             ('op', equals('update')),
             ('person', check_dict_only([
-                ('email', check_string),
-                ('user_id', check_int),
                 ('avatar_url', check_string),
                 ('avatar_url_medium', check_string),
                 ('avatar_version', check_int),
                 ('avatar_source', check_string),
+                ('user_id', check_int),
             ])),
         ])
         events = self.do_test(
@@ -1554,7 +1552,6 @@ class EventsRegisterTest(ZulipTestCase):
                 ('avatar_url', check_none_or(check_string)),
                 ('avatar_url_medium', check_none_or(check_string)),
                 ('avatar_version', check_int),
-                ('email', check_string),
                 ('user_id', check_int),
             ])),
         ])
@@ -1569,7 +1566,6 @@ class EventsRegisterTest(ZulipTestCase):
             ('type', equals('realm_user')),
             ('op', equals('update')),
             ('person', check_dict_only([
-                ('email', check_string),
                 ('full_name', check_string),
                 ('user_id', check_int),
             ])),
@@ -1591,12 +1587,11 @@ class EventsRegisterTest(ZulipTestCase):
             ('type', equals('realm_user')),
             ('op', equals('update')),
             ('person', check_dict_only([
-                ('email', check_string),
-                ('user_id', check_int),
                 ('avatar_source', check_string),
                 ('avatar_url', check_string),
                 ('avatar_url_medium', check_string),
                 ('avatar_version', check_int),
+                ('user_id', check_int),
             ])),
         ])
         do_set_realm_property(self.user_profile.realm, "email_address_visibility",
@@ -1836,7 +1831,6 @@ class EventsRegisterTest(ZulipTestCase):
             ('type', equals('realm_user')),
             ('op', equals('update')),
             ('person', check_dict_only([
-                ('email', check_string),
                 ('is_admin', check_bool),
                 ('user_id', check_int),
             ])),
@@ -1907,7 +1901,6 @@ class EventsRegisterTest(ZulipTestCase):
                 ('type', equals('realm_user')),
                 ('op', equals('update')),
                 ('person', check_dict_only([
-                    ('email', check_string),
                     ('user_id', check_int),
                     ('timezone', check_string),
                 ])),
@@ -2259,7 +2252,6 @@ class EventsRegisterTest(ZulipTestCase):
             ('type', equals('realm_bot')),
             ('op', equals('update')),
             ('bot', check_dict_only([
-                ('email', check_string),
                 ('user_id', check_int),
                 ('owner_id', check_int),
             ])),
@@ -2278,7 +2270,6 @@ class EventsRegisterTest(ZulipTestCase):
             ('type', equals('realm_bot')),
             ('op', equals('delete')),
             ('bot', check_dict_only([
-                ('email', check_string),
                 ('user_id', check_int),
             ])),
         ])
@@ -2326,7 +2317,6 @@ class EventsRegisterTest(ZulipTestCase):
             ('type', equals('realm_bot')),
             ('op', equals('update')),
             ('bot', check_dict_only([
-                ('email', check_string),
                 ('user_id', check_int),
                 ('services', check_list(check_dict_only([
                     ('base_url', check_url),
@@ -2352,7 +2342,6 @@ class EventsRegisterTest(ZulipTestCase):
             ('type', equals('realm_bot')),
             ('op', equals('remove')),
             ('bot', check_dict_only([
-                ('email', check_string),
                 ('full_name', check_string),
                 ('user_id', check_int),
             ])),

zerver/tests/test_users.py

@@ -157,7 +157,7 @@ class PermissionTest(ZulipTestCase):
         admin_users = realm.get_human_admin_users()
         self.assertTrue(othello in admin_users)
         person = events[0]['event']['person']
-        self.assertEqual(person['email'], othello.email)
+        self.assertEqual(person['user_id'], othello.id)
         self.assertEqual(person['is_admin'], True)
 
         # Taketh away
@@ -169,7 +169,7 @@ class PermissionTest(ZulipTestCase):
         admin_users = realm.get_human_admin_users()
         self.assertFalse(othello in admin_users)
         person = events[0]['event']['person']
-        self.assertEqual(person['email'], othello.email)
+        self.assertEqual(person['user_id'], othello.id)
         self.assertEqual(person['is_admin'], False)
 
         # Cannot take away from last admin
@@ -182,7 +182,7 @@ class PermissionTest(ZulipTestCase):
         admin_users = realm.get_human_admin_users()
         self.assertFalse(hamlet in admin_users)
         person = events[0]['event']['person']
-        self.assertEqual(person['email'], hamlet.email)
+        self.assertEqual(person['user_id'], hamlet.id)
         self.assertEqual(person['is_admin'], False)
         with tornado_redirected_to_list([]):
             result = self.client_patch('/json/users/{}'.format(iago.id), req)
@@ -383,7 +383,7 @@ class PermissionTest(ZulipTestCase):
         self.assertTrue(hamlet.is_guest)
         self.assertFalse(hamlet.can_access_all_realm_members())
         person = events[0]['event']['person']
-        self.assertEqual(person['email'], hamlet.email)
+        self.assertEqual(person['user_id'], hamlet.id)
         self.assertTrue(person['is_guest'])
 
     def test_change_guest_to_regular_member(self) -> None:
@@ -401,7 +401,7 @@ class PermissionTest(ZulipTestCase):
         polonius = self.example_user("polonius")
         self.assertFalse(polonius.is_guest)
         person = events[0]['event']['person']
-        self.assertEqual(person['email'], polonius.email)
+        self.assertEqual(person['user_id'], polonius.id)
         self.assertFalse(person['is_guest'])
 
     def test_change_admin_to_guest(self) -> None:
@@ -431,11 +431,11 @@ class PermissionTest(ZulipTestCase):
         self.assertFalse(hamlet.is_realm_admin)
 
         person = events[0]['event']['person']
-        self.assertEqual(person['email'], hamlet.email)
+        self.assertEqual(person['user_id'], hamlet.id)
         self.assertFalse(person['is_admin'])
 
         person = events[1]['event']['person']
-        self.assertEqual(person['email'], hamlet.email)
+        self.assertEqual(person['user_id'], hamlet.id)
         self.assertTrue(person['is_guest'])
 
     def test_change_guest_to_admin(self) -> None:
@@ -464,7 +464,7 @@ class PermissionTest(ZulipTestCase):
         self.assertTrue(polonius.is_realm_admin)
 
         person = events[0]['event']['person']
-        self.assertEqual(person['email'], polonius.email)
+        self.assertEqual(person['user_id'], polonius.id)
         self.assertTrue(person['is_admin'])
 
     def test_admin_user_can_change_profile_data(self) -> None: