streams: Remove duplicates of get_web_public_streams_queryset.

This is a somewhat subtle function, that deserves a few comments
explaining subtle details of its logic, and there's no good reason to
have multiple copies of that logic that are slightly inconsistent.

Because the main changes here are just checking for invariant
failures, the behavioral change here should be limited to ensuring
deactivated streams are not considered available even if they were
tagged as web public streams before deactivation.
This commit is contained in:
Tim Abbott 2021-09-27 16:10:40 -07:00 committed by Tim Abbott
parent 272e81988b
commit e556481ba0
3 changed files with 22 additions and 8 deletions

View File

@ -150,6 +150,7 @@ from zerver.lib.streams import (
check_stream_name,
create_stream_if_needed,
get_default_value_for_history_public_to_subscribers,
get_web_public_streams_queryset,
render_stream_description,
send_stream_creation_event,
subscribed_to_stream,
@ -6596,7 +6597,7 @@ def get_web_public_subs(realm: Realm) -> SubscriptionInfo:
return color
subscribed = []
for stream in Stream.objects.filter(realm=realm, is_web_public=True, deactivated=False):
for stream in get_web_public_streams_queryset(realm):
stream_dict = stream.to_dict()
# Add versions of the Subscription fields based on a simulated
@ -7483,7 +7484,7 @@ def get_occupied_streams(realm: Realm) -> QuerySet:
def get_web_public_streams(realm: Realm) -> List[Dict[str, Any]]: # nocoverage
query = Stream.objects.filter(realm=realm, deactivated=False, is_web_public=True)
query = get_web_public_streams_queryset(realm)
streams = Stream.get_client_data(query)
return streams
@ -7529,7 +7530,13 @@ def do_get_streams(
invite_only_check = Q(invite_only=False)
add_filter_option(invite_only_check)
if include_web_public:
web_public_check = Q(is_web_public=True)
# This should match get_web_public_streams_queryset
web_public_check = Q(
is_web_public=True,
invite_only=False,
history_public_to_subscribers=True,
deactivated=False,
)
add_filter_option(web_public_check)
if include_owner_subscribed and user_profile.is_bot:
bot_owner = user_profile.bot_owner

View File

@ -399,15 +399,20 @@ def get_public_streams_queryset(realm: Realm) -> "QuerySet[Stream]":
def get_web_public_streams_queryset(realm: Realm) -> "QuerySet[Stream]":
# In theory, is_web_public=True implies invite_only=False and
# history_public_to_subscribers=True, but it's safer to include
# this in the query.
# This should match the include_web_public code path in do_get_streams.
return Stream.objects.filter(
realm=realm,
is_web_public=True,
# In theory, nothing conflicts with allowing web-public access
# to deactivated streams. However, we should offer a way to
# review archived streams and adjust their settings before
# allowing that configuration to exist.
deactivated=False,
# In theory, is_web_public=True implies invite_only=False and
# history_public_to_subscribers=True, but it's safer to include
# these in the query.
invite_only=False,
history_public_to_subscribers=True,
is_web_public=True,
)

View File

@ -874,7 +874,9 @@ class Realm(models.Model):
if not self.web_public_streams_enabled():
return False
return Stream.objects.filter(realm=self, is_web_public=True).exists()
from zerver.lib.streams import get_web_public_streams_queryset
return get_web_public_streams_queryset(self).exists()
post_save.connect(flush_realm, sender=Realm)