From 04fdf3e4d98fe85c1ceae8c97bf5c57b21920145 Mon Sep 17 00:00:00 2001
From: Mateusz Mandera <mateusz.mandera@zulip.com>
Date: Wed, 27 Apr 2022 18:05:48 +0200
Subject: [PATCH] import_utils: Fix history_public_to_subscribers being set
 incorrectly.

history_public_to_subscribers wasn't explicitly set when creating
streams via build_stream, thus relying on the model's default of False.
This lead to public streams being created with that value set to False,
which doesn't make sense.

We can solve this by inferring the correct value based on invite_only in
the build_stream funtion itself - rather than needing to add a flag
argument to it.

This commit also includes a migration to fix public stream with the
wrong history_public_to_subscribers value.

Fixes #21784.
---
 zerver/data_import/import_util.py             |  8 +++++++
 ...ix_stream_history_public_to_subscribers.py | 23 +++++++++++++++++++
 zerver/tests/test_slack_importer.py           |  1 +
 3 files changed, 32 insertions(+)
 create mode 100644 zerver/migrations/0390_fix_stream_history_public_to_subscribers.py

diff --git a/zerver/data_import/import_util.py b/zerver/data_import/import_util.py
index 90d5ac1c27..f3548919d8 100644
--- a/zerver/data_import/import_util.py
+++ b/zerver/data_import/import_util.py
@@ -441,6 +441,13 @@ def build_stream(
     invite_only: bool = False,
     stream_post_policy: int = 1,
 ) -> ZerverFieldsT:
+
+    # Other applications don't have the distinction of "private stream with public history"
+    # vs "private stream with hidden history" - and we've traditionally imported private "streams"
+    # of other products as private streams with hidden history.
+    # So we can set the history_public_to_subscribers value based on the invite_only flag.
+    history_public_to_subscribers = not invite_only
+
     stream = Stream(
         name=name,
         deactivated=deactivated,
@@ -450,6 +457,7 @@ def build_stream(
         invite_only=invite_only,
         id=stream_id,
         stream_post_policy=stream_post_policy,
+        history_public_to_subscribers=history_public_to_subscribers,
     )
     stream_dict = model_to_dict(stream, exclude=["realm"])
     stream_dict["realm"] = realm_id
diff --git a/zerver/migrations/0390_fix_stream_history_public_to_subscribers.py b/zerver/migrations/0390_fix_stream_history_public_to_subscribers.py
new file mode 100644
index 0000000000..b22ca5edd9
--- /dev/null
+++ b/zerver/migrations/0390_fix_stream_history_public_to_subscribers.py
@@ -0,0 +1,23 @@
+from django.db import migrations
+from django.db.backends.postgresql.schema import DatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+
+
+def fix_stream_history_public_to_subscribers(
+    apps: StateApps, schema_editor: DatabaseSchemaEditor
+) -> None:
+    Stream = apps.get_model("zerver", "Stream")
+    Stream.objects.filter(
+        invite_only=False, is_in_zephyr_realm=False, history_public_to_subscribers=False
+    ).update(history_public_to_subscribers=True)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("zerver", "0389_userprofile_display_emoji_reaction_users"),
+    ]
+
+    operations = [
+        migrations.RunPython(fix_stream_history_public_to_subscribers, elidable=True),
+    ]
diff --git a/zerver/tests/test_slack_importer.py b/zerver/tests/test_slack_importer.py
index fa042c51bb..52a6b3ba9b 100644
--- a/zerver/tests/test_slack_importer.py
+++ b/zerver/tests/test_slack_importer.py
@@ -684,6 +684,7 @@ class SlackImporter(ZulipTestCase):
         self.assertEqual(zerver_stream[0]["deactivated"], True)
         self.assertEqual(zerver_stream[0]["description"], "no purpose")
         self.assertEqual(zerver_stream[0]["invite_only"], False)
+        self.assertEqual(zerver_stream[0]["history_public_to_subscribers"], True)
         self.assertEqual(zerver_stream[0]["realm"], realm_id)
         self.assertEqual(zerver_stream[2]["id"], test_added_channels[zerver_stream[2]["name"]][1])