From 8d6a8f8833ac85adb544928359a2874f7f5b49f3 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Sun, 15 Dec 2024 10:02:32 +0530 Subject: [PATCH] streams: Remove API support for changing stream_post_policy. --- api_docs/changelog.md | 4 + zerver/actions/streams.py | 76 ------------------- zerver/openapi/python_examples.py | 1 - zerver/openapi/zulip.yaml | 57 ++++---------- zerver/tests/test_subs.py | 122 ------------------------------ zerver/views/streams.py | 25 +----- 6 files changed, 19 insertions(+), 266 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index c5eef718cc..88888a2b2c 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -40,6 +40,10 @@ format used by the Zulip server that they are interacting with. this backwards-compatible `stream_post_policy` value now contains the superset of the true value that best approximates the actual permission setting. +* [`POST /users/me/subscriptions`](/api/subscribe), + [`PATCH /streams/{stream_id}`](/api/update-stream): Removed + `stream_post_policy` and `is_announcement_only` properties, as the permission + to post in the channel is now controlled by `can_send_message_group` setting. **Feature level 332** diff --git a/zerver/actions/streams.py b/zerver/actions/streams.py index f50e88ddba..e65d54127c 100644 --- a/zerver/actions/streams.py +++ b/zerver/actions/streams.py @@ -1355,82 +1355,6 @@ def send_stream_posting_permission_update_notification( ) -def send_change_stream_post_policy_notification( - stream: Stream, *, old_post_policy: int, new_post_policy: int, acting_user: UserProfile -) -> None: - sender = get_system_bot(settings.NOTIFICATION_BOT, acting_user.realm_id) - user_mention = silent_mention_syntax_for_user(acting_user) - - with override_language(stream.realm.default_language): - notification_string = _( - "{user} changed the [posting permissions]({help_link}) " - "for this channel:\n\n" - "* **Old permissions**: {old_policy}.\n" - "* **New permissions**: {new_policy}.\n" - ) - notification_string = notification_string.format( - user=user_mention, - help_link="/help/channel-posting-policy", - old_policy=Stream.POST_POLICIES[old_post_policy], - new_policy=Stream.POST_POLICIES[new_post_policy], - ) - internal_send_stream_message( - sender, stream, str(Realm.STREAM_EVENTS_NOTIFICATION_TOPIC_NAME), notification_string - ) - - -@transaction.atomic(durable=True) -def do_change_stream_post_policy( - stream: Stream, stream_post_policy: int, *, acting_user: UserProfile -) -> None: - old_post_policy = stream.stream_post_policy - stream.stream_post_policy = stream_post_policy - stream.save(update_fields=["stream_post_policy"]) - RealmAuditLog.objects.create( - realm=stream.realm, - acting_user=acting_user, - modified_stream=stream, - event_type=AuditLogEventType.CHANNEL_PROPERTY_CHANGED, - event_time=timezone_now(), - extra_data={ - RealmAuditLog.OLD_VALUE: old_post_policy, - RealmAuditLog.NEW_VALUE: stream_post_policy, - "property": "stream_post_policy", - }, - ) - - event = dict( - op="update", - type="stream", - property="stream_post_policy", - value=stream_post_policy, - stream_id=stream.id, - name=stream.name, - ) - send_event_on_commit(stream.realm, event, can_access_stream_user_ids(stream)) - - # Backwards-compatibility code: We removed the - # is_announcement_only property in early 2020, but we send a - # duplicate event for legacy mobile clients that might want the - # data. - event = dict( - op="update", - type="stream", - property="is_announcement_only", - value=stream.stream_post_policy == Stream.STREAM_POST_POLICY_ADMINS, - stream_id=stream.id, - name=stream.name, - ) - send_event_on_commit(stream.realm, event, can_access_stream_user_ids(stream)) - - send_change_stream_post_policy_notification( - stream, - old_post_policy=old_post_policy, - new_post_policy=stream_post_policy, - acting_user=acting_user, - ) - - @transaction.atomic(durable=True) def do_rename_stream(stream: Stream, new_name: str, user_profile: UserProfile) -> None: old_name = stream.name diff --git a/zerver/openapi/python_examples.py b/zerver/openapi/python_examples.py index 348e1096b6..0f87cdfec0 100644 --- a/zerver/openapi/python_examples.py +++ b/zerver/openapi/python_examples.py @@ -804,7 +804,6 @@ def update_stream(client: Client, stream_id: int) -> None: # Update settings for the channel with a given ID. request = { "stream_id": stream_id, - "stream_post_policy": 2, "is_private": True, } result = client.update_stream(request) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 4e8ea93212..924711018d 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -10214,13 +10214,16 @@ paths: Note that the ability to subscribe oneself and/or other users to a specified channel depends on the [channel's privacy settings](/help/channel-permissions). - **Changes**: Before Zulip 8.0 (feature level 208), if a user - specified by the [`principals`][principals-param] parameter was - a deactivated user, or did not exist, then an HTTP status code - of 403 was returned with `code: "UNAUTHORIZED_PRINCIPAL"` in the - error response. As of this feature level, an HTTP status code of - 400 is returned with `code: "BAD_REQUEST"` in the error response - for these cases. + **Changes**: Removed `stream_post_policy` and `is_announcement_only` + parameters in Zulip 10.0 (feature level 333), as permission to post + in the channel is now controlled by `can_send_message_group`. + + Before Zulip 8.0 (feature level 208), if a user specified by the + [`principals`][principals-param] parameter was a deactivated user, + or did not exist, then an HTTP status code of 403 was returned with + `code: "UNAUTHORIZED_PRINCIPAL"` in the error response. As of this + feature level, an HTTP status code of 400 is returned with + `code: "BAD_REQUEST"` in the error response for these cases. [principals-param]: /api/subscribe#parameter-principals x-curl-examples-parameters: @@ -10347,8 +10350,6 @@ paths: example: true history_public_to_subscribers: $ref: "#/components/schemas/HistoryPublicToSubscribers" - stream_post_policy: - $ref: "#/components/schemas/ChannelPostPolicy" message_retention_days: $ref: "#/components/schemas/MessageRetentionDays" can_remove_subscribers_group: @@ -10376,8 +10377,6 @@ paths: contentType: application/json history_public_to_subscribers: contentType: application/json - stream_post_policy: - contentType: application/json can_remove_subscribers_group: contentType: application/json can_administer_channel_group: @@ -19970,6 +19969,10 @@ paths: Note that an organization administrator's ability to change a [private channel's permissions](/help/channel-permissions#private-channels) depends on them being subscribed to the channel. + + **Changes**: Removed `stream_post_policy` and `is_announcement_only` + parameters in Zulip 10.0 (feature level 333), as permission to post + in the channel is now controlled by `can_send_message_group`. x-curl-examples-parameters: oneOf: - type: include @@ -20017,15 +20020,6 @@ paths: Change whether the channel is a private channel. type: boolean example: true - is_announcement_only: - deprecated: true - description: | - Whether the channel is limited to announcements. - - **Changes**: Deprecated in Zulip 3.0 (feature level 1). Clients - should use `stream_post_policy` instead. - type: boolean - example: true is_web_public: description: | Change whether the channel is a web-public channel. @@ -20070,8 +20064,6 @@ paths: could only be changed using the [dedicated API endpoint](/api/add-default-stream). type: boolean example: false - stream_post_policy: - $ref: "#/components/schemas/ChannelPostPolicy" message_retention_days: $ref: "#/components/schemas/MessageRetentionDays" can_remove_subscribers_group: @@ -20192,16 +20184,12 @@ paths: encoding: is_private: contentType: application/json - is_announcement_only: - contentType: application/json is_web_public: contentType: application/json history_public_to_subscribers: contentType: application/json is_default_stream: contentType: application/json - stream_post_policy: - contentType: application/json can_remove_subscribers_group: contentType: application/json can_administer_channel_group: @@ -24677,23 +24665,6 @@ components: throw an error. type: string example: Hello - ChannelPostPolicy: - description: | - [Policy][permission-level] for which users can post messages to the channel. - - - 1 = Any user can post. - - 2 = Only administrators can post. - - 3 = Only [full members][calc-full-member] can post. - - 4 = Only moderators can post. - - **Changes**: New in Zulip 3.0 (feature level 1), replacing the previous - `is_announcement_only` boolean. - - [permission-level]: /api/roles-and-permissions#permission-levels - [calc-full-member]: /api/roles-and-permissions#determining-if-a-user-is-a-full-member - type: integer - default: 1 - example: 2 HistoryPublicToSubscribers: description: | Whether the channel's message history should be available to diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index be0a0aec01..78cbe06bf7 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -493,7 +493,6 @@ class TestCreateStreams(ZulipTestCase): "invite_only": "false", "announce": "true", "principals": orjson.dumps([iago.id, aaron.id, cordelia.id, hamlet.id]).decode(), - "stream_post_policy": "1", } response = self.client_post("/json/users/me/subscriptions", data) @@ -2231,127 +2230,6 @@ class StreamAdminTest(ZulipTestCase): ) self.assert_json_error(result, "You do not have permission to administer this channel.") - def test_change_to_stream_post_policy_admins(self) -> None: - user_profile = self.example_user("hamlet") - self.login_user(user_profile) - - self.subscribe(user_profile, "stream_name1") - do_change_user_role(user_profile, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None) - - stream_id = get_stream("stream_name1", user_profile.realm).id - result = self.client_patch( - f"/json/streams/{stream_id}", {"is_announcement_only": orjson.dumps(True).decode()} - ) - self.assert_json_success(result) - stream = get_stream("stream_name1", user_profile.realm) - self.assertEqual(stream.stream_post_policy, Stream.STREAM_POST_POLICY_ADMINS) - - messages = get_topic_messages(user_profile, stream, "channel events") - expected_notification = ( - f"@_**{user_profile.full_name}|{user_profile.id}** changed the " - "[posting permissions](/help/channel-posting-policy) for this channel:\n\n" - "* **Old permissions**: All channel members can post.\n" - "* **New permissions**: Only organization administrators can post." - ) - self.assertEqual(messages[-1].content, expected_notification) - - realm_audit_log = RealmAuditLog.objects.filter( - event_type=AuditLogEventType.CHANNEL_PROPERTY_CHANGED, - modified_stream=stream, - ).last() - assert realm_audit_log is not None - expected_extra_data = { - RealmAuditLog.OLD_VALUE: Stream.STREAM_POST_POLICY_EVERYONE, - RealmAuditLog.NEW_VALUE: Stream.STREAM_POST_POLICY_ADMINS, - "property": "stream_post_policy", - } - self.assertEqual(realm_audit_log.extra_data, expected_extra_data) - - def test_change_stream_post_policy_requires_admin(self) -> None: - user_profile = self.example_user("hamlet") - self.login_user(user_profile) - - self.make_stream("stream_name1") - stream = self.subscribe(user_profile, "stream_name1") - - do_change_user_role(user_profile, UserProfile.ROLE_MEMBER, acting_user=None) - - do_set_realm_property(user_profile.realm, "waiting_period_threshold", 10, acting_user=None) - - def test_non_admin(how_old: int, is_new: bool, policy: int) -> None: - user_profile.date_joined = timezone_now() - timedelta(days=how_old) - user_profile.save() - self.assertEqual(user_profile.is_provisional_member, is_new) - stream = get_stream("stream_name1", user_profile.realm) - self.assertFalse(is_user_in_group(stream.can_administer_channel_group, user_profile)) - result = self.client_patch( - f"/json/streams/{stream.id}", {"stream_post_policy": orjson.dumps(policy).decode()} - ) - self.assert_json_error(result, "You do not have permission to administer this channel.") - - policies = [ - Stream.STREAM_POST_POLICY_ADMINS, - Stream.STREAM_POST_POLICY_MODERATORS, - Stream.STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS, - ] - - for policy in policies: - test_non_admin(how_old=15, is_new=False, policy=policy) - test_non_admin(how_old=5, is_new=True, policy=policy) - - do_change_user_role(user_profile, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None) - - for policy in policies: - stream = get_stream("stream_name1", user_profile.realm) - old_post_policy = stream.stream_post_policy - result = self.client_patch( - f"/json/streams/{stream.id}", {"stream_post_policy": orjson.dumps(policy).decode()} - ) - self.assert_json_success(result) - stream = get_stream("stream_name1", user_profile.realm) - self.assertEqual(stream.stream_post_policy, policy) - - messages = get_topic_messages(user_profile, stream, "channel events") - expected_notification = ( - f"@_**{user_profile.full_name}|{user_profile.id}** changed the " - "[posting permissions](/help/channel-posting-policy) for this channel:\n\n" - f"* **Old permissions**: {Stream.POST_POLICIES[old_post_policy]}.\n" - f"* **New permissions**: {Stream.POST_POLICIES[policy]}." - ) - - self.assertEqual(messages[-1].content, expected_notification) - - realm_audit_log = RealmAuditLog.objects.filter( - event_type=AuditLogEventType.CHANNEL_PROPERTY_CHANGED, - modified_stream=stream, - ).last() - assert realm_audit_log is not None - expected_extra_data = { - RealmAuditLog.OLD_VALUE: old_post_policy, - RealmAuditLog.NEW_VALUE: policy, - "property": "stream_post_policy", - } - self.assertEqual(realm_audit_log.extra_data, expected_extra_data) - - # Test non-admin should be able to change policy if they are - # part of can_administer_channel_group - do_change_user_role(user_profile, UserProfile.ROLE_MEMBER, acting_user=None) - user_profile_group = check_add_user_group( - user_profile.realm, "user_profile_group", [user_profile], acting_user=user_profile - ) - do_change_stream_group_based_setting( - stream, - "can_administer_channel_group", - user_profile_group, - acting_user=None, - ) - stream = get_stream("stream_name1", user_profile.realm) - old_post_policy = stream.stream_post_policy - result = self.client_patch( - f"/json/streams/{stream.id}", {"stream_post_policy": orjson.dumps(policies[0]).decode()} - ) - self.assert_json_success(result) - def test_change_stream_message_retention_days_notifications(self) -> None: user_profile = self.example_user("desdemona") self.login_user(user_profile) diff --git a/zerver/views/streams.py b/zerver/views/streams.py index d8e56d8c37..78da5136ca 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -35,7 +35,6 @@ from zerver.actions.streams import ( do_change_stream_group_based_setting, do_change_stream_message_retention_days, do_change_stream_permission, - do_change_stream_post_policy, do_change_subscription_property, do_deactivate_stream, do_rename_stream, @@ -83,7 +82,7 @@ from zerver.lib.topic import ( messages_for_topic, ) from zerver.lib.typed_endpoint import ApiParamConfig, PathOnly, typed_endpoint -from zerver.lib.typed_endpoint_validators import check_color, check_int_in_validator +from zerver.lib.typed_endpoint_validators import check_color from zerver.lib.types import AnonymousSettingGroupDict from zerver.lib.user_groups import ( GroupSettingChangeRequest, @@ -254,15 +253,7 @@ def update_stream_backend( description: Annotated[str, StringConstraints(max_length=Stream.MAX_DESCRIPTION_LENGTH)] | None = None, is_private: Json[bool] | None = None, - is_announcement_only: Json[bool] | None = None, is_default_stream: Json[bool] | None = None, - stream_post_policy: Json[ - Annotated[ - int, - check_int_in_validator(Stream.STREAM_POST_POLICY_TYPES), - ] - ] - | None = None, history_public_to_subscribers: Json[bool] | None = None, is_web_public: Json[bool] | None = None, new_name: str | None = None, @@ -390,16 +381,6 @@ def update_stream_backend( # are only changing the casing of the stream name). check_stream_name_available(user_profile.realm, new_name) do_rename_stream(stream, new_name, user_profile) - if is_announcement_only is not None: - # is_announcement_only is a legacy way to specify - # stream_post_policy. We can probably just delete this code, - # since we're not aware of clients that used it, but we're - # keeping it for backwards-compatibility for now. - stream_post_policy = Stream.STREAM_POST_POLICY_EVERYONE - if is_announcement_only: - stream_post_policy = Stream.STREAM_POST_POLICY_ADMINS - if stream_post_policy is not None: - do_change_stream_post_policy(stream, stream_post_policy, acting_user=user_profile) request_settings_dict = locals() for setting_name, permission_configuration in Stream.stream_permission_group_settings.items(): @@ -591,9 +572,6 @@ def add_subscriptions_backend( invite_only: Json[bool] = False, is_web_public: Json[bool] = False, is_default_stream: Json[bool] = False, - stream_post_policy: Json[ - Annotated[int, check_int_in_validator(Stream.STREAM_POST_POLICY_TYPES)] - ] = Stream.STREAM_POST_POLICY_EVERYONE, history_public_to_subscribers: Json[bool] | None = None, message_retention_days: Json[str] | Json[int] = RETENTION_DEFAULT, can_administer_channel_group: Json[int | AnonymousSettingGroupDict] | None = None, @@ -653,7 +631,6 @@ def add_subscriptions_backend( stream_dict_copy["invite_only"] = invite_only stream_dict_copy["is_web_public"] = is_web_public - stream_dict_copy["stream_post_policy"] = stream_post_policy stream_dict_copy["history_public_to_subscribers"] = history_public_to_subscribers stream_dict_copy["message_retention_days"] = parse_message_retention_days( message_retention_days, Stream.MESSAGE_RETENTION_SPECIAL_VALUES_MAP