users: Refactor get_members_backend endpoint to use get_raw_user_data.

Modify the get_raw_user_data method for use by the /users API endpoint
and then modify the /users endpoint to use it.

zerver/lib/cache.py

@@ -352,7 +352,8 @@ def user_profile_by_api_key_cache_key(api_key: str) -> str:
     'id', 'full_name', 'short_name', 'email',
     'avatar_source', 'avatar_version', 'is_active',
     'role', 'is_bot', 'realm_id', 'timezone',
-    'date_joined', 'bot_owner_id', 'delivery_email'
+    'date_joined', 'bot_owner_id', 'delivery_email',
+    'bot_type'
 ]  # type: List[str]
 
 def realm_user_dicts_cache_key(realm_id: int) -> str:

zerver/lib/events.py

@@ -75,7 +75,8 @@ def get_custom_profile_field_values(realm_id: int) -> Dict[int, Dict[str, Any]]:
     return profiles_by_user_id
 
 
-def get_raw_user_data(realm: Realm, user_profile: UserProfile, client_gravatar: bool,
+def get_raw_user_data(realm: Realm, user_profile: UserProfile,
+                      client_gravatar: bool, for_api: bool=False,
                       include_custom_profile_fields: bool=True) -> Dict[int, Dict[str, str]]:
     user_dicts = get_realm_user_dicts(realm.id)
 
@@ -114,14 +115,18 @@ def user_data(row: Dict[str, Any]) -> Dict[str, Any]:
                 user_profile.is_realm_admin):
             result['delivery_email'] = row['delivery_email']
 
+        if for_api:
+            # The API currently has a quirk that it expects to include
+            # a bot_type field even for human users; this field is
+            # invalid so we plan to eventually remove this.
+            result['bot_type'] = row['bot_type']
         if is_bot:
             if row['email'] in settings.CROSS_REALM_BOT_EMAILS:
                 result['is_cross_realm_bot'] = True
             elif row['bot_owner_id'] is not None:
                 result['bot_owner_id'] = row['bot_owner_id']
-        else:
-            if include_custom_profile_fields:
-                result['profile_data'] = profiles_by_user_id.get(row['id'], {})
+        elif include_custom_profile_fields:
+            result['profile_data'] = profiles_by_user_id.get(row['id'], {})
         return result
 
     return {

zerver/openapi/zulip.yaml

@@ -1013,6 +1013,14 @@ paths:
           type: boolean
           default: false
         example: true
+      - name: include_custom_profile_fields
+        in: query
+        description: Set to `true` if the client wants custom profile field
+          data to be included in the response.
+        schema:
+          type: boolean
+          default: false
+        example: true
       security:
       - basicAuth: []
       responses:

zerver/tests/test_bots.py

@@ -185,7 +185,7 @@ def test_add_bot(self) -> None:
         bots = [m for m in members if m['email'] == '[email protected]']
         self.assertEqual(len(bots), 1)
         bot = bots[0]
-        self.assertEqual(bot['bot_owner'], self.example_email('hamlet'))
+        self.assertEqual(bot['bot_owner_id'], self.example_user('hamlet').id)
         self.assertEqual(bot['user_id'], self.get_bot_user(email).id)
 
     def test_add_bot_with_username_in_use(self) -> None:
@@ -317,7 +317,7 @@ def test_add_bot_email_address_visibility(self) -> None:
         bots = [m for m in members if m['email'] == '[email protected]']
         self.assertEqual(len(bots), 1)
         bot = bots[0]
-        self.assertEqual(bot['bot_owner'], user.email)
+        self.assertEqual(bot['bot_owner_id'], user.id)
         self.assertEqual(bot['user_id'], self.get_bot_user(email).id)
 
     def test_bot_add_subscription(self) -> None:

zerver/tests/test_custom_profile_data.py

@@ -638,6 +638,53 @@ def test_list_order(self) -> None:
         self.assertListEqual(content["custom_fields"],
                              sorted(content["custom_fields"], key=lambda x: -x["id"]))
 
+    def test_get_custom_profile_fields_from_api(self) -> None:
+        iago = self.example_user("iago")
+        test_bot = self.create_test_bot("foo-bot", iago)
+        assert(test_bot)
+
+        url = "/json/users?client_gravatar=false&include_custom_profile_fields=true"
+        response = self.client_get(url)
+        self.assertEqual(response.status_code, 200)
+        raw_users_data = response.json()["members"]
+
+        iago_raw_data = None
+        test_bot_raw_data = None
+
+        for user_dict in raw_users_data:
+            if user_dict["user_id"] == iago.id:
+                iago_raw_data = user_dict
+                continue
+            if user_dict["user_id"] == test_bot.id:
+                test_bot_raw_data = user_dict
+                continue
+
+        if (not iago_raw_data) or (not test_bot_raw_data):
+            raise AssertionError("Could not find required data from the response.")
+
+        expected_keys_for_iago = {
+            "email", "user_id", "avatar_url", "is_admin", "is_guest", "is_bot",
+            "full_name", "timezone", "is_active", "date_joined", "bot_type", "profile_data"}
+        self.assertEqual(set(iago_raw_data.keys()), expected_keys_for_iago)
+        self.assertIsNone(iago_raw_data["bot_type"])  # the key should exist though
+        self.assertNotEqual(iago_raw_data["profile_data"], {})
+
+        expected_keys_for_test_bot = {
+            "email", "user_id", "avatar_url", "is_admin", "is_guest", "is_bot", "full_name",
+            "timezone", "is_active", "date_joined", "bot_type", "bot_owner_id"}
+        self.assertEqual(set(test_bot_raw_data.keys()), expected_keys_for_test_bot)
+        self.assertEqual(test_bot_raw_data["bot_type"], 1)
+        self.assertEqual(test_bot_raw_data["bot_owner_id"], iago_raw_data["user_id"])
+
+        url = "/json/users?client_gravatar=false"
+        response = self.client_get(url)
+        self.assertEqual(response.status_code, 200)
+        raw_users_data = response.json()["members"]
+        for user_dict in raw_users_data:
+            with self.assertRaises(KeyError):
+                user_dict["profile_data"]
+
+
 class ReorderCustomProfileFieldTest(CustomProfileFieldTestCase):
     def test_reorder(self) -> None:
         self.login(self.example_email("iago"))

zerver/views/users.py

@@ -9,14 +9,15 @@
 
 from zerver.decorator import require_realm_admin, require_member_or_admin
 from zerver.forms import CreateUserForm
+from zerver.lib.events import get_raw_user_data
 from zerver.lib.actions import do_change_avatar_fields, do_change_bot_owner, \
     do_change_is_admin, do_change_default_all_public_streams, \
     do_change_default_events_register_stream, do_change_default_sending_stream, \
     do_create_user, do_deactivate_user, do_reactivate_user, do_regenerate_api_key, \
     check_change_full_name, notify_created_bot, do_update_outgoing_webhook_service, \
     do_update_bot_config_data, check_change_bot_full_name, do_change_is_guest, \
     do_update_user_custom_profile_data_if_changed, check_remove_custom_profile_field_value
-from zerver.lib.avatar import avatar_url, get_gravatar_url, get_avatar_field
+from zerver.lib.avatar import avatar_url, get_gravatar_url
 from zerver.lib.bot_config import set_bot_config
 from zerver.lib.exceptions import CannotDeactivateLastUserError
 from zerver.lib.integrations import EMBEDDED_BOTS
@@ -398,82 +399,27 @@ def bot_info(bot_profile: UserProfile) -> Dict[str, Any]:
 
 @has_request_variables
 def get_members_backend(request: HttpRequest, user_profile: UserProfile,
-                        client_gravatar: bool=REQ(validator=check_bool, default=False)) -> HttpResponse:
+                        include_custom_profile_fields: bool=REQ(validator=check_bool,
+                                                                default=False),
+                        client_gravatar: bool=REQ(validator=check_bool, default=False)
+                        ) -> HttpResponse:
     '''
     The client_gravatar field here is set to True if clients can compute
     their own gravatars, which saves us bandwidth.  We want to eventually
     make this the default behavior, but we have old clients that expect
     the server to compute this for us.
     '''
-
     realm = user_profile.realm
-
-    fields = [
-        'id',
-        'email',
-        'realm_id',
-        'full_name',
-        'is_bot',
-        'is_active',
-        'bot_type',
-        'role',
-        'avatar_source',
-        'avatar_version',
-        'bot_owner__email',
-        'timezone',
-    ]
     if realm.email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS:
         # If email addresses are only available to administrators,
         # clients cannot compute gravatars, so we force-set it to false.
         client_gravatar = False
-    include_delivery_email = user_profile.is_realm_admin
-    if include_delivery_email:
-        fields.append("delivery_email")
-        fields.append("bot_owner__delivery_email")
-
-    query = UserProfile.objects.filter(
-        realm_id=realm.id
-    ).values(*fields)
-
-    def get_member(row: Dict[str, Any]) -> Dict[str, Any]:
-        email = row['email']
-        user_id = row['id']
-
-        result = dict(
-            user_id=user_id,
-            email=email,
-            full_name=row['full_name'],
-            is_bot=row['is_bot'],
-            is_active=row['is_active'],
-            bot_type=row['bot_type'],
-            is_admin=row['role'] == UserProfile.ROLE_REALM_ADMINISTRATOR,
-            is_guest=row['role'] == UserProfile.ROLE_GUEST,
-            timezone=row['timezone'],
-        )
-
-        result['avatar_url'] = get_avatar_field(
-            user_id=user_id,
-            email=email,
-            avatar_source=row['avatar_source'],
-            avatar_version=row['avatar_version'],
-            realm_id=row['realm_id'],
-            medium=False,
-            client_gravatar=client_gravatar,
-        )
-
-        if row['bot_owner__email']:
-            result['bot_owner'] = row['bot_owner__email']
-
-        if include_delivery_email:
-            result['delivery_email'] = row['delivery_email']
-            if row['bot_owner__delivery_email']:
-                result['bot_owner_delivery_email'] = row['bot_owner__delivery_email']
-
-        return result
-
-    members = [get_member(row) for row in query]
-
-    return json_success({'members': members})
+    members = get_raw_user_data(realm,
+                                user_profile=user_profile,
+                                client_gravatar=client_gravatar,
+                                for_api=True,
+                                include_custom_profile_fields=include_custom_profile_fields)
+    return json_success({'members': members.values()})
 
 @require_realm_admin
 @has_request_variables