Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

narrow: Update `with` operator behaviour when message not accessible. 

Previously, when the message of the "with" operator can not be
accessed by the user, it used to return messages in combined feed
or stream feed.

This commit updates it to rather raise a BadNarrowOperatorError
if the message of "with" operand is not accessible to user, and the
narrow terms present can not define a conversation. However, if
the narrow terms can define a conversation, then the narrow falls
back to that of without the "with" operator.
This commit is contained in:
roanster007 2024-07-17 19:55:02 +05:30
parent a88445a6d2
commit 9beecac3e5
3 changed files with 96 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api_docs/construct-narrow.md
View File

@ -107,6 +107,10 @@ exists, and can be accessed by the user, then it will return messages
with the `channel`/`topic`/`dm` operators corresponding to the current
conversation containing that message, and replacing any such filters
included in the narrow.
However, if the specified ID does not exist, or can not be accessed by
the user, then if the remaining narrow terms can identify a conversation
i.e., contain `channel` and `topic` terms or `dm` term, then messages
corresponding to the remaining narrow terms is returned.
* `with:12345`: Search for the conversation that contains the message
with ID `12345`.

40
zerver/lib/narrow.py
View File

@ -870,13 +870,38 @@ def get_channel_from_narrow_access_unchecked(
return None
# This function verifies if the current narrow has the necessary
# terms to point to a channel or a direct message conversation.
def can_narrow_define_conversation(narrow: list[NarrowParameter]) -> bool:
contains_channel_term = False
contains_topic_term = False
for term in narrow:
if term.operator in ["dm", "pm-with"]:
return True
elif term.operator in ["stream", "channel"]:
contains_channel_term = True
elif term.operator == "topic":
contains_topic_term = True
if contains_channel_term and contains_topic_term:
return True
return False
# This function implements the core logic of the `with` operator,
# which is designed to support permanent links to a topic that
# robustly function if the topic is moved.
#
# The with operator accepts a message ID as an operand. If the
# message ID does not exist or is otherwise not accessible to the
# current user, then it has no effect.
# current user, then if the remaining narrow terms can point to
# a conversation then the narrow corresponding to it is returned.
# If the remaining terms can not point to a particular conversation,
# then a BadNarrowOperatorError is raised.
#
# Otherwise, the narrow terms are mutated to remove any
# channel/topic/dm operators, replacing them with the appropriate
@ -890,6 +915,7 @@ def update_narrow_terms_containing_with_operator(
return narrow
with_operator_terms = list(filter(lambda term: term.operator == "with", narrow))
can_user_access_target_message = True
if len(with_operator_terms) > 1:
raise InvalidOperatorCombinationError(_("Duplicate 'with' operators."))
@ -908,12 +934,22 @@ def update_narrow_terms_containing_with_operator(
try:
message = access_message(maybe_user_profile, message_id)
except JsonableError:
return narrow
can_user_access_target_message = False
else:
try:
message = access_web_public_message(realm, message_id)
except MissingAuthenticationError:
can_user_access_target_message = False
# If the user can not access the target message we fall back to the
# conversation specified by those other operators if they're enough
# to specify a single conversation.
# Else, we raise a BadNarrowOperatorError.
if not can_user_access_target_message:
if can_narrow_define_conversation(narrow):
return narrow
else:
raise BadNarrowOperatorError(_("Invalid 'with' operator"))
# TODO: It would be better if the legacy names here are canonicalized
# while building a NarrowParameter.

63
zerver/tests/test_message_fetch.py
View File

@ -2686,9 +2686,14 @@ class GetOldMessagesTest(ZulipTestCase):
self.make_stream("dev team", invite_only=True, history_public_to_subscribers=False)
self.subscribe(iago, "dev team")
self.make_stream("public")
self.subscribe(hamlet, "public")
# Test `with` operator effective when targeting a topic with
# message which can be accessed by the user.
msg_id = self.send_stream_message(iago, "dev team", topic_name="test")
msg_id_2 = self.send_stream_message(hamlet, "public", topic_name="test")
dm_msg_id = self.send_personal_message(hamlet, iago, "direct message")
narrow = [
dict(operator="channel", operand="dev team"),
@ -2709,7 +2714,10 @@ class GetOldMessagesTest(ZulipTestCase):
# Test `with` operator ineffective when targeting a topic with
# message that can not be accessed by the user.
#
# Since !history_public_to_subscribers, hamlet cannot view.
# Hence, it falls back to the narrow without the `with`
# operator since it can alone define a conversation.
self.subscribe(hamlet, "dev team")
self.login("hamlet")
@ -2721,22 +2729,59 @@ class GetOldMessagesTest(ZulipTestCase):
results = self.get_and_check_messages(dict(narrow=orjson.dumps(narrow).decode()))
self.assert_length(results["messages"], 0)
# Same result with topic specified incorrectly
narrow = [
dict(operator="channel", operand="public"),
dict(operator="topic", operand="test"),
dict(operator="with", operand=msg_id),
]
results = self.get_and_check_messages(dict(narrow=orjson.dumps(narrow).decode()))
self.assert_length(results["messages"], 1)
self.assertEqual(results["messages"][0]["id"], msg_id_2)
# Since `dm` operator alone can also define conversation,
# narrow falls back to `dm` since hamlet can't access
# msg_id.
narrow = [
dict(operator="dm", operand=iago.email),
dict(operator="with", operand=msg_id),
]
results = self.get_and_check_messages(dict(narrow=orjson.dumps(narrow).decode()))
self.assert_length(results["messages"], 1)
self.assertEqual(results["messages"][0]["id"], dm_msg_id)
# However, if the narrow can not define conversation,
# and the target message is not accessible to user,
# then BadNarrowOperatorError is raised.
#
# narrow can't define conversation due to missing topic term.
narrow = [
dict(operator="channel", operand="dev team"),
dict(operator="topic", operand="wrong_guess"),
dict(operator="with", operand=msg_id),
]
results = self.get_and_check_messages(dict(narrow=orjson.dumps(narrow).decode()))
self.assert_length(results["messages"], 0)
post_params = {
"anchor": msg_id,
"num_before": 0,
"num_after": 5,
"narrow": orjson.dumps(narrow).decode(),
}
result = self.client_get("/json/messages", dict(post_params))
self.assert_json_error(result, "Invalid narrow operator: Invalid 'with' operator")
# If just with is specified, we get messages a la combined feed,
# but not the target message.
# narrow can't define conversation due to missing channel term.
narrow = [
dict(operator="topic", operand="test"),
dict(operator="with", operand=msg_id),
]
result = self.client_get("/json/messages", dict(post_params))
self.assert_json_error(result, "Invalid narrow operator: Invalid 'with' operator")
# narrow can't define conversation due to missing channel-topic
# terms or dm terms.
narrow = [
dict(operator="with", operand=msg_id),
]
results = self.get_and_check_messages(dict(narrow=orjson.dumps(narrow).decode()))
self.assertNotIn(msg_id, [message["id"] for message in results["messages"]])
result = self.client_get("/json/messages", dict(post_params))
self.assert_json_error(result, "Invalid narrow operator: Invalid 'with' operator")
# Test `with` operator is effective when targeting personal
# messages with message id, and returns messages of that narrow.
@ -2753,7 +2798,7 @@ class GetOldMessagesTest(ZulipTestCase):
results = self.get_and_check_messages(dict(narrow=orjson.dumps(with_narrow).decode()))
self.assertNotIn(msg_id, [message["id"] for message in results["messages"]])
# Now switch to a user how does have access.
# Now switch to a user who does have access.
self.login("iago")
with_narrow = [
# Important: We pass the wrong conversation.