diff --git a/tests/model/test_model.py b/tests/model/test_model.py index 89176ca1ac..0eeb583ea8 100644 --- a/tests/model/test_model.py +++ b/tests/model/test_model.py @@ -600,6 +600,34 @@ def test_success_get_messages(self, mocker, messages_successful_response, assert model.index['pointer'][repr(model.narrow)] == anchor assert model._have_last_message[repr(model.narrow)] is True + @pytest.mark.parametrize('messages, expected_messages_response', [( + {'topic_links': [{'url': 'www.foo.com', 'text': 'Bar'}]}, + {'topic_links': [{'url': 'www.foo.com', 'text': 'Bar'}]}, + ), ( + {'topic_links': ['www.foo1.com']}, + {'topic_links': [{'url': 'www.foo1.com', 'text': ''}]}, + ), ( + {'topic_links': []}, + {'topic_links': []}, + ), ( + {'subject_links': ['www.foo2.com']}, + {'topic_links': [{'url': 'www.foo2.com', 'text': ''}]}, + ), ( + {'subject_links': []}, + {'topic_links': []}, + )], ids=[ + 'Zulip_4.0+_ZFL46_response_with_topic_links', + 'Zulip_3.0+_ZFL1_response_with_topic_links', + 'Zulip_3.0+_ZFL1_response_empty_topic_links', + 'Zulip_2.1+_response_with_subject_links', + 'Zulip_2.1+_response_empty_subject_links', + ]) + def test_modernize_message_response(self, model, messages, + expected_messages_response): + + assert (model.modernize_message_response(messages) + == expected_messages_response) + def test_get_message_false_first_anchor( self, mocker, messages_successful_response, index_all_messages, initial_data, num_before=30, num_after=10 diff --git a/tests/ui/test_ui_tools.py b/tests/ui/test_ui_tools.py index fdcc69edc5..ae790b9040 100644 --- a/tests/ui/test_ui_tools.py +++ b/tests/ui/test_ui_tools.py @@ -1292,6 +1292,8 @@ def test_init(self, mocker, message_type, set_fields): assert msg_box.last_message == defaultdict(dict) for field, invalid_default in set_fields: assert getattr(msg_box, field) != invalid_default + if message_type == 'stream': + assert msg_box.topic_links == OrderedDict() assert msg_box.message_links == OrderedDict() assert msg_box.time_mentions == list() diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py index 01d1630bbc..539a295557 100644 --- a/tests/ui_tools/test_popups.py +++ b/tests/ui_tools/test_popups.py @@ -207,6 +207,7 @@ def mock_external_classes(self, mocker): self.edit_history_view = EditHistoryView( controller=self.controller, message=self.message, + topic_links=OrderedDict(), message_links=OrderedDict(), time_mentions=list(), title='Edit History', @@ -215,6 +216,7 @@ def mock_external_classes(self, mocker): def test_init(self): assert self.edit_history_view.controller == self.controller assert self.edit_history_view.message == self.message + assert self.edit_history_view.topic_links == OrderedDict() assert self.edit_history_view.message_links == OrderedDict() assert self.edit_history_view.time_mentions == list() self.controller.model.fetch_message_history.assert_called_once_with( @@ -246,6 +248,7 @@ def test_keypress_show_msg_info(self, key, widget_size): self.controller.show_msg_info.assert_called_once_with( msg=self.message, + topic_links=OrderedDict(), message_links=OrderedDict(), time_mentions=list(), ) @@ -450,7 +453,13 @@ def mock_external_classes(self, mocker, monkeypatch, message_fixture): ] self.msg_info_view = MsgInfoView(self.controller, message_fixture, 'Message Information', OrderedDict(), - list()) + OrderedDict(), list()) + + def test_init(self, message_fixture): + assert self.msg_info_view.msg == message_fixture + assert self.msg_info_view.topic_links == OrderedDict() + assert self.msg_info_view.message_links == OrderedDict() + assert self.msg_info_view.time_mentions == list() def test_keypress_any_key(self, widget_size): key = "a" @@ -482,6 +491,7 @@ def test_keypress_edit_history(self, message_fixture, key, widget_size, } msg_info_view = MsgInfoView(self.controller, message_fixture, title='Message Information', + topic_links=OrderedDict(), message_links=OrderedDict(), time_mentions=list()) size = widget_size(msg_info_view) @@ -491,6 +501,7 @@ def test_keypress_edit_history(self, message_fixture, key, widget_size, if msg_info_view.show_edit_history_label: self.controller.show_edit_history.assert_called_once_with( message=message_fixture, + topic_links=OrderedDict(), message_links=OrderedDict(), time_mentions=list(), ) @@ -552,11 +563,50 @@ def test_height_reactions(self, message_fixture, to_vary_in_each_message): varied_message = dict(message_fixture, **to_vary_in_each_message) self.msg_info_view = MsgInfoView(self.controller, varied_message, 'Message Information', OrderedDict(), - list()) + OrderedDict(), list()) # 9 = 3 labels + 1 blank line + 1 'Reactions' (category) + 4 reactions. expected_height = 9 assert self.msg_info_view.height == expected_height + @pytest.mark.parametrize([ + 'initial_link', + 'expected_text', + 'expected_attr_map', + 'expected_focus_map', + 'expected_link_width' + ], [( + OrderedDict([('https://bar.com', ('Foo', 1, True))]), + '1: Foo\nhttps://bar.com', + {None: 'popup_contrast'}, + {None: 'selected'}, + 15, + ), ( + OrderedDict([('https://foo.com', ('', 1, True))]), + '1: https://foo.com', + {None: 'popup_contrast'}, + {None: 'selected'}, + 18, + )], ids=[ + 'link_with_link_text', + 'link_without_link_text', + ] + ) + def test_create_link_buttons(self, initial_link, expected_text, + expected_attr_map, expected_focus_map, + expected_link_width): + [link_w], link_width = self.msg_info_view.create_link_buttons( + self.controller, initial_link, + ) + + assert [link_w.link] == list(initial_link) + assert (link_w._wrapped_widget.original_widget.text + == expected_text) + assert (link_w._wrapped_widget.focus_map + == expected_focus_map) + assert (link_w._wrapped_widget.attr_map + == expected_attr_map) + assert link_width == expected_link_width + def test_keypress_navigation(self, mocker, widget_size, navigation_key_expected_key_pair): key, expected_key = navigation_key_expected_key_pair diff --git a/zulipterminal/api_types.py b/zulipterminal/api_types.py index 07bf551fa4..959a7cd7cb 100644 --- a/zulipterminal/api_types.py +++ b/zulipterminal/api_types.py @@ -30,7 +30,12 @@ class Message(TypedDict, total=False): timestamp: int client: str subject: str # Only for stream msgs. - topic_links: List[str] + # NOTE: new in Zulip 3.0 / ZFL 1 + # NOTE: API response format of `topic_links` new in Zulip 4.0 / ZFL 46 + topic_links: List[Any] + # NOTE: `subject_links` new in Zulip 2.1; + # Deprecated from Zulip 3.0 / ZFL 1 + subject_links: List[str] is_me_message: bool reactions: List[Dict[str, Any]] submessages: List[Dict[str, Any]] diff --git a/zulipterminal/core.py b/zulipterminal/core.py index 14db4ba311..346e8a8b42 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -205,12 +205,13 @@ def show_topic_edit_mode(self, button: Any) -> None: self.show_pop_up(EditModeView(self, button), 'area:msg') def show_msg_info(self, msg: Message, + topic_links: 'OrderedDict[str, Tuple[str, int, bool]]', message_links: 'OrderedDict[str, Tuple[str, int, bool]]', time_mentions: List[Tuple[str, str]], ) -> None: msg_info_view = MsgInfoView(self, msg, "Message Information (up/down scrolls)", - message_links, time_mentions) + topic_links, message_links, time_mentions) self.show_pop_up(msg_info_view, 'area:msg') def show_stream_info(self, stream_id: int) -> None: @@ -241,11 +242,13 @@ def show_about(self) -> None: def show_edit_history( self, message: Message, + topic_links: 'OrderedDict[str, Tuple[str, int, bool]]', message_links: 'OrderedDict[str, Tuple[str, int, bool]]', time_mentions: List[Tuple[str, str]], ) -> None: self.show_pop_up( - EditHistoryView(self, message, message_links, time_mentions, + EditHistoryView(self, message, topic_links, + message_links, time_mentions, 'Edit History (up/down scrolls)'), 'area:msg' ) diff --git a/zulipterminal/model.py b/zulipterminal/model.py index 6b996489b7..c1b2231f22 100644 --- a/zulipterminal/model.py +++ b/zulipterminal/model.py @@ -509,6 +509,11 @@ def get_messages(self, *, } response = self.client.get_messages(message_filters=request) if response['result'] == 'success': + response['messages'] = [ + self.modernize_message_response(msg) + for msg in response['messages'] + ] + self.index = index_messages(response['messages'], self, self.index) narrow_str = repr(self.narrow) if first_anchor and response['anchor'] != 10000000000000000: @@ -530,6 +535,35 @@ def get_messages(self, *, display_error_if_present(response, self.controller) return response['msg'] + @staticmethod + def modernize_message_response(message: Message + ) -> Message: + """ + Converts received message into the modern message response format. + + This provides a good single place to handle support for older server + releases params, and making them compatible with recent releases. + + TODO: This could be extended for other message params potentially + in future. + """ + # (1) `subject_links` param is changed to `topic_links` from + # feature level 1, server version 3.0 + if 'subject_links' in message.keys(): + message['topic_links'] = message.pop('subject_links') + + # (2) Modernize `topic_links` old response (List[str]) to new response + # (List[Dict[str, str]]) + if 'topic_links' in message.keys(): + topic_links = [ + {'url': link, 'text': ''} for link in message['topic_links'] + if type(link) == str + ] + if topic_links: + message['topic_links'] = topic_links + + return message + def fetch_message_history(self, message_id: int, ) -> List[Dict[str, Union[int, str]]]: """ @@ -986,7 +1020,7 @@ def _handle_message_event(self, event: Event) -> None: Handle new messages (eg. add message to the end of the view) """ assert event['type'] == "message" - message = event['message'] + message = self.modernize_message_response(event['message']) # sometimes `flags` are missing in `event` so initialize # an empty list of flags in that case. message['flags'] = event.get('flags', []) diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py index ee38d6007a..0bd47f806b 100644 --- a/zulipterminal/ui_tools/boxes.py +++ b/zulipterminal/ui_tools/boxes.py @@ -661,6 +661,9 @@ def __init__(self, message: Message, model: 'Model', self.message_links: 'OrderedDict[str, Tuple[str, int, bool]]' = ( OrderedDict() ) + self.topic_links: 'OrderedDict[str, Tuple[str, int, bool]]' = ( + OrderedDict() + ) self.time_mentions: List[Tuple[str, str]] = list() self.last_message = last_message # if this is the first message @@ -668,6 +671,13 @@ def __init__(self, message: Message, model: 'Model', self.last_message = defaultdict(dict) if self.message['type'] == 'stream': + # Set `topic_links` if present + for link in self.message.get('topic_links', []): + # Modernized response + self.topic_links[link['url']] = ( + link['text'], len(self.topic_links) + 1, True + ) + self.stream_name = self.message['display_recipient'] self.stream_id = self.message['stream_id'] self.topic_name = self.message['subject'] @@ -1549,6 +1559,7 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]: self.model.controller.view.middle_column.set_focus('footer') elif is_command_key('MSG_INFO', key): self.model.controller.show_msg_info(self.message, + self.topic_links, self.message_links, self.time_mentions) return key diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 4c9819de44..8851a0edfb 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -1219,10 +1219,12 @@ def keypress(self, size: urwid_Size, key: str) -> str: class MsgInfoView(PopUpView): def __init__(self, controller: Any, msg: Message, title: str, + topic_links: 'OrderedDict[str, Tuple[str, int, bool]]', message_links: 'OrderedDict[str, Tuple[str, int, bool]]', time_mentions: List[Tuple[str, str]], ) -> None: self.msg = msg + self.topic_links = topic_links self.message_links = message_links self.time_mentions = time_mentions date_and_time = controller.model.formatted_local_time( @@ -1249,6 +1251,8 @@ def __init__(self, controller: Any, msg: Message, title: str, ('Edit History', f"Press {keys} to view") ) # Render the category using the existing table methods if links exist. + if topic_links: + msg_info.append(('Topic Links', [])) if message_links: msg_info.append(('Message Links', [])) if time_mentions: @@ -1270,36 +1274,74 @@ def __init__(self, controller: Any, msg: Message, title: str, len(title)) widgets = self.make_table_with_categories(msg_info, column_widths) - if message_links: - message_link_widgets = [] - message_link_width = 0 - for index, link in enumerate(message_links): - text, link_index, _ = message_links[link] - caption = f"{link_index}: {text}\n{link}" - message_link_width = max( - message_link_width, - len(max(caption.split('\n'), key=len)) - ) + # To keep track of buttons (e.g., button links) and to facilitate + # computing their slice indexes + button_widgets = [] # type: List[Any] - display_attr = None if index % 2 else 'popup_contrast' - message_link_widgets.append( - MessageLinkButton(controller, caption, link, display_attr) - ) + if topic_links: + topic_link_widgets, topic_link_width = ( + self.create_link_buttons(controller, topic_links) + ) + + # slice_index = Number of labels before topic links + 1 newline + # + 1 'Topic Links' category label. + slice_index = len(msg_info[0][1]) + 2 + slice_index += sum([len(w) + 2 for w in button_widgets]) + button_widgets.append(topic_links) + + widgets = (widgets[:slice_index] + topic_link_widgets + + widgets[slice_index:]) + popup_width = max(popup_width, topic_link_width) + + if message_links: + message_link_widgets, message_link_width = ( + self.create_link_buttons(controller, message_links) + ) # slice_index = Number of labels before message links + 1 newline # + 1 'Message Links' category label. slice_index = len(msg_info[0][1]) + 2 + slice_index += sum([len(w) + 2 for w in button_widgets]) + button_widgets.append(message_links) + widgets = (widgets[:slice_index] + message_link_widgets + widgets[slice_index:]) popup_width = max(popup_width, message_link_width) super().__init__(controller, widgets, 'MSG_INFO', popup_width, title) + @staticmethod + def create_link_buttons( + controller: Any, + links: 'OrderedDict[str, Tuple[str, int, bool]]' + ) -> Tuple[List[MessageLinkButton], int]: + link_widgets = [] + link_width = 0 + + for index, link in enumerate(links): + text, link_index, _ = links[link] + if text: + caption = f"{link_index}: {text}\n{link}" + else: + caption = f"{link_index}: {link}" + link_width = max( + link_width, + len(max(caption.split('\n'), key=len)) + ) + + display_attr = None if index % 2 else 'popup_contrast' + link_widgets.append( + MessageLinkButton(controller, caption, link, display_attr) + ) + + return link_widgets, link_width + def keypress(self, size: urwid_Size, key: str) -> str: if (is_command_key('EDIT_HISTORY', key) and self.show_edit_history_label): self.controller.show_edit_history( message=self.msg, + topic_links=self.topic_links, message_links=self.message_links, time_mentions=self.time_mentions, ) @@ -1345,11 +1387,13 @@ def keypress(self, size: urwid_Size, key: str) -> str: class EditHistoryView(PopUpView): def __init__(self, controller: Any, message: Message, + topic_links: 'OrderedDict[str, Tuple[str, int, bool]]', message_links: 'OrderedDict[str, Tuple[str, int, bool]]', time_mentions: List[Tuple[str, str]], title: str) -> None: self.controller = controller self.message = message + self.topic_links = topic_links self.message_links = message_links self.time_mentions = time_mentions width = 64 @@ -1446,6 +1490,7 @@ def keypress(self, size: urwid_Size, key: str) -> str: or is_command_key('EDIT_HISTORY', key)): self.controller.show_msg_info( msg=self.message, + topic_links=self.topic_links, message_links=self.message_links, time_mentions=self.time_mentions, )