From 27558315a2c2d30b4369f8c0e2f16d7b311c46f3 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Fri, 19 Apr 2024 20:07:29 +0530 Subject: [PATCH] settings: Use named_user_group field to access name. This commit updates code to access name from named_user_group field which points to the "NamedUserGroup" instead of directly accessing name from "UserGroup", since name field will only be present on NamedUserGroup objects in further commits. --- zerver/actions/message_send.py | 2 +- zerver/actions/streams.py | 8 ++++---- zerver/lib/email_mirror.py | 1 + zerver/lib/message.py | 10 +++++----- zerver/lib/users.py | 2 +- zerver/models/users.py | 25 ++++++++++++++++++++----- zerver/tests/test_invite.py | 2 +- zerver/tests/test_realm.py | 3 ++- zerver/tests/test_signup.py | 2 +- zerver/tests/test_users.py | 2 +- zproject/backends.py | 4 ++-- 11 files changed, 39 insertions(+), 22 deletions(-) diff --git a/zerver/actions/message_send.py b/zerver/actions/message_send.py index 89736db589..91a76ed8fd 100644 --- a/zerver/actions/message_send.py +++ b/zerver/actions/message_send.py @@ -1567,7 +1567,7 @@ def get_recipients_for_user_creation_events( if len(guest_recipients) == 0: return recipients_for_user_creation_events - if realm.can_access_all_users_group.name == SystemGroups.EVERYONE: + if realm.can_access_all_users_group.named_user_group.name == SystemGroups.EVERYONE: return recipients_for_user_creation_events if len(user_profiles) == 1: diff --git a/zerver/actions/streams.py b/zerver/actions/streams.py index e9ea206f8b..e1b2151e7b 100644 --- a/zerver/actions/streams.py +++ b/zerver/actions/streams.py @@ -195,7 +195,7 @@ def do_deactivate_stream(stream: Stream, *, acting_user: Optional[UserProfile]) event = dict(type="stream", op="delete", streams=[stream_dict]) send_event_on_commit(stream.realm, event, affected_user_ids) - if stream.realm.can_access_all_users_group.name != SystemGroups.EVERYONE: + if stream.realm.can_access_all_users_group.named_user_group.name != SystemGroups.EVERYONE: send_user_remove_events_on_stream_deactivation(stream, subscribed_users) event_time = timezone_now() @@ -797,7 +797,7 @@ def bulk_add_subscriptions( if sub_info.user.is_guest: altered_guests.add(sub_info.user.id) - if realm.can_access_all_users_group.name != SystemGroups.EVERYONE: + if realm.can_access_all_users_group.named_user_group.name != SystemGroups.EVERYONE: altered_users = list(altered_streams_dict.keys()) subscribers_of_altered_user_subscriptions = get_subscribers_of_target_user_subscriptions( altered_users @@ -837,7 +837,7 @@ def bulk_add_subscriptions( subscriber_dict=subscriber_peer_info.subscribed_ids, ) - if realm.can_access_all_users_group.name != SystemGroups.EVERYONE: + if realm.can_access_all_users_group.named_user_group.name != SystemGroups.EVERYONE: send_user_creation_events_on_adding_subscriptions( realm, altered_user_dict, @@ -1082,7 +1082,7 @@ def bulk_remove_subscriptions( removed_sub_tuples = [(sub_info.user, sub_info.stream) for sub_info in subs_to_deactivate] send_subscription_remove_events(realm, users, streams, removed_sub_tuples) - if realm.can_access_all_users_group.name != SystemGroups.EVERYONE: + if realm.can_access_all_users_group.named_user_group.name != SystemGroups.EVERYONE: altered_user_dict: Dict[UserProfile, Set[int]] = defaultdict(set) for user, stream in removed_sub_tuples: altered_user_dict[user].add(stream.id) diff --git a/zerver/lib/email_mirror.py b/zerver/lib/email_mirror.py index 34b2a162f8..3c16de5d55 100644 --- a/zerver/lib/email_mirror.py +++ b/zerver/lib/email_mirror.py @@ -114,6 +114,7 @@ def get_usable_missed_message_address(address: str) -> MissedMessageEmailAddress "user_profile", "user_profile__realm", "user_profile__realm__can_access_all_users_group", + "user_profile__realm__can_access_all_users_group__named_user_group", "message", "message__sender", "message__recipient", diff --git a/zerver/lib/message.py b/zerver/lib/message.py index e294dc0a90..cb3bdb4ee6 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -1214,27 +1214,27 @@ def stream_wildcard_mention_allowed(sender: UserProfile, stream: Stream, realm: def check_user_group_mention_allowed(sender: UserProfile, user_group_ids: List[int]) -> None: user_groups = NamedUserGroup.objects.filter(id__in=user_group_ids).select_related( - "can_mention_group" + "can_mention_group", "can_mention_group__named_user_group" ) sender_is_system_bot = is_cross_realm_bot_email(sender.delivery_email) for group in user_groups: can_mention_group = group.can_mention_group - + can_mention_group_name = can_mention_group.named_user_group.name if sender_is_system_bot: - if can_mention_group.name == SystemGroups.EVERYONE: + if can_mention_group_name == SystemGroups.EVERYONE: continue raise JsonableError( _( "You are not allowed to mention user group '{user_group_name}'. You must be a member of '{can_mention_group_name}' to mention this group." - ).format(user_group_name=group.name, can_mention_group_name=can_mention_group.name) + ).format(user_group_name=group.name, can_mention_group_name=can_mention_group_name) ) if not is_user_in_group(can_mention_group, sender, direct_member_only=False): raise JsonableError( _( "You are not allowed to mention user group '{user_group_name}'. You must be a member of '{can_mention_group_name}' to mention this group." - ).format(user_group_name=group.name, can_mention_group_name=can_mention_group.name) + ).format(user_group_name=group.name, can_mention_group_name=can_mention_group_name) ) diff --git a/zerver/lib/users.py b/zerver/lib/users.py index 78b4f684ac..ecd9084829 100644 --- a/zerver/lib/users.py +++ b/zerver/lib/users.py @@ -569,7 +569,7 @@ def user_access_restricted_in_realm(target_user: UserProfile) -> bool: return False realm = target_user.realm - if realm.can_access_all_users_group.name == SystemGroups.EVERYONE: + if realm.can_access_all_users_group.named_user_group.name == SystemGroups.EVERYONE: return False return True diff --git a/zerver/models/users.py b/zerver/models/users.py index a0abe1e80e..4e3a847fe6 100644 --- a/zerver/models/users.py +++ b/zerver/models/users.py @@ -883,7 +883,10 @@ post_save.connect(flush_user_profile, sender=UserProfile) @cache_with_key(user_profile_by_id_cache_key, timeout=3600 * 24 * 7) def get_user_profile_by_id(user_profile_id: int) -> UserProfile: return UserProfile.objects.select_related( - "realm", "realm__can_access_all_users_group", "bot_owner" + "realm", + "realm__can_access_all_users_group", + "realm__can_access_all_users_group__named_user_group", + "bot_owner", ).get(id=user_profile_id) @@ -901,7 +904,10 @@ def get_user_profile_by_email(email: str) -> UserProfile: def maybe_get_user_profile_by_api_key(api_key: str) -> Optional[UserProfile]: try: return UserProfile.objects.select_related( - "realm", "realm__can_access_all_users_group", "bot_owner" + "realm", + "realm__can_access_all_users_group", + "realm__can_access_all_users_group__named_user_group", + "bot_owner", ).get(api_key=api_key) except UserProfile.DoesNotExist: # We will cache failed lookups with None. The @@ -927,7 +933,10 @@ def get_user_by_delivery_email(email: str, realm: "Realm") -> UserProfile: those code paths. """ return UserProfile.objects.select_related( - "realm", "realm__can_access_all_users_group", "bot_owner" + "realm", + "realm__can_access_all_users_group", + "realm__can_access_all_users_group__named_user_group", + "bot_owner", ).get(delivery_email__iexact=email.strip(), realm=realm) @@ -964,7 +973,10 @@ def get_user(email: str, realm: "Realm") -> UserProfile: get_user_by_delivery_email. """ return UserProfile.objects.select_related( - "realm", "realm__can_access_all_users_group", "bot_owner" + "realm", + "realm__can_access_all_users_group", + "realm__can_access_all_users_group__named_user_group", + "bot_owner", ).get(email__iexact=email.strip(), realm=realm) @@ -979,7 +991,10 @@ def get_active_user(email: str, realm: "Realm") -> UserProfile: def get_user_profile_by_id_in_realm(uid: int, realm: "Realm") -> UserProfile: return UserProfile.objects.select_related( - "realm", "realm__can_access_all_users_group", "bot_owner" + "realm", + "realm__can_access_all_users_group", + "realm__can_access_all_users_group__named_user_group", + "bot_owner", ).get(id=uid, realm=realm) diff --git a/zerver/tests/test_invite.py b/zerver/tests/test_invite.py index ab699b9eda..3afe4ea9a2 100644 --- a/zerver/tests/test_invite.py +++ b/zerver/tests/test_invite.py @@ -107,7 +107,7 @@ class StreamSetupTest(ZulipTestCase): new_user = self.create_simple_new_user(realm, "alice@zulip.com") - with self.assert_database_query_count(12): + with self.assert_database_query_count(13): set_up_streams_for_new_human_user( user_profile=new_user, prereg_user=None, diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index b021c9e6d5..e42494df92 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -1164,7 +1164,8 @@ class RealmTest(ZulipTestCase): permission_configuration, ) in Realm.REALM_PERMISSION_GROUP_SETTINGS.items(): self.assertEqual( - getattr(realm, setting_name).name, permission_configuration.default_group_name + getattr(realm, setting_name).named_user_group.name, + permission_configuration.default_group_name, ) def test_do_create_realm_with_keyword_arguments(self) -> None: diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index bf18d1e4fc..8779a097b3 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -932,7 +932,7 @@ class LoginTest(ZulipTestCase): # seem to be any O(N) behavior. Some of the cache hits are related # to sending messages, such as getting the welcome bot, looking up # the alert words for a realm, etc. - with self.assert_database_query_count(104), self.assert_memcached_count(18): + with self.assert_database_query_count(105), self.assert_memcached_count(18): with self.captureOnCommitCallbacks(execute=True): self.register(self.nonreg_email("test"), "test") diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index def5bca463..17e0ef4fc6 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -906,7 +906,7 @@ class QueryCountTest(ZulipTestCase): prereg_user = PreregistrationUser.objects.get(email="fred@zulip.com") - with self.assert_database_query_count(93): + with self.assert_database_query_count(94): with self.assert_memcached_count(23): with self.capture_send_event_calls(expected_num_events=11) as events: fred = do_create_user( diff --git a/zproject/backends.py b/zproject/backends.py index 9abe3cec43..c361be3e4a 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -986,11 +986,11 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend): existing_group_name_set_for_user = set( UserGroupMembership.objects.filter( user_group__realm=user_profile.realm, - user_group__name__in=set( + user_group__named_user_group__name__in=set( settings.LDAP_SYNCHRONIZED_GROUPS_BY_REALM[user_profile.realm.string_id] ), user_profile=user_profile, - ).values_list("user_group__name", flat=True) + ).values_list("user_group__named_user_group__name", flat=True) ) ldap_logger.debug(