custom_profile_fields: Bulk fetch of UserProfile.

bulk fetch query of UserPfrofile against which
user_ids are validated, instead of looping
over user_ids and fetchingeach UserPfrofile resulting
in O(n) queries.

zerver/models/custom_profile_fields.py

@@ -29,29 +29,31 @@ from zerver.lib.validator import (
     validate_select_field,
 )
 from zerver.models.realms import Realm
-from zerver.models.users import UserProfile, get_user_profile_by_id_in_realm
+from zerver.models.users import UserProfile
 
 
 def check_valid_user_ids(realm_id: int, val: object, allow_deactivated: bool = False) -> List[int]:
     user_ids = check_list(check_int)("User IDs", val)
-    realm = Realm.objects.get(id=realm_id)
-    for user_id in user_ids:
-        # TODO: Structurally, we should be doing a bulk fetch query to
-        # get the users here, not doing these in a loop.  But because
-        # this is a rarely used feature and likely to never have more
-        # than a handful of users, it's probably mostly OK.
-        try:
-            user_profile = get_user_profile_by_id_in_realm(user_id, realm)
-        except UserProfile.DoesNotExist:
-            raise ValidationError(_("Invalid user ID: {user_id}").format(user_id=user_id))
+    user_profiles = UserProfile.objects.filter(realm_id=realm_id, id__in=user_ids)
 
-        if not allow_deactivated and not user_profile.is_active:
+    valid_users_ids = set(user_profiles.values_list("id", flat=True))
+    invalid_users_ids = [invalid_id for invalid_id in user_ids if invalid_id not in valid_users_ids]
+
+    if invalid_users_ids:
         raise ValidationError(
-                _("User with ID {user_id} is deactivated").format(user_id=user_id)
+            _("Invalid user IDs: {invalid_ids}").format(
+                invalid_ids=", ".join(map(str, invalid_users_ids))
+            )
         )
 
-        if user_profile.is_bot:
-            raise ValidationError(_("User with ID {user_id} is a bot").format(user_id=user_id))
+    for user in user_profiles:
+        if not allow_deactivated and not user.is_active:
+            raise ValidationError(
+                _("User with ID {user_id} is deactivated").format(user_id=user.id)
+            )
+
+        if user.is_bot:
+            raise ValidationError(_("User with ID {user_id} is a bot").format(user_id=user.id))
 
     return user_ids
 

zerver/tests/test_custom_profile_data.py

@@ -718,7 +718,7 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
         field_name = "Mentor"
         invalid_user_id = 1000
         self.assert_error_update_invalid_value(
-            field_name, [invalid_user_id], f"Invalid user ID: {invalid_user_id}"
+            field_name, [invalid_user_id], f"Invalid user IDs: {invalid_user_id}"
         )
 
     def test_update_profile_data_successfully(self) -> None:

zerver/tests/test_users.py

@@ -1158,13 +1158,19 @@ class UserProfileTest(ZulipTestCase):
         othello = self.example_user("othello")
         bot = self.example_user("default_bot")
 
-        # Invalid user ID
+        # Invalid user IDs
         invalid_uid: object = 1000
+        another_invalid_uid: object = 1001
         with self.assertRaisesRegex(ValidationError, r"User IDs is not a list"):
             check_valid_user_ids(realm.id, invalid_uid)
-        with self.assertRaisesRegex(ValidationError, rf"Invalid user ID: {invalid_uid}"):
+        with self.assertRaisesRegex(ValidationError, rf"Invalid user IDs: {invalid_uid}"):
             check_valid_user_ids(realm.id, [invalid_uid])
 
+        with self.assertRaisesRegex(
+            ValidationError, rf"Invalid user IDs: {invalid_uid}, {another_invalid_uid}"
+        ):
+            check_valid_user_ids(realm.id, [invalid_uid, another_invalid_uid])
+
         invalid_uid = "abc"
         with self.assertRaisesRegex(ValidationError, r"User IDs\[0\] is not an integer"):
             check_valid_user_ids(realm.id, [invalid_uid])
@@ -1174,7 +1180,7 @@ class UserProfileTest(ZulipTestCase):
             check_valid_user_ids(realm.id, [invalid_uid])
 
         # User is in different realm
-        with self.assertRaisesRegex(ValidationError, rf"Invalid user ID: {hamlet.id}"):
+        with self.assertRaisesRegex(ValidationError, rf"Invalid user IDs: {hamlet.id}"):
             check_valid_user_ids(get_realm("zephyr").id, [hamlet.id])
 
         # User is not active