models: Add Stream.history_public_to_subscribers.

This commit adds a new field history_public_to_subscribers to the
Stream model, which serves a similar function to the old
settings.PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS; we still use that
setting as the default value for new streams to avoid breaking
backwards-compatibility for those users before we are ready with an
actual UI for users to choose directly.

This also comes with a migration to set the value of the new field for
existing streams with an algorithm matching that used at runtime.

With significant changes by Tim Abbott.

This is an initial part of our efforts on #9232.

zerver/lib/actions.py

@@ -1563,8 +1563,26 @@ def send_stream_creation_event(stream: Stream, user_ids: List[int]) -> None:
 
 def create_stream_if_needed(realm: Realm,
                             stream_name: Text,
+                            *,
                             invite_only: bool=False,
+                            history_public_to_subscribers: Optional[bool]=None,
                             stream_description: Text="") -> Tuple[Stream, bool]:
+    if invite_only:
+        if history_public_to_subscribers is None:
+            # TODO: Once we have a UI for this feature, we'll remove
+            # settings.PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS and set
+            # this to be False here
+            history_public_to_subscribers = settings.PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS
+    else:
+        # If we later decide to support public streams without
+        # history, we can remove this code path.
+        history_public_to_subscribers = True
+
+    if realm.is_zephyr_mirror_realm:
+        # In the Zephyr mirroring model, history is unconditionally
+        # not public to subscribers, even for public streams.
+        history_public_to_subscribers = False
+
     (stream, created) = Stream.objects.get_or_create(
         realm=realm,
         name__iexact=stream_name,
@@ -1572,6 +1590,7 @@ def create_stream_if_needed(realm: Realm,
             name=stream_name,
             description=stream_description,
             invite_only=invite_only,
+            history_public_to_subscribers=history_public_to_subscribers,
             is_in_zephyr_realm=realm.is_zephyr_mirror_realm
         )
     )
@@ -1589,7 +1608,9 @@ def ensure_stream(realm: Realm,
                   stream_name: Text,
                   invite_only: bool=False,
                   stream_description: Text="") -> Stream:
-    return create_stream_if_needed(realm, stream_name, invite_only, stream_description)[0]
+    return create_stream_if_needed(realm, stream_name,
+                                   invite_only=invite_only,
+                                   stream_description=stream_description)[0]
 
 def create_streams_if_needed(realm: Realm,
                              stream_dicts: List[Mapping[str, Any]]) -> Tuple[List[Stream], List[Stream]]:
@@ -1598,10 +1619,13 @@ def create_streams_if_needed(realm: Realm,
     added_streams = []  # type: List[Stream]
     existing_streams = []  # type: List[Stream]
     for stream_dict in stream_dicts:
-        stream, created = create_stream_if_needed(realm,
+        stream, created = create_stream_if_needed(
-                                                  stream_dict["name"],
+            realm,
-                                                  invite_only=stream_dict.get("invite_only", False),
+            stream_dict["name"],
-                                                  stream_description=stream_dict.get("description", ""))
+            invite_only=stream_dict.get("invite_only", False),
+            history_public_to_subscribers=stream_dict.get("history_public_to_subscribers"),
+            stream_description=stream_dict.get("description", "")
+        )
 
         if created:
             added_streams.append(stream)
@@ -2912,9 +2936,27 @@ def do_change_bot_type(user_profile: UserProfile, value: int) -> None:
     user_profile.bot_type = value
     user_profile.save(update_fields=["bot_type"])
 
-def do_change_stream_invite_only(stream: Stream, invite_only: bool) -> None:
+def do_change_stream_invite_only(stream: Stream, invite_only: bool,
+                                 history_public_to_subscribers: Optional[bool]=None) -> None:
+    if invite_only:
+        if history_public_to_subscribers is None:
+            # TODO: Once we have a UI for this feature, we'll remove
+            # settings.PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS and set
+            # this to be False here
+            history_public_to_subscribers = settings.PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS
+    else:
+        # If we later decide to support public streams without
+        # history, we can remove this code path.
+        history_public_to_subscribers = True
+
+    if stream.realm.is_zephyr_mirror_realm:
+        # In the Zephyr mirroring model, history is unconditionally
+        # not public to subscribers, even for public streams.
+        history_public_to_subscribers = False
+
     stream.invite_only = invite_only
-    stream.save(update_fields=['invite_only'])
+    stream.history_public_to_subscribers = history_public_to_subscribers
+    stream.save(update_fields=['invite_only', 'history_public_to_subscribers'])
 
 def do_rename_stream(stream: Stream, new_name: Text, log: bool=True) -> Dict[str, Text]:
     old_name = stream.name

zerver/lib/test_classes.py

@@ -525,7 +525,8 @@ class ZulipTestCase(TestCase):
         return open(fn).read()
 
     def make_stream(self, stream_name: Text, realm: Optional[Realm]=None,
-                    invite_only: Optional[bool]=False) -> Stream:
+                    invite_only: Optional[bool]=False,
+                    history_public_to_subscribers: Optional[bool]=False) -> Stream:
         if realm is None:
             realm = self.DEFAULT_REALM
 
@@ -534,6 +535,7 @@ class ZulipTestCase(TestCase):
                 realm=realm,
                 name=stream_name,
                 invite_only=invite_only,
+                history_public_to_subscribers=history_public_to_subscribers,
             )
         except IntegrityError:  # nocoverage -- this is for bugs in the tests
             raise Exception('''

zerver/migrations/0164_stream_history_public_to_subscribers.py

@@ -0,0 +1,41 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.11 on 2018-04-28 22:31
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations, models
+from django.db.backends.postgresql_psycopg2.schema import DatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+
+
+def set_initial_value_for_history_public_to_subscribers(
+        apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None:
+    stream_model = apps.get_model("zerver", "Stream")
+    streams = stream_model.objects.all()
+
+    for stream in streams:
+        if stream.invite_only:
+            stream.history_public_to_subscribers = settings.PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS
+        else:
+            stream.history_public_to_subscribers = True
+
+        if stream.is_in_zephyr_realm:
+            stream.history_public_to_subscribers = False
+
+        stream.save(update_fields=["history_public_to_subscribers"])
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('zerver', '0163_remove_userprofile_default_desktop_notifications'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='stream',
+            name='history_public_to_subscribers',
+            field=models.BooleanField(default=False),
+        ),
+        migrations.RunPython(set_initial_value_for_history_public_to_subscribers,
+                             reverse_code=migrations.RunPython.noop),
+    ]

zerver/models.py

@@ -938,6 +938,7 @@ class Stream(models.Model):
     name = models.CharField(max_length=MAX_NAME_LENGTH, db_index=True)  # type: Text
     realm = models.ForeignKey(Realm, db_index=True, on_delete=CASCADE)  # type: Realm
     invite_only = models.NullBooleanField(default=False)  # type: Optional[bool]
+    history_public_to_subscribers = models.BooleanField(default=False)  # type: bool
 
     # The unique thing about Zephyr public streams is that we never list their
     # users.  We may try to generalize this concept later, but for now
@@ -970,9 +971,7 @@ class Stream(models.Model):
         return self.is_public()
 
     def is_history_public_to_subscribers(self) -> bool:
-        if settings.PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS:
+        return self.history_public_to_subscribers
-            return True
-        return self.is_public()
 
     class Meta:
         unique_together = ("name", "realm")

zerver/tests/test_messages.py

@@ -2420,17 +2420,26 @@ class StarTests(ZulipTestCase):
         result = self.change_star(message_ids)
         self.assert_json_error(result, 'Invalid message(s)')
 
-        with self.settings(PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS=True):
+        stream_name = "private_stream_2"
-            # With PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS, you still
+        self.make_stream(stream_name, invite_only=True,
-            # can't see it if you didn't receive the message and are
+                         history_public_to_subscribers=True)
-            # not subscribed.
+        self.subscribe(self.example_user("hamlet"), stream_name)
-            result = self.change_star(message_ids)
+        self.login(self.example_email("hamlet"))
-            self.assert_json_error(result, 'Invalid message(s)')
+        message_ids = [
+            self.send_stream_message(self.example_email("hamlet"), stream_name, "test"),
+        ]
 
-            # But if you subscribe, then you can star the message
+        # With stream.history_public_to_subscribers = True, you still
-            self.subscribe(self.example_user("cordelia"), stream_name)
+        # can't see it if you didn't receive the message and are
-            result = self.change_star(message_ids)
+        # not subscribed.
-            self.assert_json_success(result)
+        self.login(self.example_email("cordelia"))
+        result = self.change_star(message_ids)
+        self.assert_json_error(result, 'Invalid message(s)')
+
+        # But if you subscribe, then you can star the message
+        self.subscribe(self.example_user("cordelia"), stream_name)
+        result = self.change_star(message_ids)
+        self.assert_json_success(result)
 
     def test_new_message(self) -> None:
         """

zerver/tests/test_narrow.py

@@ -384,10 +384,17 @@ class IncludeHistoryTest(ZulipTestCase):
         ]
         self.assertFalse(ok_to_include_history(narrow, user_profile))
 
-        with self.settings(PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS=True):
+        # Verify that with stream.history_public_to_subscribers, subscribed
-            # Verify that with this setting, subscribed users can access history.
+        # users can access history.
-            self.assertFalse(ok_to_include_history(narrow, user_profile))
+        self.make_stream('private_stream_2', realm=user_profile.realm,
-            self.assertTrue(ok_to_include_history(narrow, subscribed_user_profile))
+                         invite_only=True, history_public_to_subscribers=True)
+        subscribed_user_profile = self.example_user("cordelia")
+        self.subscribe(subscribed_user_profile, 'private_stream_2')
+        narrow = [
+            dict(operator='stream', operand='private_stream_2'),
+        ]
+        self.assertFalse(ok_to_include_history(narrow, user_profile))
+        self.assertTrue(ok_to_include_history(narrow, subscribed_user_profile))
 
         # History doesn't apply to PMs.
         narrow = [

zerver/tests/test_subs.py

@@ -4,6 +4,7 @@ from typing import Any, Dict, List, Mapping, Optional, Sequence, Set, Text
 
 from django.conf import settings
 from django.http import HttpRequest, HttpResponse
+from django.test import override_settings
 from django.utils.timezone import now as timezone_now
 
 from zerver.lib import cache
@@ -141,6 +142,83 @@ class TestCreateStreams(ZulipTestCase):
         for stream in existing_streams:
             self.assertTrue(stream.invite_only)
 
+    def test_history_public_to_subscribers_on_stream_creation(self) -> None:
+        realm = get_realm('zulip')
+        stream_dicts = [
+            {
+                "name": "publicstream",
+                "description": "Public stream with public history"
+            },
+            {
+                "name": "privatestream",
+                "description": "Private stream with non-public history",
+                "invite_only": True
+            },
+            {
+                "name": "privatewithhistory",
+                "description": "Private stream with public history",
+                "invite_only": True,
+                "history_public_to_subscribers": True
+            },
+            {
+                "name": "publictrywithouthistory",
+                "description": "Public stream without public history (disallowed)",
+                "invite_only": False,
+                "history_public_to_subscribers": False
+            },
+        ]  # type: List[Mapping[str, Any]]
+
+        created, existing = create_streams_if_needed(realm, stream_dicts)
+
+        self.assertEqual(len(created), 4)
+        self.assertEqual(len(existing), 0)
+        for stream in created:
+            if stream.name == 'publicstream':
+                self.assertTrue(stream.history_public_to_subscribers)
+            if stream.name == 'privatestream':
+                self.assertFalse(stream.history_public_to_subscribers)
+            if stream.name == 'privatewithhistory':
+                self.assertTrue(stream.history_public_to_subscribers)
+            if stream.name == 'publictrywithouthistory':
+                self.assertTrue(stream.history_public_to_subscribers)
+
+    @override_settings(PRIVATE_STREAM_HISTORY_FOR_SUBSCRIBERS=True)
+    def test_history_public_to_subscribers_on_stream_creation_with_setting(self) -> None:
+        realm = get_realm('zulip')
+
+        stream, created = create_stream_if_needed(realm, "private_stream", invite_only=True)
+        self.assertTrue(created)
+        self.assertTrue(stream.invite_only)
+        self.assertTrue(stream.history_public_to_subscribers)
+
+        stream, created = create_stream_if_needed(realm, "history_stream",
+                                                  invite_only=True,
+                                                  history_public_to_subscribers=False)
+        self.assertTrue(created)
+        self.assertTrue(stream.invite_only)
+        self.assertFalse(stream.history_public_to_subscribers)
+
+        # You can't make a public stream limited in this way
+        stream, created = create_stream_if_needed(realm, "public_history_stream",
+                                                  invite_only=False,
+                                                  history_public_to_subscribers=False)
+        self.assertTrue(created)
+        self.assertFalse(stream.invite_only)
+        self.assertTrue(stream.history_public_to_subscribers)
+
+    def test_history_public_to_subscribers_zephyr_realm(self) -> None:
+        realm = get_realm('zephyr')
+
+        stream, created = create_stream_if_needed(realm, "private_stream", invite_only=True)
+        self.assertTrue(created)
+        self.assertTrue(stream.invite_only)
+        self.assertFalse(stream.history_public_to_subscribers)
+
+        stream, created = create_stream_if_needed(realm, "public_stream", invite_only=False)
+        self.assertTrue(created)
+        self.assertFalse(stream.invite_only)
+        self.assertFalse(stream.history_public_to_subscribers)
+
     def test_welcome_message(self) -> None:
         realm = get_realm('zulip')
         name = u'New Stream'
@@ -208,6 +286,7 @@ class StreamAdminTest(ZulipTestCase):
         realm = user_profile.realm
         stream = get_stream('private_stream', realm)
         self.assertFalse(stream.invite_only)
+        self.assertTrue(stream.history_public_to_subscribers)
 
     def test_make_stream_private(self) -> None:
         user_profile = self.example_user('hamlet')
@@ -226,6 +305,68 @@ class StreamAdminTest(ZulipTestCase):
         self.assert_json_success(result)
         stream = get_stream('public_stream', realm)
         self.assertTrue(stream.invite_only)
+        self.assertFalse(stream.history_public_to_subscribers)
+
+    def test_make_stream_public_zephyr_mirror(self) -> None:
+        user_profile = self.mit_user('starnine')
+        email = user_profile.email
+        self.login(email, realm=get_realm("zephyr"))
+        realm = user_profile.realm
+        self.make_stream('target_stream', realm=realm, invite_only=True)
+        self.subscribe(user_profile, 'target_stream')
+
+        do_change_is_admin(user_profile, True)
+        params = {
+            'stream_name': ujson.dumps('target_stream'),
+            'is_private': ujson.dumps(False)
+        }
+        stream_id = get_stream('target_stream', realm).id
+        result = self.client_patch("/json/streams/%d" % (stream_id,), params,
+                                   subdomain="zephyr")
+        self.assert_json_success(result)
+        stream = get_stream('target_stream', realm)
+        self.assertFalse(stream.invite_only)
+        self.assertFalse(stream.history_public_to_subscribers)
+
+    def test_make_stream_private_with_public_history(self) -> None:
+        user_profile = self.example_user('hamlet')
+        email = user_profile.email
+        self.login(email)
+        realm = user_profile.realm
+        self.make_stream('public_history_stream', realm=realm)
+
+        do_change_is_admin(user_profile, True)
+        params = {
+            'stream_name': ujson.dumps('public_history_stream'),
+            'is_private': ujson.dumps(True),
+            'history_public_to_subscribers': ujson.dumps(True),
+        }
+        stream_id = get_stream('public_history_stream', realm).id
+        result = self.client_patch("/json/streams/%d" % (stream_id,), params)
+        self.assert_json_success(result)
+        stream = get_stream('public_history_stream', realm)
+        self.assertTrue(stream.invite_only)
+        self.assertTrue(stream.history_public_to_subscribers)
+
+    def test_try_make_stream_public_with_private_history(self) -> None:
+        user_profile = self.example_user('hamlet')
+        email = user_profile.email
+        self.login(email)
+        realm = user_profile.realm
+        self.make_stream('public_stream', realm=realm)
+
+        do_change_is_admin(user_profile, True)
+        params = {
+            'stream_name': ujson.dumps('public_stream'),
+            'is_private': ujson.dumps(False),
+            'history_public_to_subscribers': ujson.dumps(False),
+        }
+        stream_id = get_stream('public_stream', realm).id
+        result = self.client_patch("/json/streams/%d" % (stream_id,), params)
+        self.assert_json_success(result)
+        stream = get_stream('public_stream', realm)
+        self.assertFalse(stream.invite_only)
+        self.assertTrue(stream.history_public_to_subscribers)
 
     def test_deactivate_stream_backend(self) -> None:
         user_profile = self.example_user('hamlet')

zerver/views/streams.py

@@ -146,6 +146,7 @@ def update_stream_backend(
         stream_id: int,
         description: Optional[str]=REQ(validator=check_string, default=None),
         is_private: Optional[bool]=REQ(validator=check_bool, default=None),
+        history_public_to_subscribers: Optional[bool]=REQ(validator=check_bool, default=None),
         new_name: Optional[str]=REQ(validator=check_string, default=None),
 ) -> HttpResponse:
     # We allow realm administrators to to update the stream name and
@@ -167,7 +168,7 @@ def update_stream_backend(
     # subscribed to make a private stream public.
     if is_private is not None:
         (stream, recipient, sub) = access_stream_by_id(user_profile, stream_id)
-        do_change_stream_invite_only(stream, is_private)
+        do_change_stream_invite_only(stream, is_private, history_public_to_subscribers)
     return json_success()
 
 def list_subscriptions_backend(request: HttpRequest, user_profile: UserProfile) -> HttpResponse:
@@ -288,6 +289,7 @@ def add_subscriptions_backend(
         streams_raw: Iterable[Mapping[str, str]]=REQ(
             "subscriptions", validator=check_list(check_dict([('name', check_string)]))),
         invite_only: bool=REQ(validator=check_bool, default=False),
+        history_public_to_subscribers: Optional[bool]=REQ(validator=check_bool, default=None),
         announce: bool=REQ(validator=check_bool, default=False),
         principals: List[str]=REQ(validator=check_list(check_string), default=[]),
         authorization_errors_fatal: bool=REQ(validator=check_bool, default=True),
@@ -300,6 +302,7 @@ def add_subscriptions_backend(
         # Strip the stream name here.
         stream_dict_copy['name'] = stream_dict_copy['name'].strip()
         stream_dict_copy["invite_only"] = invite_only
+        stream_dict_copy["history_public_to_subscribers"] = history_public_to_subscribers
         stream_dicts.append(stream_dict_copy)
 
     # Validation of the streams arguments, including enforcement of