Skip to content

Commit

Permalink
model/views: Add Visual desktop notification checkbox in Stream Info.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@zee-bit
zee-bit committed Jan 31, 2021
1 parent a5a3e61 commit 3691fc4
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 25 deletions.
108 changes: 89 additions & 19 deletions tests/model/test_model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def model(self, mocker, initial_data, user_profile,
return_value=[])
mocker.patch('zulipterminal.model.Model.'
'_stream_info_from_subscriptions',
return_value=({}, set(), [], []))
return_value=({}, set(), [], [], set()))
# NOTE: PATCH WHERE USED NOT WHERE DEFINED
self.classify_unread_counts = mocker.patch(
'zulipterminal.model.classify_unread_counts',
Expand Down Expand Up @@ -130,7 +130,7 @@ def test_init_InvalidAPIKey_response(self, mocker, initial_data):
return_value=[])
mocker.patch('zulipterminal.model.Model.'
'_stream_info_from_subscriptions',
return_value=({}, set(), [], []))
return_value=({}, set(), [], [], set()))
self.classify_unread_counts = mocker.patch(
'zulipterminal.model.classify_unread_counts',
return_value=[])
Expand All @@ -152,7 +152,7 @@ def test_init_ZulipError_exception(self, mocker, initial_data,
return_value=[])
mocker.patch('zulipterminal.model.Model.'
'_stream_info_from_subscriptions',
return_value=({}, set(), [], []))
return_value=({}, set(), [], [], set()))
self.classify_unread_counts = mocker.patch(
'zulipterminal.model.classify_unread_counts',
return_value=[])
Expand Down Expand Up @@ -557,7 +557,7 @@ def test_success_get_messages(self, mocker, messages_successful_response,
return_value=[])
mocker.patch('zulipterminal.model.Model.'
'_stream_info_from_subscriptions',
return_value=({}, set(), [], []))
return_value=({}, set(), [], [], set()))
self.classify_unread_counts = mocker.patch(
'zulipterminal.model.classify_unread_counts',
return_value=[])
Expand Down Expand Up @@ -597,7 +597,7 @@ def test_get_message_false_first_anchor(
return_value=[])
mocker.patch('zulipterminal.model.Model.'
'_stream_info_from_subscriptions',
return_value=({}, set(), [], []))
return_value=({}, set(), [], [], set()))
self.classify_unread_counts = mocker.patch(
'zulipterminal.model.classify_unread_counts',
return_value=[])
Expand Down Expand Up @@ -631,7 +631,7 @@ def test_fail_get_messages(self, mocker, error_response,
return_value=[])
mocker.patch('zulipterminal.model.Model.'
'_stream_info_from_subscriptions',
return_value=({}, set(), [], []))
return_value=({}, set(), [], [], set()))
self.classify_unread_counts = mocker.patch(
'zulipterminal.model.classify_unread_counts',
return_value=[])
Expand Down Expand Up @@ -666,6 +666,29 @@ def test_toggle_stream_muted_status(self, mocker, model,
self.display_error_if_present.assert_called_once_with(response,
self.controller)

@pytest.mark.parametrize('initial_desktop_notified_streams, value', [
({315}, True),
({205, 315}, False),
(set(), True),
({205}, False),
], ids=['desktop_notify_enable_205', 'desktop_notify_disable_205',
'first_notify_enable_205', 'last_notify_disable_205'])
def test_toggle_stream_desktop_notification(
self, mocker, model, initial_desktop_notified_streams,
value, response={'result': 'success'}):
model.desktop_notifs_enabled_streams = initial_desktop_notified_streams
model.client.update_subscription_settings.return_value = response
model.toggle_stream_desktop_notification(205)
request = [{
'stream_id': 205,
'property': 'desktop_notifications',
'value': value
}]
(model.client.update_subscription_settings
.assert_called_once_with(request))
self.display_error_if_present.assert_called_once_with(response,
self.controller)

@pytest.mark.parametrize('flags_before, expected_operator', [
([], 'add'),
(['starred'], 'remove'),
Expand Down Expand Up @@ -716,7 +739,7 @@ def test__update_initial_data_raises_exception(self, mocker, initial_data):
return_value=[])
mocker.patch('zulipterminal.model.Model.'
'_stream_info_from_subscriptions',
return_value=({}, set(), [], []))
return_value=({}, set(), [], [], set()))
self.classify_unread_counts = mocker.patch(
'zulipterminal.model.classify_unread_counts',
return_value=[])
Expand Down Expand Up @@ -750,26 +773,32 @@ def test_get_all_users(self, mocker, initial_data, user_list, user_dict,
self.client.register.return_value = initial_data
mocker.patch('zulipterminal.model.Model.'
'_stream_info_from_subscriptions',
return_value=({}, set(), [], []))
return_value=({}, set(), [], [], set()))
self.classify_unread_counts = mocker.patch(
'zulipterminal.model.classify_unread_counts',
return_value=[])
model = Model(self.controller)
assert model.user_dict == user_dict
assert model.users == user_list

@pytest.mark.parametrize('muted', powerset([1, 2, 99, 1000]))
@pytest.mark.parametrize('muted, desktop_notifs_enabled', list(
zip(powerset([1, 2, 99, 1000]), powerset([1, 2, 99, 1000])))
)
def test__stream_info_from_subscriptions(self, initial_data, streams,
muted):
subs = [dict(entry, in_home_view=entry['stream_id'] not in muted)
muted, desktop_notifs_enabled):
subs = [dict(entry, in_home_view=entry['stream_id'] not in muted,
desktop_notifications=entry['stream_id'] in
desktop_notifs_enabled)
for entry in initial_data['subscriptions']]
by_id, muted_streams, pinned, unpinned = (
(by_id, muted_streams, pinned, unpinned,
desktop_notifs_enabled_streams) = (
Model._stream_info_from_subscriptions(subs))
assert len(by_id)
assert all(msg_id == msg['stream_id'] for msg_id, msg in by_id.items())
assert muted_streams == muted
assert pinned == [] # FIXME generalize/parametrize
assert unpinned == streams # FIXME generalize/parametrize
assert desktop_notifs_enabled_streams == desktop_notifs_enabled

def test__handle_message_event_with_Falsey_log(self, mocker,
model, message_fixture):
Expand Down Expand Up @@ -939,7 +968,8 @@ def test__update_topic_index(self, topic_name, topic_order_initial,
assert model.index['topics'][86] == topic_order_final

# TODO: Ideally message_fixture would use standardized ids?
@pytest.mark.parametrize(['user_id', 'vary_each_msg', 'stream_setting',
@pytest.mark.parametrize(['user_id', 'vary_each_msg',
'desktop_notification_status',
'types_when_notify_called'], [
(5140, {'flags': ['mentioned', 'wildcard_mentioned']}, True,
[]), # message_fixture sender_id is 5140
Expand All @@ -962,15 +992,13 @@ def test_notify_users_calling_msg_type(self, mocker, model,
message_fixture,
user_id,
vary_each_msg,
stream_setting,
desktop_notification_status,
types_when_notify_called):
message_fixture.update(vary_each_msg)
model.user_id = user_id
if 'stream_id' in message_fixture:
model.stream_dict.update(
{message_fixture['stream_id']:
{'desktop_notifications': stream_setting}}
)
mocker.patch('zulipterminal.model.Model.'
'is_desktop_notifications_enabled',
return_value=desktop_notification_status)
notify = mocker.patch('zulipterminal.model.notify')

model.notify_user(message_fixture)
Expand Down Expand Up @@ -1770,6 +1798,34 @@ def set_from_list_of_dict(data):
update_left_panel.assert_called_once_with()
model.controller.update_screen.assert_called_once_with()

@pytest.mark.parametrize('event, final_desktop_notified_streams, ', [
(
{'property': 'desktop_notifications',
'op': 'update',
'stream_id': 19,
'value': False},
{15}
),
(
{'property': 'desktop_notifications',
'op': 'update',
'stream_id': 30,
'value': True},
{15, 19, 30}
)
], ids=[
'remove_19', 'add_30'
])
def test__handle_subscription_event_desktop_notified_streams(
self, model, mocker, stream_button, event,
final_desktop_notified_streams):
model.desktop_notifs_enabled_streams = {15, 19}
model._handle_subscription_event(event)

assert (model.desktop_notifs_enabled_streams
== final_desktop_notified_streams)
model.controller.update_screen.assert_called_once_with()

@pytest.mark.parametrize(['event', 'feature_level',
'stream_id', 'expected_subscribers'], [
({'op': 'peer_add', 'stream_id': 99, 'user_id': 12}, None,
Expand Down Expand Up @@ -1903,6 +1959,20 @@ def test_is_muted_stream(self, muted_streams, stream_id, is_muted,
model.muted_streams = muted_streams
assert model.is_muted_stream(stream_id) == is_muted

@pytest.mark.parametrize(['desktop_notified_streams',
'stream_id', 'is_enabled'], [
({1}, 1, True),
({1, 3, 7}, 2, False),
(set(), 1, False),
], ids=['single_stream', 'multiple_streams', 'no_stream'])
def test_is_desktop_notifications_enabled(self, desktop_notified_streams,
stream_id, is_enabled,
stream_dict, model):
model.stream_dict = stream_dict
model.desktop_notifs_enabled_streams = desktop_notified_streams
assert (model.is_desktop_notifications_enabled(stream_id)
== is_enabled)

@pytest.mark.parametrize('topic, is_muted', [
((1, 'stream muted & unmuted topic'), False),
((2, 'muted topic'), True),
Expand Down
19 changes: 17 additions & 2 deletions tests/ui_tools/test_popups.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,8 @@ def mock_external_classes(self, mocker, monkeypatch):
return_value=(64, 64))
self.controller.model.is_muted_stream.return_value = False
self.controller.model.is_pinned_stream.return_value = False
(self.controller.model.is_desktop_notifications_enabled.
return_value) = False
mocker.patch(VIEWS + ".urwid.SimpleFocusListWalker", return_value=[])
self.stream_id = 10
self.controller.model.stream_dict = {
Expand Down Expand Up @@ -609,7 +611,7 @@ def test_keypress_navigation(self, mocker, widget_size,

@pytest.mark.parametrize('key', (*keys_for_command('ENTER'), ' '))
def test_checkbox_toggle_mute_stream(self, mocker, key, widget_size):
mute_checkbox = self.stream_info_view.widgets[7]
mute_checkbox = self.stream_info_view.widgets[-3]
toggle_mute_status = self.controller.model.toggle_stream_muted_status
stream_id = self.stream_info_view.stream_id
size = widget_size(mute_checkbox)
Expand All @@ -620,7 +622,7 @@ def test_checkbox_toggle_mute_stream(self, mocker, key, widget_size):

@pytest.mark.parametrize('key', (*keys_for_command('ENTER'), ' '))
def test_checkbox_toggle_pin_stream(self, mocker, key, widget_size):
pin_checkbox = self.stream_info_view.widgets[8]
pin_checkbox = self.stream_info_view.widgets[-2]
toggle_pin_status = self.controller.model.toggle_stream_pinned_status
stream_id = self.stream_info_view.stream_id
size = widget_size(pin_checkbox)
Expand All @@ -629,6 +631,19 @@ def test_checkbox_toggle_pin_stream(self, mocker, key, widget_size):

toggle_pin_status.assert_called_once_with(stream_id)

@pytest.mark.parametrize('key', (*keys_for_command('ENTER'), ' '))
def test_checkbox_toggle_desktop_notification(self, mocker,
key, widget_size):
desktop_notify_checkbox = self.stream_info_view.widgets[-1]
toggle_desktop_notify_status = (
self.controller.model.toggle_stream_desktop_notification)
stream_id = self.stream_info_view.stream_id
size = widget_size(desktop_notify_checkbox)

desktop_notify_checkbox.keypress(size, key)

toggle_desktop_notify_status.assert_called_once_with(stream_id)


class TestStreamMembersView:
@pytest.fixture(autouse=True)
Expand Down
30 changes: 27 additions & 3 deletions zulipterminal/model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ def __init__(self, controller: Any) -> None:
subscriptions = self.initial_data['subscriptions']
stream_data = Model._stream_info_from_subscriptions(subscriptions)
(self.stream_dict, self.muted_streams,
self.pinned_streams, self.unpinned_streams) = stream_data
self.pinned_streams, self.unpinned_streams,
self.desktop_notifs_enabled_streams) = stream_data

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

def is_desktop_notifications_enabled(self, stream_id: int) -> bool:
return stream_id in self.desktop_notifs_enabled_streams

def is_muted_topic(self, stream_id: int, topic: str) -> bool:
"""
Returns True if topic is muted via muted_topics.
Expand Down Expand Up @@ -685,7 +689,8 @@ def user_name_from_id(self, user_id: int) -> str:
@staticmethod
def _stream_info_from_subscriptions(
subscriptions: List[Dict[str, Any]]
) -> Tuple[Dict[int, Any], Set[int], List[StreamData], List[StreamData]]:
) -> Tuple[Dict[int, Any], Set[int], List[StreamData],
List[StreamData], Set[int]]:

def make_reduced_stream_data(stream: Dict[str, Any]) -> StreamData:
# stream_id has been changed to id.
Expand Down Expand Up @@ -715,6 +720,8 @@ def make_reduced_stream_data(stream: Dict[str, Any]) -> StreamData:
if stream['in_home_view'] is False},
pinned_streams,
unpinned_streams,
{stream['stream_id'] for stream in subscriptions
if stream['desktop_notifications'] is True},
)

def _group_info_from_realm_user_groups(self,
Expand Down Expand Up @@ -764,6 +771,15 @@ def toggle_stream_pinned_status(self, stream_id: int) -> bool:
def is_user_subscribed_to_stream(self, stream_id: int) -> bool:
return stream_id in self.stream_dict

def toggle_stream_desktop_notification(self, stream_id: int) -> None:
request = [{
'stream_id': stream_id,
'property': 'desktop_notifications',
'value': not self.is_desktop_notifications_enabled(stream_id)
}]
response = self.client.update_subscription_settings(request)
display_error_if_present(response, self.controller)

def _handle_subscription_event(self, event: Event) -> None:
"""
Handle changes in subscription (eg. muting/unmuting,
Expand Down Expand Up @@ -820,6 +836,14 @@ def get_stream_by_id(streams: List[StreamData], stream_id: int
sort_streams(self.pinned_streams)
self.controller.view.left_panel.update_stream_view()
self.controller.update_screen()
elif event.get('property', None) == 'desktop_notifications':
stream_id = event['stream_id']

if event['value']:
self.desktop_notifs_enabled_streams.add(stream_id)
else:
self.desktop_notifs_enabled_streams.remove(stream_id)
self.controller.update_screen()
elif event['op'] in ('peer_add', 'peer_remove'):
# NOTE: ZFL 35 commit was not atomic with API change
# (ZFL >=35 can use new plural style)
Expand Down Expand Up @@ -897,7 +921,7 @@ def notify_user(self, message: Message) -> str:
{'mentioned', 'wildcard_mentioned'}.intersection(
set(message['flags'])
)
or self.stream_dict[message['stream_id']]['desktop_notifications']
or self.is_desktop_notifications_enabled(message['stream_id'])
):
recipient = '{display_recipient} -> {subject}'.format(**message)

Expand Down
15 changes: 14 additions & 1 deletion zulipterminal/ui_tools/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1133,15 +1133,23 @@ def __init__(self, controller: Any, stream_id: int) -> None:
checked_symbol=CHECK_MARK)
urwid.connect_signal(pinned_setting, 'change',
self.toggle_pinned_status)
desktop_notification = urwid.CheckBox(
label="Visual desktop notification",
state=controller.model.is_desktop_notifications_enabled(stream_id),
checked_symbol=CHECK_MARK)
urwid.connect_signal(desktop_notification, 'change',
self.toggle_desktop_notification)

# Manual because calculate_table_widths does not support checkboxes.
# Add 4 to checkbox label to accommodate the checkbox itself.
popup_width = max(popup_width, len(muted_setting.label) + 4,
len(pinned_setting.label) + 4)
len(pinned_setting.label) + 4,
len(desktop_notification.label) + 4)
self.widgets = self.make_table_with_categories(stream_info_content,
column_widths)
self.widgets.append(muted_setting)
self.widgets.append(pinned_setting)
self.widgets.append(desktop_notification)
super().__init__(controller, self.widgets, 'STREAM_DESC', popup_width,
title)

Expand All @@ -1151,6 +1159,11 @@ def toggle_mute_status(self, button: Any, new_state: bool) -> None:
def toggle_pinned_status(self, button: Any, new_state: bool) -> None:
self.controller.model.toggle_stream_pinned_status(self.stream_id)

def toggle_desktop_notification(self, button: Any,
new_state: bool) -> None:
self.controller.model.toggle_stream_desktop_notification(
self.stream_id)

def keypress(self, size: urwid_Size, key: str) -> str:
if is_command_key('STREAM_MEMBERS', key):
self.controller.show_stream_members(stream_id=self.stream_id)
Expand Down

0 comments on commit 3691fc4

Please sign in to comment.