stream_exists_backend: Use access_stream_by_name.

tools/lint-all

@@ -364,6 +364,7 @@ def build_custom_checkers(by_lang):
              ('zerver/views/invite.py',
               'return json_error(data=error_data, msg=ret_error)'),
              ('zerver/views/streams.py', 'return json_error(property_conversion)'),
+             ('zerver/views/streams.py', 'return json_error(e.error, data=result, status=404)'),
              # We can't do anything about this.
              ('zerver/views/realm_emoji.py', 'return json_error(e.messages[0])'),
              ('zerver/views/realm_filters.py', 'return json_error(e.messages[0], data={"errors": dict(e)})'),

zerver/tests/test_subs.py

@@ -570,8 +570,7 @@ def test_admin_remove_others_from_unsubbed_private_stream(self):
         result = self.attempt_unsubscribe_of_principal(
             is_admin=True, is_subbed=False, invite_only=True,
             other_user_subbed=True)
-        self.assert_json_error(
-            result, "Cannot administer invite-only streams this way")
+        self.assert_json_error(result, "Cannot administer invite-only streams this way")
 
     def test_create_stream_by_admins_only_setting(self):
         # type: () -> None
@@ -1927,13 +1926,26 @@ def test_existing_subscriptions_autosubscription(self):
         """
         Call /json/subscriptions/exist on an existing stream and autosubscribe to it.
         """
-        stream_name = self.streams[0]
+        stream_name = "new_public_stream"
+        result = self.common_subscribe_to_streams("[email protected]", [stream_name],
+                                                  invite_only=False)
         result = self.client_post("/json/subscriptions/exists",
-                                  {"stream": stream_name, "autosubscribe": True})
+                                  {"stream": stream_name, "autosubscribe": "false"})
         self.assert_json_success(result)
         json = ujson.loads(result.content)
         self.assertIn("exists", json)
         self.assertTrue(json["exists"])
+        self.assertIn("subscribed", json)
+        self.assertFalse(json["subscribed"])
+
+        result = self.client_post("/json/subscriptions/exists",
+                                  {"stream": stream_name, "autosubscribe": "true"})
+        self.assert_json_success(result)
+        json = ujson.loads(result.content)
+        self.assertIn("exists", json)
+        self.assertTrue(json["exists"])
+        self.assertIn("subscribed", json)
+        self.assertTrue(json["subscribed"])
 
     def test_existing_subscriptions_autosubscription_private_stream(self):
         # type: () -> None
@@ -1946,17 +1958,24 @@ def test_existing_subscriptions_autosubscription_private_stream(self):
         stream = get_stream(stream_name, self.realm)
 
         result = self.client_post("/json/subscriptions/exists",
-                                  {"stream": stream_name, "autosubscribe": True})
+                                  {"stream": stream_name, "autosubscribe": "true"})
+        # We can't see invite-only streams here
+        self.assert_json_error(result, "Invalid stream name 'Saxony'", status_code=404)
+        # Importantly, we are not now subscribed
+        self.assertEqual(Subscription.objects.filter(
+            recipient__type=Recipient.STREAM,
+            recipient__type_id=stream.id).count(), 1)
+
+        # A user who is subscribed still sees the stream exists
+        self.login("[email protected]")
+        result = self.client_post("/json/subscriptions/exists",
+                                  {"stream": stream_name, "autosubscribe": "false"})
         self.assert_json_success(result)
         json = ujson.loads(result.content)
         self.assertIn("exists", json)
         self.assertTrue(json["exists"])
         self.assertIn("subscribed", json)
-        # Importantly, we are not now subscribed
-        self.assertFalse(json["subscribed"])
-        self.assertEqual(Subscription.objects.filter(
-            recipient__type=Recipient.STREAM,
-            recipient__type_id=stream.id).count(), 1)
+        self.assertTrue(json["subscribed"])
 
     def get_subscription(self, user_profile, stream_name):
         # type: (UserProfile, Text) -> Subscription

zerver/views/streams.py

@@ -32,12 +32,6 @@
 import six
 from typing import Text
 
-def is_active_subscriber(user_profile, recipient):
-    # type: (UserProfile, Recipient) -> bool
-    return Subscription.objects.filter(user_profile=user_profile,
-                                       recipient=recipient,
-                                       active=True).exists()
-
 def list_to_streams(streams_raw, user_profile, autocreate=False):
     # type: (Iterable[Mapping[str, Any]], UserProfile, Optional[bool]) -> Tuple[List[Stream], List[Stream]]
     """Converts list of dicts to a list of Streams, validating input in the process
@@ -439,45 +433,33 @@ def get_topics_backend(request, user_profile,
     # so the most recent topic is the first element of the list.
     return json_success(dict(topics=result))
 
-
 @authenticated_json_post_view
 @has_request_variables
-def json_stream_exists(request, user_profile, stream=REQ(),
-                       autosubscribe=REQ(default=False)):
+def json_stream_exists(request, user_profile, stream_name=REQ("stream"),
+                       autosubscribe=REQ(validator=check_bool, default=False)):
     # type: (HttpRequest, UserProfile, Text, bool) -> HttpResponse
-    if not valid_stream_name(stream):
+    if not valid_stream_name(stream_name):
         return json_error(_("Invalid characters in stream name"))
-    try:
-        stream_id = Stream.objects.get(realm=user_profile.realm, name=stream).id
-    except Stream.DoesNotExist:
-        stream_id = None
-    return stream_exists_backend(request, user_profile, stream_id, autosubscribe)
 
-def stream_exists_backend(request, user_profile, stream_id, autosubscribe):
-    # type: (HttpRequest, UserProfile, int, bool) -> HttpResponse
     try:
-        stream = get_and_validate_stream_by_id(stream_id, user_profile.realm)
-    except JsonableError:
-        stream = None
-    result = {"exists": bool(stream)}
-    if stream is not None:
-        recipient = get_recipient(Recipient.STREAM, stream.id)
-        if not stream.invite_only and autosubscribe:
-            bulk_add_subscriptions([stream], [user_profile])
-        result["subscribed"] = is_active_subscriber(
-            user_profile=user_profile,
-            recipient=recipient)
-
-        return json_success(result) # results are ignored for HEAD requests
-    return json_response(data=result, status=404)
-
-def get_and_validate_stream_by_id(stream_id, realm):
-    # type: (int, Realm) -> Stream
-    try:
-        stream = Stream.objects.get(pk=stream_id, realm_id=realm.id)
-    except Stream.DoesNotExist:
-        raise JsonableError(_("Invalid stream id"))
-    return stream
+        (stream, recipient, sub) = access_stream_by_name(user_profile, stream_name)
+    except JsonableError as e:
+        result = {"exists": False}
+        return json_error(e.error, data=result, status=404)
+
+    # access_stream functions return a subscription if and only if we
+    # are already subscribed.
+    result = {"exists": True,
+              "subscribed": sub is not None}
+
+    # If we got here, we're either subscribed or the stream is public.
+    # So if we're not yet subscribed and autosubscribe is enabled, we
+    # should join.
+    if sub is None and autosubscribe:
+        bulk_add_subscriptions([stream], [user_profile])
+        result["subscribed"] = True
+
+    return json_success(result) # results are ignored for HEAD requests
 
 @has_request_variables
 def json_get_stream_id(request, user_profile, stream_name=REQ('stream')):