From 884aa2b1841791203048f0e93d6ac05bc4a3c20c Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Sun, 29 Jan 2017 21:42:09 -0800 Subject: [PATCH] streams: Eliminate last use of get_stream in views. --- tools/lint-all | 2 -- zerver/lib/streams.py | 10 ++++++++-- zerver/views/streams.py | 8 +++----- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/tools/lint-all b/tools/lint-all index 259a1bde16..0223f4c58b 100755 --- a/tools/lint-all +++ b/tools/lint-all @@ -419,8 +419,6 @@ def build_custom_checkers(by_lang): # that you are no longer subscribed to, so need get_stream. 'exclude': set(['zerver/views/messages.py']), 'exclude_line': set([ - # This is a check for whether a stream rename is invalid because it already exists - ('zerver/views/streams.py', 'if get_stream(new_name, user_profile.realm) is not None:'), # This is a check for whether a stream rename is invalid because it already exists ('zerver/lib/actions.py', 'existing_deactivated_stream = get_stream(new_name, stream.realm)'), # This one in check_message is kinda terrible, since it's diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index 5d9b8e6d3e..62711f0edc 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -5,10 +5,10 @@ from typing import Any, Iterable, List, Mapping, Set, Text, Tuple from django.http import HttpRequest, HttpResponse from django.utils.translation import ugettext as _ -from zerver.lib.actions import create_streams_if_needed +from zerver.lib.actions import check_stream_name, create_streams_if_needed from zerver.lib.request import JsonableError from zerver.models import UserProfile, Stream, Subscription, \ - Recipient, bulk_get_recipients, get_recipient, get_stream, \ + Realm, Recipient, bulk_get_recipients, get_recipient, get_stream, \ bulk_get_streams, valid_stream_name def access_stream_common(user_profile, stream, error): @@ -55,6 +55,12 @@ def access_stream_by_id(user_profile, stream_id): (recipient, sub) = access_stream_common(user_profile, stream, error) return (stream, recipient, sub) +def check_stream_name_available(realm, name): + # type: (Realm, Text) -> None + check_stream_name(name) + if get_stream(name, realm) is not None: + raise JsonableError(_("Stream name '%s' is already taken") % (name,)) + def access_stream_by_name(user_profile, stream_name): # type: (UserProfile, Text) -> Tuple[Stream, Recipient, Subscription] error = _("Invalid stream name '%s'" % (stream_name,)) diff --git a/zerver/views/streams.py b/zerver/views/streams.py index 0228dc1149..68752614b7 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -12,14 +12,14 @@ from zerver.decorator import authenticated_json_post_view, \ get_user_profile_by_email, require_realm_admin, to_non_negative_int from zerver.lib.actions import bulk_remove_subscriptions, \ do_change_subscription_property, internal_prep_message, \ - check_stream_name, gather_subscriptions, subscribed_to_stream, \ + gather_subscriptions, subscribed_to_stream, \ bulk_add_subscriptions, do_send_messages, get_subscriber_emails, do_rename_stream, \ do_deactivate_stream, do_change_stream_invite_only, do_add_default_stream, \ do_change_stream_description, do_get_streams, \ do_remove_default_stream, get_topic_history_for_stream from zerver.lib.response import json_success, json_error, json_response from zerver.lib.streams import access_stream_by_id, access_stream_by_name, \ - filter_stream_authorization, list_to_streams + check_stream_name_available, filter_stream_authorization, list_to_streams from zerver.lib.validator import check_string, check_list, check_dict, \ check_bool, check_variable_type from zerver.models import UserProfile, Stream, Realm, Subscription, \ @@ -99,11 +99,9 @@ def update_stream_backend(request, user_profile, stream_id, if new_name is not None: new_name = new_name.strip() # Will raise if the new name has invalid characters. - check_stream_name(new_name) if stream.name.lower() == new_name.lower(): return json_error(_("Stream already has that name!")) - if get_stream(new_name, user_profile.realm) is not None: - raise JsonableError(_("Stream name '%s' is already taken") % (new_name,)) + check_stream_name_available(user_profile.realm, new_name) do_rename_stream(stream, new_name) if is_private is not None: do_change_stream_invite_only(stream, is_private)