delete_topic: Delete UserTopic rows when a topic is deleted.

Earlier, when a topic was deleted then UserTopic rows corresponding
to that topic were not deleted resulting in a bug where the topic
is listed in the '/#settings/topics' panel even after deletion.

This commit fixes the incorrect behavior to delete the concerned
UserTopic rows.

We need to take special care of the case when a topic is deleted
in private channel with protected history. We delete the UserTopic
records for exactly the users for whom after the topic deletion
action, they no longer have access to any messages in the topic.

tools/linter_lib/custom_check.py

@@ -246,6 +246,7 @@ python_rules = RuleList(
             "exclude": FILES_WITH_LEGACY_SUBJECT,
             "exclude_line": {
                 ("zerver/lib/message.py", "message__subject__iexact=message.topic_name(),"),
+                ("zerver/views/streams.py", "message__subject__iexact=topic_name,"),
             },
             "include_only": {
                 "zerver/data_import/",

zerver/tests/test_message_topics.py

@@ -4,7 +4,8 @@ from django.utils.timezone import now as timezone_now
 
 from zerver.actions.streams import do_change_stream_permission
 from zerver.lib.test_classes import ZulipTestCase
-from zerver.models import Message, UserMessage
+from zerver.lib.user_topics import set_topic_visibility_policy, topic_has_visibility_policy
+from zerver.models import Message, UserMessage, UserTopic
 from zerver.models.clients import get_client
 from zerver.models.realms import get_realm
 from zerver.models.streams import get_stream
@@ -249,11 +250,19 @@ class TopicDeleteTest(ZulipTestCase):
             acting_user=user_profile,
         )
 
-        # ADMIN USER subscribed now
+        # NON-ADMIN USER follows the topic
+        set_topic_visibility_policy(
+            user_profile, [[stream_name, topic_name]], UserTopic.VisibilityPolicy.FOLLOWED
+        )
+
+        # ADMIN USER subscribed now and follows the topic
         user_profile = self.example_user("iago")
         self.subscribe(user_profile, stream_name)
         self.login_user(user_profile)
         new_last_msg_id = self.send_stream_message(user_profile, stream_name, topic_name=topic_name)
+        set_topic_visibility_policy(
+            user_profile, [[stream_name, topic_name]], UserTopic.VisibilityPolicy.FOLLOWED
+        )
 
         # Now admin deletes all messages in topic -- which should only
         # delete new_last_msg_id, i.e. the one sent since they joined.
@@ -268,6 +277,26 @@ class TopicDeleteTest(ZulipTestCase):
         self.assertTrue(result_dict["complete"])
         self.assertTrue(Message.objects.filter(id=last_msg_id).exists())
 
+        # Verify that we delete the UserTopic row only for 'iago' (ADMIN USER) as they can't
+        # access any messages in the topic. 'hamlet' (NON ADMIN USER) can still access the
+        # protected messages hence the UserTopic row for him is not deleted.
+        self.assertTrue(
+            topic_has_visibility_policy(
+                self.example_user("hamlet"),
+                stream.id,
+                topic_name,
+                UserTopic.VisibilityPolicy.FOLLOWED,
+            )
+        )
+        self.assertFalse(
+            topic_has_visibility_policy(
+                self.example_user("iago"),
+                stream.id,
+                topic_name,
+                UserTopic.VisibilityPolicy.FOLLOWED,
+            )
+        )
+
         # Try to delete all messages in the topic again. There are no messages accessible
         # to the administrator, so this should do nothing.
         result = self.client_post(

zerver/views/streams.py

@@ -40,6 +40,7 @@ from zerver.actions.streams import (
     do_rename_stream,
     get_subscriber_ids,
 )
+from zerver.actions.user_topics import bulk_do_set_user_topic_visibility_policy
 from zerver.context_processors import get_valid_realm_from_request
 from zerver.decorator import (
     check_if_user_can_manage_default_streams,
@@ -92,9 +93,10 @@ from zerver.lib.user_groups import (
     parse_group_setting_value,
     validate_group_setting_value_change,
 )
+from zerver.lib.user_topics import get_users_with_user_topic_visibility_policy
 from zerver.lib.users import bulk_access_users_by_email, bulk_access_users_by_id
 from zerver.lib.utils import assert_is_not_none
-from zerver.models import Realm, Stream, UserProfile
+from zerver.models import Realm, Stream, UserMessage, UserProfile, UserTopic
 from zerver.models.users import get_system_bot
 
 
@@ -989,6 +991,38 @@ def delete_in_topic(
                 break
             do_delete_messages(user_profile.realm, messages_to_delete, acting_user=user_profile)
 
+    # Since the topic no longer exists, remove the user topic rows.
+    users_with_stale_user_topic_rows = [
+        user_topic.user_profile
+        for user_topic in get_users_with_user_topic_visibility_policy(stream.id, topic_name)
+    ]
+
+    if not stream.is_history_public_to_subscribers():
+        # In a private channel with protected history, delete the UserTopic
+        # records for exactly the users for whom after the topic deletion
+        # action, they no longer have access to any messages in the topic.
+        user_ids_with_access_to_protected_messages = set(
+            UserMessage.objects.filter(
+                user_profile__in=users_with_stale_user_topic_rows,
+                message__recipient_id=assert_is_not_none(stream.recipient_id),
+                message__subject__iexact=topic_name,
+            ).values_list("user_profile", flat=True)
+        )
+        users_with_stale_user_topic_rows = list(
+            filter(
+                lambda user_profile: user_profile.id
+                not in user_ids_with_access_to_protected_messages,
+                users_with_stale_user_topic_rows,
+            )
+        )
+
+    bulk_do_set_user_topic_visibility_policy(
+        users_with_stale_user_topic_rows,
+        stream,
+        topic_name,
+        visibility_policy=UserTopic.VisibilityPolicy.INHERIT,
+    )
+
     return json_success(request, data={"complete": True})