From b188e6fa047e16e6842ae710ff2654d794a20a85 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Mon, 1 Nov 2021 15:55:07 -0700 Subject: [PATCH] management: Add a reactivate-stream command. Fixes #601. --- zerver/actions/streams.py | 100 ++++++++++++++++++ .../management/commands/reactivate_stream.py | 91 ++++++++++++++++ zerver/tests/test_subs.py | 91 ++++++++++++++++ 3 files changed, 282 insertions(+) create mode 100644 zerver/management/commands/reactivate_stream.py diff --git a/zerver/actions/streams.py b/zerver/actions/streams.py index 1e6e62df5c..1a155aa7ed 100644 --- a/zerver/actions/streams.py +++ b/zerver/actions/streams.py @@ -6,6 +6,7 @@ from typing import Any, Collection, Dict, Iterable, List, Mapping, Optional, Set import orjson from django.conf import settings from django.db import transaction +from django.db.models import Q, QuerySet from django.utils.timezone import now as timezone_now from django.utils.translation import gettext as _ from django.utils.translation import override as override_language @@ -131,6 +132,105 @@ def do_deactivate_stream(stream: Stream, *, acting_user: Optional[UserProfile]) ) +def deactivated_streams_by_old_name(realm: Realm, stream_name: str) -> QuerySet[Stream]: + fixed_length_prefix = ".......!DEACTIVATED:" + truncated_name = stream_name[0 : Stream.MAX_NAME_LENGTH - len(fixed_length_prefix)] + + old_names: List[str] = [] + for bang_length in range(1, 21): + name = "!" * bang_length + "DEACTIVATED:" + stream_name + old_names.append(name[0 : Stream.MAX_NAME_LENGTH]) + + possible_streams = Stream.objects.filter(realm=realm, deactivated=True).filter( + # We go looking for names as they are post-1b6f68bb59dc; 8 + # characters, followed by `!DEACTIVATED:`, followed by at + # most MAX_NAME_LENGTH-(length of the prefix) of the name + # they provided: + Q(name__regex=rf"^{fixed_length_prefix}{truncated_name}") + # Finally, we go looking for the pre-1b6f68bb59dc version, + # which is any number of `!` followed by `DEACTIVATED:` + # and a prefix of the old stream name + | Q(name__in=old_names), + ) + + return possible_streams + + +@transaction.atomic(savepoint=False) +def do_reactivate_stream( + stream: Stream, new_name: str, *, acting_user: Optional[UserProfile] +) -> None: + realm = stream.realm + if not stream.deactivated: + raise JsonableError(_("Stream is not currently deactivated")) + if Stream.objects.filter(realm=realm, name=new_name).exists(): + raise JsonableError( + _("Stream named {stream_name} already exists").format(stream_name=new_name) + ) + assert stream.recipient_id is not None + + stream.deactivated = False + stream.name = new_name + + # We only set invite_only=True during deactivation, which can lead + # to the invalid state of to invite-only but also web-public + # streams. Explicitly reset the access; we do not use + # do_change_stream_permission because no users need be notified, + # and it cannot handle the broken state that may currently exist. + stream.is_web_public = False + stream.invite_only = True + stream.history_public_to_subscribers = True + stream.save( + update_fields=[ + "name", + "deactivated", + "is_web_public", + "invite_only", + "history_public_to_subscribers", + ] + ) + + # Update caches + cache_set(display_recipient_cache_key(stream.recipient_id), new_name) + messages = Message.objects.filter(recipient_id=stream.recipient_id).only("id") + cache_delete_many(to_dict_cache_key_id(message.id) for message in messages) + + # Unset the is_web_public cache on attachments, since the stream is now private. + Attachment.objects.filter(messages__recipient_id=stream.recipient_id).update(is_web_public=None) + ArchivedAttachment.objects.filter(messages__recipient_id=stream.recipient_id).update( + is_web_public=None + ) + + RealmAuditLog.objects.create( + realm=realm, + acting_user=acting_user, + modified_stream=stream, + event_type=RealmAuditLog.STREAM_REACTIVATED, + event_time=timezone_now(), + ) + + # All admins always get to know about private streams' existence, + # but we only subscribe the realm owners. + send_stream_creation_event( + realm, stream, [user.id for user in realm.get_admin_users_and_bots()] + ) + bulk_add_subscriptions( + realm=realm, + streams=[stream], + users=realm.get_human_owner_users(), + acting_user=acting_user, + ) + + sender = get_system_bot(settings.NOTIFICATION_BOT, stream.realm_id) + with override_language(stream.realm.default_language): + internal_send_stream_message( + sender, + stream, + str(Realm.STREAM_EVENTS_NOTIFICATION_TOPIC), + _("Stream {stream_name} un-archived.").format(stream_name=new_name), + ) + + def bulk_delete_cache_keys(message_ids_to_clear: List[int]) -> None: while len(message_ids_to_clear) > 0: batch = message_ids_to_clear[0:5000] diff --git a/zerver/management/commands/reactivate_stream.py b/zerver/management/commands/reactivate_stream.py new file mode 100644 index 0000000000..3d33b5bda5 --- /dev/null +++ b/zerver/management/commands/reactivate_stream.py @@ -0,0 +1,91 @@ +from argparse import ArgumentParser +from typing import Any, Optional + +from django.core.management.base import CommandError + +from zerver.actions.streams import deactivated_streams_by_old_name, do_reactivate_stream +from zerver.lib.management import ZulipBaseCommand +from zerver.models import RealmAuditLog, Stream + + +class Command(ZulipBaseCommand): + help = """Reactivate a stream that was deactivated.""" + + def add_arguments(self, parser: ArgumentParser) -> None: + specify_stream = parser.add_mutually_exclusive_group(required=True) + specify_stream.add_argument( + "-s", + "--stream", + help="Name of a deactivated stream in the realm.", + ) + specify_stream.add_argument( + "--stream-id", + help="ID of a deactivated stream in the realm.", + ) + parser.add_argument( + "-n", + "--new-name", + required=False, + help="Name to reactivate as; defaults to the old name.", + ) + self.add_realm_args(parser, required=True) + + def handle(self, *args: Any, **options: Optional[str]) -> None: + realm = self.get_realm(options) + assert realm is not None # Should be ensured by parser + + # Looking up the stream is complicated, since they get renamed + # when they are deactivated, in a transformation which may be + # lossy. + + if options["stream_id"] is not None: + stream = Stream.objects.get(id=options["stream_id"]) + if not stream.deactivated: + raise CommandError( + f"Stream id {stream.id}, named '{stream.name}', is not deactivated" + ) + if options["new_name"] is None: + raise CommandError("--new-name flag is required with --stream-id") + new_name = options["new_name"] + else: + stream_name = options["stream"] + assert stream_name is not None + + possible_streams = deactivated_streams_by_old_name(realm, stream_name) + if len(possible_streams) == 0: + raise CommandError("No matching deactivated streams found!") + + if len(possible_streams) > 1: + # Print ids of all possible streams, support passing by id + print("Matching streams:") + for stream in possible_streams: + last_deactivation = ( + RealmAuditLog.objects.filter( + realm=realm, + modified_stream=stream, + event_type=RealmAuditLog.STREAM_DEACTIVATED, + ) + .order_by("-id") + .first() + ) + assert last_deactivation is not None + print( + f" ({stream.id}) {stream.name}, deactivated on {last_deactivation.event_time}" + ) + raise CommandError( + "More than one matching stream found! Specify which with --stream-id" + ) + + stream = possible_streams[0] + if options["new_name"] is not None: + new_name = options["new_name"] + else: + new_name = stream_name + + if Stream.objects.filter(realm=realm, name=new_name).exists(): + raise CommandError( + f"Stream with name '{new_name}' already exists; pass a different --new-name" + ) + + assert stream is not None + do_reactivate_stream(stream, new_name, acting_user=None) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 71eb95d55a..d08806e4d0 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -28,9 +28,11 @@ from zerver.actions.realm_settings import do_change_realm_plan_type, do_set_real from zerver.actions.streams import ( bulk_add_subscriptions, bulk_remove_subscriptions, + deactivated_streams_by_old_name, do_change_stream_group_based_setting, do_change_stream_post_policy, do_deactivate_stream, + do_reactivate_stream, ) from zerver.actions.user_groups import add_subgroups_to_user_group, check_add_user_group from zerver.actions.users import do_change_user_role, do_deactivate_user @@ -1334,6 +1336,95 @@ class StreamAdminTest(ZulipTestCase): self.assertTrue(new_stream_usermessage.flags.read) self.assertFalse(denmark_usermessage.flags.read) + def test_deactivated_streams_by_old_name(self) -> None: + realm = get_realm("zulip") + stream = self.make_stream("new_stream") + do_deactivate_stream(stream, acting_user=None) + self.assertEqual(set(deactivated_streams_by_old_name(realm, "new_stream")), {stream}) + + second_stream = self.make_stream("new_stream") + do_deactivate_stream(second_stream, acting_user=None) + self.assertEqual( + set(deactivated_streams_by_old_name(realm, "new_stream")), {stream, second_stream} + ) + + self.make_stream("!DEACTIVATED:old_style") # This is left active + old_style = self.make_stream("old_style") + do_deactivate_stream(old_style, acting_user=None) + old_style.name = "!!DEACTIVATED:old_style" + old_style.save() + self.assertEqual(set(deactivated_streams_by_old_name(realm, "old_style")), {old_style}) + + def test_reactivate_stream_active_stream(self) -> None: + stream = self.make_stream("new_stream") + with self.assertRaisesRegex(JsonableError, "Stream is not currently deactivated"): + do_reactivate_stream(stream, new_name="new_stream", acting_user=None) + + def test_reactivate_stream_existing_name(self) -> None: + stream = self.make_stream("new_stream") + self.make_stream("existing") + do_deactivate_stream(stream, acting_user=None) + with self.assertRaisesRegex(JsonableError, "Stream named existing already exists"): + do_reactivate_stream(stream, new_name="existing", acting_user=None) + + def test_reactivate_stream(self) -> None: + desdemona = self.example_user("desdemona") + iago = self.example_user("iago") + hamlet = self.example_user("hamlet") + cordelia = self.example_user("cordelia") + + stream = self.make_stream("new_stream", is_web_public=True) + self.subscribe(hamlet, stream.name) + self.subscribe(cordelia, stream.name) + do_deactivate_stream(stream, acting_user=None) + with self.capture_send_event_calls(expected_num_events=4) as events: + do_reactivate_stream(stream, new_name="new_stream", acting_user=None) + + # Tell all admins and owners that the stream exists + self.assertEqual(events[0]["event"]["op"], "create") + self.assertEqual(events[0]["event"]["streams"][0]["name"], "new_stream") + self.assertEqual(events[0]["event"]["streams"][0]["stream_id"], stream.id) + self.assertEqual(set(events[0]["users"]), {iago.id, desdemona.id}) + + # Tell the owners that they're subscribed to it + self.assertEqual(events[1]["event"]["op"], "add") + self.assertEqual(events[1]["event"]["subscriptions"][0]["name"], "new_stream") + self.assertEqual(events[1]["event"]["subscriptions"][0]["stream_id"], stream.id) + self.assertEqual(events[1]["users"], [desdemona.id]) + + # Send a message there logging the reactivation + self.assertEqual(events[2]["event"]["type"], "message") + + # iago (as an admin) gets to know that desdemona (the owner) is now subscribed. + self.assertEqual( + events[3], + { + "event": { + "op": "peer_add", + "stream_ids": [stream.id], + "type": "subscription", + "user_ids": [desdemona.id], + }, + "users": [iago.id], + }, + ) + + stream = Stream.objects.get(id=stream.id) + self.assertFalse(stream.deactivated) + self.assertTrue(stream.invite_only) + self.assertFalse(stream.is_web_public) + self.assertTrue(stream.history_public_to_subscribers) + + self.assertEqual( + [desdemona.id], + [ + sub.user_profile_id + for sub in get_active_subscriptions_for_stream_id( + stream.id, include_deactivated_users=True + ) + ], + ) + def test_vacate_private_stream_removes_default_stream(self) -> None: stream = self.make_stream("new_stream", invite_only=True) self.subscribe(self.example_user("hamlet"), stream.name)