Accept stream_id for muting endpoints.

zerver/lib/streams.py

@@ -10,6 +10,13 @@ from zerver.models import UserProfile, Stream, Subscription, \
     Realm, Recipient, bulk_get_recipients, get_stream_recipient, get_stream, \
     bulk_get_streams, get_realm_stream, DefaultStreamGroup
 
+def check_for_exactly_one_stream_arg(stream_id: Optional[int], stream: Optional[str]) -> None:
+    if stream_id is None and stream is None:
+        raise JsonableError(_("Please supply 'stream'."))
+
+    if stream_id is not None and stream is not None:
+        raise JsonableError(_("Please choose one: 'stream' or 'stream_id'."))
+
 def access_stream_for_delete_or_update(user_profile: UserProfile, stream_id: int) -> Stream:
 
     # We should only ever use this for realm admins, who are allowed
@@ -114,7 +121,9 @@ def access_stream_by_name(user_profile: UserProfile,
                                             allow_realm_admin=allow_realm_admin)
     return (stream, recipient, sub)
 
-def access_stream_for_unmute_topic(user_profile: UserProfile, stream_name: str, error: str) -> Stream:
+def access_stream_for_unmute_topic_by_name(user_profile: UserProfile,
+                                           stream_name: str,
+                                           error: str) -> Stream:
     """
     It may seem a little silly to have this helper function for unmuting
     topics, but it gets around a linter warning, and it helps to be able
@@ -134,6 +143,15 @@ def access_stream_for_unmute_topic(user_profile: UserProfile, stream_name: str,
         raise JsonableError(error)
     return stream
 
+def access_stream_for_unmute_topic_by_id(user_profile: UserProfile,
+                                         stream_id: int,
+                                         error: str) -> Stream:
+    try:
+        stream = Stream.objects.get(id=stream_id, realm_id=user_profile.realm_id)
+    except Stream.DoesNotExist:
+        raise JsonableError(error)
+    return stream
+
 def can_access_stream_history_by_name(user_profile: UserProfile, stream_name: str) -> bool:
     """Determine whether the provided user is allowed to access the
     history of the target stream.  The stream is specified by name.

zerver/tests/test_muting.py

@@ -20,6 +20,7 @@ from zerver.models import (
 from zerver.lib.topic_mutes import (
     add_topic_mute,
     get_topic_mutes,
+    remove_topic_mute,
     topic_is_muted,
 )
 
@@ -57,74 +58,120 @@ class MutedTopicsTests(ZulipTestCase):
         self.assertEqual(user_ids, {hamlet.id, cordelia.id})
 
     def test_add_muted_topic(self) -> None:
-        email = self.example_email('hamlet')
+        user = self.example_user('hamlet')
-        self.login(email)
+        self.login(user.email)
+
+        stream = get_stream('Verona', user.realm)
 
         url = '/api/v1/users/me/subscriptions/muted_topics'
-        data = {'stream': 'Verona', 'topic': 'Verona3', 'op': 'add'}
-        result = self.api_patch(email, url, data)
-        self.assert_json_success(result)
 
-        user = self.example_user('hamlet')
+        payloads = [
-        self.assertIn([u'Verona', u'Verona3'], get_topic_mutes(user))
+            {'stream': stream.name, 'topic': 'Verona3', 'op': 'add'},
+            {'stream_id': stream.id, 'topic': 'Verona3', 'op': 'add'},
+        ]
 
-        stream = get_stream(u'Verona', user.realm)
+        for data in payloads:
-        self.assertTrue(topic_is_muted(user, stream.id, 'Verona3'))
+            result = self.api_patch(user.email, url, data)
-        self.assertTrue(topic_is_muted(user, stream.id, 'verona3'))
+            self.assert_json_success(result)
+
+            self.assertIn([stream.name, 'Verona3'], get_topic_mutes(user))
+
+            self.assertTrue(topic_is_muted(user, stream.id, 'Verona3'))
+            self.assertTrue(topic_is_muted(user, stream.id, 'verona3'))
+
+            remove_topic_mute(
+                user_profile=user,
+                stream_id=stream.id,
+                topic_name='Verona3',
+            )
 
     def test_remove_muted_topic(self) -> None:
-        self.user_profile = self.example_user('hamlet')
+        user = self.example_user('hamlet')
-        email = self.user_profile.email
+        email = user.email
+        realm = user.realm
         self.login(email)
 
-        realm = self.user_profile.realm
         stream = get_stream(u'Verona', realm)
         recipient = get_stream_recipient(stream.id)
-        add_topic_mute(
-            user_profile=self.user_profile,
-            stream_id=stream.id,
-            recipient_id=recipient.id,
-            topic_name=u'Verona3',
-        )
 
         url = '/api/v1/users/me/subscriptions/muted_topics'
-        data = {'stream': 'Verona', 'topic': 'vERONA3', 'op': 'remove'}
+        payloads = [
-        result = self.api_patch(email, url, data)
+            {'stream': stream.name, 'topic': 'vERONA3', 'op': 'remove'},
+            {'stream_id': stream.id, 'topic': 'vEroNA3', 'op': 'remove'},
+        ]
 
-        self.assert_json_success(result)
+        for data in payloads:
-        user = self.example_user('hamlet')
+            add_topic_mute(
-        self.assertNotIn([[u'Verona', u'Verona3']], get_topic_mutes(user))
+                user_profile=user,
+                stream_id=stream.id,
+                recipient_id=recipient.id,
+                topic_name='Verona3',
+            )
+            self.assertIn([stream.name, 'Verona3'], get_topic_mutes(user))
+
+            result = self.api_patch(email, url, data)
+
+            self.assert_json_success(result)
+            self.assertNotIn([stream.name, 'Verona3'], get_topic_mutes(user))
+            self.assertFalse(topic_is_muted(user, stream.id, 'verona3'))
 
     def test_muted_topic_add_invalid(self) -> None:
-        self.user_profile = self.example_user('hamlet')
+        user = self.example_user('hamlet')
-        email = self.user_profile.email
+        email = user.email
+        realm = user.realm
         self.login(email)
 
-        realm = self.user_profile.realm
+        stream = get_stream('Verona', realm)
-        stream = get_stream(u'Verona', realm)
         recipient = get_stream_recipient(stream.id)
         add_topic_mute(
-            user_profile=self.user_profile,
+            user_profile=user,
             stream_id=stream.id,
             recipient_id=recipient.id,
             topic_name=u'Verona3',
         )
 
         url = '/api/v1/users/me/subscriptions/muted_topics'
-        data = {'stream': 'Verona', 'topic': 'Verona3', 'op': 'add'}
+
+        data = {'stream': stream.name, 'topic': 'Verona3', 'op': 'add'}  # type: Dict[str, Any]
         result = self.api_patch(email, url, data)
         self.assert_json_error(result, "Topic already muted")
 
+        data = {'stream_id': 999999999, 'topic': 'Verona3', 'op': 'add'}
+        result = self.api_patch(email, url, data)
+        self.assert_json_error(result, "Invalid stream id")
+
+        data = {'topic': 'Verona3', 'op': 'add'}
+        result = self.api_patch(email, url, data)
+        self.assert_json_error(result, "Please supply 'stream'.")
+
+        data = {'stream': stream.name, 'stream_id': stream.id, 'topic': 'Verona3', 'op': 'add'}
+        result = self.api_patch(email, url, data)
+        self.assert_json_error(result, "Please choose one: 'stream' or 'stream_id'.")
+
     def test_muted_topic_remove_invalid(self) -> None:
-        self.user_profile = self.example_user('hamlet')
+        user = self.example_user('hamlet')
-        email = self.user_profile.email
+        email = user.email
+        realm = user.realm
         self.login(email)
+        stream = get_stream('Verona', realm)
 
         url = '/api/v1/users/me/subscriptions/muted_topics'
-        data = {'stream': 'BOGUS', 'topic': 'Verona3', 'op': 'remove'}
+        data = {'stream': 'BOGUS', 'topic': 'Verona3', 'op': 'remove'}  # type: Dict[str, Any]
         result = self.api_patch(email, url, data)
         self.assert_json_error(result, "Topic is not muted")
 
-        data = {'stream': 'Verona', 'topic': 'BOGUS', 'op': 'remove'}
+        data = {'stream': stream.name, 'topic': 'BOGUS', 'op': 'remove'}
         result = self.api_patch(email, url, data)
         self.assert_json_error(result, "Topic is not muted")
+
+        data = {'stream_id': 999999999, 'topic': 'BOGUS', 'op': 'remove'}
+        result = self.api_patch(email, url, data)
+        self.assert_json_error(result, "Topic is not muted")
+
+        data = {'topic': 'Verona3', 'op': 'remove'}
+        result = self.api_patch(email, url, data)
+        self.assert_json_error(result, "Please supply 'stream'.")
+
+        data = {'stream': stream.name, 'stream_id': stream.id, 'topic': 'Verona3', 'op': 'remove'}
+        result = self.api_patch(email, url, data)
+        self.assert_json_error(result, "Please choose one: 'stream' or 'stream_id'.")

zerver/views/muting.py

@@ -1,6 +1,6 @@
 
 from django.http import HttpResponse, HttpRequest
-from typing import List
+from typing import Optional
 
 import ujson
 
@@ -9,13 +9,25 @@ from zerver.lib.actions import do_mute_topic, do_unmute_topic
 from zerver.lib.request import has_request_variables, REQ
 from zerver.lib.response import json_success, json_error
 from zerver.lib.topic_mutes import topic_is_muted
-from zerver.lib.streams import access_stream_by_name, access_stream_for_unmute_topic
+from zerver.lib.streams import (
-from zerver.lib.validator import check_string, check_list
+    access_stream_by_id,
+    access_stream_by_name,
+    access_stream_for_unmute_topic_by_id,
+    access_stream_for_unmute_topic_by_name,
+    check_for_exactly_one_stream_arg,
+)
+from zerver.lib.validator import check_int
 from zerver.models import get_stream, Stream, UserProfile
 
-def mute_topic(user_profile: UserProfile, stream_name: str,
+def mute_topic(user_profile: UserProfile,
+               stream_id: Optional[int],
+               stream_name: Optional[str],
                topic_name: str) -> HttpResponse:
-    (stream, recipient, sub) = access_stream_by_name(user_profile, stream_name)
+    if stream_name is not None:
+        (stream, recipient, sub) = access_stream_by_name(user_profile, stream_name)
+    else:
+        assert stream_id is not None
+        (stream, recipient, sub) = access_stream_by_id(user_profile, stream_id)
 
     if topic_is_muted(user_profile, stream.id, topic_name):
         return json_error(_("Topic already muted"))
@@ -23,10 +35,17 @@ def mute_topic(user_profile: UserProfile, stream_name: str,
     do_mute_topic(user_profile, stream, recipient, topic_name)
     return json_success()
 
-def unmute_topic(user_profile: UserProfile, stream_name: str,
+def unmute_topic(user_profile: UserProfile,
+                 stream_id: Optional[int],
+                 stream_name: Optional[str],
                  topic_name: str) -> HttpResponse:
     error = _("Topic is not muted")
-    stream = access_stream_for_unmute_topic(user_profile, stream_name, error)
+
+    if stream_name is not None:
+        stream = access_stream_for_unmute_topic_by_name(user_profile, stream_name, error)
+    else:
+        assert stream_id is not None
+        stream = access_stream_for_unmute_topic_by_id(user_profile, stream_id, error)
 
     if not topic_is_muted(user_profile, stream.id, topic_name):
         return json_error(error)
@@ -35,10 +54,26 @@ def unmute_topic(user_profile: UserProfile, stream_name: str,
     return json_success()
 
 @has_request_variables
-def update_muted_topic(request: HttpRequest, user_profile: UserProfile, stream: str=REQ(),
+def update_muted_topic(request: HttpRequest,
-                       topic: str=REQ(), op: str=REQ()) -> HttpResponse:
+                       user_profile: UserProfile,
+                       stream_id: Optional[int]=REQ(validator=check_int, default=None),
+                       stream: Optional[str]=REQ(default=None),
+                       topic: str=REQ(),
+                       op: str=REQ()) -> HttpResponse:
+
+    check_for_exactly_one_stream_arg(stream_id=stream_id, stream=stream)
 
     if op == 'add':
-        return mute_topic(user_profile, stream, topic)
+        return mute_topic(
+            user_profile=user_profile,
+            stream_id=stream_id,
+            stream_name=stream,
+            topic_name=topic,
+        )
     elif op == 'remove':
-        return unmute_topic(user_profile, stream, topic)
+        return unmute_topic(
+            user_profile=user_profile,
+            stream_id=stream_id,
+            stream_name=stream,
+            topic_name=topic,
+        )