diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index be5cfb0ac4..2917ec4a26 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -186,6 +186,13 @@ def can_access_stream_history_by_name(user_profile: UserProfile, stream_name: st return False return can_access_stream_history(user_profile, stream) +def can_access_stream_history_by_id(user_profile: UserProfile, stream_id: int) -> bool: + try: + stream = get_stream_by_id_in_realm(stream_id, user_profile.realm) + except Stream.DoesNotExist: + return False + return can_access_stream_history(user_profile, stream) + def filter_stream_authorization(user_profile: UserProfile, streams: Iterable[Stream]) -> Tuple[List[Stream], List[Stream]]: streams_subscribed = set() # type: Set[int] diff --git a/zerver/tests/test_narrow.py b/zerver/tests/test_narrow.py index 43b311be32..47fc8225b9 100644 --- a/zerver/tests/test_narrow.py +++ b/zerver/tests/test_narrow.py @@ -1283,14 +1283,17 @@ class GetOldMessagesTest(ZulipTestCase): messages = get_user_messages(self.example_user('hamlet')) stream_messages = [msg for msg in messages if msg.is_stream_message()] stream_name = get_display_recipient(stream_messages[0].recipient) + assert isinstance(stream_name, str) + stream_id = get_stream(stream_name, stream_messages[0].get_realm()).id stream_recipient_id = stream_messages[0].recipient.id - narrow = [dict(operator='stream', operand=stream_name)] - result = self.get_and_check_messages(dict(narrow=ujson.dumps(narrow))) + for operand in [stream_name, stream_id]: + narrow = [dict(operator='stream', operand=operand)] + result = self.get_and_check_messages(dict(narrow=ujson.dumps(narrow))) - for message in result["messages"]: - self.assertEqual(message["type"], "stream") - self.assertEqual(message["recipient_id"], stream_recipient_id) + for message in result["messages"]: + self.assertEqual(message["type"], "stream") + self.assertEqual(message["recipient_id"], stream_recipient_id) def test_get_visible_messages_with_narrow_stream(self) -> None: self.login(self.example_email('hamlet')) @@ -2097,16 +2100,41 @@ class GetOldMessagesTest(ZulipTestCase): self.assert_json_error_contains(result, "Invalid narrow operator: unknown operator") - def test_non_string_narrow_operand_in_dict(self) -> None: - """ - We expect search operands to be strings, not integers. - """ + def test_invalid_narrow_operand_in_dict(self) -> None: self.login(self.example_email("hamlet")) - not_a_string = 42 - narrow = [dict(operator='stream', operand=not_a_string)] - params = dict(anchor=0, num_before=0, num_after=0, narrow=ujson.dumps(narrow)) - result = self.client_get("/json/messages", params) - self.assert_json_error_contains(result, 'elem["operand"] is not a string') + + # str or int is required for sender, group-pm-with, stream + invalid_operands = [['1'], [2], None] + error_msg = 'elem["operand"] is not a string or integer' + for operand in ['sender', 'group-pm-with', 'stream']: + self.exercise_bad_narrow_operand_using_dict_api(operand, invalid_operands, error_msg) + + # str or int list is required for pm-with operator + invalid_operands = [None] + error_msg = 'elem["operand"] is not a string or an integer list' + self.exercise_bad_narrow_operand_using_dict_api('pm-with', invalid_operands, error_msg) + + invalid_operands = [['2']] + error_msg = 'elem["operand"][0] is not an integer' + self.exercise_bad_narrow_operand_using_dict_api('pm-with', invalid_operands, error_msg) + + # For others only str is acceptable + invalid_operands = [2, None, [1]] + error_msg = 'elem["operand"] is not a string' + for operand in ['is', 'near', 'has', 'id']: + self.exercise_bad_narrow_operand_using_dict_api(operand, invalid_operands, error_msg) + + # The exercise_bad_narrow_operand helper method uses legacy tuple format to + # test bad narrow, this method uses the current dict api format + def exercise_bad_narrow_operand_using_dict_api(self, operator: str, + operands: Sequence[Any], + error_msg: str) -> None: + + for operand in operands: + narrow = [dict(operator=operator, operand=operand)] + params = dict(anchor=0, num_before=0, num_after=0, narrow=ujson.dumps(narrow)) + result = self.client_get('/json/messages', params) + self.assert_json_error_contains(result, error_msg) def exercise_bad_narrow_operand(self, operator: str, operands: Sequence[Any], @@ -2143,6 +2171,10 @@ class GetOldMessagesTest(ZulipTestCase): self.exercise_bad_narrow_operand("stream", ['non-existent stream'], "Invalid narrow operator: unknown stream") + non_existing_stream_id = 1232891381239 + self.exercise_bad_narrow_operand_using_dict_api('stream', [non_existing_stream_id], + 'Invalid narrow operator: unknown stream') + def test_bad_narrow_nonexistent_email(self) -> None: self.login(self.example_email("hamlet")) self.exercise_bad_narrow_operand("pm-with", ['non-existent-user@zulip.com'], @@ -2436,6 +2468,13 @@ class GetOldMessagesTest(ZulipTestCase): muting_conditions = exclude_muting_conditions(user_profile, narrow) self.assertEqual(muting_conditions, []) + # Also test that passing stream ID works + narrow = [ + dict(operator='stream', operand=get_stream('Scotland', realm).id) + ] + muting_conditions = exclude_muting_conditions(user_profile, narrow) + self.assertEqual(muting_conditions, []) + # Ok, now set up our muted topics to include a topic relevant to our narrow. muted_topics = [ ['Scotland', 'golf'], diff --git a/zerver/views/messages.py b/zerver/views/messages.py index 6d112cd0e0..743daca3f3 100644 --- a/zerver/views/messages.py +++ b/zerver/views/messages.py @@ -31,7 +31,7 @@ from zerver.lib.message import ( from zerver.lib.response import json_success, json_error from zerver.lib.sqlalchemy_utils import get_sqlalchemy_connection from zerver.lib.streams import access_stream_by_id, can_access_stream_history_by_name, \ - get_stream_by_narrow_operand_access_unchecked + can_access_stream_history_by_id, get_stream_by_narrow_operand_access_unchecked from zerver.lib.timestamp import datetime_to_timestamp, convert_to_UTC from zerver.lib.timezone import get_timezone from zerver.lib.topic import ( @@ -204,14 +204,14 @@ class NarrowBuilder: s[i] = '\\' + c return ''.join(s) - def by_stream(self, query: Query, operand: str, maybe_negate: ConditionTransform) -> Query: + def by_stream(self, query: Query, operand: Union[str, int], maybe_negate: ConditionTransform) -> Query: try: # Because you can see your own message history for # private streams you are no longer subscribed to, we # need get_stream_by_narrow_operand_access_unchecked here. stream = get_stream_by_narrow_operand_access_unchecked(operand, self.user_profile.realm) except Stream.DoesNotExist: - raise BadNarrowOperator('unknown stream ' + operand) + raise BadNarrowOperator('unknown stream ' + str(operand)) if self.user_profile.realm.is_zephyr_mirror_realm: # MIT users expect narrowing to "social" to also show messages to @@ -522,7 +522,7 @@ def narrow_parameter(json: str) -> OptionalNarrowListT: # that supports user IDs. Relevant code is located in static/js/message_fetch.js # in handle_operators_supporting_id_based_api function where you will need to update # operators_supporting_id, or operators_supporting_ids array. - operators_supporting_id = ['sender', 'group-pm-with'] + operators_supporting_id = ['sender', 'group-pm-with', 'stream'] operators_supporting_ids = ['pm-with'] operator = elem.get('operator', '') @@ -568,8 +568,11 @@ def ok_to_include_history(narrow: OptionalNarrowListT, user_profile: UserProfile if narrow is not None: for term in narrow: if term['operator'] == "stream" and not term.get('negated', False): - if can_access_stream_history_by_name(user_profile, term['operand']): - include_history = True + operand = term['operand'] # type: Union[str, int] + if isinstance(operand, str): + include_history = can_access_stream_history_by_name(user_profile, operand) + else: + include_history = can_access_stream_history_by_id(user_profile, operand) # Disable historical messages if the user is narrowing on anything # that's a property on the UserMessage table. There cannot be # historical messages in these cases anyway.