topic: Enforce is_channel_message=True on topic queries.

This commit is contained in:
Alex Vandiver
2025-03-17 18:16:38 +00:00
committed by Tim Abbott
parent 33e1d583bf
commit d978363a75
10 changed files with 68 additions and 28 deletions

View File

@@ -1246,6 +1246,7 @@ def check_time_limit_for_change_all_propagate_mode(
user_profile=user_profile,
message__recipient_id=message.recipient_id,
message__subject__iexact=message.topic_name(),
message__is_channel_message=True,
).values_list("message_id", flat=True)
messages_allowed_to_move: list[int] = list(
Message.objects.filter(

View File

@@ -1263,11 +1263,14 @@ def already_sent_mirrored_message_id(message: Message) -> int | None:
time_window = timedelta(seconds=0)
messages = Message.objects.filter(
# Uses index: zerver_message_realm_recipient_subject
# Uses index: zerver_message_realm_recipient_subject for
# channel messages or zerver_message_realm_sender_recipient for
# DMs
realm_id=message.realm_id,
sender=message.sender,
recipient=message.recipient,
subject=message.topic_name(),
is_channel_message=message.is_channel_message,
content=message.content,
sending_client=message.sending_client,
date_sent__gte=message.date_sent - time_window,

View File

@@ -1629,6 +1629,7 @@ def visibility_policy_for_send_message(
user_profile=sender,
message__recipient_id=message.recipient_id,
message__subject__iexact=message.topic_name(),
message__is_channel_message=True,
).exclude(message_id=message.id)
if (

View File

@@ -837,7 +837,11 @@ class NarrowBuilder:
term = term[1:-1]
term = "%" + connection.ops.prep_for_like_query(term) + "%"
cond: ClauseElement = or_(
column("content", Text).ilike(term), topic_column_sa().ilike(term)
column("content", Text).ilike(term),
and_(
topic_column_sa().ilike(term),
column("is_channel_message", Boolean),
),
)
query = query.where(maybe_negate(cond))

View File

@@ -64,7 +64,7 @@ MESSAGE__TOPIC = "message__subject"
def filter_by_topic_name_via_message(
query: QuerySet[UserMessage], topic_name: str
) -> QuerySet[UserMessage]:
return query.filter(message__subject__iexact=topic_name)
return query.filter(message__is_channel_message=True, message__subject__iexact=topic_name)
def messages_for_topic(
@@ -75,6 +75,7 @@ def messages_for_topic(
realm_id=realm_id,
recipient_id=stream_recipient_id,
subject__iexact=topic_name,
is_channel_message=True,
)
@@ -103,6 +104,7 @@ def get_first_message_for_user_in_topic(
user_profile=user_profile,
message__recipient_id=recipient_id,
message__subject__iexact=topic_name,
message__is_channel_message=True,
)
.values_list("message_id", flat=True)
.first()
@@ -135,6 +137,7 @@ def user_message_exists_for_topic(
user_profile=user_profile,
message__recipient_id=recipient_id,
message__subject__iexact=topic_name,
message__is_channel_message=True,
).exists()
@@ -163,6 +166,7 @@ def update_messages_for_topic_edit(
realm_id=old_stream.realm_id,
recipient_id=assert_is_not_none(old_stream.recipient_id),
subject__iexact=message_edit_request.orig_topic_name,
is_channel_message=True,
)
if message_edit_request.propagate_mode == "change_all":
messages = messages.exclude(id=edited_message.id)
@@ -277,7 +281,8 @@ def get_topic_history_for_public_stream(
FROM "zerver_message"
WHERE (
"zerver_message"."realm_id" = %s AND
"zerver_message"."recipient_id" = %s
"zerver_message"."recipient_id" = %s AND
"zerver_message"."is_channel_message"
)
GROUP BY (
"zerver_message"."subject"
@@ -319,7 +324,8 @@ def get_topic_history_for_stream(
WHERE (
"zerver_usermessage"."user_profile_id" = %s AND
"zerver_message"."realm_id" = %s AND
"zerver_message"."recipient_id" = %s
"zerver_message"."recipient_id" = %s AND
"zerver_message"."is_channel_message"
)
GROUP BY (
"zerver_message"."subject"
@@ -357,6 +363,7 @@ def participants_for_topic(realm_id: int, recipient_id: int, topic_name: str) ->
realm_id=realm_id,
recipient_id=recipient_id,
subject__iexact=topic_name,
is_channel_message=True,
)
participants = set(
UserProfile.objects.filter(

View File

@@ -8,12 +8,18 @@ from zerver.models import UserTopic
def topic_match_sa(topic_name: str) -> ColumnElement[Boolean]:
# _sa is short for SQLAlchemy, which we use mostly for
# queries that search messages
topic_cond = func.upper(column("subject", Text)) == func.upper(literal(topic_name))
topic_cond = and_(
func.upper(column("subject", Text)) == func.upper(literal(topic_name)),
column("is_channel_message", Boolean),
)
return topic_cond
def get_resolved_topic_condition_sa() -> ColumnElement[Boolean]:
resolved_topic_cond = column("subject", Text).startswith(RESOLVED_TOPIC_PREFIX)
resolved_topic_cond = and_(
column("subject", Text).startswith(RESOLVED_TOPIC_PREFIX),
column("is_channel_message", Boolean),
)
return resolved_topic_cond
@@ -32,6 +38,7 @@ def get_followed_topic_condition_sa(user_id: int) -> ColumnElement[Boolean]:
== literal(UserTopic.VisibilityPolicy.FOLLOWED),
func.upper(literal_column("zerver_usertopic.topic_name"))
== func.upper(literal_column("zerver_message.subject")),
literal_column("zerver_message.is_channel_message", Boolean),
literal_column("zerver_usertopic.recipient_id")
== literal_column("zerver_message.recipient_id"),
)

View File

@@ -147,7 +147,10 @@ This is most often used for legal compliance.
limits = reduce(
or_,
[Q(content__icontains=term) | Q(subject__icontains=term) for term in terms],
[
Q(content__icontains=term) | Q(is_channel_message=True, subject__icontains=term)
for term in terms
],
limits,
)

View File

@@ -329,6 +329,7 @@ def get_context_for_message(message: Message) -> QuerySet[Message]:
realm_id=message.realm_id,
recipient_id=message.recipient_id,
subject__iexact=message.subject,
is_channel_message=True,
id__lt=message.id,
date_sent__gt=message.date_sent - timedelta(minutes=15),
).order_by("-id")[:10]

View File

@@ -274,24 +274,28 @@ class NarrowBuilderTest(ZulipTestCase):
def test_add_term_using_is_operator_for_resolved_topics(self) -> None:
term = NarrowParameter(operator="is", operand="resolved")
self._do_add_term_test(term, "WHERE (subject LIKE %(subject_1)s || '%%'")
self._do_add_term_test(
term, "WHERE (subject LIKE %(subject_1)s || '%%') AND is_channel_message"
)
def test_add_term_using_is_operator_for_negated_resolved_topics(self) -> None:
term = NarrowParameter(operator="is", operand="resolved", negated=True)
self._do_add_term_test(term, "WHERE (subject NOT LIKE %(subject_1)s || '%%'")
self._do_add_term_test(
term, "WHERE NOT ((subject LIKE %(subject_1)s || '%%') AND is_channel_message)"
)
def test_add_term_using_is_operator_for_followed_topics(self) -> None:
term = NarrowParameter(operator="is", operand="followed", negated=False)
self._do_add_term_test(
term,
"EXISTS (SELECT 1 \nFROM zerver_usertopic \nWHERE zerver_usertopic.user_profile_id = %(param_1)s AND zerver_usertopic.visibility_policy = %(param_2)s AND upper(zerver_usertopic.topic_name) = upper(zerver_message.subject) AND zerver_usertopic.recipient_id = zerver_message.recipient_id)",
"EXISTS (SELECT 1 \nFROM zerver_usertopic \nWHERE zerver_usertopic.user_profile_id = %(param_1)s AND zerver_usertopic.visibility_policy = %(param_2)s AND upper(zerver_usertopic.topic_name) = upper(zerver_message.subject) AND zerver_message.is_channel_message AND zerver_usertopic.recipient_id = zerver_message.recipient_id)",
)
def test_add_term_using_is_operator_for_negated_followed_topics(self) -> None:
term = NarrowParameter(operator="is", operand="followed", negated=True)
self._do_add_term_test(
term,
"NOT (EXISTS (SELECT 1 \nFROM zerver_usertopic \nWHERE zerver_usertopic.user_profile_id = %(param_1)s AND zerver_usertopic.visibility_policy = %(param_2)s AND upper(zerver_usertopic.topic_name) = upper(zerver_message.subject) AND zerver_usertopic.recipient_id = zerver_message.recipient_id))",
"NOT (EXISTS (SELECT 1 \nFROM zerver_usertopic \nWHERE zerver_usertopic.user_profile_id = %(param_1)s AND zerver_usertopic.visibility_policy = %(param_2)s AND upper(zerver_usertopic.topic_name) = upper(zerver_message.subject) AND zerver_message.is_channel_message AND zerver_usertopic.recipient_id = zerver_message.recipient_id))",
)
def test_add_term_using_is_operator_for_muted_topics(self) -> None:
@@ -310,19 +314,27 @@ class NarrowBuilderTest(ZulipTestCase):
def test_add_term_using_topic_operator_and_lunch_operand(self) -> None:
term = NarrowParameter(operator="topic", operand="lunch")
self._do_add_term_test(term, "WHERE upper(subject) = upper(%(param_1)s)")
self._do_add_term_test(
term, "WHERE upper(subject) = upper(%(param_1)s) AND is_channel_message"
)
def test_add_term_using_topic_operator_lunch_operand_and_negated(self) -> None: # NEGATED
term = NarrowParameter(operator="topic", operand="lunch", negated=True)
self._do_add_term_test(term, "WHERE upper(subject) != upper(%(param_1)s)")
self._do_add_term_test(
term, "WHERE NOT (upper(subject) = upper(%(param_1)s) AND is_channel_message)"
)
def test_add_term_using_topic_operator_and_personal_operand(self) -> None:
term = NarrowParameter(operator="topic", operand="personal")
self._do_add_term_test(term, "WHERE upper(subject) = upper(%(param_1)s)")
self._do_add_term_test(
term, "WHERE upper(subject) = upper(%(param_1)s) AND is_channel_message"
)
def test_add_term_using_topic_operator_personal_operand_and_negated(self) -> None: # NEGATED
term = NarrowParameter(operator="topic", operand="personal", negated=True)
self._do_add_term_test(term, "WHERE upper(subject) != upper(%(param_1)s)")
self._do_add_term_test(
term, "WHERE NOT (upper(subject) = upper(%(param_1)s) AND is_channel_message)"
)
def test_add_term_using_sender_operator(self) -> None:
term = NarrowParameter(operator="sender", operand=self.othello_email)
@@ -550,7 +562,7 @@ class NarrowBuilderTest(ZulipTestCase):
term = NarrowParameter(operator="search", operand='"french fries"')
self._do_add_term_test(
term,
"WHERE (content ILIKE %(content_1)s OR subject ILIKE %(subject_1)s) AND (search_tsvector @@ plainto_tsquery(%(param_4)s, %(param_5)s))",
"WHERE (content ILIKE %(content_1)s OR subject ILIKE %(subject_1)s AND is_channel_message) AND (search_tsvector @@ plainto_tsquery(%(param_4)s, %(param_5)s))",
)
@override_settings(USING_PGROONGA=False)
@@ -558,7 +570,7 @@ class NarrowBuilderTest(ZulipTestCase):
term = NarrowParameter(operator="search", operand='"french fries"', negated=True)
self._do_add_term_test(
term,
"WHERE NOT (content ILIKE %(content_1)s OR subject ILIKE %(subject_1)s) AND NOT (search_tsvector @@ plainto_tsquery(%(param_4)s, %(param_5)s))",
"WHERE NOT (content ILIKE %(content_1)s OR subject ILIKE %(subject_1)s AND is_channel_message) AND NOT (search_tsvector @@ plainto_tsquery(%(param_4)s, %(param_5)s))",
)
@override_settings(USING_PGROONGA=True)
@@ -4554,7 +4566,7 @@ class GetOldMessagesTest(ZulipTestCase):
channel = get_stream("Scotland", realm)
assert channel.recipient is not None
recipient_id = channel.recipient.id
cond = f"AND NOT (recipient_id = {recipient_id} AND upper(subject) = upper('golf'))"
cond = f"AND NOT (recipient_id = {recipient_id} AND upper(subject) = upper('golf') AND is_channel_message)"
self.assertIn(cond, queries[0].sql)
# Next, verify the use_first_unread_anchor setting invokes
@@ -4612,7 +4624,7 @@ class GetOldMessagesTest(ZulipTestCase):
expected_query = """\
SELECT id AS message_id \n\
FROM zerver_message \n\
WHERE NOT (recipient_id = %(recipient_id_1)s AND upper(subject) = upper(%(param_1)s))\
WHERE NOT (recipient_id = %(recipient_id_1)s AND upper(subject) = upper(%(param_1)s) AND is_channel_message)\
"""
self.assertEqual(get_sqlalchemy_sql(query), expected_query)
@@ -4639,8 +4651,8 @@ WHERE NOT (recipient_id = %(recipient_id_1)s AND upper(subject) = upper(%(param_
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)) \
WHERE NOT (recipient_id = %(recipient_id_1)s AND upper(subject) = upper(%(param_1)s) AND is_channel_message \
OR recipient_id = %(recipient_id_2)s AND upper(subject) = upper(%(param_2)s) AND is_channel_message) \
AND (recipient_id NOT IN (__[POSTCOMPILE_recipient_id_3]))\
"""
self.assertEqual(get_sqlalchemy_sql(query), expected_query)
@@ -4670,10 +4682,10 @@ AND (recipient_id NOT IN (__[POSTCOMPILE_recipient_id_3]))\
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)) \
WHERE NOT (recipient_id = %(recipient_id_1)s AND upper(subject) = upper(%(param_1)s) AND is_channel_message \
OR recipient_id = %(recipient_id_2)s AND upper(subject) = upper(%(param_2)s) AND is_channel_message) \
AND NOT (recipient_id IN (__[POSTCOMPILE_recipient_id_3]) \
AND NOT (recipient_id = %(recipient_id_4)s AND upper(subject) = upper(%(param_3)s)))\
AND NOT (recipient_id = %(recipient_id_4)s AND upper(subject) = upper(%(param_3)s) AND is_channel_message))\
"""
self.assertEqual(get_sqlalchemy_sql(query), expected_query)
params = get_sqlalchemy_query_params(query)
@@ -4946,7 +4958,7 @@ WHERE user_profile_id = {hamlet_id} AND (zerver_recipient.type_id != 2 OR (EXIST
FROM zerver_stream \n\
WHERE zerver_stream.recipient_id = zerver_recipient.id AND (NOT zerver_stream.invite_only AND NOT zerver_stream.is_in_zephyr_realm OR zerver_stream.can_subscribe_group_id IN {hamlet_groups} OR zerver_stream.can_add_subscribers_group_id IN {hamlet_groups}))) OR (EXISTS (SELECT \n\
FROM zerver_subscription \n\
WHERE zerver_subscription.user_profile_id = {hamlet_id} AND zerver_subscription.recipient_id = zerver_recipient.id AND zerver_subscription.active))) AND upper(subject) = upper('blah') ORDER BY message_id ASC \n\
WHERE zerver_subscription.user_profile_id = {hamlet_id} AND zerver_subscription.recipient_id = zerver_recipient.id AND zerver_subscription.active))) AND upper(subject) = upper('blah') AND is_channel_message ORDER BY message_id ASC \n\
LIMIT 10) AS anon_1 ORDER BY message_id ASC\
"""
sql = sql_template.format(**query_ids)
@@ -4958,7 +4970,7 @@ WHERE zerver_subscription.user_profile_id = {hamlet_id} AND zerver_subscription.
SELECT anon_1.message_id \n\
FROM (SELECT id AS message_id \n\
FROM zerver_message \n\
WHERE realm_id = 2 AND recipient_id = {scotland_recipient} AND upper(subject) = upper('blah') ORDER BY zerver_message.id ASC \n\
WHERE realm_id = 2 AND recipient_id = {scotland_recipient} AND upper(subject) = upper('blah') AND is_channel_message ORDER BY zerver_message.id ASC \n\
LIMIT 10) AS anon_1 ORDER BY message_id ASC\
"""
sql = sql_template.format(**query_ids)
@@ -5075,7 +5087,7 @@ WHERE user_profile_id = {hamlet_id} AND (zerver_recipient.type_id != 2 OR (EXIST
FROM zerver_stream \n\
WHERE zerver_stream.recipient_id = zerver_recipient.id AND (NOT zerver_stream.invite_only AND NOT zerver_stream.is_in_zephyr_realm OR zerver_stream.can_subscribe_group_id IN {hamlet_groups} OR zerver_stream.can_add_subscribers_group_id IN {hamlet_groups}))) OR (EXISTS (SELECT \n\
FROM zerver_subscription \n\
WHERE zerver_subscription.user_profile_id = {hamlet_id} AND zerver_subscription.recipient_id = zerver_recipient.id AND zerver_subscription.active))) AND (content ILIKE '%jumping%' OR subject ILIKE '%jumping%') AND (search_tsvector @@ plainto_tsquery('zulip.english_us_search', '"jumping" quickly')) ORDER BY message_id ASC \n\
WHERE zerver_subscription.user_profile_id = {hamlet_id} AND zerver_subscription.recipient_id = zerver_recipient.id AND zerver_subscription.active))) AND (content ILIKE '%jumping%' OR subject ILIKE '%jumping%' AND is_channel_message) AND (search_tsvector @@ plainto_tsquery('zulip.english_us_search', '"jumping" quickly')) ORDER BY message_id ASC \n\
LIMIT 10) AS anon_1 ORDER BY message_id ASC\
"""
sql = sql_template.format(**query_ids)

View File

@@ -1063,6 +1063,7 @@ def delete_in_topic(
user_profile__in=users_with_stale_user_topic_rows,
message__recipient_id=assert_is_not_none(stream.recipient_id),
message__subject__iexact=topic_name,
message__is_channel_message=True,
).values_list("user_profile", flat=True)
)
users_with_stale_user_topic_rows = list(