From 92ce2d0e31db366d2366aa2d3861964c3719c8a0 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Tue, 1 Dec 2020 13:13:09 +0000 Subject: [PATCH] events: Fix apply_event for streams. In 1bcb8d8ee84d8d6b558a427d9f5385972f98519e I made it so the webapp doesn't include "streams" in its state from `fetch_initial_state_data`, but I didn't address all the places in apply_event. --- zerver/lib/events.py | 25 ++++++++------ zerver/tests/test_events.py | 65 +++++++++++++++++++------------------ 2 files changed, 49 insertions(+), 41 deletions(-) diff --git a/zerver/lib/events.py b/zerver/lib/events.py index bcb0f10d97..849ba3da24 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -606,12 +606,16 @@ def apply_event(state: Dict[str, Any], # Add stream to never_subscribed (if not invite_only) state['never_subscribed'].append(stream_data) - state['streams'].append(stream) - state['streams'].sort(key=lambda elt: elt["name"]) + if 'streams' in state: + state['streams'].append(stream) + + if 'streams' in state: + state['streams'].sort(key=lambda elt: elt["name"]) if event['op'] == 'delete': deleted_stream_ids = {stream['stream_id'] for stream in event['streams']} - state['streams'] = [s for s in state['streams'] if s['stream_id'] not in deleted_stream_ids] + if 'streams' in state: + state['streams'] = [s for s in state['streams'] if s['stream_id'] not in deleted_stream_ids] state['never_subscribed'] = [stream for stream in state['never_subscribed'] if stream['stream_id'] not in deleted_stream_ids] @@ -624,13 +628,14 @@ def apply_event(state: Dict[str, Any], if event['property'] == "description": obj['rendered_description'] = event['rendered_description'] # Also update the pure streams data - for stream in state['streams']: - if stream['name'].lower() == event['name'].lower(): - prop = event['property'] - if prop in stream: - stream[prop] = event['value'] - if prop == 'description': - stream['rendered_description'] = event['rendered_description'] + if 'streams' in state: + for stream in state['streams']: + if stream['name'].lower() == event['name'].lower(): + prop = event['property'] + if prop in stream: + stream[prop] = event['value'] + if prop == 'description': + stream['rendered_description'] = event['rendered_description'] elif event['type'] == 'default_streams': state['realm_default_streams'] = event['default_streams'] elif event['type'] == 'default_stream_groups': diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index d5ae6ebfd3..ea0bdedded 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -1439,47 +1439,50 @@ class NormalActionsTest(BaseAction): check_hotspots('events[0]', events[0]) def test_rename_stream(self) -> None: - stream = self.make_stream('old_name') - new_name = 'stream with a brand new name' - self.subscribe(self.user_profile, stream.name) - action = lambda: do_rename_stream(stream, new_name, self.user_profile) - events = self.verify_action(action, num_events=3) + for i, include_streams in enumerate([True, False]): + old_name = f'old name{i}' + new_name = f'new name{i}' - check_stream_update('events[0]', events[0]) - self.assertEqual(events[0]['name'], 'old_name') + stream = self.make_stream(old_name) + self.subscribe(self.user_profile, stream.name) + action = lambda: do_rename_stream(stream, new_name, self.user_profile) + events = self.verify_action(action, num_events=3, include_streams=include_streams) - check_stream_update('events[1]', events[1]) - self.assertEqual(events[1]['name'], 'old_name') + check_stream_update('events[0]', events[0]) + self.assertEqual(events[0]['name'], old_name) - check_message('events[2]', events[2]) + check_stream_update('events[1]', events[1]) + self.assertEqual(events[1]['name'], old_name) - fields = dict( - sender_email='notification-bot@zulip.com', - display_recipient=new_name, - sender_full_name='Notification Bot', - is_me_message=False, - type='stream', - client='Internal', - ) + check_message('events[2]', events[2]) - fields[TOPIC_NAME] = 'stream events' + fields = dict( + sender_email='notification-bot@zulip.com', + display_recipient=new_name, + sender_full_name='Notification Bot', + is_me_message=False, + type='stream', + client='Internal', + ) - msg = events[2]['message'] - for k, v in fields.items(): - self.assertEqual(msg[k], v) + fields[TOPIC_NAME] = 'stream events' + + msg = events[2]['message'] + for k, v in fields.items(): + self.assertEqual(msg[k], v) def test_deactivate_stream_neversubscribed(self) -> None: - stream = self.make_stream('old_name') - - action = lambda: do_deactivate_stream(stream) - events = self.verify_action(action) - - check_stream_delete('events[0]', events[0]) + for i, include_streams in enumerate([True, False]): + stream = self.make_stream(f"stream{i}") + action = lambda: do_deactivate_stream(stream) + events = self.verify_action(action, include_streams=include_streams) + check_stream_delete('events[0]', events[0]) def test_subscribe_other_user_never_subscribed(self) -> None: - action = lambda: self.subscribe(self.example_user("othello"), "test_stream") - events = self.verify_action(action, num_events=2) - check_subscription_peer_add('events[1]', events[1]) + for i, include_streams in enumerate([True, False]): + action = lambda: self.subscribe(self.example_user("othello"), f"test_stream{i}") + events = self.verify_action(action, num_events=2, include_streams=True) + check_subscription_peer_add('events[1]', events[1]) def test_remove_other_user_never_subscribed(self) -> None: self.subscribe(self.example_user("othello"), "test_stream")