settings: Add display setting for demoting inactive streams.

This adds a setting to control Zulip's default behavior of sorting to
bottom and graying out inactive streams.  The previous logic is still
the default "automatic", but this gives users more control.  See the
models.py comment for details.

Fixes #11524.
This commit is contained in:
sahil839 2019-03-17 06:48:51 -07:00 committed by Tim Abbott
parent 710fc6767f
commit 5a130097bf
17 changed files with 191 additions and 20 deletions

View File

@ -607,6 +607,12 @@ var event_fixtures = {
setting: true,
},
update_display_settings__demote_inactive_streams: {
type: 'update_display_settings',
setting_name: 'demote_inactive_streams',
setting: 2,
},
update_display_settings__translate_emoticons: {
type: 'update_display_settings',
setting_name: 'translate_emoticons',
@ -1389,6 +1395,14 @@ with_overrides(function (override) {
page_params.fluid_layout_width = false;
dispatch(event);
assert_same(page_params.fluid_layout_width, true);
global.with_stub(function (stub) {
event = event_fixtures.update_display_settings__demote_inactive_streams;
override('stream_list.update_streams_sidebar', stub.f);
page_params.demote_inactive_streams = 1;
dispatch(event);
assert_same(page_params.demote_inactive_streams, 2);
});
});
with_overrides(function (override) {

View File

@ -55,6 +55,8 @@ const denmark_stream = {
// to make our tests more self-containted.
zrequire('stream_data');
set_global('i18n', global.stub_i18n);
zrequire('settings_display');
run_test('stream_data', () => {
assert.equal(stream_data.get_sub_by_name('Denmark'), undefined);

View File

@ -9,6 +9,7 @@ set_global('$', function () {
set_global('blueslip', global.make_zblueslip());
set_global('document', null);
set_global('i18n', global.stub_i18n);
global.stub_out_jquery();
zrequire('color_data');
@ -24,6 +25,7 @@ zrequire('Filter', 'js/filter');
zrequire('MessageListData', 'js/message_list_data');
zrequire('MessageListView', 'js/message_list_view');
zrequire('message_list');
zrequire('settings_display', 'js/settings_display');
run_test('basics', () => {
var denmark = {
@ -291,15 +293,45 @@ run_test('is_active', () => {
var sub;
page_params.demote_inactive_streams =
settings_display.demote_inactive_streams_values.automatic.code;
sub = {name: 'pets', subscribed: false, stream_id: 111};
stream_data.add_sub('pets', sub);
assert(stream_data.is_active(sub));
stream_data.set_filter_out_inactives(true);
stream_data.subscribe_myself(sub);
assert(stream_data.is_active(sub));
stream_data.unsubscribe_myself(sub);
assert(stream_data.is_active(sub));
sub.pin_to_top = true;
assert(stream_data.is_active(sub));
sub.pin_to_top = false;
var opts = {
stream_id: 222,
message_id: 108,
topic_name: 'topic2',
};
topic_data.add_message(opts);
assert(stream_data.is_active(sub));
page_params.demote_inactive_streams =
settings_display.demote_inactive_streams_values.always.code;
sub = {name: 'pets', subscribed: false, stream_id: 111};
stream_data.add_sub('pets', sub);
assert(!stream_data.is_active(sub));
sub.pin_to_top = true;
assert(stream_data.is_active(sub));
sub.pin_to_top = false;
stream_data.subscribe_myself(sub);
assert(stream_data.is_active(sub));
@ -309,13 +341,29 @@ run_test('is_active', () => {
sub = {name: 'lunch', subscribed: false, stream_id: 222};
stream_data.add_sub('lunch', sub);
assert(!stream_data.is_active(sub));
assert(stream_data.is_active(sub));
topic_data.add_message(opts);
assert(stream_data.is_active(sub));
page_params.demote_inactive_streams =
settings_display.demote_inactive_streams_values.never.code;
sub = {name: 'pets', subscribed: false, stream_id: 111};
stream_data.add_sub('pets', sub);
assert(stream_data.is_active(sub));
stream_data.subscribe_myself(sub);
assert(stream_data.is_active(sub));
stream_data.unsubscribe_myself(sub);
assert(stream_data.is_active(sub));
sub.pin_to_top = true;
assert(stream_data.is_active(sub));
var opts = {
stream_id: 222,
message_id: 108,
topic_name: 'topic2',
};
topic_data.add_message(opts);
assert(stream_data.is_active(sub));
@ -695,6 +743,7 @@ run_test('initialize', () => {
}
initialize();
page_params.demote_inactive_streams = 1;
page_params.realm_notifications_stream_id = -1;
stream_data.initialize();
assert(!stream_data.is_filtering_inactives());

View File

@ -18,6 +18,8 @@ zrequire('list_cursor');
zrequire('stream_list');
zrequire('topic_zoom');
zrequire('ui');
set_global('i18n', global.stub_i18n);
zrequire('settings_display');
stream_color.initialize();
@ -33,10 +35,15 @@ set_global('keydown_util', {
handle: noop,
});
set_global('page_params', {
is_admin: false,
realm_users: [],
});
run_test('create_sidebar_row', () => {
// Make a couple calls to create_sidebar_row() and make sure they
// generate the right markup as well as play nice with get_stream_li().
page_params.demote_inactive_streams = 1;
var devel = {
name: 'devel',
stream_id: 100,

View File

@ -121,6 +121,7 @@ zrequire('starred_messages');
zrequire('user_status');
zrequire('user_status_ui');
zrequire('ui_init');
zrequire('settings_display');
set_global('$', global.make_zjquery());

View File

@ -376,6 +376,7 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) {
case 'update_display_settings':
var user_display_settings = [
'default_language',
'demote_inactive_streams',
'dense_mode',
'emojiset',
'high_contrast_mode',
@ -404,6 +405,9 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) {
if (event.setting_name === 'high_contrast_mode') {
$("body").toggleClass("high-contrast");
}
if (event.setting_name === 'demote_inactive_streams') {
stream_list.update_streams_sidebar();
}
if (event.setting_name === 'dense_mode') {
$("body").toggleClass("less_dense_mode");
$("body").toggleClass("more_dense_mode");

View File

@ -130,6 +130,7 @@ exports.build_page = function () {
timezones: moment.tz.names(),
can_create_new_bots: settings_bots.can_create_new_bots(),
settings_label: settings.settings_label,
demote_inactive_streams_values: settings_display.demote_inactive_streams_values,
});
$(".settings-box").html(rendered_settings_tab);

View File

@ -28,12 +28,29 @@ exports.set_night_mode = function (bool) {
change_display_setting(data, '#display-settings-status');
};
exports.demote_inactive_streams_values = {
automatic: {
code: 1,
description: i18n.t("Automatic"),
},
always: {
code: 2,
description: i18n.t("Always"),
},
never: {
code: 3,
description: i18n.t("Never"),
},
};
exports.set_up = function () {
meta.loaded = true;
$("#display-settings-status").hide();
$("#user_timezone").val(page_params.timezone);
$("#demote_inactive_streams").val(page_params.demote_inactive_streams);
$(".emojiset_choice[value=" + page_params.emojiset + "]").prop("checked", true);
$("#default_language_modal [data-dismiss]").click(function () {
@ -83,6 +100,11 @@ exports.set_up = function () {
change_display_setting(data, '#display-settings-status');
});
$('#demote_inactive_streams').change(function () {
var data = {demote_inactive_streams: this.value};
change_display_setting(data, '#display-settings-status');
});
$("#night_mode").change(function () {
exports.set_night_mode(this.checked);
});

View File

@ -20,8 +20,16 @@ exports.clear_subscriptions = function () {
exports.clear_subscriptions();
exports.set_filter_out_inactives = function (want_filter) {
filter_out_inactives = want_filter || false;
exports.set_filter_out_inactives = function () {
if (page_params.demote_inactive_streams ===
settings_display.demote_inactive_streams_values.automatic.code) {
filter_out_inactives = exports.subscribed_subs().length >= 30;
} else if (page_params.demote_inactive_streams ===
settings_display.demote_inactive_streams_values.always.code) {
filter_out_inactives = true;
} else {
filter_out_inactives = false;
}
};
// for testing:
@ -30,7 +38,8 @@ exports.is_filtering_inactives = function () {
};
exports.is_active = function (sub) {
if (!filter_out_inactives) {
exports.set_filter_out_inactives();
if (!filter_out_inactives || sub.pin_to_top) {
// If users don't want to filter inactive streams
// to the bottom, we respect that setting and don't
// treat any streams as dormant.
@ -730,8 +739,7 @@ exports.initialize = function () {
page_params.notifications_stream = "";
}
var has_many_streams = exports.subscribed_subs().length >= 30;
exports.set_filter_out_inactives(has_many_streams);
exports.set_filter_out_inactives();
// Garbage collect data structures that were only used for initialization.
delete page_params.subscriptions;

View File

@ -56,6 +56,15 @@
"setting_name" "fluid_layout_width"
"is_checked" page_params.fluid_layout_width
"label" settings_label.fluid_layout_width}}
<div class="input-group">
<label for="demote_inactive_streams" class="dropdown-title">{{t "Demote inactive streams" }}:</label>
<select name="demote_inactive_streams" id="demote_inactive_streams">
{{#each demote_inactive_streams_values}}
<option value='{{ this.code }}'>{{ this.description }}</option>
{{/each}}
</select>
</div>
</div>
<div id="user-time-settings">

View File

@ -3680,7 +3680,7 @@ def do_change_enter_sends(user_profile: UserProfile, enter_sends: bool) -> None:
def do_set_user_display_setting(user_profile: UserProfile,
setting_name: str,
setting_value: Union[bool, str]) -> None:
setting_value: Union[bool, str, int]) -> None:
property_type = UserProfile.property_types[setting_name]
assert isinstance(setting_value, property_type)
setattr(user_profile, setting_name, setting_value)

View File

@ -0,0 +1,20 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.20 on 2019-03-08 19:50
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('zerver', '0227_inline_url_embed_preview_default_off'),
]
operations = [
migrations.AddField(
model_name='userprofile',
name='demote_inactive_streams',
field=models.PositiveSmallIntegerField(default=1),
),
]

View File

@ -875,6 +875,20 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
starred_message_counts = models.BooleanField(default=False) # type: bool
fluid_layout_width = models.BooleanField(default=False) # type: bool
# UI setting controlling Zulip's behavior of demoting in the sort
# order and graying out streams with no recent traffic. The
# default behavior, automatic, enables this behavior once a user
# is subscribed to 30+ streams in the webapp.
DEMOTE_STREAMS_AUTOMATIC = 1
DEMOTE_STREAMS_ALWAYS = 2
DEMOTE_STREAMS_NEVER = 3
DEMOTE_STREAMS_CHOICES = [
DEMOTE_STREAMS_AUTOMATIC,
DEMOTE_STREAMS_ALWAYS,
DEMOTE_STREAMS_NEVER
]
demote_inactive_streams = models.PositiveSmallIntegerField(default=DEMOTE_STREAMS_AUTOMATIC)
# A timezone name from the `tzdata` database, as found in pytz.all_timezones.
#
# The longest existing name is 32 characters long, so max_length=40 seems
@ -924,6 +938,7 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
# Define the types of the various automatically managed properties
property_types = dict(
default_language=str,
demote_inactive_streams=int,
dense_mode=bool,
emojiset=str,
left_side_userlist=bool,

View File

@ -1799,7 +1799,8 @@ class EventsRegisterTest(ZulipTestCase):
test_changes = dict(
emojiset = [u'apple', u'twitter'],
default_language = [u'es', u'de', u'en'],
timezone = [u'US/Mountain', u'US/Samoa', u'Pacific/Galapogos', u'']
timezone = [u'US/Mountain', u'US/Samoa', u'Pacific/Galapogos', u''],
demote_inactive_streams = [2, 3, 1],
) # type: Dict[str, Any]
property_type = UserProfile.property_types[setting_name]
@ -1807,6 +1808,8 @@ class EventsRegisterTest(ZulipTestCase):
validator = check_bool
elif property_type is str:
validator = check_string
elif property_type is int:
validator = check_int
else:
raise AssertionError("Unexpected property type %s" % (property_type,))

View File

@ -60,6 +60,7 @@ class HomeTest(ZulipTestCase):
"default_language",
"default_language_name",
"delivery_email",
"demote_inactive_streams",
"dense_mode",
"development_environment",
"email",

View File

@ -4,7 +4,7 @@ import ujson
from django.http import HttpResponse
from django.test import override_settings
from mock import patch
from typing import Any, Dict
from typing import Any, Dict, Union
from zerver.lib.initial_password import initial_password
from zerver.lib.test_classes import ZulipTestCase
@ -259,6 +259,7 @@ class ChangeSettingsTest(ZulipTestCase):
default_language = 'de',
emojiset = 'google',
timezone = 'US/Mountain',
demote_inactive_streams = 2,
) # type: Dict[str, Any]
email = self.example_email('hamlet')
@ -267,9 +268,13 @@ class ChangeSettingsTest(ZulipTestCase):
# Error if a setting in UserProfile.property_types does not have test values
if test_value is None:
raise AssertionError('No test created for %s' % (setting_name,))
invalid_value = 'invalid_' + setting_name
if setting_name == 'demote_inactive_streams':
invalid_value = 4 # type: Union[int, str]
else:
invalid_value = 'invalid_' + setting_name
data = {setting_name: ujson.dumps(test_value)}
result = self.client_patch("/json/settings/display", data)
self.assert_json_success(result)
user_profile = self.example_user('hamlet')
@ -278,11 +283,16 @@ class ChangeSettingsTest(ZulipTestCase):
# Test to make sure invalid settings are not accepted
# and saved in the db.
data = {setting_name: ujson.dumps(invalid_value)}
result = self.client_patch("/json/settings/display", data)
# the json error for multiple word setting names (ex: default_language)
# displays as 'Invalid language'. Using setting_name.split('_') to format.
if setting_name == 'demote_inactive_streams':
self.assert_json_error(result, "Invalid setting value '%s'" % (invalid_value,))
else:
self.assert_json_error(result, "Invalid %s '%s'" % (setting_name.split('_')[-1],
invalid_value))
user_profile = self.example_user('hamlet')
self.assertNotEqual(getattr(user_profile, setting_name), invalid_value)

View File

@ -18,7 +18,7 @@ from zerver.lib.send_email import send_email, FromAddress
from zerver.lib.i18n import get_available_language_codes
from zerver.lib.response import json_success, json_error
from zerver.lib.upload import upload_avatar_image
from zerver.lib.validator import check_bool, check_string
from zerver.lib.validator import check_bool, check_string, check_int
from zerver.lib.request import JsonableError
from zerver.lib.timezone import get_all_timezones
from zerver.models import UserProfile, name_changes_disabled, avatar_changes_disabled
@ -126,6 +126,7 @@ def update_display_settings_backend(
default_language: Optional[bool]=REQ(validator=check_string, default=None),
left_side_userlist: Optional[bool]=REQ(validator=check_bool, default=None),
emojiset: Optional[str]=REQ(validator=check_string, default=None),
demote_inactive_streams: Optional[int]=REQ(validator=check_int, default=None),
timezone: Optional[str]=REQ(validator=check_string, default=None)) -> HttpResponse:
if (default_language is not None and
@ -140,6 +141,10 @@ def update_display_settings_backend(
emojiset not in UserProfile.emojiset_choices()):
raise JsonableError(_("Invalid emojiset '%s'") % (emojiset,))
if (demote_inactive_streams is not None and
demote_inactive_streams not in UserProfile.DEMOTE_STREAMS_CHOICES):
raise JsonableError(_("Invalid setting value '%s'") % (demote_inactive_streams,))
request_settings = {k: v for k, v in list(locals().items()) if k in user_profile.property_types}
result = {} # type: Dict[str, Any]
for k, v in list(request_settings.items()):