From 0bae59fa4b2c199a9c7d2ef0d7e8191fdc8c98fa Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Mon, 17 Jun 2024 16:18:16 +0530 Subject: [PATCH] streams: Check permission to create channels based on group setting. This commit updates code to use new group based setting when checking permission to create private channels. --- zerver/lib/events.py | 23 +++++++++++++---------- zerver/models/users.py | 5 +++-- zerver/tests/test_event_system.py | 4 ++-- zerver/tests/test_home.py | 6 +++--- zerver/tests/test_subs.py | 12 +++--------- 5 files changed, 24 insertions(+), 26 deletions(-) diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 7a0555a6f0..3531a6e1a9 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -543,7 +543,7 @@ def fetch_initial_state_data( client_gravatar=False, ) - state["can_create_private_streams"] = settings_user.can_create_private_streams() + state["can_create_private_streams"] = settings_user.can_create_private_streams(realm) state["can_create_public_streams"] = settings_user.can_create_public_streams(realm) state["can_create_web_public_streams"] = settings_user.can_create_web_public_streams() # TODO/compatibility: Deprecated in Zulip 5.0 (feature level @@ -1202,7 +1202,6 @@ def apply_event( ) policy_permission_dict = { - "create_private_stream_policy": "can_create_private_streams", "create_web_public_stream_policy": "can_create_web_public_streams", "invite_to_stream_policy": "can_subscribe_other_users", "invite_to_realm_policy": "can_invite_others_to_realm", @@ -1242,15 +1241,19 @@ def apply_event( ) state["realm_email_auth_enabled"] = value["Email"]["enabled"] - if key == "can_create_public_channel_group": - state["realm_create_public_stream_policy"] = ( - get_corresponding_policy_value_for_group_setting( - user_profile.realm, - "can_create_public_channel_group", - Realm.COMMON_POLICY_TYPES, + if key in ["can_create_public_channel_group", "can_create_private_channel_group"]: + if key == "can_create_public_channel_group": + state["realm_create_public_stream_policy"] = ( + get_corresponding_policy_value_for_group_setting( + user_profile.realm, + "can_create_public_channel_group", + Realm.COMMON_POLICY_TYPES, + ) ) - ) - state["can_create_public_streams"] = user_profile.has_permission(key) + state["can_create_public_streams"] = user_profile.has_permission(key) + else: + state["can_create_private_streams"] = user_profile.has_permission(key) + state["can_create_streams"] = ( state["can_create_private_streams"] or state["can_create_public_streams"] diff --git a/zerver/models/users.py b/zerver/models/users.py index 0d1698a220..70a7255ad3 100644 --- a/zerver/models/users.py +++ b/zerver/models/users.py @@ -749,6 +749,7 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings): if policy_name not in [ "add_custom_emoji_policy", + "can_create_private_channel_group", "can_create_public_channel_group", "create_multiuse_invite_group", "create_private_stream_policy", @@ -808,8 +809,8 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings): def can_create_public_streams(self, realm: Optional["Realm"] = None) -> bool: return self.has_permission("can_create_public_channel_group", realm) - def can_create_private_streams(self) -> bool: - return self.has_permission("create_private_stream_policy") + def can_create_private_streams(self, realm: Optional["Realm"] = None) -> bool: + return self.has_permission("can_create_private_channel_group", realm) def can_create_web_public_streams(self) -> bool: if not self.realm.web_public_streams_enabled(): diff --git a/zerver/tests/test_event_system.py b/zerver/tests/test_event_system.py index 5ab5f9f01d..da279d58f8 100644 --- a/zerver/tests/test_event_system.py +++ b/zerver/tests/test_event_system.py @@ -1170,7 +1170,7 @@ class FetchQueriesTest(ZulipTestCase): # count in production. realm = get_realm_with_settings(realm_id=user.realm_id) - with self.assert_database_query_count(43): + with self.assert_database_query_count(44): with mock.patch("zerver.lib.events.always_want") as want_mock: fetch_initial_state_data(user, realm=realm) @@ -1194,7 +1194,7 @@ class FetchQueriesTest(ZulipTestCase): realm_filters=0, realm_linkifiers=0, realm_playgrounds=1, - realm_user=4, + realm_user=5, realm_user_groups=5, realm_user_settings_defaults=1, recent_private_conversations=1, diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index 2e6767c367..06d59102be 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -260,7 +260,7 @@ class HomeTest(ZulipTestCase): self.client_post("/json/bots", bot_info) # Verify succeeds once logged-in - with self.assert_database_query_count(54): + with self.assert_database_query_count(55): with patch("zerver.lib.cache.cache_set") as cache_mock: result = self._get_home_page(stream="Denmark") self.check_rendered_logged_in_app(result) @@ -565,7 +565,7 @@ class HomeTest(ZulipTestCase): def test_num_queries_for_realm_admin(self) -> None: # Verify number of queries for Realm admin isn't much higher than for normal users. self.login("iago") - with self.assert_database_query_count(54): + with self.assert_database_query_count(55): with patch("zerver.lib.cache.cache_set") as cache_mock: result = self._get_home_page() self.check_rendered_logged_in_app(result) @@ -596,7 +596,7 @@ class HomeTest(ZulipTestCase): self._get_home_page() # Then for the second page load, measure the number of queries. - with self.assert_database_query_count(49): + with self.assert_database_query_count(50): result = self._get_home_page() # Do a sanity check that our new streams were in the payload. diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 90898ffe14..036ca0e86a 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -4532,8 +4532,8 @@ class SubscriptionAPITest(ZulipTestCase): self.common_subscribe_to_streams(cordelia, ["new_stream5"], invite_only=invite_only) def test_user_settings_for_creating_private_streams(self) -> None: - self._test_user_settings_for_creating_streams( - "create_private_stream_policy", + self._test_group_based_settings_for_creating_streams( + "can_create_private_channel_group", invite_only=True, is_web_public=False, ) @@ -4592,12 +4592,6 @@ class SubscriptionAPITest(ZulipTestCase): # Other streams that weren't created using the api should have no creator. self.assertIsNone(stream["creator_id"]) - def test_private_stream_policies(self) -> None: - def validation_func(user_profile: UserProfile) -> bool: - return user_profile.can_create_private_streams() - - self.check_has_permission_policies("create_private_stream_policy", validation_func) - def test_web_public_stream_policies(self) -> None: def validation_func(user_profile: UserProfile) -> bool: return user_profile.can_create_web_public_streams() @@ -5599,7 +5593,7 @@ class SubscriptionAPITest(ZulipTestCase): ) # Test creating private stream. - with self.assert_database_query_count(38): + with self.assert_database_query_count(40): self.common_subscribe_to_streams( self.test_user, [new_streams[1]],