do_deactivate_stream: Do not rename streams during archiving.

Functions related to archived streams are also updated.

zerver/actions/streams.py

@@ -1,4 +1,3 @@
-import hashlib
 from collections import defaultdict
 from collections.abc import Collection, Iterable, Mapping
 from typing import Any, TypeAlias
@@ -146,20 +145,8 @@ def do_deactivate_stream(stream: Stream, *, acting_user: UserProfile | None) ->
     # the code to unset is_web_public field on attachments below.
     stream.deactivated = True
     stream.invite_only = True
-    # Preserve as much as possible the original stream name while giving it a
-    # special prefix that both indicates that the stream is deactivated and
-    # frees up the original name for reuse.
-    old_name = stream.name
 
-    # Prepend a substring of the hashed stream ID to the new stream name
+    stream.save(update_fields=["deactivated", "invite_only"])
-    streamID = str(stream.id)
-    stream_id_hash_object = hashlib.sha512(streamID.encode())
-    hashed_stream_id = stream_id_hash_object.hexdigest()[0:7]
-
-    new_name = (hashed_stream_id + "!DEACTIVATED:" + old_name)[: Stream.MAX_NAME_LENGTH]
-
-    stream.name = new_name[: Stream.MAX_NAME_LENGTH]
-    stream.save(update_fields=["name", "deactivated", "invite_only"])
 
     assert stream.recipient_id is not None
     if was_web_public:
@@ -191,7 +178,7 @@ def do_deactivate_stream(stream: Stream, *, acting_user: UserProfile | None) ->
         do_remove_streams_from_default_stream_group(stream.realm, group, [stream])
 
     stream_dict = stream_to_dict(stream)
-    stream_dict.update(dict(name=old_name, invite_only=was_invite_only))
+    stream_dict.update(dict(invite_only=was_invite_only))
     event = dict(type="stream", op="delete", streams=[stream_dict])
     send_event_on_commit(stream.realm, event, affected_user_ids)
 
@@ -222,7 +209,8 @@ def deactivated_streams_by_old_name(realm: Realm, stream_name: str) -> QuerySet[
         # 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}")
+        Q(name=stream_name)
+        | 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
@@ -237,7 +225,7 @@ def do_unarchive_stream(stream: Stream, new_name: str, *, acting_user: UserProfi
     realm = stream.realm
     if not stream.deactivated:
         raise JsonableError(_("Channel is not currently deactivated"))
-    if Stream.objects.filter(realm=realm, name=new_name).exists():
+    if stream.name != new_name and Stream.objects.filter(realm=realm, name=new_name).exists():
         raise JsonableError(
             _("Channel named {channel_name} already exists").format(channel_name=new_name)
         )

zerver/tests/test_realm.py

@@ -656,8 +656,7 @@ class RealmTest(ZulipTestCase):
         stats = merge_streams(realm, denmark, atlantis)
         self.assertEqual(stats, (1, 1, 1))
 
-        with self.assertRaises(Stream.DoesNotExist):
+        self.assertEqual(get_stream("Atlantis", realm).deactivated, True)
-            get_stream("Atlantis", realm)
 
         stats = merge_streams(realm, denmark, new_stream_announcements_stream)
         self.assertEqual(stats, (2, new_stream_announcements_stream_messages_count, 10))

zerver/tests/test_subs.py

@@ -1487,12 +1487,6 @@ class StreamAdminTest(ZulipTestCase):
         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)
@@ -2479,11 +2473,6 @@ class StreamAdminTest(ZulipTestCase):
         realm = stream.realm
         stream_id = stream.id
 
-        # Simulate that a stream by the same name has already been
-        # deactivated, just to exercise our renaming logic:
-        # Since we do not know the id of these simulated stream we prepend the name with a random hashed_stream_id
-        ensure_stream(realm, "DB32B77!DEACTIVATED:" + active_name, acting_user=None)
-
         with self.capture_send_event_calls(expected_num_events=1) as events:
             result = self.client_delete("/json/streams/" + str(stream_id))
         self.assert_json_success(result)
@@ -2498,16 +2487,18 @@ class StreamAdminTest(ZulipTestCase):
         self.assertEqual(event["op"], "delete")
         self.assertEqual(event["streams"][0]["stream_id"], stream.id)
 
-        with self.assertRaises(Stream.DoesNotExist):
-            Stream.objects.get(realm=get_realm("zulip"), name=active_name)
-
-        # A deleted stream's name is changed, is deactivated, is invite-only,
-        # and has no subscribers.
         hashed_stream_id = hashlib.sha512(str(stream_id).encode()).hexdigest()[0:7]
-        deactivated_stream_name = hashed_stream_id + "!DEACTIVATED:" + active_name
+        old_deactivated_stream_name = hashed_stream_id + "!DEACTIVATED:" + active_name
+
+        with self.assertRaises(Stream.DoesNotExist):
+            Stream.objects.get(realm=get_realm("zulip"), name=old_deactivated_stream_name)
+
+        # An archived stream is deactivated, is invite-only,
+        # and has no subscribers.
+        deactivated_stream_name = active_name
         deactivated_stream = get_stream(deactivated_stream_name, realm)
         self.assertTrue(deactivated_stream.deactivated)
-        self.assertTrue(deactivated_stream.invite_only)
+        self.assertEqual(deactivated_stream.invite_only, True)
         self.assertEqual(deactivated_stream.name, deactivated_stream_name)
         subscribers = self.users_subscribed_to_stream(deactivated_stream_name, realm)
         self.assertEqual(subscribers, [])
@@ -2515,10 +2506,9 @@ class StreamAdminTest(ZulipTestCase):
         # It doesn't show up in the list of public streams anymore.
         result = self.client_get("/json/streams", {"include_subscribed": "false"})
         public_streams = [s["name"] for s in self.assert_json_success(result)["streams"]]
-        self.assertNotIn(active_name, public_streams)
         self.assertNotIn(deactivated_stream_name, public_streams)
 
-        # Even if you could guess the new name, you can't subscribe to it.
+        # You can't subscribe to archived stream.
         result = self.common_subscribe_to_streams(
             self.example_user("hamlet"), [deactivated_stream_name], allow_fail=True
         )