From 77d4be56a454409f6fea5a1f71973f1c161061b3 Mon Sep 17 00:00:00 2001 From: Sahil Batra <35494118+sahil839@users.noreply.github.com> Date: Wed, 3 Jun 2020 04:41:36 +0530 Subject: [PATCH] users: Modify do_create_user and create_user to accept role. We change do_create_user and create_user to accept role as a parameter instead of 'is_realm_admin' and 'is_guest'. These changes are done to minimize data conversions between role and boolean fields. --- .../commands/populate_analytics_db.py | 2 +- analytics/tests/test_counts.py | 2 +- zerver/lib/actions.py | 6 ++---- zerver/lib/create_user.py | 18 +++++++++++------- zerver/views/registration.py | 11 +++++------ zilencer/management/commands/add_new_realm.py | 2 +- zproject/backends.py | 5 ++--- 7 files changed, 23 insertions(+), 23 deletions(-) diff --git a/analytics/management/commands/populate_analytics_db.py b/analytics/management/commands/populate_analytics_db.py index d4bfcc2bc3..462f41447b 100644 --- a/analytics/management/commands/populate_analytics_db.py +++ b/analytics/management/commands/populate_analytics_db.py @@ -59,7 +59,7 @@ class Command(BaseCommand): with mock.patch("zerver.lib.create_user.timezone_now", return_value=installation_time): shylock = create_user('shylock@analytics.ds', 'Shylock', realm, full_name='Shylock', short_name='shylock', - is_realm_admin=True) + role=UserProfile.ROLE_REALM_ADMINISTRATOR) do_change_user_role(shylock, UserProfile.ROLE_REALM_ADMINISTRATOR) stream = Stream.objects.create( name='all', realm=realm, date_created=installation_time) diff --git a/analytics/tests/test_counts.py b/analytics/tests/test_counts.py index 46780a6507..7e306cd45b 100644 --- a/analytics/tests/test_counts.py +++ b/analytics/tests/test_counts.py @@ -67,7 +67,7 @@ class AnalyticsTestCase(TestCase): return create_user(kwargs['email'], 'password', kwargs['realm'], active=kwargs['is_active'], full_name=kwargs['full_name'], short_name=kwargs['short_name'], - is_realm_admin=True, **pass_kwargs) + role=UserProfile.ROLE_REALM_ADMINISTRATOR, **pass_kwargs) def create_stream_with_recipient(self, **kwargs: Any) -> Tuple[Stream, Recipient]: self.name_counter += 1 diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 08e530e3eb..e95f383349 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -498,8 +498,7 @@ def create_users(realm: Realm, name_list: Iterable[Tuple[str, str]], bot_type: O bulk_create_users(realm, user_set, bot_type) def do_create_user(email: str, password: Optional[str], realm: Realm, full_name: str, - short_name: str, bot_type: Optional[int]=None, - is_realm_admin: bool=False, is_guest: bool=False, + short_name: str, bot_type: Optional[int]=None, role: Optional[int]=None, bot_owner: Optional[UserProfile]=None, tos_version: Optional[str]=None, timezone: str="", avatar_source: str=UserProfile.AVATAR_FROM_GRAVATAR, default_sending_stream: Optional[Stream]=None, @@ -513,8 +512,7 @@ def do_create_user(email: str, password: Optional[str], realm: Realm, full_name: user_profile = create_user(email=email, password=password, realm=realm, full_name=full_name, short_name=short_name, - is_realm_admin=is_realm_admin, is_guest=is_guest, - bot_type=bot_type, bot_owner=bot_owner, + role=role, bot_type=bot_type, bot_owner=bot_owner, tos_version=tos_version, timezone=timezone, avatar_source=avatar_source, default_sending_stream=default_sending_stream, default_events_register_stream=default_events_register_stream, diff --git a/zerver/lib/create_user.py b/zerver/lib/create_user.py index c26b6f3cf9..57c8abc25c 100644 --- a/zerver/lib/create_user.py +++ b/zerver/lib/create_user.py @@ -1,7 +1,7 @@ from django.contrib.auth.models import UserManager from django.utils.timezone import now as timezone_now from zerver.models import UserProfile, Recipient, Subscription, Realm, Stream, \ - get_fake_email_domain + PreregistrationUser, get_fake_email_domain from zerver.lib.upload import copy_avatar from zerver.lib.hotspots import copy_hotpots from zerver.lib.utils import generate_api_key @@ -36,6 +36,13 @@ def get_display_email_address(user_profile: UserProfile, realm: Realm) -> str: return "user%s@%s" % (user_profile.id, get_fake_email_domain()) return user_profile.delivery_email +def get_role_for_new_user(invited_as: int, realm_creation: bool=False) -> int: + if invited_as == PreregistrationUser.INVITE_AS['REALM_ADMIN'] or realm_creation: + return UserProfile.ROLE_REALM_ADMINISTRATOR + elif invited_as == PreregistrationUser.INVITE_AS['GUEST_USER']: + return UserProfile.ROLE_GUEST + return UserProfile.ROLE_MEMBER + # create_user_profile is based on Django's User.objects.create_user, # except that we don't save to the database so it can used in # bulk_creates @@ -76,8 +83,7 @@ def create_user_profile(realm: Realm, email: str, password: Optional[str], def create_user(email: str, password: Optional[str], realm: Realm, full_name: str, short_name: str, active: bool = True, - is_realm_admin: bool = False, - is_guest: bool = False, + role: Optional[int] = None, bot_type: Optional[int] = None, bot_owner: Optional[UserProfile] = None, tos_version: Optional[str] = None, timezone: str = "", @@ -90,14 +96,12 @@ def create_user(email: str, password: Optional[str], realm: Realm, user_profile = create_user_profile(realm, email, password, active, bot_type, full_name, short_name, bot_owner, is_mirror_dummy, tos_version, timezone) - if is_realm_admin: - user_profile.role = UserProfile.ROLE_REALM_ADMINISTRATOR - if is_guest: - user_profile.role = UserProfile.ROLE_GUEST user_profile.avatar_source = avatar_source user_profile.timezone = timezone user_profile.default_sending_stream = default_sending_stream user_profile.default_events_register_stream = default_events_register_stream + if role is not None: + user_profile.role = role # Allow the ORM default to be used if not provided if default_all_public_streams is not None: user_profile.default_all_public_streams = default_all_public_streams diff --git a/zerver/views/registration.py b/zerver/views/registration.py index d192d3de55..5f01cb75af 100644 --- a/zerver/views/registration.py +++ b/zerver/views/registration.py @@ -12,8 +12,7 @@ from zerver.context_processors import get_realm_from_request, login_context from zerver.models import UserProfile, Realm, Stream, MultiuseInvite, \ name_changes_disabled, email_to_username, \ get_realm, get_user_by_delivery_email, get_default_stream_groups, DisposableEmailError, \ - DomainNotAllowedForRealmError, get_source_profile, EmailContainsPlusError, \ - PreregistrationUser + DomainNotAllowedForRealmError, get_source_profile, EmailContainsPlusError from zerver.lib.email_validation import email_allowed_for_realm, \ validate_email_not_already_in_realm from zerver.lib.send_email import send_email, FromAddress @@ -25,6 +24,7 @@ from zerver.forms import RegistrationForm, HomepageForm, RealmCreationForm, \ from django_auth_ldap.backend import LDAPBackend, _LDAPUser from zerver.decorator import require_post, \ do_login +from zerver.lib.create_user import get_role_for_new_user from zerver.lib.onboarding import send_initial_realm_messages, setup_realm_internal_bots from zerver.lib.sessions import get_expirable_session_var from zerver.lib.subdomains import get_subdomain, is_root_domain_available @@ -88,8 +88,8 @@ def accounts_register(request: HttpRequest) -> HttpResponse: email = prereg_user.email realm_creation = prereg_user.realm_creation password_required = prereg_user.password_required - is_realm_admin = prereg_user.invited_as == PreregistrationUser.INVITE_AS['REALM_ADMIN'] or realm_creation - is_guest = prereg_user.invited_as == PreregistrationUser.INVITE_AS['GUEST_USER'] + + role = get_role_for_new_user(prereg_user.invited_as, realm_creation) try: validators.validate_email(email) @@ -338,8 +338,7 @@ def accounts_register(request: HttpRequest) -> HttpResponse: if user_profile is None: user_profile = do_create_user(email, password, realm, full_name, short_name, prereg_user=prereg_user, - is_realm_admin=is_realm_admin, - is_guest=is_guest, + role=role, tos_version=settings.TOS_VERSION, timezone=timezone, newsletter_data={"IP": request.META['REMOTE_ADDR']}, diff --git a/zilencer/management/commands/add_new_realm.py b/zilencer/management/commands/add_new_realm.py index 1037f4f5b5..7570e0c75e 100644 --- a/zilencer/management/commands/add_new_realm.py +++ b/zilencer/management/commands/add_new_realm.py @@ -18,7 +18,7 @@ class Command(ZulipBaseCommand): name = '%02d-user' % ( UserProfile.objects.filter(email__contains='user@').count(),) user = do_create_user('%s@%s.zulip.com' % (name, string_id), - 'password', realm, name, name, is_realm_admin=True) + 'password', realm, name, name, role=UserProfile.ROLE_REALM_ADMINISTRATOR) bulk_add_subscriptions([realm.signup_notifications_stream], [user]) send_initial_realm_messages(realm) diff --git a/zproject/backends.py b/zproject/backends.py index 3f7de69a9a..13707745d7 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -57,6 +57,7 @@ from zerver.lib.actions import do_create_user, do_reactivate_user, do_deactivate do_update_user_custom_profile_data_if_changed from zerver.lib.avatar import is_avatar_new, avatar_url from zerver.lib.avatar_hash import user_avatar_content_hash +from zerver.lib.create_user import get_role_for_new_user from zerver.lib.dev_ldap_directory import init_fakeldap from zerver.lib.email_validation import email_allowed_for_realm, \ validate_email_not_already_in_realm @@ -706,9 +707,7 @@ class ZulipLDAPAuthBackend(ZulipLDAPAuthBackendBase): invited_as = self._prereg_user.invited_as realm_creation = self._prereg_user.realm_creation opts['prereg_user'] = self._prereg_user - opts['is_realm_admin'] = ( - invited_as == PreregistrationUser.INVITE_AS['REALM_ADMIN']) or realm_creation - opts['is_guest'] = invited_as == PreregistrationUser.INVITE_AS['GUEST_USER'] + opts['role'] = get_role_for_new_user(invited_as, realm_creation) opts['realm_creation'] = realm_creation # TODO: Ideally, we should add a mechanism for the user # entering which default stream groups they've selected in