From 9aaa61963eea6d5b0a04ca1695fc1b2acfc16b51 Mon Sep 17 00:00:00 2001 From: sahil839 Date: Fri, 15 May 2020 04:03:24 +0530 Subject: [PATCH] stream: Allow non admins to set stream post policy when creating streams. This commit allows non admins to set stream post policy while creating streams. Restriction was there to prevent user from creating a stream in which the user cannot post himself but this will be taken care of with stream admin feature. --- static/js/stream_create.js | 7 +--- static/templates/stream_types.hbs | 28 ++++++------- zerver/lib/streams.py | 14 ------- zerver/tests/test_subs.py | 68 +++++++------------------------ 4 files changed, 29 insertions(+), 88 deletions(-) diff --git a/static/js/stream_create.js b/static/js/stream_create.js index af009bc922..61e12497c9 100644 --- a/static/js/stream_create.js +++ b/static/js/stream_create.js @@ -165,13 +165,8 @@ function create_stream() { data.invite_only = JSON.stringify(invite_only); data.history_public_to_subscribers = JSON.stringify(history_public_to_subscribers); - let stream_post_policy = parseInt($('#stream_creation_form input[name=stream-post-policy]:checked').val(), 10); + const stream_post_policy = parseInt($('#stream_creation_form input[name=stream-post-policy]:checked').val(), 10); - // Because the stream_post_policy field is hidden when non-administrators create streams, - // we need to set the default value here. - if (isNaN(stream_post_policy)) { - stream_post_policy = stream_data.stream_post_policy_values.everyone.code; - } data.stream_post_policy = JSON.stringify(stream_post_policy); const announce = stream_data.realm_has_notifications_stream() && diff --git a/static/templates/stream_types.hbs b/static/templates/stream_types.hbs index 0a88c0b916..f0fc7d8414 100644 --- a/static/templates/stream_types.hbs +++ b/static/templates/stream_types.hbs @@ -22,19 +22,17 @@ {{t 'Private, protected history: must be invited by a member; new members can only see messages sent after they join; hidden from non-administrator users' }} - {{#if is_admin}} -

{{t 'Who can post to the stream?'}} - - - -

- {{#each stream_post_policy_values}} -
  • - -
    • - {{/each}} - {{/if}} +

    {{t 'Who can post to the stream?'}} + + + +

    + {{#each stream_post_policy_values}} +
  • + +
    • + {{/each}} diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index a74a8e44b6..a534dd3926 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -415,22 +415,10 @@ def list_to_streams(streams_raw: Iterable[Mapping[str, Any]], missing_stream_dicts: List[Mapping[str, Any]] = [] existing_stream_map = bulk_get_streams(user_profile.realm, stream_set) - member_creating_announcement_only_stream = False - for stream_dict in streams_raw: stream_name = stream_dict["name"] stream = existing_stream_map.get(stream_name.lower()) if stream is None: - # Non admins cannot create STREAM_POST_POLICY_ADMINS streams. - if ((stream_dict.get("stream_post_policy", False) == - Stream.STREAM_POST_POLICY_ADMINS) and not user_profile.is_realm_admin): - member_creating_announcement_only_stream = True - # New members cannot create STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS streams, - # unless they are admins who are also new members of the organization. - if ((stream_dict.get("stream_post_policy", False) == - Stream.STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS) and user_profile.is_new_member): - if not user_profile.is_realm_admin: - member_creating_announcement_only_stream = True missing_stream_dicts.append(stream_dict) else: existing_streams.append(stream) @@ -446,8 +434,6 @@ def list_to_streams(streams_raw: Iterable[Mapping[str, Any]], elif not autocreate: raise JsonableError(_("Stream(s) (%s) do not exist") % ", ".join( stream_dict["name"] for stream_dict in missing_stream_dicts)) - elif member_creating_announcement_only_stream: - raise JsonableError(_('User cannot create a stream with these settings.')) # We already filtered out existing streams, so dup_streams # will normally be an empty list below, but we protect against somebody diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 9512aac185..e3a4465f8e 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -65,6 +65,7 @@ from zerver.lib.actions import ( can_access_stream_user_ids, validate_user_access_to_subscribers_helper, get_average_weekly_stream_traffic, round_to_2_significant_digits, + do_change_stream_post_policy, ) from zerver.views.streams import ( @@ -2594,75 +2595,36 @@ class SubscriptionAPITest(ZulipTestCase): def test_subscribe_to_stream_post_policy_admins_stream(self) -> None: """ Members can subscribe to streams where only admins can post - but not create those streams, only realm admins can """ member = self.example_user("AARON") - result = self.common_subscribe_to_streams(member, ["general"]) + stream = self.make_stream('stream1') + do_change_stream_post_policy(stream, Stream.STREAM_POST_POLICY_ADMINS) + result = self.common_subscribe_to_streams(member, ["stream1"]) self.assert_json_success(result) - streams_raw = [{ - 'name': 'new_stream', - 'stream_post_policy': Stream.STREAM_POST_POLICY_ADMINS, - }] - with self.assertRaisesRegex( - JsonableError, "User cannot create a stream with these settings."): - list_to_streams(streams_raw, member, autocreate=True) - - admin = self.example_user("iago") - result = list_to_streams(streams_raw, admin, autocreate=True) - self.assert_length(result[0], 0) - self.assert_length(result[1], 1) - self.assertEqual(result[1][0].name, 'new_stream') - self.assertTrue(result[1][0].stream_post_policy == Stream.STREAM_POST_POLICY_ADMINS) + json = result.json() + self.assertEqual(json["subscribed"], {member.email: ["stream1"]}) + self.assertEqual(json["already_subscribed"], {}) def test_subscribe_to_stream_post_policy_restrict_new_members_stream(self) -> None: """ - New members can subscribe to streams where they can neither post - nor create those streams, only realm admins can can. + New members can subscribe to streams where they can not post """ new_member_email = self.nonreg_email('test') self.register(new_member_email, "test") new_member = self.nonreg_user('test') do_set_realm_property(new_member.realm, 'waiting_period_threshold', 10) - streams_raw = [{ - 'name': 'new_stream', - 'stream_post_policy': Stream.STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS, - }] - - # new members cannot create STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS streams. self.assertTrue(new_member.is_new_member) - with self.assertRaisesRegex( - JsonableError, "User cannot create a stream with these settings."): - list_to_streams(streams_raw, new_member, autocreate=True) - # Non admins can create STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS streams. - # However, they must not be a new user. - non_admin = self.example_user("AARON") - non_admin.date_joined = timezone_now() - timedelta(days=11) - non_admin.save() - self.assertFalse(non_admin.is_new_member) - self.assertFalse(non_admin.is_realm_admin) - result = list_to_streams(streams_raw, non_admin, autocreate=True) - self.assert_length(result[0], 0) - self.assert_length(result[1], 1) - self.assertEqual(result[1][0].name, 'new_stream') - self.assertTrue(result[1][0].stream_post_policy == Stream.STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS) + stream = self.make_stream('stream1') + do_change_stream_post_policy(stream, Stream.STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS) + result = self.common_subscribe_to_streams(new_member, ["stream1"]) + self.assert_json_success(result) - streams_raw = [{ - 'name': 'newer_stream', - 'stream_post_policy': Stream.STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS, - }] - - # Admins can create STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS streams, - # irrespective of whether they are new members or not. - admin = self.example_user("iago") - self.assertTrue(admin.is_realm_admin) - result = list_to_streams(streams_raw, admin, autocreate=True) - self.assert_length(result[0], 0) - self.assert_length(result[1], 1) - self.assertEqual(result[1][0].name, 'newer_stream') - self.assertTrue(result[1][0].stream_post_policy == Stream.STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS) + json = result.json() + self.assertEqual(json["subscribed"], {new_member.email: ["stream1"]}) + self.assertEqual(json["already_subscribed"], {}) def test_guest_user_subscribe(self) -> None: """Guest users cannot subscribe themselves to anything"""