streams: Add RealmAuditLog entries for permission changes.

zerver/lib/actions.py

@@ -5041,11 +5041,9 @@ def do_change_stream_permission(
     is_web_public: Optional[bool] = None,
     acting_user: UserProfile,
 ) -> None:
-    old_policy_name = get_stream_permission_policy_name(
-        invite_only=stream.invite_only,
-        history_public_to_subscribers=stream.history_public_to_subscribers,
-        is_web_public=stream.is_web_public,
-    )
+    old_invite_only_value = stream.invite_only
+    old_history_public_to_subscribers_value = stream.history_public_to_subscribers
+    old_is_web_public_value = stream.is_web_public
 
     # A note on these assertions: It's possible we'd be better off
     # making all callers of this function pass the full set of
@@ -5073,12 +5071,56 @@ def do_change_stream_permission(
         stream.history_public_to_subscribers = history_public_to_subscribers
         stream.is_web_public = False
 
+    with transaction.atomic():
         stream.save(update_fields=["invite_only", "history_public_to_subscribers", "is_web_public"])
 
-    new_policy_name = get_stream_permission_policy_name(
-        invite_only=stream.invite_only,
-        history_public_to_subscribers=stream.history_public_to_subscribers,
-        is_web_public=stream.is_web_public,
+        event_time = timezone_now()
+        if old_invite_only_value != stream.invite_only:
+            RealmAuditLog.objects.create(
+                realm=stream.realm,
+                acting_user=acting_user,
+                modified_stream=stream,
+                event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+                event_time=event_time,
+                extra_data=orjson.dumps(
+                    {
+                        RealmAuditLog.OLD_VALUE: old_invite_only_value,
+                        RealmAuditLog.NEW_VALUE: stream.invite_only,
+                        "property": "invite_only",
+                    }
+                ).decode(),
+            )
+
+        if old_history_public_to_subscribers_value != stream.history_public_to_subscribers:
+            RealmAuditLog.objects.create(
+                realm=stream.realm,
+                acting_user=acting_user,
+                modified_stream=stream,
+                event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+                event_time=event_time,
+                extra_data=orjson.dumps(
+                    {
+                        RealmAuditLog.OLD_VALUE: old_history_public_to_subscribers_value,
+                        RealmAuditLog.NEW_VALUE: stream.history_public_to_subscribers,
+                        "property": "history_public_to_subscribers",
+                    }
+                ).decode(),
+            )
+
+        if old_is_web_public_value != stream.is_web_public:
+            RealmAuditLog.objects.create(
+                realm=stream.realm,
+                acting_user=acting_user,
+                modified_stream=stream,
+                event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+                event_time=event_time,
+                extra_data=orjson.dumps(
+                    {
+                        RealmAuditLog.OLD_VALUE: old_is_web_public_value,
+                        RealmAuditLog.NEW_VALUE: stream.is_web_public,
+                        "property": "is_web_public",
+                    }
+                ).decode(),
             )
 
     event = dict(
@@ -5092,6 +5134,17 @@ def do_change_stream_permission(
         name=stream.name,
     )
     send_event(stream.realm, event, can_access_stream_user_ids(stream))
+
+    old_policy_name = get_stream_permission_policy_name(
+        invite_only=old_invite_only_value,
+        history_public_to_subscribers=old_history_public_to_subscribers_value,
+        is_web_public=old_is_web_public_value,
+    )
+    new_policy_name = get_stream_permission_policy_name(
+        invite_only=stream.invite_only,
+        history_public_to_subscribers=stream.history_public_to_subscribers,
+        is_web_public=stream.is_web_public,
+    )
     send_change_stream_permission_notification(
         stream,
         old_policy_name=old_policy_name,

zerver/tests/test_subs.py

@@ -475,6 +475,36 @@ class StreamAdminTest(ZulipTestCase):
         )
         self.assertEqual(messages[0].content, expected_notification)
 
+        history_public_to_subscribers_log = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+            modified_stream=stream,
+        ).last()
+        assert history_public_to_subscribers_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(history_public_to_subscribers_log.extra_data, expected_extra_data)
+
+        invite_only_log = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+            modified_stream=stream,
+        ).order_by("-id")[1]
+        assert invite_only_log is not None
+
+        expected_extra_data = orjson.dumps(
+            {
+                RealmAuditLog.OLD_VALUE: True,
+                RealmAuditLog.NEW_VALUE: False,
+                "property": "invite_only",
+            }
+        ).decode()
+        self.assertEqual(invite_only_log.extra_data, expected_extra_data)
+
         do_change_user_role(user_profile, UserProfile.ROLE_MEMBER, acting_user=None)
         params = {
             "stream_name": orjson.dumps("private_stream_2").decode(),
@@ -509,6 +539,36 @@ class StreamAdminTest(ZulipTestCase):
         )
         self.assertEqual(messages[0].content, expected_notification)
 
+        history_public_to_subscribers_log = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+            modified_stream=stream,
+        ).last()
+        assert history_public_to_subscribers_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(history_public_to_subscribers_log.extra_data, expected_extra_data)
+
+        invite_only_log = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+            modified_stream=stream,
+        ).order_by("-id")[1]
+        assert invite_only_log is not None
+
+        expected_extra_data = orjson.dumps(
+            {
+                RealmAuditLog.OLD_VALUE: True,
+                RealmAuditLog.NEW_VALUE: False,
+                "property": "invite_only",
+            }
+        ).decode()
+        self.assertEqual(invite_only_log.extra_data, expected_extra_data)
+
     def test_make_stream_private(self) -> None:
         user_profile = self.example_user("hamlet")
         self.login_user(user_profile)
@@ -536,6 +596,36 @@ class StreamAdminTest(ZulipTestCase):
         )
         self.assertEqual(messages[0].content, expected_notification)
 
+        history_public_to_subscribers_log = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+            modified_stream=stream,
+        ).last()
+        assert history_public_to_subscribers_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(history_public_to_subscribers_log.extra_data, expected_extra_data)
+
+        invite_only_log = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+            modified_stream=stream,
+        ).order_by("-id")[1]
+        assert invite_only_log is not None
+
+        expected_extra_data = orjson.dumps(
+            {
+                RealmAuditLog.OLD_VALUE: False,
+                RealmAuditLog.NEW_VALUE: True,
+                "property": "invite_only",
+            }
+        ).decode()
+        self.assertEqual(invite_only_log.extra_data, expected_extra_data)
+
         default_stream = self.make_stream("default_stream", realm=realm)
         do_add_default_stream(default_stream)
         params = {
@@ -580,6 +670,36 @@ class StreamAdminTest(ZulipTestCase):
         )
         self.assertEqual(messages[0].content, expected_notification)
 
+        history_public_to_subscribers_log = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+            modified_stream=stream,
+        ).last()
+        assert history_public_to_subscribers_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(history_public_to_subscribers_log.extra_data, expected_extra_data)
+
+        invite_only_log = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STREAM_PROPERTY_CHANGED,
+            modified_stream=stream,
+        ).order_by("-id")[1]
+        assert invite_only_log is not None
+
+        expected_extra_data = orjson.dumps(
+            {
+                RealmAuditLog.OLD_VALUE: False,
+                RealmAuditLog.NEW_VALUE: True,
+                "property": "invite_only",
+            }
+        ).decode()
+        self.assertEqual(invite_only_log.extra_data, expected_extra_data)
+
     def test_create_web_public_stream(self) -> None:
         user_profile = self.example_user("hamlet")
         owner = self.example_user("desdemona")
@@ -654,7 +774,22 @@ class StreamAdminTest(ZulipTestCase):
         )
         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: True,
+                RealmAuditLog.NEW_VALUE: False,
+                "property": "invite_only",
+            }
+        ).decode()
+        self.assertEqual(realm_audit_log.extra_data, expected_extra_data)
+
     def test_make_stream_private_with_public_history(self) -> None:
+        # Convert a public stream to a private stream with shared history
         user_profile = self.example_user("hamlet")
         self.login_user(user_profile)
         realm = user_profile.realm
@@ -681,6 +816,62 @@ class StreamAdminTest(ZulipTestCase):
         )
         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": "invite_only",
+            }
+        ).decode()
+        self.assertEqual(realm_audit_log.extra_data, expected_extra_data)
+
+        # Convert a private stream with protected history to a private stream
+        # with shared history.
+        self.make_stream(
+            "private_protected_stream",
+            realm=realm,
+            invite_only=True,
+            history_public_to_subscribers=False,
+        )
+        params = {
+            "stream_name": orjson.dumps("public_protected_stream").decode(),
+            "is_private": orjson.dumps(True).decode(),
+            "history_public_to_subscribers": orjson.dumps(True).decode(),
+        }
+        stream_id = self.subscribe(user_profile, "private_protected_stream").id
+        result = self.client_patch(f"/json/streams/{stream_id}", params)
+        self.assert_json_success(result)
+        stream = get_stream("private_protected_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"@_**King Hamlet|{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)
+
     def test_make_stream_web_public(self) -> None:
         user_profile = self.example_user("hamlet")
         self.login_user(user_profile)
@@ -755,6 +946,20 @@ class StreamAdminTest(ZulipTestCase):
         )
         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": "is_web_public",
+            }
+        ).decode()
+        self.assertEqual(realm_audit_log.extra_data, expected_extra_data)
+
     def test_try_make_stream_public_with_private_history(self) -> None:
         user_profile = self.example_user("hamlet")
         self.login_user(user_profile)