streams: Allow setting can_remove_subscribers_group_id while creating streams.

This commit adds API support to set can_remove_subscribers_group setting
when creating streams.

api_docs/changelog.md

@@ -25,6 +25,10 @@ format used by the Zulip server that they are interacting with.
 * [`PATCH /streams/{stream_id}`](/api/update-stream): Added
   `can_remove_subscribers_group_id` parameter to support
   changing `can_remove_subscribers_group` setting.
+* [`POST /users/me/subscriptions`](/api/subscribe): Added
+  `can_remove_subscribers_group_id` parameter to set
+  `can_remove_subscribers_group` setting while creating
+  streams.
 
 **Feature level 160**
 

version.py

@@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
 # Changes should be accompanied by documentation explaining what the
 # new level means in api_docs/changelog.md, as well as "**Changes**"
 # entries in the endpoint's documentation in `zulip.yaml`.
-API_FEATURE_LEVEL = 159
+API_FEATURE_LEVEL = 161
 
 # Bump the minor PROVISION_VERSION to indicate that folks should provision
 # only when going from an old version of the code to a newer version. Bump

zerver/lib/streams.py

@@ -62,6 +62,7 @@ class StreamDict(TypedDict, total=False):
     stream_post_policy: int
     history_public_to_subscribers: Optional[bool]
     message_retention_days: Optional[int]
+    can_remove_subscribers_group: Optional[UserGroup]
 
 
 def get_stream_permission_policy_name(
@@ -125,15 +126,19 @@ def create_stream_if_needed(
     history_public_to_subscribers: Optional[bool] = None,
     stream_description: str = "",
     message_retention_days: Optional[int] = None,
+    can_remove_subscribers_group: Optional[UserGroup] = None,
     acting_user: Optional[UserProfile] = None,
 ) -> Tuple[Stream, bool]:
     history_public_to_subscribers = get_default_value_for_history_public_to_subscribers(
         realm, invite_only, history_public_to_subscribers
     )
-    administrators_user_group = UserGroup.objects.get(
-        name=UserGroup.ADMINISTRATORS_GROUP_NAME, is_system_group=True, realm=realm
-    )
 
+    if can_remove_subscribers_group is None:
+        can_remove_subscribers_group = UserGroup.objects.get(
+            name=UserGroup.ADMINISTRATORS_GROUP_NAME, is_system_group=True, realm=realm
+        )
+
+    assert can_remove_subscribers_group is not None
     with transaction.atomic():
         (stream, created) = Stream.objects.get_or_create(
             realm=realm,
@@ -147,7 +152,7 @@ def create_stream_if_needed(
                 history_public_to_subscribers=history_public_to_subscribers,
                 is_in_zephyr_realm=realm.is_zephyr_mirror_realm,
                 message_retention_days=message_retention_days,
-                can_remove_subscribers_group=administrators_user_group,
+                can_remove_subscribers_group=can_remove_subscribers_group,
             ),
         )
 
@@ -196,6 +201,7 @@ def create_streams_if_needed(
             history_public_to_subscribers=stream_dict.get("history_public_to_subscribers"),
             stream_description=stream_dict.get("description", ""),
             message_retention_days=stream_dict.get("message_retention_days", None),
+            can_remove_subscribers_group=stream_dict.get("can_remove_subscribers_group", None),
             acting_user=acting_user,
         )
 

zerver/openapi/zulip.yaml

@@ -7855,6 +7855,7 @@ paths:
         - $ref: "#/components/parameters/HistoryPublicToSubscribers"
         - $ref: "#/components/parameters/StreamPostPolicy"
         - $ref: "#/components/parameters/MessageRetentionDays"
+        - $ref: "#/components/parameters/CanRemoveSubscribersGroupId"
       responses:
         "200":
           description: Success.

zerver/tests/test_subs.py

@@ -269,6 +269,9 @@ class TestCreateStreams(ZulipTestCase):
         self.assertTrue(self.example_user("desdemona").id in events[0]["users"])
         self.assertEqual(events[0]["event"]["streams"][0]["name"], "Private stream")
 
+        moderators_system_group = UserGroup.objects.get(
+            name="@role:moderators", realm=realm, is_system_group=True
+        )
         new_streams, existing_streams = create_streams_if_needed(
             realm,
             [
@@ -278,6 +281,7 @@ class TestCreateStreams(ZulipTestCase):
                     "invite_only": True,
                     "stream_post_policy": Stream.STREAM_POST_POLICY_ADMINS,
                     "message_retention_days": -1,
+                    "can_remove_subscribers_group": moderators_system_group,
                 }
                 for (stream_name, stream_description) in zip(stream_names, stream_descriptions)
             ],
@@ -294,6 +298,7 @@ class TestCreateStreams(ZulipTestCase):
             self.assertTrue(stream.invite_only)
             self.assertTrue(stream.stream_post_policy == Stream.STREAM_POST_POLICY_ADMINS)
             self.assertTrue(stream.message_retention_days == -1)
+            self.assertEqual(stream.can_remove_subscribers_group.id, moderators_system_group.id)
 
         new_streams, existing_streams = create_streams_if_needed(
             realm,
@@ -466,6 +471,78 @@ class TestCreateStreams(ZulipTestCase):
         # But it should be marked as read for Iago, the stream creator.
         self.assert_length(iago_unread_messages, 0)
 
+    def test_can_remove_subscribers_group_on_stream_creation(self) -> None:
+        user = self.example_user("hamlet")
+        realm = user.realm
+        self.login_user(user)
+        moderators_system_group = UserGroup.objects.get(
+            name="@role:moderators", realm=realm, is_system_group=True
+        )
+        admins_system_group = UserGroup.objects.get(
+            name="@role:administrators", realm=realm, is_system_group=True
+        )
+
+        post_data = {
+            "subscriptions": orjson.dumps(
+                [{"name": "new_stream1", "description": "First new stream"}]
+            ).decode(),
+            "can_remove_subscribers_group_id": orjson.dumps(moderators_system_group.id).decode(),
+        }
+        result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
+        self.assert_json_success(result)
+        stream = get_stream("new_stream1", realm)
+        self.assertEqual(stream.can_remove_subscribers_group.id, moderators_system_group.id)
+
+        post_data = {
+            "subscriptions": orjson.dumps(
+                [{"name": "new_stream2", "description": "Second new stream"}]
+            ).decode(),
+        }
+        result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
+        self.assert_json_success(result)
+        stream = get_stream("new_stream2", realm)
+        self.assertEqual(stream.can_remove_subscribers_group.id, admins_system_group.id)
+
+        hamletcharacters_group = UserGroup.objects.get(name="hamletcharacters", realm=realm)
+        post_data = {
+            "subscriptions": orjson.dumps(
+                [{"name": "new_stream3", "description": "Third new stream"}]
+            ).decode(),
+            "can_remove_subscribers_group_id": orjson.dumps(hamletcharacters_group.id).decode(),
+        }
+        result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
+        self.assert_json_error(
+            result, "'can_remove_subscribers_group' must be a system user group."
+        )
+
+        internet_group = UserGroup.objects.get(
+            name="@role:internet", is_system_group=True, realm=realm
+        )
+        post_data = {
+            "subscriptions": orjson.dumps(
+                [{"name": "new_stream3", "description": "Third new stream"}]
+            ).decode(),
+            "can_remove_subscribers_group_id": orjson.dumps(internet_group.id).decode(),
+        }
+        result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
+        self.assert_json_error(
+            result,
+            "'can_remove_subscribers_group' setting cannot be set to '@role:internet' group.",
+        )
+
+        owners_group = UserGroup.objects.get(name="@role:owners", is_system_group=True, realm=realm)
+        post_data = {
+            "subscriptions": orjson.dumps(
+                [{"name": "new_stream3", "description": "Third new stream"}]
+            ).decode(),
+            "can_remove_subscribers_group_id": orjson.dumps(owners_group.id).decode(),
+        }
+        result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
+        self.assert_json_error(
+            result,
+            "'can_remove_subscribers_group' setting cannot be set to '@role:owners' group.",
+        )
+
 
 class RecipientTest(ZulipTestCase):
     def test_recipient(self) -> None:
@@ -4325,7 +4402,7 @@ class SubscriptionAPITest(ZulipTestCase):
 
         # Now add ourselves
         with self.tornado_redirected_to_list(events, expected_num_events=2):
-            with self.assert_database_query_count(12):
+            with self.assert_database_query_count(13):
                 self.common_subscribe_to_streams(
                     self.test_user,
                     streams_to_sub,
@@ -4650,7 +4727,7 @@ class SubscriptionAPITest(ZulipTestCase):
         # any tornado subscription events
         events: List[Mapping[str, Any]] = []
         with self.tornado_redirected_to_list(events, expected_num_events=0):
-            with self.assert_database_query_count(4):
+            with self.assert_database_query_count(5):
                 self.common_subscribe_to_streams(
                     mit_user,
                     stream_names,
@@ -4698,7 +4775,7 @@ class SubscriptionAPITest(ZulipTestCase):
 
         test_user_ids = [user.id for user in test_users]
 
-        with self.assert_database_query_count(19):
+        with self.assert_database_query_count(20):
             with cache_tries_captured() as cache_tries:
                 with mock.patch("zerver.views.streams.send_messages_for_new_subscribers"):
                     self.common_subscribe_to_streams(
@@ -5611,7 +5688,7 @@ class GetSubscribersTest(ZulipTestCase):
             polonius.id,
         ]
 
-        with self.assert_database_query_count(46):
+        with self.assert_database_query_count(47):
             self.common_subscribe_to_streams(
                 self.user_profile,
                 streams,

zerver/views/streams.py

@@ -99,6 +99,7 @@ from zerver.lib.validator import (
 from zerver.models import (
     Realm,
     Stream,
+    UserGroup,
     UserMessage,
     UserProfile,
     get_active_user,
@@ -555,6 +556,7 @@ def add_subscriptions_backend(
     message_retention_days: Union[str, int] = REQ(
         json_validator=check_string_or_int, default=RETENTION_DEFAULT
     ),
+    can_remove_subscribers_group_id: Optional[int] = REQ(json_validator=check_int, default=None),
     announce: bool = REQ(json_validator=check_bool, default=False),
     principals: Union[Sequence[str], Sequence[int]] = REQ(
         json_validator=check_principals,
@@ -565,6 +567,19 @@ def add_subscriptions_backend(
     realm = user_profile.realm
     stream_dicts = []
     color_map = {}
+
+    if can_remove_subscribers_group_id is not None:
+        can_remove_subscribers_group = access_user_group_for_setting(
+            can_remove_subscribers_group_id,
+            user_profile,
+            setting_name="can_remove_subscribers_group",
+            require_system_group=True,
+        )
+    else:
+        can_remove_subscribers_group = UserGroup.objects.get(
+            name="@role:administrators", realm=user_profile.realm, is_system_group=True
+        )
+
     for stream_dict in streams_raw:
         # 'color' field is optional
         # check for its presence in the streams_raw first
@@ -585,6 +600,7 @@ def add_subscriptions_backend(
         stream_dict_copy["message_retention_days"] = parse_message_retention_days(
             message_retention_days, Stream.MESSAGE_RETENTION_SPECIAL_VALUES_MAP
         )
+        stream_dict_copy["can_remove_subscribers_group"] = can_remove_subscribers_group
 
         stream_dicts.append(stream_dict_copy)