streams: Refactor code to set group-based stream settings.

We add stream_permission_group_settings object which is
similar to property_types framework used for realm settings.

This commit also adds GroupPermissionSetting dataclass for
defining settings inside stream_permission_group_settings.

We add "do_change_stream_group_based_setting" function which
is called in loop to update all the group-based stream settings
and it is now used to update 'can_remove_subscribers_group'
setting instead of "do_change_can_remove_subscribers_group".

We also change the variable name for event_type field of
RealmAuditLog objects to STREAM_GROUP_BASED_SETTING_CHANGED
since this will be used for all group-based stream settings.

'property' field is also added to extra_data field to identify
the setting for which RealmAuditLog object was created.

We will add a migration in further commits which will add the
property field to existing RealmAuditLog objects created for
changing can_remove_subscribers_group setting.

zerver/actions/streams.py

@@ -1317,33 +1317,39 @@ def do_change_stream_message_retention_days(
     )
 
 
-def do_change_can_remove_subscribers_group(
-    stream: Stream, user_group: UserGroup, *, acting_user: Optional[UserProfile] = None
+def do_change_stream_group_based_setting(
+    stream: Stream,
+    setting_name: str,
+    user_group: UserGroup,
+    *,
+    acting_user: Optional[UserProfile] = None,
 ) -> None:
-    old_user_group = stream.can_remove_subscribers_group
+    old_user_group = getattr(stream, setting_name)
     old_user_group_id = None
     if old_user_group is not None:
         old_user_group_id = old_user_group.id
 
-    stream.can_remove_subscribers_group = user_group
+    setattr(stream, setting_name, user_group)
     stream.save()
+
     RealmAuditLog.objects.create(
         realm=stream.realm,
         acting_user=acting_user,
         modified_stream=stream,
-        event_type=RealmAuditLog.STREAM_CAN_REMOVE_SUBSCRIBERS_GROUP_CHANGED,
+        event_type=RealmAuditLog.STREAM_GROUP_BASED_SETTING_CHANGED,
         event_time=timezone_now(),
         extra_data=orjson.dumps(
             {
                 RealmAuditLog.OLD_VALUE: old_user_group_id,
                 RealmAuditLog.NEW_VALUE: user_group.id,
+                "property": setting_name,
             }
         ).decode(),
     )
     event = dict(
         op="update",
         type="stream",
-        property="can_remove_subscribers_group_id",
+        property=setting_name + "_id",
         value=user_group.id,
         stream_id=stream.id,
         name=stream.name,

zerver/lib/types.py

@@ -272,3 +272,10 @@ class RealmPlaygroundDict(TypedDict):
     name: str
     pygments_language: str
     url_prefix: str
+
+
+@dataclass
+class GroupPermissionSetting:
+    require_system_group: bool
+    allow_internet_group: bool
+    allow_owners_group: bool

zerver/models.py

@@ -91,6 +91,7 @@ from zerver.lib.types import (
     ExtendedFieldElement,
     ExtendedValidator,
     FieldElement,
+    GroupPermissionSetting,
     LinkifierDict,
     ProfileData,
     ProfileDataElementBase,
@@ -2541,6 +2542,14 @@ class Stream(models.Model):
     # stream based on what messages they have cached.
     first_message_id = models.IntegerField(null=True, db_index=True)
 
+    stream_permission_group_settings = {
+        "can_remove_subscribers_group": GroupPermissionSetting(
+            require_system_group=True,
+            allow_internet_group=False,
+            allow_owners_group=False,
+        ),
+    }
+
     def __str__(self) -> str:
         return f"<Stream: {self.name}>"
 
@@ -4382,7 +4391,7 @@ class AbstractRealmAuditLog(models.Model):
     STREAM_REACTIVATED = 604
     STREAM_MESSAGE_RETENTION_DAYS_CHANGED = 605
     STREAM_PROPERTY_CHANGED = 607
-    STREAM_CAN_REMOVE_SUBSCRIBERS_GROUP_CHANGED = 608
+    STREAM_GROUP_BASED_SETTING_CHANGED = 608
 
     # The following values are only for RemoteZulipServerAuditLog
     # Values should be exactly 10000 greater than the corresponding

zerver/tests/test_events.py

@@ -78,8 +78,8 @@ from zerver.actions.realm_settings import (
 from zerver.actions.streams import (
     bulk_add_subscriptions,
     bulk_remove_subscriptions,
-    do_change_can_remove_subscribers_group,
     do_change_stream_description,
+    do_change_stream_group_based_setting,
     do_change_stream_message_retention_days,
     do_change_stream_permission,
     do_change_stream_post_policy,
@@ -3018,8 +3018,11 @@ class SubscribeActionTest(BaseAction):
             is_system_group=True,
             realm=self.user_profile.realm,
         )
-        action = lambda: do_change_can_remove_subscribers_group(
-            stream, moderators_group, acting_user=self.example_user("hamlet")
+        action = lambda: do_change_stream_group_based_setting(
+            stream,
+            "can_remove_subscribers_group",
+            moderators_group,
+            acting_user=self.example_user("hamlet"),
         )
         events = self.verify_action(action, include_subscribers=include_subscribers, num_events=1)
         check_stream_update("events[0]", events[0])

zerver/tests/test_subs.py

@@ -29,7 +29,7 @@ from zerver.actions.realm_settings import do_change_realm_plan_type, do_set_real
 from zerver.actions.streams import (
     bulk_add_subscriptions,
     bulk_remove_subscriptions,
-    do_change_can_remove_subscribers_group,
+    do_change_stream_group_based_setting,
     do_change_stream_post_policy,
     do_deactivate_stream,
 )
@@ -2585,8 +2585,11 @@ class StreamAdminTest(ZulipTestCase):
         ) -> None:
             self.login_user(user)
             self.subscribe(cordelia, stream.name)
-            do_change_can_remove_subscribers_group(
-                stream, can_remove_subscribers_group, acting_user=None
+            do_change_stream_group_based_setting(
+                stream,
+                "can_remove_subscribers_group",
+                can_remove_subscribers_group,
+                acting_user=None,
             )
             result = self.client_delete(
                 "/json/users/me/subscriptions",

zerver/views/streams.py

@@ -30,8 +30,8 @@ from zerver.actions.message_send import (
 from zerver.actions.streams import (
     bulk_add_subscriptions,
     bulk_remove_subscriptions,
-    do_change_can_remove_subscribers_group,
     do_change_stream_description,
+    do_change_stream_group_based_setting,
     do_change_stream_message_retention_days,
     do_change_stream_permission,
     do_change_stream_post_policy,
@@ -384,19 +384,32 @@ def update_stream_backend(
     if stream_post_policy is not None:
         do_change_stream_post_policy(stream, stream_post_policy, acting_user=user_profile)
 
-    if can_remove_subscribers_group_id is not None:
-        if sub is None and stream.invite_only:
-            # Admins cannot change this setting for unsubscribed private streams.
-            raise JsonableError(_("Invalid stream ID"))
+    for setting_name, permissions_configuration in Stream.stream_permission_group_settings.items():
+        request_settings_dict = locals()
+        setting_group_id_name = setting_name + "_id"
 
-        user_group = access_user_group_for_setting(
-            can_remove_subscribers_group_id,
-            user_profile,
-            setting_name="can_remove_subscribers_group",
-            require_system_group=True,
-        )
+        if setting_group_id_name not in request_settings_dict:  # nocoverage
+            continue
 
-        do_change_can_remove_subscribers_group(stream, user_group, acting_user=user_profile)
+        if request_settings_dict[setting_group_id_name] is not None and request_settings_dict[
+            setting_group_id_name
+        ] != getattr(stream, setting_name):
+            if sub is None and stream.invite_only:
+                # Admins cannot change this setting for unsubscribed private streams.
+                raise JsonableError(_("Invalid stream ID"))
+
+            user_group_id = request_settings_dict[setting_group_id_name]
+            user_group = access_user_group_for_setting(
+                user_group_id,
+                user_profile,
+                setting_name=setting_name,
+                require_system_group=permissions_configuration.require_system_group,
+                allow_internet_group=permissions_configuration.allow_internet_group,
+                allow_owners_group=permissions_configuration.allow_owners_group,
+            )
+            do_change_stream_group_based_setting(
+                stream, setting_name, user_group, acting_user=user_profile
+            )
 
     return json_success(request)