diff --git a/tests/conftest.py b/tests/conftest.py index ad67a439b5..e994862333 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -551,7 +551,8 @@ def index_topic(empty_index): Expected index of initial_data when model.narrow = [['stream', '7'], ['topic', 'Test']] """ - diff = {'topic_msg_ids': defaultdict(dict, {205: {'Test': {537286}}})} + # NOTE: Use canonicalized topic name for indexing. + diff = {'topic_msg_ids': defaultdict(dict, {205: {'test': {537286}}})} return dict(empty_index, **diff) @@ -799,8 +800,8 @@ def classified_unread_counts(): 'all_msg': 12, 'all_pms': 8, 'unread_topics': { - (1000, 'Some general unread topic'): 3, - (99, 'Some private unread topic'): 1 + (1000, 'some general unread topic'): 3, + (99, 'some private unread topic'): 1 }, 'unread_pms': { 1: 2, diff --git a/tests/core/test_core.py b/tests/core/test_core.py index 4404983026..2f20c339c9 100644 --- a/tests/core/test_core.py +++ b/tests/core/test_core.py @@ -122,7 +122,8 @@ def test_narrow_to_topic(self, mocker, controller, widget = (controller.view.message_view.log .extend.call_args_list[0][0][0][0]) stream_id, topic_name = msg_box.stream_id, msg_box.topic_name - id_list = index_topic['topic_msg_ids'][stream_id][topic_name] + # NOTE: Use canonicalized topic name for lookup. + id_list = index_topic['topic_msg_ids'][stream_id][topic_name.lower()] assert {widget.original_widget.message['id']} == id_list def test_narrow_to_user(self, mocker, controller, user_button, index_user): diff --git a/tests/helper/test_helper.py b/tests/helper/test_helper.py index d5888a6e8a..5a18e7cb33 100644 --- a/tests/helper/test_helper.py +++ b/tests/helper/test_helper.py @@ -189,16 +189,16 @@ def test_powerset(iterable, map_func, expected_powerset): @pytest.mark.parametrize('muted_streams, muted_topics, vary_in_unreads', [ - ([99], [['Some general stream', 'Some general unread topic']], { + ([99], [['Some general stream', 'some general unread topic']], { 'all_msg': 8, 'streams': {99: 1}, - 'unread_topics': {(99, 'Some private unread topic'): 1}, + 'unread_topics': {(99, 'some private unread topic'): 1}, 'all_mentions': 0, }), - ([1000], [['Secret stream', 'Some private unread topic']], { + ([1000], [['Secret stream', 'some private unread topic']], { 'all_msg': 8, 'streams': {1000: 3}, - 'unread_topics': {(1000, 'Some general unread topic'): 3}, + 'unread_topics': {(1000, 'some general unread topic'): 3}, 'all_mentions': 0, }), ([1], [], {'all_mentions': 0}) @@ -214,7 +214,7 @@ def test_classify_unread_counts(mocker, initial_data, stream_dict, model.initial_data = initial_data model.is_muted_topic = mocker.Mock(side_effect=( lambda stream_id, topic: - [model.stream_dict[stream_id]['name'], topic] in muted_topics + [model.stream_dict[stream_id]['name'], topic.lower()] in muted_topics )) model.muted_streams = muted_streams assert classify_unread_counts(model) == dict(classified_unread_counts, diff --git a/tests/model/test_model.py b/tests/model/test_model.py index 9ed2756b2d..f73f86aadd 100644 --- a/tests/model/test_model.py +++ b/tests/model/test_model.py @@ -286,7 +286,8 @@ def test_set_narrow_not_already_set(self, model, initial_narrow, narrow, ['topic', 'BOO']], { 'topic_msg_ids': { 1: { - 'BOO': {0, 1} + # NOTE: Use canonicalized topic name for indexing. + 'boo': {0, 1} } } }, {0, 1}), @@ -294,7 +295,8 @@ def test_set_narrow_not_already_set(self, model, initial_narrow, narrow, ['topic', 'BOOBOO']], { 'topic_msg_ids': { 1: { - 'BOO': {0, 1} + # NOTE: Use canonicalized topic name for indexing. + 'boo': {0, 1} } } }, set()), diff --git a/tests/ui/test_ui_tools.py b/tests/ui/test_ui_tools.py index 9172eadf5b..5729d7c266 100644 --- a/tests/ui/test_ui_tools.py +++ b/tests/ui/test_ui_tools.py @@ -1144,8 +1144,8 @@ def mock_external_classes(self, mocker): 1000: 1, }, 'unread_topics': { - (205, 'TOPIC1'): 34, - (205, 'TOPIC2'): 100, + (205, 'topic1'): 34, + (205, 'topic2'): 100, }, 'all_mentions': 1, } @@ -1200,7 +1200,7 @@ def test_topics_view(self, mocker, stream_button, width=40): topic_button = mocker.patch(VIEWS + '.TopicButton') topics_view = mocker.patch(VIEWS + '.TopicsView') line_box = mocker.patch(VIEWS + '.urwid.LineBox') - topic_list = ['TOPIC1', 'TOPIC2', 'TOPIC3'] + topic_list = ['topic1', 'topic2', 'topic3'] unread_count_list = [34, 100, 0] self.view.model.topics_in_stream = ( mocker.Mock(return_value=topic_list) diff --git a/zulipterminal/helper.py b/zulipterminal/helper.py index 9a409f7e82..f4de35d302 100644 --- a/zulipterminal/helper.py +++ b/zulipterminal/helper.py @@ -93,7 +93,7 @@ 'all_msg': int, 'all_pms': int, 'all_mentions': int, - 'unread_topics': Dict[Tuple[int, str], int], # stream_id, topic + 'unread_topics': Dict[Tuple[int, str], int], # stream_id, canonical topic 'unread_pms': Dict[int, int], # sender_id 'unread_huddles': Dict[FrozenSet[int], int], # Group pms 'streams': Dict[int, int], # stream_id @@ -139,8 +139,10 @@ def update_unreads(unreads: Dict[KeyT, int], key: KeyT) -> None: for message in changed_messages: if message['type'] == 'stream': stream_id = message['stream_id'] + # Use lowercase topic names in unread_topics to backup the + # invariant in index. update_unreads(unread_counts['unread_topics'], - (stream_id, message['subject'])) + (stream_id, canonicalize_topic(message['subject']))) update_unreads(unread_counts['streams'], stream_id) # self-pm has only one display_recipient # 1-1 pms have 2 display_recipient @@ -201,7 +203,7 @@ def _set_count_in_view(controller: Any, new_count: int, # If topic_view is open for incoming messages's stream, # We update the respective TopicButton count accordingly. for topic_button in topic_buttons_log: - if topic_button.topic_name == msg_topic: + if compare_lowercase(topic_button.topic_name, msg_topic): topic_button.update_count(topic_button.count + new_count) else: @@ -245,11 +247,13 @@ def index_messages(messages: List[Message], { 'pointer': { '[]': 30 # str(ZulipModel.narrow) + # NOTE: canonicalize_topic() is used for indexing. '[["stream", "verona"]]': 32, ... } 'topic_msg_ids': { 123: { # stream_id + # NOTE: canonicalize_topic() is used for indexing. 'topic name': { 51234, # message id 56454, @@ -409,12 +413,19 @@ def index_messages(messages: List[Message], (index['stream_msg_ids_by_stream_id'][msg['stream_id']] .add(msg['id'])) - if (msg['type'] == 'stream' and len(narrow) == 2 - and narrow[1][1] == msg['subject']): - topics_in_stream = index['topic_msg_ids'][msg['stream_id']] - if not topics_in_stream.get(msg['subject']): - topics_in_stream[msg['subject']] = set() - topics_in_stream[msg['subject']].add(msg['id']) + if msg['type'] == 'stream' and len(narrow) == 2: + narrow_topic = narrow[1][1] + message_topic = msg['subject'] + # The comparison is in lowercase to avoid missing other case + # sensitive topic variants. For instance, case, CASE, cAsE. + if compare_lowercase(narrow_topic, message_topic): + topics_in_stream = index['topic_msg_ids'][msg['stream_id']] + # To have an invariant, use canonicalize_topic() to index IDs + # in topic_msg_ids. + message_topic = canonicalize_topic(message_topic) + if not topics_in_stream.get(message_topic): + topics_in_stream[message_topic] = set() + topics_in_stream[message_topic].add(msg['id']) return index @@ -450,8 +461,10 @@ def classify_unread_counts(model: Any) -> UnreadCounts: continue if model.is_muted_topic(stream_id, stream['topic']): continue - stream_topic = (stream_id, stream['topic']) - unread_counts['unread_topics'][stream_topic] = count + stream_topic = (stream_id, canonicalize_topic(stream['topic'])) + unread_counts['unread_topics'][stream_topic] = ( + count + unread_counts['unread_topics'].get(stream_topic, 0) + ) if not unread_counts['streams'].get(stream_id): unread_counts['streams'][stream_id] = count else: @@ -629,3 +642,15 @@ def display_error_if_present(response: Dict[str, Any], controller: Any ) -> None: if response['result'] == 'error' and hasattr(controller, 'view'): controller.view.set_footer_text(response['msg'], 3) + + +def canonicalize_topic(topic_name: str) -> str: + """ + Returns a canonicalized topic name to be used for indexing and lookups + locally. + """ + return topic_name.lower() + + +def compare_lowercase(a: str, b: str) -> bool: + return a.lower() == b.lower() diff --git a/zulipterminal/model.py b/zulipterminal/model.py index 44208a8402..91ad3effa2 100644 --- a/zulipterminal/model.py +++ b/zulipterminal/model.py @@ -2,6 +2,7 @@ import time from collections import OrderedDict, defaultdict from concurrent.futures import Future, ThreadPoolExecutor, wait +from copy import deepcopy from typing import ( Any, Callable, DefaultDict, Dict, FrozenSet, Iterable, List, Optional, Set, Tuple, Union, @@ -13,8 +14,9 @@ from zulipterminal.config.keys import keys_for_command from zulipterminal.helper import ( - Message, StreamData, asynch, canonicalize_color, classify_unread_counts, - display_error_if_present, index_messages, initial_index, notify, set_count, + Message, StreamData, asynch, canonicalize_color, canonicalize_topic, + classify_unread_counts, compare_lowercase, display_error_if_present, + index_messages, initial_index, notify, set_count, ) from zulipterminal.ui_tools.utils import create_msg_box_list @@ -124,8 +126,9 @@ def __init__(self, controller: Any) -> None: muted_topics = self.initial_data['muted_topics'] assert set(map(len, muted_topics)) in (set(), {2}, {3}) self._muted_topics = { - (stream_name, topic): (None if self.server_feature_level is None - else date_muted[0]) + (stream_name, canonicalize_topic(topic)): ( + None if self.server_feature_level is None else date_muted[0] + ) for stream_name, topic, *date_muted in muted_topics } # type: Dict[Tuple[str, str], Optional[int]] @@ -138,15 +141,26 @@ def __init__(self, controller: Any) -> None: self.new_user_input = True self._start_presence_updates() + def narrow_with_canonical_topic(self) -> List[Any]: + """ + Returns the narrow with its topic name replaced with the invariant + version that we maintain locally. + """ + narrow = deepcopy(self.narrow) + if len(narrow) == 2 and narrow[1][0] == 'topic': + narrow[1][1] = canonicalize_topic(narrow[1][1]) + return narrow + def get_focus_in_current_narrow(self) -> Union[int, Set[None]]: """ Returns the focus in the current narrow. For no existing focus this returns {}, otherwise the message ID. """ - return self.index['pointer'][repr(self.narrow)] + return self.index['pointer'][repr(self.narrow_with_canonical_topic())] def set_focus_in_current_narrow(self, focus_message: int) -> None: - self.index['pointer'][repr(self.narrow)] = focus_message + narrow_str = repr(self.narrow_with_canonical_topic()) + self.index['pointer'][narrow_str] = focus_message def is_search_narrow(self) -> bool: """ @@ -219,7 +233,7 @@ def get_message_ids_in_current_narrow(self) -> Set[int]: if len(narrow) == 1: ids = index['stream_msg_ids_by_stream_id'][stream_id] elif len(narrow) == 2: - topic = narrow[1][1] + topic = canonicalize_topic(narrow[1][1]) ids = index['topic_msg_ids'][stream_id].get(topic, set()) elif narrow[0][1] == 'private': ids = index['private_msg_ids'] @@ -372,7 +386,7 @@ def get_messages(self, *, response = self.client.get_messages(message_filters=request) if response['result'] == 'success': self.index = index_messages(response['messages'], self, self.index) - narrow_str = repr(self.narrow) + narrow_str = repr(self.narrow_with_canonical_topic()) if first_anchor and response['anchor'] != 10000000000000000: self.index['pointer'][narrow_str] = response['anchor'] if 'found_newest' in response: @@ -396,6 +410,11 @@ def _fetch_topics_in_streams(self, stream_list: Iterable[int]) -> str: for stream_id in stream_list: response = self.client.get_stream_topics(stream_id) if response['result'] == 'success': + # NOTE: The server only sends the latest topic name version for + # case sensitive topic names. + # See (https://github.com/zulip/zulip/blob + # /2e5f860d41bb441597f7f088a477d5946cab4b13/zerver/lib + # /topic.py#L144). self.index['topics'][stream_id] = [topic['name'] for topic in response['topics']] else: @@ -427,7 +446,7 @@ def is_muted_topic(self, stream_id: int, topic: str) -> bool: Returns True if topic is muted via muted_topics. """ stream_name = self.stream_dict[stream_id]['name'] - topic_to_search = (stream_name, topic) + topic_to_search = (stream_name, canonicalize_topic(topic)) return topic_to_search in self._muted_topics.keys() def _update_initial_data(self) -> None: @@ -862,8 +881,9 @@ def _handle_message_event(self, event: Event) -> None: if 'read' not in message['flags']: set_count([message['id']], self.controller, 1) + narrow_str = repr(self.narrow_with_canonical_topic()) if (hasattr(self.controller, 'view') - and self._have_last_message[repr(self.narrow)]): + and self._have_last_message[narrow_str]): msg_log = self.controller.view.message_view.log if msg_log: last_message = msg_log[-1].original_widget.message @@ -896,7 +916,8 @@ def _handle_message_event(self, event: Event) -> None: if (append_to_stream and (len(self.narrow) == 1 or (len(self.narrow) == 2 - and self.narrow[1][1] == message['subject']))): + and compare_lowercase(self.narrow[1][1], + message['subject'])))): msg_log.append(msg_w) elif (message['type'] == 'private' and len(self.narrow) == 1 @@ -915,8 +936,10 @@ def _update_topic_index(self, stream_id: int, topic_name: str) -> None: """ topic_list = self.topics_in_stream(stream_id) for topic_iterator, topic in enumerate(topic_list): - if topic == topic_name: - topic_list.insert(0, topic_list.pop(topic_iterator)) + if compare_lowercase(topic, topic_name): + topic_list.pop(topic_iterator) + # Use the latest topic name version to update. + topic_list.insert(0, topic_name) break else: # No previous topics with same topic names are found @@ -1025,7 +1048,8 @@ def _update_rendered_view(self, msg_id: int) -> None: # Remove the message if it no longer belongs in the current # narrow. if (len(self.narrow) == 2 - and msg_box.message['subject'] != self.narrow[1][1]): + and not compare_lowercase(msg_box.message['subject'], + self.narrow[1][1])): view.message_view.log.remove(msg_w) # Change narrow if there are no messages left in the # current narrow. diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py index 0a9547d455..0660d2bc48 100644 --- a/zulipterminal/ui_tools/boxes.py +++ b/zulipterminal/ui_tools/boxes.py @@ -20,8 +20,8 @@ STREAM_TOPIC_SEPARATOR, TIME_MENTION_MARKER, ) from zulipterminal.helper import ( - Message, format_string, match_emoji, match_group, match_stream, - match_topics, match_user, + Message, compare_lowercase, format_string, match_emoji, match_group, + match_stream, match_topics, match_user, ) from zulipterminal.ui_tools.tables import render_table from zulipterminal.urwid_types import urwid_Size @@ -432,7 +432,7 @@ def need_recipient_header(self) -> bool: if self.message['type'] == 'stream': return not ( last_msg['type'] == 'stream' - and self.topic_name == last_msg['subject'] + and compare_lowercase(self.topic_name, last_msg['subject']) and self.stream_name == last_msg['display_recipient'] ) elif self.message['type'] == 'private': diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index df4c8a829c..1c9297c4e1 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -11,7 +11,10 @@ from zulipterminal.config.symbols import ( CHECK_MARK, LIST_TITLE_BAR_LINE, PINNED_STREAMS_DIVIDER, ) -from zulipterminal.helper import Message, asynch, match_stream, match_user +from zulipterminal.helper import ( + Message, asynch, canonicalize_topic, compare_lowercase, match_stream, + match_user, +) from zulipterminal.ui_tools.boxes import PanelSearchBox from zulipterminal.ui_tools.buttons import ( HomeButton, MentionedButton, MessageLinkButton, PMButton, StarredButton, @@ -390,8 +393,20 @@ def update_topics_list(self, stream_id: int, topic_name: str, # More recent topics are found towards the beginning # of the list. for topic_iterator, topic_button in enumerate(self.log): - if topic_button.topic_name == topic_name: - self.log.insert(0, self.log.pop(topic_iterator)) + if compare_lowercase(topic_button.topic_name, topic_name): + updated_topic_button = self.log.pop(topic_iterator) + # Use the latest topic name version to update the topic list if + # it has been updated. + if topic_button.topic_name != topic_name: + updated_topic_button = TopicButton( + stream_id=stream_id, + topic=topic_name, + controller=self.view.controller, + width=self.view.LEFT_WIDTH, + count=self.view.model.unread_counts['unread_topics']. + get((stream_id, canonicalize_topic(topic_name)), 0), + ) + self.log.insert(0, updated_topic_button) self.list_box.set_focus_valign('bottom') if sender_id == self.view.model.user_id: self.list_box.set_focus(0) @@ -790,7 +805,7 @@ def topics_view(self, stream_button: Any) -> Any: controller=self.controller, width=self.width, count=self.model.unread_counts['unread_topics']. - get((stream_id, topic), 0) + get((stream_id, canonicalize_topic(topic)), 0) ) for topic in topics ]