user_topics: Refactor remove_topic_mute.

This commit replaces 'remove_topic_mute' with
'set_user_topic_visibility_policy_in_database' and
updates it to delete UserTopic row with any configured
visibility_policy and not just muting.

zerver/actions/user_topics.py

@@ -8,7 +8,6 @@ from zerver.lib.exceptions import JsonableError
 from zerver.lib.timestamp import datetime_to_timestamp
 from zerver.lib.user_topics import (
     get_topic_mutes,
-    remove_topic_mute,
     set_user_topic_visibility_policy_in_database,
 )
 from zerver.models import Stream, UserProfile, UserTopic
@@ -30,9 +29,11 @@ def do_set_user_topic_visibility_policy(
 
     if visibility_policy == UserTopic.VISIBILITY_POLICY_INHERIT:
         try:
-            remove_topic_mute(user_profile, stream.id, topic)
+            set_user_topic_visibility_policy_in_database(
+                user_profile, stream.id, topic, visibility_policy=visibility_policy
+            )
         except UserTopic.DoesNotExist:
-            raise JsonableError(_("Topic is not muted"))
+            raise JsonableError(_("Nothing to be done"))
     else:
         assert stream.recipient_id is not None
         set_user_topic_visibility_policy_in_database(

zerver/lib/user_topics.py

@@ -115,10 +115,20 @@ def set_user_topic_visibility_policy_in_database(
     topic_name: str,
     *,
     visibility_policy: int,
-    recipient_id: int,
+    recipient_id: Optional[int] = None,
     last_updated: Optional[datetime.datetime] = None,
     ignore_duplicate: bool = False,
 ) -> None:
+    if visibility_policy == UserTopic.VISIBILITY_POLICY_INHERIT:
+        # Will throw UserTopic.DoesNotExist if the user doesn't
+        # already have a visibility policy for this topic.
+        UserTopic.objects.get(
+            user_profile=user_profile,
+            stream_id=stream_id,
+            topic_name__iexact=topic_name,
+        ).delete()
+        return
+
     assert last_updated is not None
     (row, created) = UserTopic.objects.get_or_create(
         user_profile=user_profile,
@@ -155,16 +165,6 @@ def set_user_topic_visibility_policy_in_database(
     row.save(update_fields=["visibility_policy", "last_updated"])
 
 
-def remove_topic_mute(user_profile: UserProfile, stream_id: int, topic_name: str) -> None:
-    row = UserTopic.objects.get(
-        user_profile=user_profile,
-        stream_id=stream_id,
-        topic_name__iexact=topic_name,
-        visibility_policy=UserTopic.MUTED,
-    )
-    row.delete()
-
-
 def topic_is_muted(user_profile: UserProfile, stream_id: int, topic_name: str) -> bool:
     is_muted = UserTopic.objects.filter(
         user_profile=user_profile,

zerver/tests/test_user_topics.py

@@ -2,6 +2,7 @@ from datetime import datetime, timezone
 from typing import Any, Dict, List
 from unittest import mock
 
+from django.db import transaction
 from django.utils.timezone import now as timezone_now
 
 from zerver.actions.user_topics import do_set_user_topic_visibility_policy
@@ -184,9 +185,15 @@ class MutedTopicsTests(ZulipTestCase):
         result = self.api_patch(user, url, data)
         self.assert_json_error(result, "Topic is not muted")
 
-        data = {"stream": stream.name, "topic": "BOGUS", "op": "remove"}
-        result = self.api_patch(user, url, data)
-        self.assert_json_error(result, "Topic is not muted")
+        with transaction.atomic():
+            # This API call needs a new nested transaction with 'savepoint=True',
+            # because it calls 'set_user_topic_visibility_policy_in_database',
+            # which on failure rollbacks the test-transaction.
+            # If it is not used, the test-transaction will be rolled back during this API call,
+            # and the next API call will result in a "TransactionManagementError."
+            data = {"stream": stream.name, "topic": "BOGUS", "op": "remove"}
+            result = self.api_patch(user, url, data)
+            self.assert_json_error(result, "Nothing to be done")
 
         data = {"stream_id": 999999999, "topic": "BOGUS", "op": "remove"}
         result = self.api_patch(user, url, data)