i18n: Fix default language for users created via API/LDAP.

This fixes a regression introduced in
9954db4b59, where the realm's default
language would be ignored for users created via API/LDAP/SAML,
resulting in all such users having English as their default language.

The API/LDAP/SAML account creation code paths don't have a request,
and thus cannot pull default language from the user's browser.

We have the `realm.default_language` field intended for this use case,
but it was not being passed through the system.

Rather than pass `realm.default_language` through from each caller, we
make the low-level user creation code set this field, as that seems
more robust to the creation of future callers.
This commit is contained in:
Tim Abbott 2023-09-29 10:09:45 -07:00 committed by Mateusz Mandera
parent 7669fe8446
commit c6fe799369
8 changed files with 30 additions and 5 deletions

View File

@ -406,7 +406,7 @@ def do_create_user(
tos_version: Optional[str] = None, tos_version: Optional[str] = None,
timezone: str = "", timezone: str = "",
avatar_source: str = UserProfile.AVATAR_FROM_GRAVATAR, avatar_source: str = UserProfile.AVATAR_FROM_GRAVATAR,
default_language: str = "en", default_language: Optional[str] = None,
default_sending_stream: Optional[Stream] = None, default_sending_stream: Optional[Stream] = None,
default_events_register_stream: Optional[Stream] = None, default_events_register_stream: Optional[Stream] = None,
default_all_public_streams: Optional[bool] = None, default_all_public_streams: Optional[bool] = None,

View File

@ -58,6 +58,7 @@ def bulk_create_users(
False, False,
tos_version, tos_version,
timezone, timezone,
default_language=realm.default_language,
tutorial_status=UserProfile.TUTORIAL_FINISHED, tutorial_status=UserProfile.TUTORIAL_FINISHED,
email_address_visibility=email_address_visibility, email_address_visibility=email_address_visibility,
) )

View File

@ -7,6 +7,7 @@ from django.contrib.auth.models import UserManager
from django.utils.timezone import now as timezone_now from django.utils.timezone import now as timezone_now
from zerver.lib.hotspots import copy_hotspots from zerver.lib.hotspots import copy_hotspots
from zerver.lib.i18n import get_default_language_for_new_user
from zerver.lib.timezone import canonicalize_timezone from zerver.lib.timezone import canonicalize_timezone
from zerver.lib.upload import copy_avatar from zerver.lib.upload import copy_avatar
from zerver.models import ( from zerver.models import (
@ -92,7 +93,7 @@ def create_user_profile(
is_mirror_dummy: bool, is_mirror_dummy: bool,
tos_version: Optional[str], tos_version: Optional[str],
timezone: str, timezone: str,
default_language: str = "en", default_language: str,
tutorial_status: str = UserProfile.TUTORIAL_WAITING, tutorial_status: str = UserProfile.TUTORIAL_WAITING,
force_id: Optional[int] = None, force_id: Optional[int] = None,
force_date_joined: Optional[datetime] = None, force_date_joined: Optional[datetime] = None,
@ -152,7 +153,7 @@ def create_user(
timezone: str = "", timezone: str = "",
avatar_source: str = UserProfile.AVATAR_FROM_GRAVATAR, avatar_source: str = UserProfile.AVATAR_FROM_GRAVATAR,
is_mirror_dummy: bool = False, is_mirror_dummy: bool = False,
default_language: str = "en", default_language: Optional[str] = None,
default_sending_stream: Optional[Stream] = None, default_sending_stream: Optional[Stream] = None,
default_events_register_stream: Optional[Stream] = None, default_events_register_stream: Optional[Stream] = None,
default_all_public_streams: Optional[bool] = None, default_all_public_streams: Optional[bool] = None,
@ -174,6 +175,12 @@ def create_user(
# so we hardcode them to EMAIL_ADDRESS_VISIBILITY_EVERYONE. # so we hardcode them to EMAIL_ADDRESS_VISIBILITY_EVERYONE.
user_email_address_visibility = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE user_email_address_visibility = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE
# Users created via the API or LDAP/SAML syncing code paths will
# usually not have a default_language value, and should fall back
# to the realm default.
if default_language is None:
default_language = get_default_language_for_new_user(realm, request=None)
user_profile = create_user_profile( user_profile = create_user_profile(
realm, realm,
email, email,

View File

@ -85,7 +85,13 @@ def get_browser_language_code(request: HttpRequest) -> Optional[str]:
return None return None
def get_default_language_for_new_user(realm: Realm, *, request: HttpRequest) -> str: def get_default_language_for_new_user(realm: Realm, *, request: Optional[HttpRequest]) -> str:
if request is None:
# Users created via the API or LDAP will not have a
# browser/request associated with them, and should just use
# the realm's default language.
return realm.default_language
browser_language_code = get_browser_language_code(request) browser_language_code = get_browser_language_code(request)
if browser_language_code is not None: if browser_language_code is not None:
return browser_language_code return browser_language_code

View File

@ -6312,7 +6312,9 @@ class TestLDAP(ZulipLDAPTestCase):
} }
) )
def test_login_success_when_user_does_not_exist_with_valid_subdomain(self) -> None: def test_login_success_when_user_does_not_exist_with_valid_subdomain(self) -> None:
RealmDomain.objects.create(realm=self.backend._realm, domain="acme.com") realm = self.backend._realm
do_set_realm_property(realm, "default_language", "ja", acting_user=None)
RealmDomain.objects.create(realm=realm, domain="acme.com")
with self.settings(LDAP_APPEND_DOMAIN="acme.com"): with self.settings(LDAP_APPEND_DOMAIN="acme.com"):
user_profile = self.backend.authenticate( user_profile = self.backend.authenticate(
request=mock.MagicMock(), request=mock.MagicMock(),
@ -6324,6 +6326,8 @@ class TestLDAP(ZulipLDAPTestCase):
self.assertEqual(user_profile.delivery_email, "newuser@acme.com") self.assertEqual(user_profile.delivery_email, "newuser@acme.com")
self.assertEqual(user_profile.full_name, "New LDAP fullname") self.assertEqual(user_profile.full_name, "New LDAP fullname")
self.assertEqual(user_profile.realm.string_id, "zulip") self.assertEqual(user_profile.realm.string_id, "zulip")
# Verify the user got the realm's default language.
self.assertEqual(user_profile.default_language, "ja")
# Verify avatar gets created # Verify avatar gets created
self.assertEqual(user_profile.avatar_source, UserProfile.AVATAR_FROM_USER) self.assertEqual(user_profile.avatar_source, UserProfile.AVATAR_FROM_USER)

View File

@ -173,6 +173,7 @@ class MirroredMessageUsersTest(ZulipTestCase):
kwargs["bot_owner"] = None kwargs["bot_owner"] = None
kwargs["tos_version"] = None kwargs["tos_version"] = None
kwargs["timezone"] = timezone_now() kwargs["timezone"] = timezone_now()
kwargs["default_language"] = "en"
kwargs["email_address_visibility"] = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE kwargs["email_address_visibility"] = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE
create_user_profile(**kwargs).save() create_user_profile(**kwargs).save()
raise IntegrityError raise IntegrityError

View File

@ -3996,6 +3996,9 @@ class UserSignUpTest(ZulipTestCase):
req.META["HTTP_ACCEPT_LANGUAGE"] = "" req.META["HTTP_ACCEPT_LANGUAGE"] = ""
self.assertEqual(get_default_language_for_new_user(realm, request=req), "hi") self.assertEqual(get_default_language_for_new_user(realm, request=req), "hi")
# Test code path for users created via the API or LDAP
self.assertEqual(get_default_language_for_new_user(realm, request=None), "hi")
class DeactivateUserTest(ZulipTestCase): class DeactivateUserTest(ZulipTestCase):
def test_deactivate_user(self) -> None: def test_deactivate_user(self) -> None:

View File

@ -938,6 +938,7 @@ class AdminCreateUserTest(ZulipTestCase):
realm = admin.realm realm = admin.realm
self.login_user(admin) self.login_user(admin)
do_change_user_role(admin, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None) do_change_user_role(admin, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None)
do_set_realm_property(realm, "default_language", "ja", acting_user=None)
valid_params = dict( valid_params = dict(
email="romeo@zulip.net", email="romeo@zulip.net",
password="xxxx", password="xxxx",
@ -1026,6 +1027,8 @@ class AdminCreateUserTest(ZulipTestCase):
self.assertEqual(new_user.full_name, "Romeo Montague") self.assertEqual(new_user.full_name, "Romeo Montague")
self.assertEqual(new_user.id, result["user_id"]) self.assertEqual(new_user.id, result["user_id"])
self.assertEqual(new_user.tos_version, UserProfile.TOS_VERSION_BEFORE_FIRST_LOGIN) self.assertEqual(new_user.tos_version, UserProfile.TOS_VERSION_BEFORE_FIRST_LOGIN)
# Make sure the new user got the realm's default language
self.assertEqual(new_user.default_language, "ja")
# Make sure the recipient field is set correctly. # Make sure the recipient field is set correctly.
self.assertEqual( self.assertEqual(