messages: Support passing user ID for stream operator.

ok_to_include_history fuction was updated to expect stream ID.

Fixes part of #9474.
This commit is contained in:
Priyank Patel
2019-08-07 15:32:19 +00:00
committed by Tim Abbott
parent 5eab5bbc3e
commit 3680393b47
3 changed files with 69 additions and 20 deletions

View File

@@ -186,6 +186,13 @@ def can_access_stream_history_by_name(user_profile: UserProfile, stream_name: st
return False return False
return can_access_stream_history(user_profile, stream) 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, def filter_stream_authorization(user_profile: UserProfile,
streams: Iterable[Stream]) -> Tuple[List[Stream], List[Stream]]: streams: Iterable[Stream]) -> Tuple[List[Stream], List[Stream]]:
streams_subscribed = set() # type: Set[int] streams_subscribed = set() # type: Set[int]

View File

@@ -1283,9 +1283,12 @@ class GetOldMessagesTest(ZulipTestCase):
messages = get_user_messages(self.example_user('hamlet')) messages = get_user_messages(self.example_user('hamlet'))
stream_messages = [msg for msg in messages if msg.is_stream_message()] stream_messages = [msg for msg in messages if msg.is_stream_message()]
stream_name = get_display_recipient(stream_messages[0].recipient) 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 stream_recipient_id = stream_messages[0].recipient.id
narrow = [dict(operator='stream', operand=stream_name)] for operand in [stream_name, stream_id]:
narrow = [dict(operator='stream', operand=operand)]
result = self.get_and_check_messages(dict(narrow=ujson.dumps(narrow))) result = self.get_and_check_messages(dict(narrow=ujson.dumps(narrow)))
for message in result["messages"]: for message in result["messages"]:
@@ -2097,16 +2100,41 @@ class GetOldMessagesTest(ZulipTestCase):
self.assert_json_error_contains(result, self.assert_json_error_contains(result,
"Invalid narrow operator: unknown operator") "Invalid narrow operator: unknown operator")
def test_non_string_narrow_operand_in_dict(self) -> None: def test_invalid_narrow_operand_in_dict(self) -> None:
"""
We expect search operands to be strings, not integers.
"""
self.login(self.example_email("hamlet")) self.login(self.example_email("hamlet"))
not_a_string = 42
narrow = [dict(operator='stream', operand=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)) params = dict(anchor=0, num_before=0, num_after=0, narrow=ujson.dumps(narrow))
result = self.client_get("/json/messages", params) result = self.client_get('/json/messages', params)
self.assert_json_error_contains(result, 'elem["operand"] is not a string') self.assert_json_error_contains(result, error_msg)
def exercise_bad_narrow_operand(self, operator: str, def exercise_bad_narrow_operand(self, operator: str,
operands: Sequence[Any], operands: Sequence[Any],
@@ -2143,6 +2171,10 @@ class GetOldMessagesTest(ZulipTestCase):
self.exercise_bad_narrow_operand("stream", ['non-existent stream'], self.exercise_bad_narrow_operand("stream", ['non-existent stream'],
"Invalid narrow operator: unknown 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: def test_bad_narrow_nonexistent_email(self) -> None:
self.login(self.example_email("hamlet")) self.login(self.example_email("hamlet"))
self.exercise_bad_narrow_operand("pm-with", ['non-existent-user@zulip.com'], 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) muting_conditions = exclude_muting_conditions(user_profile, narrow)
self.assertEqual(muting_conditions, []) 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. # Ok, now set up our muted topics to include a topic relevant to our narrow.
muted_topics = [ muted_topics = [
['Scotland', 'golf'], ['Scotland', 'golf'],

View File

@@ -31,7 +31,7 @@ from zerver.lib.message import (
from zerver.lib.response import json_success, json_error from zerver.lib.response import json_success, json_error
from zerver.lib.sqlalchemy_utils import get_sqlalchemy_connection from zerver.lib.sqlalchemy_utils import get_sqlalchemy_connection
from zerver.lib.streams import access_stream_by_id, can_access_stream_history_by_name, \ 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.timestamp import datetime_to_timestamp, convert_to_UTC
from zerver.lib.timezone import get_timezone from zerver.lib.timezone import get_timezone
from zerver.lib.topic import ( from zerver.lib.topic import (
@@ -204,14 +204,14 @@ class NarrowBuilder:
s[i] = '\\' + c s[i] = '\\' + c
return ''.join(s) 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: try:
# Because you can see your own message history for # Because you can see your own message history for
# private streams you are no longer subscribed to, we # private streams you are no longer subscribed to, we
# need get_stream_by_narrow_operand_access_unchecked here. # need get_stream_by_narrow_operand_access_unchecked here.
stream = get_stream_by_narrow_operand_access_unchecked(operand, self.user_profile.realm) stream = get_stream_by_narrow_operand_access_unchecked(operand, self.user_profile.realm)
except Stream.DoesNotExist: except Stream.DoesNotExist:
raise BadNarrowOperator('unknown stream ' + operand) raise BadNarrowOperator('unknown stream ' + str(operand))
if self.user_profile.realm.is_zephyr_mirror_realm: if self.user_profile.realm.is_zephyr_mirror_realm:
# MIT users expect narrowing to "social" to also show messages to # 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 # 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 # 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, 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'] operators_supporting_ids = ['pm-with']
operator = elem.get('operator', '') operator = elem.get('operator', '')
@@ -568,8 +568,11 @@ def ok_to_include_history(narrow: OptionalNarrowListT, user_profile: UserProfile
if narrow is not None: if narrow is not None:
for term in narrow: for term in narrow:
if term['operator'] == "stream" and not term.get('negated', False): if term['operator'] == "stream" and not term.get('negated', False):
if can_access_stream_history_by_name(user_profile, term['operand']): operand = term['operand'] # type: Union[str, int]
include_history = True 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 # Disable historical messages if the user is narrowing on anything
# that's a property on the UserMessage table. There cannot be # that's a property on the UserMessage table. There cannot be
# historical messages in these cases anyway. # historical messages in these cases anyway.