narrow: Fix unmuted and followed topics not visible in combined feed.

This commit is contained in:
Aman Agrawal
2024-12-05 16:16:18 +05:30
committed by Tim Abbott
parent 276e547824
commit 4b82c3258f
3 changed files with 103 additions and 36 deletions

View File

@@ -56,7 +56,7 @@ from zerver.lib.topic_sqlalchemy import (
topic_match_sa,
)
from zerver.lib.types import Validator
from zerver.lib.user_topics import exclude_topic_mutes
from zerver.lib.user_topics import exclude_stream_and_topic_mutes
from zerver.lib.validator import (
check_bool,
check_required_string,
@@ -1005,23 +1005,7 @@ def exclude_muting_conditions(
except Stream.DoesNotExist:
pass
# Channel-level muting only applies when looking at views that
# include multiple channels, since we do want users to be able to
# browser messages within a muted channel.
if channel_id is None:
rows = Subscription.objects.filter(
user_profile=user_profile,
active=True,
is_muted=True,
recipient__type=Recipient.STREAM,
).values("recipient_id")
muted_recipient_ids = [row["recipient_id"] for row in rows]
if len(muted_recipient_ids) > 0:
# Only add the condition if we have muted channels to simplify/avoid warnings.
condition = not_(column("recipient_id", Integer).in_(muted_recipient_ids))
conditions.append(condition)
conditions = exclude_topic_mutes(conditions, user_profile, channel_id)
conditions = exclude_stream_and_topic_mutes(conditions, user_profile, channel_id)
# Muted user logic for hiding messages is implemented entirely
# client-side. This is by design, as it allows UI to hint that

View File

@@ -14,7 +14,7 @@ from sqlalchemy.types import Integer
from zerver.lib.timestamp import datetime_to_timestamp
from zerver.lib.topic_sqlalchemy import topic_match_sa
from zerver.lib.types import UserTopicDict
from zerver.models import UserProfile, UserTopic
from zerver.models import Recipient, Subscription, UserProfile, UserTopic
from zerver.models.streams import get_stream
@@ -228,7 +228,7 @@ def topic_has_visibility_policy(
return has_visibility_policy
def exclude_topic_mutes(
def exclude_stream_and_topic_mutes(
conditions: list[ClauseElement], user_profile: UserProfile, stream_id: int | None
) -> list[ClauseElement]:
# Note: Unlike get_topic_mutes, here we always want to
@@ -244,27 +244,78 @@ def exclude_topic_mutes(
# by not considering topic mutes outside the stream.
query = query.filter(stream_id=stream_id)
rows = query.values(
excluded_topic_rows = query.values(
"recipient_id",
"topic_name",
)
if not rows:
return conditions
class RecipientTopicDict(TypedDict):
recipient_id: int
topic_name: str
def mute_cond(row: RecipientTopicDict) -> ClauseElement:
def topic_cond(row: RecipientTopicDict) -> ClauseElement:
recipient_id = row["recipient_id"]
topic_name = row["topic_name"]
stream_cond = column("recipient_id", Integer) == recipient_id
topic_cond = topic_match_sa(topic_name)
return and_(stream_cond, topic_cond)
condition = not_(or_(*map(mute_cond, rows)))
return [*conditions, condition]
# Add this query later to reduce the number of messages it has to run on.
if excluded_topic_rows:
exclude_muted_topics_condition = not_(or_(*map(topic_cond, excluded_topic_rows)))
conditions = [*conditions, exclude_muted_topics_condition]
# Channel-level muting only applies when looking at views that
# include multiple channels, since we do want users to be able to
# browser messages within a muted channel.
if stream_id is None:
rows = Subscription.objects.filter(
user_profile=user_profile,
active=True,
is_muted=True,
recipient__type=Recipient.STREAM,
).values("recipient_id")
muted_recipient_ids = [row["recipient_id"] for row in rows]
if len(muted_recipient_ids) == 0:
return conditions
# Add entries with visibility_policy FOLLOWED or UNMUTED in muted_recipient_ids
query = UserTopic.objects.filter(
user_profile=user_profile,
recipient_id__in=muted_recipient_ids,
visibility_policy__in=[
UserTopic.VisibilityPolicy.FOLLOWED,
UserTopic.VisibilityPolicy.UNMUTED,
],
)
included_topic_rows = query.values(
"recipient_id",
"topic_name",
)
# Exclude muted_recipient_ids unless they match include_followed_or_unmuted_topics_condition
muted_stream_condition = column("recipient_id", Integer).in_(muted_recipient_ids)
if included_topic_rows:
include_followed_or_unmuted_topics_condition = or_(
*map(topic_cond, included_topic_rows)
)
exclude_muted_streams_condition = not_(
and_(
muted_stream_condition,
not_(include_followed_or_unmuted_topics_condition),
)
)
else:
# If no included topics, exclude all muted streams
exclude_muted_streams_condition = not_(muted_stream_condition)
conditions = [*conditions, exclude_muted_streams_condition]
return conditions
def build_get_topic_visibility_policy(

View File

@@ -4540,6 +4540,7 @@ WHERE NOT (recipient_id = %(recipient_id_1)s AND upper(subject) = upper(%(param_
self.assertEqual(params["param_1"], "golf")
mute_channel(realm, user_profile, "Verona")
channel_verona_id = get_recipient_id_for_channel_name(realm, "Verona")
# Using a bogus channel name should be similar to using no narrow at
# all, and we'll exclude all mutes.
@@ -4554,25 +4555,56 @@ WHERE NOT (recipient_id = %(recipient_id_1)s AND upper(subject) = upper(%(param_
expected_query = """\
SELECT id \n\
FROM zerver_message \n\
WHERE (recipient_id NOT IN (__[POSTCOMPILE_recipient_id_1])) \
AND NOT \
(recipient_id = %(recipient_id_2)s AND upper(subject) = upper(%(param_1)s) OR \
recipient_id = %(recipient_id_3)s AND upper(subject) = upper(%(param_2)s))\
WHERE NOT (recipient_id = %(recipient_id_1)s AND upper(subject) = upper(%(param_1)s) \
OR recipient_id = %(recipient_id_2)s AND upper(subject) = upper(%(param_2)s)) \
AND (recipient_id NOT IN (__[POSTCOMPILE_recipient_id_3]))\
"""
self.assertEqual(get_sqlalchemy_sql(query), expected_query)
params = get_sqlalchemy_query_params(query)
self.assertEqual(params["recipient_id_3"], [channel_verona_id])
self.assertEqual(
params["recipient_id_1"], [get_recipient_id_for_channel_name(realm, "Verona")]
)
self.assertEqual(
params["recipient_id_2"], get_recipient_id_for_channel_name(realm, "Scotland")
params["recipient_id_1"], get_recipient_id_for_channel_name(realm, "Scotland")
)
self.assertEqual(params["param_1"], "golf")
self.assertEqual(
params["recipient_id_3"], get_recipient_id_for_channel_name(realm, "web stuff")
params["recipient_id_2"], get_recipient_id_for_channel_name(realm, "web stuff")
)
self.assertEqual(params["param_2"], "css")
# check that followed topic is included in the query.
followed_topics = [
["Verona", "Hi"],
]
set_topic_visibility_policy(
user_profile, followed_topics, UserTopic.VisibilityPolicy.FOLLOWED
)
muting_conditions = exclude_muting_conditions(user_profile, narrow)
query = select(column("id", Integer)).select_from(table("zerver_message"))
query = query.where(and_(*muting_conditions))
expected_query = """\
SELECT id \n\
FROM zerver_message \n\
WHERE NOT (recipient_id = %(recipient_id_1)s AND upper(subject) = upper(%(param_1)s) \
OR recipient_id = %(recipient_id_2)s AND upper(subject) = upper(%(param_2)s)) \
AND NOT (recipient_id IN (__[POSTCOMPILE_recipient_id_3]) \
AND NOT (recipient_id = %(recipient_id_4)s AND upper(subject) = upper(%(param_3)s)))\
"""
self.assertEqual(get_sqlalchemy_sql(query), expected_query)
params = get_sqlalchemy_query_params(query)
self.assertEqual(params["recipient_id_3"], [channel_verona_id])
self.assertEqual(
params["recipient_id_1"], get_recipient_id_for_channel_name(realm, "Scotland")
)
self.assertEqual(params["param_1"], "golf")
self.assertEqual(
params["recipient_id_2"], get_recipient_id_for_channel_name(realm, "web stuff")
)
self.assertEqual(params["param_2"], "css")
self.assertEqual(params["recipient_id_4"], channel_verona_id)
self.assertEqual(params["param_3"], "Hi")
def test_get_messages_queries(self) -> None:
query_ids = self.get_query_ids()