Skip to content

Commit 12d2537

Browse files
committed
model/views: Add Visual desktop notification checkbox in Stream Info.
This commit adds a new checkbox 'Visual desktop notification' in the Stream settings section inside StreamInfoView. It uses the subscription field of initial_data's response to set the initial state of the checkbox, and henceforth events to sync its settings between client <-> server. The notify_user code has been updated to interact properly with the checkbox settings. Tests amended. Partially Fixes #887.
1 parent a5a3e61 commit 12d2537

File tree

4 files changed

+168
-32
lines changed

4 files changed

+168
-32
lines changed

tests/model/test_model.py

Lines changed: 108 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def model(self, mocker, initial_data, user_profile,
3434
return_value=[])
3535
mocker.patch('zulipterminal.model.Model.'
3636
'_stream_info_from_subscriptions',
37-
return_value=({}, set(), [], []))
37+
return_value=({}, set(), [], [], set()))
3838
# NOTE: PATCH WHERE USED NOT WHERE DEFINED
3939
self.classify_unread_counts = mocker.patch(
4040
'zulipterminal.model.classify_unread_counts',
@@ -130,7 +130,7 @@ def test_init_InvalidAPIKey_response(self, mocker, initial_data):
130130
return_value=[])
131131
mocker.patch('zulipterminal.model.Model.'
132132
'_stream_info_from_subscriptions',
133-
return_value=({}, set(), [], []))
133+
return_value=({}, set(), [], [], set()))
134134
self.classify_unread_counts = mocker.patch(
135135
'zulipterminal.model.classify_unread_counts',
136136
return_value=[])
@@ -152,7 +152,7 @@ def test_init_ZulipError_exception(self, mocker, initial_data,
152152
return_value=[])
153153
mocker.patch('zulipterminal.model.Model.'
154154
'_stream_info_from_subscriptions',
155-
return_value=({}, set(), [], []))
155+
return_value=({}, set(), [], [], set()))
156156
self.classify_unread_counts = mocker.patch(
157157
'zulipterminal.model.classify_unread_counts',
158158
return_value=[])
@@ -557,7 +557,7 @@ def test_success_get_messages(self, mocker, messages_successful_response,
557557
return_value=[])
558558
mocker.patch('zulipterminal.model.Model.'
559559
'_stream_info_from_subscriptions',
560-
return_value=({}, set(), [], []))
560+
return_value=({}, set(), [], [], set()))
561561
self.classify_unread_counts = mocker.patch(
562562
'zulipterminal.model.classify_unread_counts',
563563
return_value=[])
@@ -597,7 +597,7 @@ def test_get_message_false_first_anchor(
597597
return_value=[])
598598
mocker.patch('zulipterminal.model.Model.'
599599
'_stream_info_from_subscriptions',
600-
return_value=({}, set(), [], []))
600+
return_value=({}, set(), [], [], set()))
601601
self.classify_unread_counts = mocker.patch(
602602
'zulipterminal.model.classify_unread_counts',
603603
return_value=[])
@@ -631,7 +631,7 @@ def test_fail_get_messages(self, mocker, error_response,
631631
return_value=[])
632632
mocker.patch('zulipterminal.model.Model.'
633633
'_stream_info_from_subscriptions',
634-
return_value=({}, set(), [], []))
634+
return_value=({}, set(), [], [], set()))
635635
self.classify_unread_counts = mocker.patch(
636636
'zulipterminal.model.classify_unread_counts',
637637
return_value=[])
@@ -666,6 +666,29 @@ def test_toggle_stream_muted_status(self, mocker, model,
666666
self.display_error_if_present.assert_called_once_with(response,
667667
self.controller)
668668

669+
@pytest.mark.parametrize('initial_desktop_notified_streams, value', [
670+
({315}, True),
671+
({205, 315}, False),
672+
(set(), True),
673+
({205}, False),
674+
], ids=['desktop_notify_enable_205', 'desktop_notify_disable_205',
675+
'first_notify_enable_205', 'last_notify_disable_205'])
676+
def test_toggle_stream_desktop_notification(
677+
self, mocker, model, initial_desktop_notified_streams,
678+
value, response={'result': 'success'}):
679+
model.desktop_notifs_enabled_streams = initial_desktop_notified_streams
680+
model.client.update_subscription_settings.return_value = response
681+
model.toggle_stream_desktop_notification(205)
682+
request = [{
683+
'stream_id': 205,
684+
'property': 'desktop_notifications',
685+
'value': value
686+
}]
687+
(model.client.update_subscription_settings
688+
.assert_called_once_with(request))
689+
self.display_error_if_present.assert_called_once_with(response,
690+
self.controller)
691+
669692
@pytest.mark.parametrize('flags_before, expected_operator', [
670693
([], 'add'),
671694
(['starred'], 'remove'),
@@ -716,7 +739,7 @@ def test__update_initial_data_raises_exception(self, mocker, initial_data):
716739
return_value=[])
717740
mocker.patch('zulipterminal.model.Model.'
718741
'_stream_info_from_subscriptions',
719-
return_value=({}, set(), [], []))
742+
return_value=({}, set(), [], [], set()))
720743
self.classify_unread_counts = mocker.patch(
721744
'zulipterminal.model.classify_unread_counts',
722745
return_value=[])
@@ -750,26 +773,32 @@ def test_get_all_users(self, mocker, initial_data, user_list, user_dict,
750773
self.client.register.return_value = initial_data
751774
mocker.patch('zulipterminal.model.Model.'
752775
'_stream_info_from_subscriptions',
753-
return_value=({}, set(), [], []))
776+
return_value=({}, set(), [], [], set()))
754777
self.classify_unread_counts = mocker.patch(
755778
'zulipterminal.model.classify_unread_counts',
756779
return_value=[])
757780
model = Model(self.controller)
758781
assert model.user_dict == user_dict
759782
assert model.users == user_list
760783

761-
@pytest.mark.parametrize('muted', powerset([1, 2, 99, 1000]))
784+
@pytest.mark.parametrize('muted, desktop_notifs_enabled', list(
785+
zip(powerset([1, 2, 99, 1000]), powerset([1, 2, 99, 1000])))
786+
)
762787
def test__stream_info_from_subscriptions(self, initial_data, streams,
763-
muted):
764-
subs = [dict(entry, in_home_view=entry['stream_id'] not in muted)
788+
muted, desktop_notifs_enabled):
789+
subs = [dict(entry, in_home_view=entry['stream_id'] not in muted,
790+
desktop_notifications=entry['stream_id'] in
791+
desktop_notifs_enabled)
765792
for entry in initial_data['subscriptions']]
766-
by_id, muted_streams, pinned, unpinned = (
793+
(by_id, muted_streams, pinned, unpinned,
794+
desktop_notifs_enabled_streams) = (
767795
Model._stream_info_from_subscriptions(subs))
768796
assert len(by_id)
769797
assert all(msg_id == msg['stream_id'] for msg_id, msg in by_id.items())
770798
assert muted_streams == muted
771799
assert pinned == [] # FIXME generalize/parametrize
772800
assert unpinned == streams # FIXME generalize/parametrize
801+
assert desktop_notifs_enabled_streams == desktop_notifs_enabled
773802

774803
def test__handle_message_event_with_Falsey_log(self, mocker,
775804
model, message_fixture):
@@ -939,50 +968,61 @@ def test__update_topic_index(self, topic_name, topic_order_initial,
939968
assert model.index['topics'][86] == topic_order_final
940969

941970
# TODO: Ideally message_fixture would use standardized ids?
942-
@pytest.mark.parametrize(['user_id', 'vary_each_msg', 'stream_setting',
943-
'types_when_notify_called'], [
971+
@pytest.mark.parametrize(['user_id', 'vary_each_msg',
972+
'desktop_notification_status',
973+
'types_when_notify_called',
974+
'is_stream_muted'], [
944975
(5140, {'flags': ['mentioned', 'wildcard_mentioned']}, True,
945-
[]), # message_fixture sender_id is 5140
976+
[], False), # message_fixture sender_id is 5140
946977
(5179, {'flags': ['mentioned']}, False,
947-
['stream', 'private']),
978+
['stream', 'private'], False),
948979
(5179, {'flags': ['wildcard_mentioned']}, False,
949-
['stream', 'private']),
980+
['stream', 'private'], False),
950981
(5179, {'flags': []}, True,
951-
['stream']),
982+
['stream'], False),
952983
(5179, {'flags': []}, False,
953-
['private']),
984+
['private'], False),
985+
(5140, {'flags': []}, True,
986+
['stream'], True),
987+
(5179, {'flags': ['mentioned']}, True,
988+
['stream'], True)
954989
], ids=[
955990
'not_notified_since_self_message',
956991
'notified_stream_and_private_since_directly_mentioned',
957992
'notified_stream_and_private_since_wildcard_mentioned',
958993
'notified_stream_since_stream_has_desktop_notifications',
959994
'notified_private_since_private_message',
995+
'not_notified_stream_since_muted_stream',
996+
'notified_muted_stream_since_directly_mentioned'
960997
])
961998
def test_notify_users_calling_msg_type(self, mocker, model,
962999
message_fixture,
9631000
user_id,
9641001
vary_each_msg,
965-
stream_setting,
1002+
desktop_notification_status,
1003+
is_stream_muted,
9661004
types_when_notify_called):
9671005
message_fixture.update(vary_each_msg)
9681006
model.user_id = user_id
969-
if 'stream_id' in message_fixture:
970-
model.stream_dict.update(
971-
{message_fixture['stream_id']:
972-
{'desktop_notifications': stream_setting}}
973-
)
1007+
mocker.patch('zulipterminal.model.Model.'
1008+
'is_desktop_notifications_enabled',
1009+
return_value=desktop_notification_status)
1010+
mocker.patch('zulipterminal.model.Model.'
1011+
'is_muted_stream', return_value=is_stream_muted)
9741012
notify = mocker.patch('zulipterminal.model.notify')
9751013

9761014
model.notify_user(message_fixture)
9771015

1016+
target = None
9781017
if message_fixture['type'] in types_when_notify_called:
9791018
who = message_fixture['type']
980-
if who == 'stream':
1019+
if who == 'stream' and not is_stream_muted:
9811020
target = 'PTEST -> Test'
9821021
elif who == 'private':
9831022
target = 'you'
9841023
if len(message_fixture['display_recipient']) > 2:
9851024
target += ', Bar Bar'
1025+
if target is not None:
9861026
title = 'Test Organization Name:\nFoo Foo (to {})'.format(target)
9871027
# TODO: Test message content too?
9881028
notify.assert_called_once_with(title, mocker.ANY)
@@ -1770,6 +1810,34 @@ def set_from_list_of_dict(data):
17701810
update_left_panel.assert_called_once_with()
17711811
model.controller.update_screen.assert_called_once_with()
17721812

1813+
@pytest.mark.parametrize('event, final_desktop_notified_streams, ', [
1814+
(
1815+
{'property': 'desktop_notifications',
1816+
'op': 'update',
1817+
'stream_id': 19,
1818+
'value': False},
1819+
{15}
1820+
),
1821+
(
1822+
{'property': 'desktop_notifications',
1823+
'op': 'update',
1824+
'stream_id': 30,
1825+
'value': True},
1826+
{15, 19, 30}
1827+
)
1828+
], ids=[
1829+
'remove_19', 'add_30'
1830+
])
1831+
def test__handle_subscription_event_desktop_notified_streams(
1832+
self, model, mocker, stream_button, event,
1833+
final_desktop_notified_streams):
1834+
model.desktop_notifs_enabled_streams = {15, 19}
1835+
model._handle_subscription_event(event)
1836+
1837+
assert (model.desktop_notifs_enabled_streams
1838+
== final_desktop_notified_streams)
1839+
model.controller.update_screen.assert_called_once_with()
1840+
17731841
@pytest.mark.parametrize(['event', 'feature_level',
17741842
'stream_id', 'expected_subscribers'], [
17751843
({'op': 'peer_add', 'stream_id': 99, 'user_id': 12}, None,
@@ -1903,6 +1971,20 @@ def test_is_muted_stream(self, muted_streams, stream_id, is_muted,
19031971
model.muted_streams = muted_streams
19041972
assert model.is_muted_stream(stream_id) == is_muted
19051973

1974+
@pytest.mark.parametrize(['desktop_notified_streams',
1975+
'stream_id', 'is_enabled'], [
1976+
({1}, 1, True),
1977+
({1, 3, 7}, 2, False),
1978+
(set(), 1, False),
1979+
], ids=['single_stream', 'multiple_streams', 'no_stream'])
1980+
def test_is_desktop_notifications_enabled(self, desktop_notified_streams,
1981+
stream_id, is_enabled,
1982+
stream_dict, model):
1983+
model.stream_dict = stream_dict
1984+
model.desktop_notifs_enabled_streams = desktop_notified_streams
1985+
assert (model.is_desktop_notifications_enabled(stream_id)
1986+
== is_enabled)
1987+
19061988
@pytest.mark.parametrize('topic, is_muted', [
19071989
((1, 'stream muted & unmuted topic'), False),
19081990
((2, 'muted topic'), True),

tests/ui_tools/test_popups.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,8 @@ def mock_external_classes(self, mocker, monkeypatch):
565565
return_value=(64, 64))
566566
self.controller.model.is_muted_stream.return_value = False
567567
self.controller.model.is_pinned_stream.return_value = False
568+
(self.controller.model.is_desktop_notifications_enabled.
569+
return_value) = False
568570
mocker.patch(VIEWS + ".urwid.SimpleFocusListWalker", return_value=[])
569571
self.stream_id = 10
570572
self.controller.model.stream_dict = {
@@ -609,7 +611,7 @@ def test_keypress_navigation(self, mocker, widget_size,
609611

610612
@pytest.mark.parametrize('key', (*keys_for_command('ENTER'), ' '))
611613
def test_checkbox_toggle_mute_stream(self, mocker, key, widget_size):
612-
mute_checkbox = self.stream_info_view.widgets[7]
614+
mute_checkbox = self.stream_info_view.widgets[-3]
613615
toggle_mute_status = self.controller.model.toggle_stream_muted_status
614616
stream_id = self.stream_info_view.stream_id
615617
size = widget_size(mute_checkbox)
@@ -620,7 +622,7 @@ def test_checkbox_toggle_mute_stream(self, mocker, key, widget_size):
620622

621623
@pytest.mark.parametrize('key', (*keys_for_command('ENTER'), ' '))
622624
def test_checkbox_toggle_pin_stream(self, mocker, key, widget_size):
623-
pin_checkbox = self.stream_info_view.widgets[8]
625+
pin_checkbox = self.stream_info_view.widgets[-2]
624626
toggle_pin_status = self.controller.model.toggle_stream_pinned_status
625627
stream_id = self.stream_info_view.stream_id
626628
size = widget_size(pin_checkbox)
@@ -629,6 +631,19 @@ def test_checkbox_toggle_pin_stream(self, mocker, key, widget_size):
629631

630632
toggle_pin_status.assert_called_once_with(stream_id)
631633

634+
@pytest.mark.parametrize('key', (*keys_for_command('ENTER'), ' '))
635+
def test_checkbox_toggle_desktop_notification(self, mocker,
636+
key, widget_size):
637+
desktop_notify_checkbox = self.stream_info_view.widgets[-1]
638+
toggle_desktop_notify_status = (
639+
self.controller.model.toggle_stream_desktop_notification)
640+
stream_id = self.stream_info_view.stream_id
641+
size = widget_size(desktop_notify_checkbox)
642+
643+
desktop_notify_checkbox.keypress(size, key)
644+
645+
toggle_desktop_notify_status.assert_called_once_with(stream_id)
646+
632647

633648
class TestStreamMembersView:
634649
@pytest.fixture(autouse=True)

zulipterminal/model.py

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ def __init__(self, controller: Any) -> None:
139139
subscriptions = self.initial_data['subscriptions']
140140
stream_data = Model._stream_info_from_subscriptions(subscriptions)
141141
(self.stream_dict, self.muted_streams,
142-
self.pinned_streams, self.unpinned_streams) = stream_data
142+
self.pinned_streams, self.unpinned_streams,
143+
self.desktop_notifs_enabled_streams) = stream_data
143144

144145
# NOTE: The expected response has been upgraded from
145146
# [stream_name, topic] to [stream_name, topic, date_muted] in
@@ -504,6 +505,9 @@ def exception_safe_result(future: 'Future[str]') -> str:
504505
def is_muted_stream(self, stream_id: int) -> bool:
505506
return stream_id in self.muted_streams
506507

508+
def is_desktop_notifications_enabled(self, stream_id: int) -> bool:
509+
return stream_id in self.desktop_notifs_enabled_streams
510+
507511
def is_muted_topic(self, stream_id: int, topic: str) -> bool:
508512
"""
509513
Returns True if topic is muted via muted_topics.
@@ -685,7 +689,8 @@ def user_name_from_id(self, user_id: int) -> str:
685689
@staticmethod
686690
def _stream_info_from_subscriptions(
687691
subscriptions: List[Dict[str, Any]]
688-
) -> Tuple[Dict[int, Any], Set[int], List[StreamData], List[StreamData]]:
692+
) -> Tuple[Dict[int, Any], Set[int], List[StreamData],
693+
List[StreamData], Set[int]]:
689694

690695
def make_reduced_stream_data(stream: Dict[str, Any]) -> StreamData:
691696
# stream_id has been changed to id.
@@ -715,6 +720,8 @@ def make_reduced_stream_data(stream: Dict[str, Any]) -> StreamData:
715720
if stream['in_home_view'] is False},
716721
pinned_streams,
717722
unpinned_streams,
723+
{stream['stream_id'] for stream in subscriptions
724+
if stream['desktop_notifications'] is True},
718725
)
719726

720727
def _group_info_from_realm_user_groups(self,
@@ -764,6 +771,15 @@ def toggle_stream_pinned_status(self, stream_id: int) -> bool:
764771
def is_user_subscribed_to_stream(self, stream_id: int) -> bool:
765772
return stream_id in self.stream_dict
766773

774+
def toggle_stream_desktop_notification(self, stream_id: int) -> None:
775+
request = [{
776+
'stream_id': stream_id,
777+
'property': 'desktop_notifications',
778+
'value': not self.is_desktop_notifications_enabled(stream_id)
779+
}]
780+
response = self.client.update_subscription_settings(request)
781+
display_error_if_present(response, self.controller)
782+
767783
def _handle_subscription_event(self, event: Event) -> None:
768784
"""
769785
Handle changes in subscription (eg. muting/unmuting,
@@ -820,6 +836,14 @@ def get_stream_by_id(streams: List[StreamData], stream_id: int
820836
sort_streams(self.pinned_streams)
821837
self.controller.view.left_panel.update_stream_view()
822838
self.controller.update_screen()
839+
elif event.get('property', None) == 'desktop_notifications':
840+
stream_id = event['stream_id']
841+
842+
if event['value']:
843+
self.desktop_notifs_enabled_streams.add(stream_id)
844+
else:
845+
self.desktop_notifs_enabled_streams.remove(stream_id)
846+
self.controller.update_screen()
823847
elif event['op'] in ('peer_add', 'peer_remove'):
824848
# NOTE: ZFL 35 commit was not atomic with API change
825849
# (ZFL >=35 can use new plural style)
@@ -897,7 +921,9 @@ def notify_user(self, message: Message) -> str:
897921
{'mentioned', 'wildcard_mentioned'}.intersection(
898922
set(message['flags'])
899923
)
900-
or self.stream_dict[message['stream_id']]['desktop_notifications']
924+
or (not self.is_muted_stream(message['stream_id'])
925+
and self.is_desktop_notifications_enabled(
926+
message['stream_id']))
901927
):
902928
recipient = '{display_recipient} -> {subject}'.format(**message)
903929

0 commit comments

Comments
 (0)