Skip to content

Commit 636e31b

Browse files
committed
ui: Add starred message count.
Initial Count Rendering: 1. get_starred function in model.py gets list of starred messages from zulip and updates the index. 2. get_starred is called in _update_initial_data on line 564 in model.py. I assume this function runs at the beginning when ZT is started and that's it. Or does it run throughout? 3. In buttons.py line 146 the StarredButton now accepts count as an arguement. 4. In views, I set the initial count by taking the length of model.index['starred_msg_ids'] Updating the Count Throughout the Session: 2. In _handle_message_flags_event (model.py #1133) starred_button's count is updated to reflect that latest changes. I have some questions about whether this was implemented properly. Theme: Add new starred_count item for styling. Style for various themes. Tests: Add test to check the new starred_button setup and the new model changes. Fixes #578.
1 parent 6b22249 commit 636e31b

File tree

8 files changed

+173
-13
lines changed

8 files changed

+173
-13
lines changed

tests/conftest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,7 @@ def initial_data(logged_on_user, users_fixture, streams_fixture):
547547
# adding extra tests unnecessarily.
548548
'zulip_version': MINIMUM_SUPPORTED_SERVER_VERSION[0],
549549
'zulip_feature_level': MINIMUM_SUPPORTED_SERVER_VERSION[1],
550+
'starred_messages': [1117554, 1117558, 1117574],
550551
}
551552

552553

tests/model/test_model.py

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ def test_register_initial_desired_events(self, mocker, initial_data):
177177
'presence',
178178
'subscription',
179179
'message',
180+
'starred_messages',
180181
'update_message_flags',
181182
'muted_topics',
182183
'realm_user',
@@ -1466,6 +1467,7 @@ def test_update_star_status_no_index(self, mocker, model,
14661467
assert model.index == dict(messages={})
14671468
model._update_rendered_view.assert_not_called()
14681469
set_count.assert_not_called()
1470+
self.controller.view.starred_button.update_count.assert_not_called()
14691471

14701472
def test_update_star_status_invalid_operation(
14711473
self, mocker, model, update_message_flags_operation,
@@ -1486,6 +1488,7 @@ def test_update_star_status_invalid_operation(
14861488
model._handle_update_message_flags_event(event)
14871489
model._update_rendered_view.assert_not_called()
14881490
set_count.assert_not_called()
1491+
self.controller.view.starred_button.update_count.assert_not_called()
14891492

14901493
@pytest.mark.parametrize('event_message_ids, indexed_ids', [
14911494
([1], [1]),
@@ -1511,12 +1514,12 @@ def test_update_star_status(self, mocker, model, event_op,
15111514
flags_before, flags_after,
15121515
update_message_flags_operation):
15131516
operation, model.server_feature_level = update_message_flags_operation
1514-
15151517
model.index = dict(messages={msg_id: {'flags': flags_before}
15161518
for msg_id in indexed_ids},
15171519
starred_msg_ids=set([msg_id
15181520
for msg_id in indexed_ids
15191521
if 'starred' in flags_before]))
1522+
15201523
event = {
15211524
'type': 'update_message_flags',
15221525
'messages': event_message_ids,
@@ -1547,6 +1550,68 @@ def test_update_star_status(self, mocker, model, event_op,
15471550

15481551
set_count.assert_not_called()
15491552

1553+
@pytest.mark.parametrize('event_message_ids, indexed_ids', [
1554+
([1], [1]),
1555+
([1, 2], [1]),
1556+
([1, 2], [1, 2]),
1557+
([1], [1, 2]),
1558+
])
1559+
def test_update_star_count_in_view_remove(self, mocker, model,
1560+
event_message_ids,
1561+
indexed_ids,
1562+
update_message_flags_operation):
1563+
operation, model.server_feature_level = update_message_flags_operation
1564+
model.index = dict(messages={msg_id: {'flags': ['starred']}
1565+
for msg_id in indexed_ids},
1566+
starred_msg_ids=set(indexed_ids))
1567+
1568+
event = {
1569+
'type': 'update_message_flags',
1570+
'messages': event_message_ids,
1571+
'flag': 'starred',
1572+
operation: 'remove',
1573+
'all': False,
1574+
}
1575+
mocker.patch('zulipterminal.model.Model._update_rendered_view')
1576+
set_count = mocker.patch('zulipterminal.model.set_count')
1577+
update_star_count = self.controller.view.starred_button.update_count
1578+
1579+
model._handle_update_message_flags_event(event)
1580+
1581+
update_star_count.assert_called_with(
1582+
self.controller.view.starred_button.count - 1)
1583+
1584+
@pytest.mark.parametrize('event_message_ids, indexed_ids', [
1585+
([1], [1]),
1586+
([1, 2], [1]),
1587+
([1, 2], [1, 2]),
1588+
([1], [1, 2]),
1589+
])
1590+
def test_update_star_count_in_view_add(self, mocker, model,
1591+
event_message_ids,
1592+
indexed_ids,
1593+
update_message_flags_operation):
1594+
operation, model.server_feature_level = update_message_flags_operation
1595+
model.index = dict(messages={msg_id: {'flags': []}
1596+
for msg_id in indexed_ids},
1597+
starred_msg_ids=set())
1598+
1599+
event = {
1600+
'type': 'update_message_flags',
1601+
'messages': event_message_ids,
1602+
'flag': 'starred',
1603+
operation: 'add',
1604+
'all': False,
1605+
}
1606+
mocker.patch('zulipterminal.model.Model._update_rendered_view')
1607+
set_count = mocker.patch('zulipterminal.model.set_count')
1608+
update_star_count = self.controller.view.starred_button.update_count
1609+
1610+
model._handle_update_message_flags_event(event)
1611+
1612+
update_star_count.assert_called_with(
1613+
self.controller.view.starred_button.count + 1)
1614+
15501615
@pytest.mark.parametrize('event_message_ids, indexed_ids', [
15511616
([1], [1]),
15521617
([1, 2], [1]),
@@ -1573,7 +1638,10 @@ def test_update_read_status(self, mocker, model, event_op,
15731638
operation, model.server_feature_level = update_message_flags_operation
15741639

15751640
model.index = dict(messages={msg_id: {'flags': flags_before}
1576-
for msg_id in indexed_ids})
1641+
for msg_id in indexed_ids},
1642+
starred_msg_ids=set([msg_id
1643+
for msg_id in indexed_ids
1644+
if 'starred' in flags_before]))
15771645
event = {
15781646
'type': 'update_message_flags',
15791647
'messages': event_message_ids,

tests/ui/test_ui_tools.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,6 +1190,9 @@ def mock_external_classes(self, mocker):
11901190
},
11911191
'all_mentions': 1,
11921192
}
1193+
self.view.model.initial_data = {
1194+
"starred_messages": [1117554, 1117558, 1117574],
1195+
}
11931196
self.view.controller = mocker.Mock()
11941197
self.super_mock = mocker.patch(VIEWS + ".urwid.Pile.__init__")
11951198

@@ -1198,6 +1201,7 @@ def test_menu_view(self, mocker, width=40):
11981201
VIEWS + ".LeftColumnView.streams_view")
11991202
home_button = mocker.patch(VIEWS + ".HomeButton")
12001203
pm_button = mocker.patch(VIEWS + ".PMButton")
1204+
starred_button = mocker.patch(VIEWS + ".StarredButton")
12011205
mocker.patch(VIEWS + ".urwid.ListBox")
12021206
mocker.patch(VIEWS + ".urwid.SimpleFocusListWalker")
12031207
mocker.patch(STREAMBUTTON + ".mark_muted")
@@ -1206,6 +1210,8 @@ def test_menu_view(self, mocker, width=40):
12061210
count=2, width=width)
12071211
pm_button.assert_called_once_with(left_col_view.controller,
12081212
count=0, width=width)
1213+
starred_button.assert_called_once_with(left_col_view.controller,
1214+
count=3, width=width)
12091215

12101216
@pytest.mark.parametrize('pinned', powerset([1, 2, 99, 1000]))
12111217
def test_streams_view(self, mocker, streams, pinned, width=40):

tests/ui_tools/test_buttons.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from zulipterminal.config.keys import keys_for_command
77
from zulipterminal.ui_tools.buttons import (
88
MessageLinkButton,
9+
StarredButton,
910
StreamButton,
1011
TopButton,
1112
TopicButton,
@@ -92,6 +93,60 @@ def test_text_content(self, mocker,
9293
assert len(text[0]) == len(expected_text) == (width - 1)
9394
assert text[0] == expected_text
9495

96+
def test_starred_arg(self, mocker):
97+
"""Make sure only starred_button is marked as starred"""
98+
show_function = mocker.Mock()
99+
assert TopButton(controller=mocker.Mock(),
100+
caption='caption',
101+
show_function=show_function,
102+
width=12,
103+
count=10,
104+
starred=True).starred is True
105+
assert StarredButton(controller=mocker.Mock(),
106+
width=12,
107+
count=10,).starred is True
108+
assert TopButton(controller=mocker.Mock(),
109+
caption='caption',
110+
show_function=show_function,
111+
width=12,
112+
count=10,).starred is False
113+
114+
def test_update_count_not_starred(self, mocker):
115+
"""Make sure update_count is called with correct parameters"""
116+
show_function = mocker.Mock()
117+
other_button = TopButton(controller=mocker.Mock(),
118+
caption='caption',
119+
show_function=show_function,
120+
width=12,
121+
count=10,)
122+
other_button.update_widget = mocker.patch(TOPBUTTON + ".update_widget")
123+
other_button.update_count(11)
124+
assert ((('unread_count', '11'), None)
125+
== other_button.update_widget.call_args[0])
126+
127+
128+
class TestStarredButton:
129+
# Is this where these tests should be?
130+
def test_starred_is_true(self, mocker):
131+
"""Make sure starred_button is marked as starred"""
132+
assert StarredButton(controller=mocker.Mock(),
133+
width=12,
134+
count=10,).starred is True
135+
136+
def test_starred_update_count(self, mocker):
137+
"""Make sure update_count is called with correct parameters"""
138+
starred_button = StarredButton(controller=mocker.Mock(),
139+
width=12,
140+
count=10,)
141+
starred_button.update_widget = mocker.patch(
142+
TOPBUTTON + ".update_widget")
143+
starred_button.update_count(11)
144+
assert ((('starred_count', '11'), None)
145+
== starred_button.update_widget.call_args[0])
146+
# assert starred_button.update_widget.assert_called_once_with(
147+
# ('starred_count', '11'), None)
148+
# ^ this wasn't working but it should, any ideas?
149+
95150

96151
class TestStreamButton:
97152
@pytest.mark.parametrize('is_private, expected_prefix', [

zulipterminal/config/themes.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@
163163
None, DEF['light_blue:bold'], DEF['black']),
164164
('unread_count', 'yellow', 'black',
165165
None, DEF['yellow'], DEF['black']),
166+
('starred_count', 'light gray', 'black',
167+
None, DEF['light_gray'], DEF['black']),
166168
('table_head', 'white, bold', 'black',
167169
None, DEF['white:bold'], DEF['black']),
168170
('filter_results', 'white', 'dark green',
@@ -255,6 +257,8 @@
255257
None, LIGHTBLUE, BLACK),
256258
('unread_count', 'yellow', 'black',
257259
None, YELLOW, BLACK),
260+
('starred_count', 'light gray', 'black',
261+
None, GRAY, BLACK),
258262
('table_head', 'white, bold', 'black',
259263
None, WHITEBOLD, BLACK),
260264
('filter_results', 'black', 'light green',
@@ -314,6 +318,7 @@
314318
('starred', 'light red, bold', 'white'),
315319
('popup_category', 'dark gray, bold', 'light gray'),
316320
('unread_count', 'dark blue, bold', 'white'),
321+
('starred_count', 'black', 'white'),
317322
('table_head', 'black, bold', 'white'),
318323
('filter_results', 'white', 'dark green'),
319324
('edit_topic', 'white', 'dark gray'),
@@ -360,6 +365,7 @@
360365
('starred', 'light red, bold', 'light blue'),
361366
('popup_category', 'light gray, bold', 'light blue'),
362367
('unread_count', 'yellow', 'light blue'),
368+
('starred_count', 'black', 'light blue'),
363369
('table_head', 'black, bold', 'light blue'),
364370
('filter_results', 'white', 'dark green'),
365371
('edit_topic', 'white', 'dark blue'),

zulipterminal/model.py

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ def __init__(self, controller: Any) -> None:
9494
'presence',
9595
'subscription',
9696
'message',
97+
'starred_messages',
9798
'update_message_flags',
9899
'muted_topics',
99100
'realm_user', # Enables cross_realm_bots
@@ -129,7 +130,6 @@ def __init__(self, controller: Any) -> None:
129130
self.server_feature_level = (
130131
self.initial_data.get('zulip_feature_level')
131132
)
132-
133133
self.users = self.get_all_users()
134134

135135
self.stream_dict: Dict[int, Any] = {}
@@ -627,7 +627,6 @@ def get_other_subscribers_in_stream(self, stream_id: Optional[int]=None,
627627

628628
if stream_id:
629629
assert self.is_user_subscribed_to_stream(stream_id)
630-
631630
return [sub
632631
for sub in self.stream_dict[stream_id]['subscribers']
633632
if sub != self.user_id]
@@ -1176,19 +1175,34 @@ def _handle_update_message_flags_event(self, event: Event) -> None:
11761175
indexed_message_ids = set(self.index['messages'])
11771176
message_ids_to_mark = set(event['messages'])
11781177

1178+
# why do we only run this for messages already in the index?
11791179
for message_id in message_ids_to_mark & indexed_message_ids:
11801180
msg = self.index['messages'][message_id]
11811181
if operation == 'add':
11821182
if flag_to_change not in msg['flags']:
11831183
msg['flags'].append(flag_to_change)
1184-
if flag_to_change == 'starred':
1184+
if flag_to_change == 'starred':
1185+
# my thought is if flag_to_change is already in
1186+
# msg['flags'] then the count in view is accurate
1187+
# and we should only update it if flag_to_change isn't
1188+
# in msg['flags']
1189+
self.controller.view.starred_button.update_count(
1190+
self.controller.view.starred_button.count + 1)
1191+
if (message_id not in self.index['starred_msg_ids']
1192+
and flag_to_change == 'starred'):
11851193
self.index['starred_msg_ids'].add(message_id)
11861194
elif operation == 'remove':
11871195
if flag_to_change in msg['flags']:
11881196
msg['flags'].remove(flag_to_change)
1189-
if (message_id in self.index['starred_msg_ids']
1190-
and flag_to_change == 'starred'):
1191-
self.index['starred_msg_ids'].remove(message_id)
1197+
if flag_to_change == 'starred':
1198+
# my thought is if flag_to_change isn't in
1199+
# msg['flags'] then the count in view is accurate
1200+
# and we should only update it if flag_to_change was
1201+
# in msg['flags']
1202+
self.controller.view.starred_button.update_count(
1203+
self.controller.view.starred_button.count - 1)
1204+
if message_id in self.index['starred_msg_ids']:
1205+
self.index['starred_msg_ids'].remove(message_id)
11921206
else:
11931207
raise RuntimeError(event, msg['flags'])
11941208

@@ -1298,6 +1312,8 @@ def _register_desired_events(self, *, fetch_data: bool=False) -> str:
12981312
if response['result'] == 'success':
12991313
if fetch_data:
13001314
self.initial_data.update(response)
1315+
with open("alternative_initial_data.txt", "w") as file:
1316+
file.write(str(self.initial_data))
13011317
self.max_message_id = response['max_message_id']
13021318
self.queue_id = response['queue_id']
13031319
self.last_event_id = response['last_event_id']

zulipterminal/ui_tools/buttons.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ def __init__(
3131
prefix_character: Union[str, Tuple[Any, str]]='\N{BULLET}',
3232
text_color: Optional[str]=None,
3333
count: int=0,
34+
starred: Optional[bool]=False,
3435
) -> None:
3536
if isinstance(prefix_character, tuple):
3637
prefix = prefix_character[1]
@@ -41,6 +42,7 @@ def __init__(
4142
self.prefix_character = prefix_character
4243
self.post_prefix_spacing = ' ' if prefix else ''
4344
self.count = count
45+
self.starred = starred
4446

4547
prefix_length = 0 if prefix == '' else 2
4648
# Space either side, at least one space between
@@ -61,7 +63,11 @@ def update_count(self, count: int, text_color: Optional[str]=None) -> None:
6163
count_text = ''
6264
else:
6365
count_text = str(count)
64-
self.update_widget(('unread_count', count_text), new_color)
66+
67+
if self.starred is True:
68+
self.update_widget(('starred_count', count_text), new_color)
69+
else:
70+
self.update_widget(('unread_count', count_text), new_color)
6571

6672
def update_widget(self, count_text: Tuple[str, str],
6773
text_color: Optional[str]) -> Any:
@@ -151,7 +157,7 @@ def __init__(self, controller: Any, width: int, count: int=0) -> None:
151157

152158

153159
class StarredButton(TopButton):
154-
def __init__(self, controller: Any, width: int) -> None:
160+
def __init__(self, controller: Any, width: int, count: int=0) -> None:
155161
button_text = (
156162
f"Starred messages [{primary_key_for_command('ALL_STARRED')}]"
157163
)
@@ -162,7 +168,8 @@ def __init__(self, controller: Any, width: int) -> None:
162168
show_function=controller.narrow_to_all_starred,
163169
width=width,
164170
prefix_character='',
165-
count=0, # Starred messages are already marked read
171+
count=count, # Starred messages are already marked read
172+
starred=True,
166173
)
167174

168175

zulipterminal/ui_tools/views.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -767,9 +767,10 @@ def menu_view(self) -> Any:
767767
count=self.model.unread_counts['all_mentions'])
768768

769769
# Starred messages are by definition read already
770+
count = len(self.model.initial_data["starred_messages"])
770771
self.view.starred_button = StarredButton(self.controller,
771-
width=self.width)
772-
772+
width=self.width,
773+
count=count)
773774
menu_btn_list = [
774775
self.view.home_button,
775776
self.view.pm_button,

0 commit comments

Comments
 (0)