streams: Allow changing history access without is_private parameter.

We now allow changing access to history of the stream by only passing
"history_public_to_subscribers" parameter. Previously, "is_private"
parameter was also required to change history_public_to_subscribers
otherwise the request was silently ignored.

We also raise error when only history_public_to_subscribers parameter
is passed with value False without "is_private: True" for a public
or web-public stream since we do not allow public streams with
protected history.

templates/zerver/api/changelog.md

@@ -20,6 +20,16 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 6.0
 
+**Feature level 136**
+
+* [`PATCH /streams/{stream_id}`](/api/update-stream): The endpoint
+  now returns an error for a request to make a public stream with
+  protected history which was previously ignored silently.
+* [`PATCH /streams/{stream_id}`](/api/update-stream): Added support
+  to change access to history of the stream by only passing
+  `history_public_to_subscribers` parameter without `is_private`
+  and `is_web_public` parameters.
+
 **Feature level 135**
 
 * [`DELETE /user/{user_id}`](/api/deactivate-user): Added

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 templates/zerver/api/changelog.md, as well as
 # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`.
-API_FEATURE_LEVEL = 135
+API_FEATURE_LEVEL = 136
 
 # 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/actions/streams.py

@@ -855,8 +855,12 @@ def do_change_stream_permission(
         stream.invite_only = False
         stream.history_public_to_subscribers = True
     else:
-        assert invite_only is not None
         # is_web_public is falsey
+        if invite_only is None:
+            # This is necessary to get correct default value for
+            # history_public_to_subscribers when invite_only is
+            # None.
+            invite_only = stream.invite_only
         history_public_to_subscribers = get_default_value_for_history_public_to_subscribers(
             stream.realm,
             invite_only,

zerver/openapi/zulip.yaml

@@ -13751,8 +13751,27 @@ paths:
             type: boolean
           example: true
           required: false
+        - name: history_public_to_subscribers
+          in: query
+          description: |
+            Whether the stream's message history should be available to
+            newly subscribed members, or users can only access messages
+            they actually received while subscribed to the stream.
+
+            Corresponds to the [shared history](/help/stream-permissions)
+            option in documentation.
+
+            It's an error for this parameter to be false for a public or
+            web-public stream and when is_private is false.
+
+            **Changes**: Before Zulip 6.0 (feature level 136), `history_public_to_subscribers`
+            was silently ignored unless the request also contained either `is_private` or
+            `is_web_public`.
+          schema:
+            type: boolean
+          example: false
+          required: false
         - $ref: "#/components/parameters/StreamPostPolicy"
-        - $ref: "#/components/parameters/HistoryPublicToSubscribers"
         - $ref: "#/components/parameters/MessageRetentionDays"
       responses:
         "200":
@@ -13762,16 +13781,28 @@ paths:
           content:
             application/json:
               schema:
-                allOf:
+                oneOf:
-                  - $ref: "#/components/schemas/JsonError"
+                  - allOf:
-                  - example:
+                      - $ref: "#/components/schemas/JsonError"
-                      {
+                      - example:
-                        "code": "BAD_REQUEST",
+                          {
-                        "msg": "Invalid stream ID",
+                            "code": "BAD_REQUEST",
-                        "result": "error",
+                            "msg": "Invalid stream ID",
-                      }
+                            "result": "error",
-                    description: |
+                          }
-                      An example JSON response for when the supplied stream does not exist:
+                        description: |
+                          An example JSON response for when the supplied stream does not exist:
+                  - allOf:
+                      - $ref: "#/components/schemas/JsonError"
+                      - example:
+                          {
+                            "code": "BAD_REQUEST",
+                            "msg": "Invalid parameters",
+                            "result": "error",
+                          }
+                        description: |
+                          An example JSON response for when invalid combination of stream permission
+                          parameters are passed.
   /streams/{stream_id}/delete_topic:
     post:
       operationId: delete-topic

zerver/tests/test_subs.py

@@ -879,6 +879,77 @@ class StreamAdminTest(ZulipTestCase):
         ).decode()
         self.assertEqual(realm_audit_log.extra_data, expected_extra_data)
 
+    def test_change_history_access_for_private_streams(self) -> None:
+        user_profile = self.example_user("iago")
+        self.login_user(user_profile)
+        realm = user_profile.realm
+        self.make_stream("private_stream", realm=realm, invite_only=True)
+        stream_id = self.subscribe(user_profile, "private_stream").id
+
+        params = {
+            "history_public_to_subscribers": orjson.dumps(True).decode(),
+        }
+        result = self.client_patch(f"/json/streams/{stream_id}", params)
+        self.assert_json_success(result)
+
+        stream = get_stream("private_stream", realm)
+        self.assertTrue(stream.invite_only)
+        self.assertTrue(stream.history_public_to_subscribers)
+
+        messages = get_topic_messages(user_profile, stream, "stream events")
+        self.assert_length(messages, 1)
+        expected_notification = (
+            f"@_**Iago|{user_profile.id}** changed the [access permissions](/help/stream-permissions) "
+            "for this stream from **Private, protected history** to **Private, shared history**."
+        )
+        self.assertEqual(messages[0].content, expected_notification)
+
+        realm_audit_log = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+            modified_stream=stream,
+        ).last()
+        assert realm_audit_log is not None
+        expected_extra_data = orjson.dumps(
+            {
+                RealmAuditLog.OLD_VALUE: False,
+                RealmAuditLog.NEW_VALUE: True,
+                "property": "history_public_to_subscribers",
+            }
+        ).decode()
+        self.assertEqual(realm_audit_log.extra_data, expected_extra_data)
+
+        params = {
+            "history_public_to_subscribers": orjson.dumps(False).decode(),
+        }
+        result = self.client_patch(f"/json/streams/{stream_id}", params)
+        self.assert_json_success(result)
+
+        stream = get_stream("private_stream", realm)
+        self.assertTrue(stream.invite_only)
+        self.assertFalse(stream.history_public_to_subscribers)
+
+        messages = get_topic_messages(user_profile, stream, "stream events")
+        self.assert_length(messages, 2)
+        expected_notification = (
+            f"@_**Iago|{user_profile.id}** changed the [access permissions](/help/stream-permissions) "
+            "for this stream from **Private, shared history** to **Private, protected history**."
+        )
+        self.assertEqual(messages[1].content, expected_notification)
+
+        realm_audit_log = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+            modified_stream=stream,
+        ).last()
+        assert realm_audit_log is not None
+        expected_extra_data = orjson.dumps(
+            {
+                RealmAuditLog.OLD_VALUE: True,
+                RealmAuditLog.NEW_VALUE: False,
+                "property": "history_public_to_subscribers",
+            }
+        ).decode()
+        self.assertEqual(realm_audit_log.extra_data, expected_extra_data)
+
     def test_stream_permission_changes_updates_updates_attachments(self) -> None:
         self.login("desdemona")
         fp = StringIO("zulip!")
@@ -1037,6 +1108,16 @@ class StreamAdminTest(ZulipTestCase):
         result = self.client_patch(f"/json/streams/{stream_id}", params)
         self.assert_json_error(result, "Invalid parameters")
 
+        params = {
+            "history_public_to_subscribers": orjson.dumps(False).decode(),
+        }
+        result = self.client_patch(f"/json/streams/{stream_id}", params)
+        self.assert_json_error(result, "Invalid parameters")
+
+        web_public_stream = self.make_stream("web_public_stream", realm=realm, is_web_public=True)
+        result = self.client_patch(f"/json/streams/{web_public_stream.id}", params)
+        self.assert_json_error(result, "Invalid parameters")
+
     def test_subscriber_ids_with_stream_history_access(self) -> None:
         hamlet = self.example_user("hamlet")
         polonius = self.example_user("polonius")

zerver/views/streams.py

@@ -335,7 +335,15 @@ def update_stream_backend(
         if is_private or history_public_to_subscribers is False:
             raise JsonableError(_("Invalid parameters"))
 
-    if is_private is not None or is_web_public is not None:
+    if history_public_to_subscribers is False and not stream.realm.is_zephyr_mirror_realm:
+        if is_private is None and not stream.invite_only:
+            raise JsonableError(_("Invalid parameters"))
+
+    if (
+        is_private is not None
+        or is_web_public is not None
+        or history_public_to_subscribers is not None
+    ):
         do_change_stream_permission(
             stream,
             invite_only=is_private,