From 947658def48d9d4d7b83d0e8fed84eb79dc48829 Mon Sep 17 00:00:00 2001 From: apoorvapendse Date: Sat, 12 Jul 2025 13:21:15 +0530 Subject: [PATCH] streams: Extract `topics_policy` validation. Fixes: Point 3 of https://github.com/zulip/zulip/pull/33405#issuecomment-3064452310. Signed-off-by: apoorvapendse --- zerver/lib/streams.py | 29 +++++++++++++++++++++++++++++ zerver/views/streams.py | 31 +++++++------------------------ 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index 5f64a1876d..89ed31122d 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -12,6 +12,7 @@ from django.utils.translation import gettext as _ from zerver.lib.default_streams import get_default_stream_ids_for_realm from zerver.lib.exceptions import ( CannotAdministerChannelError, + CannotSetTopicsPolicyError, IncompatibleParametersError, JsonableError, OrganizationOwnerRequiredError, @@ -24,6 +25,7 @@ from zerver.lib.stream_subscription import ( from zerver.lib.stream_traffic import get_average_weekly_stream_traffic, get_streams_traffic from zerver.lib.string_validation import check_stream_name from zerver.lib.timestamp import datetime_to_timestamp +from zerver.lib.topic import get_topic_display_name from zerver.lib.types import APIStreamDict, UserGroupMembersData from zerver.lib.user_groups import ( UserGroupMembershipDetails, @@ -123,6 +125,33 @@ def get_stream_topics_policy(realm: Realm, stream: Stream) -> int: return stream.topics_policy +def validate_topics_policy( + topics_policy: str | None, + user_profile: UserProfile, + # Pass None when creating a channel and the channel being edited when editing a channel's settings + stream: Stream | None = None, +) -> StreamTopicsPolicyEnum | None: + if topics_policy is not None and isinstance(topics_policy, StreamTopicsPolicyEnum): + if ( + topics_policy != StreamTopicsPolicyEnum.inherit + and not user_profile.can_set_topics_policy() + ): + raise JsonableError(_("Insufficient permission")) + + # Cannot set `topics_policy` to `empty_topic_only` when there are messages + # in non-empty topics in the current channel. + if ( + stream is not None + and topics_policy == StreamTopicsPolicyEnum.empty_topic_only + and channel_has_named_topics(stream) + ): + raise CannotSetTopicsPolicyError( + get_topic_display_name("", user_profile.default_language) + ) + return topics_policy + return None + + def get_default_value_for_history_public_to_subscribers( realm: Realm, invite_only: bool, diff --git a/zerver/views/streams.py b/zerver/views/streams.py index f612f849dd..3fe774617d 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -57,7 +57,6 @@ from zerver.lib.default_streams import get_default_stream_ids_for_realm from zerver.lib.email_mirror_helpers import encode_email_address, get_channel_email_token from zerver.lib.exceptions import ( CannotManageDefaultChannelError, - CannotSetTopicsPolicyError, JsonableError, OrganizationOwnerRequiredError, ) @@ -75,7 +74,6 @@ from zerver.lib.streams import ( access_stream_for_delete_or_update_requiring_metadata_access, access_web_public_stream, channel_events_topic_name, - channel_has_named_topics, check_stream_name_available, do_get_streams, filter_stream_authorization_for_adding_subscribers, @@ -85,10 +83,10 @@ from zerver.lib.streams import ( list_to_streams, stream_to_dict, user_has_content_access, + validate_topics_policy, ) from zerver.lib.subscription_info import gather_subscriptions from zerver.lib.topic import ( - get_topic_display_name, get_topic_history_for_public_stream, get_topic_history_for_stream, maybe_rename_general_chat_to_empty_topic, @@ -398,20 +396,9 @@ def update_stream_backend( if not user_profile.can_create_web_public_streams(): raise JsonableError(_("Insufficient permission")) - if topics_policy is not None and isinstance(topics_policy, StreamTopicsPolicyEnum): - if not user_profile.can_set_topics_policy(): - raise JsonableError(_("Insufficient permission")) - - # Cannot set `topics_policy` to `empty_topic_only` when there are messages - # in non-empty topics in the current channel. - if topics_policy == StreamTopicsPolicyEnum.empty_topic_only and channel_has_named_topics( - stream - ): - raise CannotSetTopicsPolicyError( - get_topic_display_name("", user_profile.default_language) - ) - - do_set_stream_property(stream, "topics_policy", topics_policy.value, user_profile) + validated_topics_policy = validate_topics_policy(topics_policy, user_profile, stream) + if validated_topics_policy is not None: + do_set_stream_property(stream, "topics_policy", validated_topics_policy.value, user_profile) if ( is_private is not None @@ -768,13 +755,9 @@ def add_subscriptions_backend( stream_dict_copy["message_retention_days"] = parse_message_retention_days( message_retention_days, Stream.MESSAGE_RETENTION_SPECIAL_VALUES_MAP ) - if topics_policy is not None and isinstance(topics_policy, StreamTopicsPolicyEnum): - if ( - topics_policy != StreamTopicsPolicyEnum.inherit - and not user_profile.can_set_topics_policy() - ): - raise JsonableError(_("Insufficient permission")) - stream_dict_copy["topics_policy"] = topics_policy.value + validated_topics_policy = validate_topics_policy(topics_policy, user_profile) + if validated_topics_policy is not None: + stream_dict_copy["topics_policy"] = validated_topics_policy.value stream_dict_copy["can_add_subscribers_group"] = group_settings_map[ "can_add_subscribers_group" ]