Skip to content

Commit 3baad7b

Browse files
klfoulk16klfoulk16
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: I have a few questions about test_model, but besides that I've updated the tests accordingly.
1 parent 6b22249 commit 3baad7b

File tree

9 files changed

+174
-13
lines changed

9 files changed

+174
-13
lines changed

‎alternative_initial_data.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{'full_name': 'Human Myself', 'email': '[email protected]', 'user_id': 1001, 'realm_name': 'Test Organization Name', 'unsubscribed': [{'audible_notifications': False, 'description': 'announce', 'stream_id': 7, 'is_old_stream': True, 'desktop_notifications': False, 'pin_to_top': False, 'stream_weekly_traffic': 0, 'invite_only': False, 'name': 'announce', 'push_notifications': False, 'email_address': '', 'color': '#bfd56f', 'in_home_view': True}], 'result': 'success', 'queue_id': '1522420755:786', 'realm_users': [{'user_id': 1001, 'full_name': 'Human Myself', 'email': '[email protected]', 'short_name': 'Human'}, {'user_id': 11, 'full_name': 'Human 1', 'email': '[email protected]', 'avatar_url': None, 'is_active': True, 'bot_type': None, 'is_bot': False, 'is_admin': False}, {'user_id': 12, 'full_name': 'Human 2', 'email': '[email protected]', 'avatar_url': None, 'is_active': True, 'bot_type': None, 'is_bot': False, 'is_admin': False}], 'cross_realm_bots': [{'full_name': 'Notification Bot', 'timezone': '', 'is_bot': True, 'date_joined': '2015-12-28T19:58:29.035543+00:00', 'email': '[email protected]', 'user_id': 5, 'is_admin': False, 'avatar_url': 'dummy_avatar_url'}, {'full_name': 'Email Gateway', 'timezone': '', 'is_bot': True, 'date_joined': '2015-12-28T19:58:29.037658+00:00', 'email': '[email protected]', 'user_id': 6, 'is_admin': False, 'avatar_url': 'dummy_avatar_url'}, {'full_name': 'Welcome Bot', 'timezone': '', 'is_bot': True, 'date_joined': '2015-12-28T19:58:29.033231+00:00', 'email': '[email protected]', 'user_id': 4, 'is_admin': False, 'avatar_url': 'dummy_avatar_url'}, {'full_name': 'Zulip Feedback Bot', 'timezone': '', 'is_bot': True, 'date_joined': '2015-12-28T19:58:28.972281+00:00', 'email': '[email protected]', 'user_id': 1, 'is_admin': False, 'avatar_url': 'dummy_avatar_url'}], 'subscriptions': [{'name': 'Some general stream', 'invite_only': False, 'color': '#b0a5fd', 'pin_to_top': False, 'stream_id': 1000, 'in_home_view': True, 'audible_notifications': False, 'description': 'General Stream', 'is_old_stream': True, 'desktop_notifications': False, 'stream_weekly_traffic': 0, 'push_notifications': False, 'email_address': '[email protected]', 'subscribers': [1001, 11, 12]}, {'description': 'Some private stream', 'stream_id': 99, 'pin_to_top': False, 'invite_only': True, 'name': 'Secret stream', 'email_address': '[email protected]', 'color': '#ccc', 'in_home_view': True, 'audible_notifications': False, 'is_old_stream': True, 'desktop_notifications': False, 'stream_weekly_traffic': 0, 'push_notifications': False, 'subscribers': [1001, 11]}, {'name': 'Stream 1', 'invite_only': False, 'color': '#b0a5fd', 'pin_to_top': False, 'stream_id': 1, 'in_home_view': True, 'audible_notifications': False, 'description': 'A description of stream 1', 'is_old_stream': True, 'desktop_notifications': False, 'stream_weekly_traffic': 0, 'push_notifications': False, 'email_address': '[email protected]', 'subscribers': [1001, 11, 12]}, {'name': 'Stream 2', 'invite_only': False, 'color': '#b0a5fd', 'pin_to_top': False, 'stream_id': 2, 'in_home_view': True, 'audible_notifications': False, 'description': 'A description of stream 2', 'is_old_stream': True, 'desktop_notifications': False, 'stream_weekly_traffic': 0, 'push_notifications': False, 'email_address': '[email protected]', 'subscribers': [1001, 11, 12]}], 'msg': '', 'max_message_id': 552761, 'never_subscribed': [{'invite_only': False, 'description': 'Announcements from the Zulip GCI Mentors', 'stream_id': 87, 'name': 'GCI announce', 'is_old_stream': True, 'stream_weekly_traffic': 0}, {'invite_only': False, 'description': 'General discussion', 'stream_id': 74, 'name': 'GCI general', 'is_old_stream': True, 'stream_weekly_traffic': 0}], 'unread_msgs': {'pms': [{'sender_id': 1, 'unread_message_ids': [1, 2]}, {'sender_id': 2, 'unread_message_ids': [3]}], 'count': 0, 'mentions': [], 'streams': [{'stream_id': 1000, 'topic': 'Some general unread topic', 'unread_message_ids': [4, 5, 6], 'sender_ids': [1, 2]}, {'stream_id': 99, 'topic': 'Some private unread topic', 'unread_message_ids': [7], 'sender_ids': [1, 2]}], 'huddles': [{'user_ids_string': '1001,11,12', 'unread_message_ids': [11, 12, 13]}, {'user_ids_string': '1001,11,12,13', 'unread_message_ids': [101, 102]}]}, 'presences': {'[email protected]': {'ZulipElectron': {'pushable': False, 'client': 'ZulipElectron', 'status': 'idle', 'timestamp': 1522484059}, 'ZulipMobile': {'pushable': False, 'client': 'ZulipMobile', 'status': 'idle', 'timestamp': 1522384165}, 'aggregated': {'timestamp': 1522484059, 'client': 'ZulipElectron', 'status': 'idle'}}, '[email protected]': {'website': {'pushable': True, 'client': 'website', 'status': 'active', 'timestamp': 1522458138}, 'ZulipMobile': {'pushable': True, 'client': 'ZulipMobile', 'status': 'active', 'timestamp': 1522480103}, 'aggregated': {'timestamp': 1522480103, 'client': 'ZulipMobile', 'status': 'active'}}}, 'twenty_four_hour_time': True, 'last_event_id': -1, 'muted_topics': [], 'realm_user_groups': [], 'zulip_version': '2.1', 'zulip_feature_level': None, 'starred_messages': [1117554, 1117558, 1117574]}

‎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'),

0 commit comments

Comments
 (0)