Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

org settings: Enable message_retention_days in org settings UI. 

Since production testing of `message_retention_days` is finished, we can
enable this feature in the organization settings page. We already had this
setting in frontend but it was bit rotten and not rendered in templates.

Here we replaced our past text-input based setting with a
dropdown-with-text-input setting approach which is more consistent with our
existing UI.

Along with frontend changes, we also incorporated a backend change to
handle making retention period forever. This change introduces a new
convertor `to_positive_or_allowed_int` which only allows positive integers
and an allowed value for settings like `message_retention_days` which can
be a positive integer or has the value `Realm.RETAIN_MESSAGE_FOREVER` when
we change the setting to retain message forever.

This change made `to_not_negative_int_or_none` redundant so removed it as
well.

Fixes: #14854
This commit is contained in:
Pragati Agrawal 2020-05-07 16:49:54 +05:30 committed by Tim Abbott
parent 12504075ee
commit bd9b74436c
15 changed files with 107 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
analytics/views.py
View File

@ -31,7 +31,7 @@ from analytics.models import BaseCount, InstallationCount, \
RealmCount, StreamCount, UserCount, last_successful_fill, installation_epoch
from confirmation.models import Confirmation, confirmation_url, _properties
from zerver.decorator import require_server_admin, require_server_admin_api, \
to_non_negative_int, to_utc_datetime, zulip_login_required, require_non_guest_user
to_utc_datetime, zulip_login_required, require_non_guest_user
from zerver.lib.exceptions import JsonableError
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_success
@ -41,6 +41,7 @@ from zerver.views.invite import get_invitee_emails_set
from zerver.lib.subdomains import get_subdomain_from_hostname
from zerver.lib.actions import do_change_plan_type, do_deactivate_realm, \
do_send_realm_reactivation_email, do_scrub_realm
from zerver.lib.validator import to_non_negative_int
from confirmation.settings import STATUS_ACTIVE
if settings.BILLING_ENABLED:

2
frontend_tests/node_tests/settings_org.js
View File

@ -804,6 +804,8 @@ run_test('set_up', () => {
};
$("#id_realm_message_content_edit_limit_minutes").set_parent($.create('<stub edit limit parent>'));
$("#id_realm_message_content_delete_limit_minutes").set_parent($.create('<stub delete limit parent>'));
$("#id_realm_message_retention_days").set_parent($.create('<stub retention period parent>'));
$("#id_realm_message_retention_setting").change = noop;
$("#message_content_in_email_notifications_label").set_parent($.create('<stub in-content setting checkbox>'));
$("#enable_digest_emails_label").set_parent($.create('<stub digest setting checkbox>'));
$("#id_realm_digest_weekday").set_parent($.create('<stub digest weekday setting dropdown>'));

1
static/js/settings_config.js
View File

@ -156,6 +156,7 @@ const time_limit_dropdown_values = new Map([
exports.msg_edit_limit_dropdown_values = time_limit_dropdown_values;
exports.msg_delete_limit_dropdown_values = time_limit_dropdown_values;
exports.retain_message_forever = -1;
// NOTIFICATIONS

42
static/js/settings_org.js
View File

@ -119,6 +119,14 @@ function get_property_value(property_name) {
return "custom_limit";
}
if (property_name === 'realm_message_retention_setting') {
if (page_params.realm_message_retention_days === null ||
page_params.realm_message_retention_days === settings_config.retain_message_forever) {
return "retain_forever";
}
return "retain_for_period";
}
if (property_name === 'realm_msg_delete_limit_setting') {
if (!page_params.realm_allow_message_deleting) {
return "never";
@ -234,6 +242,13 @@ function set_msg_delete_limit_dropdown() {
value === "custom_limit");
}
function set_message_retention_setting_dropdown() {
const value = get_property_value("realm_message_retention_setting");
$("#id_realm_message_retention_setting").val(value);
change_element_block_display_property('id_realm_message_retention_days',
value === "retain_for_period");
}
function set_org_join_restrictions_dropdown() {
const value = get_property_value("realm_org_join_restrictions");
$("#id_realm_org_join_restrictions").val(value);
@ -327,6 +342,8 @@ function update_dependent_subsettings(property_name) {
} else if (property_name === 'realm_msg_edit_limit_setting' ||
property_name === 'realm_message_content_edit_limit_minutes') {
set_msg_edit_limit_dropdown();
} else if (property_name === 'realm_message_retention_days') {
set_message_retention_setting_dropdown();
} else if (property_name === 'realm_msg_delete_limit_setting' ||
property_name === 'realm_message_content_delete_limit_minutes') {
set_msg_delete_limit_dropdown();
@ -576,6 +593,7 @@ exports.build_page = function () {
set_video_chat_provider_dropdown();
set_msg_edit_limit_dropdown();
set_msg_delete_limit_dropdown();
set_message_retention_setting_dropdown();
set_org_join_restrictions_dropdown();
set_message_content_in_email_notifications_visiblity();
set_digest_emails_weekday_visibility();
@ -671,19 +689,17 @@ exports.build_page = function () {
parseInt(exports.notifications_stream_widget.value(), 10));
data.signup_notifications_stream_id = JSON.stringify(
parseInt(exports.signup_notifications_stream_widget.value(), 10));
} else if (subsection === 'other_settings') {
let new_message_retention_days = $("#id_realm_message_retention_days").val();
if (parseInt(new_message_retention_days, 10).toString() !== new_message_retention_days
&& new_message_retention_days !== "") {
new_message_retention_days = "";
} else if (subsection === 'message_retention') {
const message_retention_setting_value = $('#id_realm_message_retention_setting').val();
if (message_retention_setting_value === 'retain_forever') {
data.message_retention_days = settings_config.retain_message_forever;
} else {
data.message_retention_days = exports.get_input_element_value(
$('#id_realm_message_retention_days'));
}
} else if (subsection === 'other_settings') {
const code_block_language_value = exports.default_code_language_widget.value();
data.default_code_block_language = JSON.stringify(code_block_language_value);
data.message_retention_days = new_message_retention_days !== "" ?
JSON.stringify(parseInt(new_message_retention_days, 10)) : null;
} else if (subsection === 'other_permissions') {
const add_emoji_permission = $("#id_realm_add_emoji_by_admins_only").val();
@ -791,6 +807,12 @@ exports.build_page = function () {
msg_delete_limit_dropdown_value === 'custom_limit');
});
$("#id_realm_message_retention_setting").change(function (e) {
const message_retention_setting_dropdown_value = e.target.value;
change_element_block_display_property('id_realm_message_retention_days',
message_retention_setting_dropdown_value === 'retain_for_period');
});
$("#id_realm_waiting_period_setting").change(function () {
const waiting_period_threshold = this.value;
change_element_block_display_property('id_realm_waiting_period_threshold',

45
static/templates/settings/organization_settings_admin.hbs
View File

@ -142,6 +142,39 @@
</div>
</div>
<div id="org-message-retention" class="org-subsection-parent">
<div class="subsection-header">
<h3>{{t "Message retention" }}</h3>
{{> settings_save_discard_widget section_name="message-retention" }}
</div>
{{> upgrade_tip_widget }}
<div class="inline-block organization-settings-parent">
<div class="input-group">
<label for="id_realm_message_retention_setting" class="dropdown-title">{{t "Message retention period" }}</label>
<select name="realm_message_retention_setting"
id="id_realm_message_retention_setting" class="prop-element"
{{#unless zulip_plan_is_not_limited}}disabled{{/unless}}>
<option value="retain_forever">{{t 'Retain forever' }}</option>
<option value="retain_for_period">{{t 'Retain for N days after posting' }}</option>
</select>
<div class="dependent-inline-block">
<label for="id_realm_message_retention_days" class="inline-block realm-time-limit-label">
{{t 'N' }}:
</label>
<input type="text" id="id_realm_message_retention_days" autocomplete="off"
name="realm_message_retention_days"
class="admin-realm-message-retention-days prop-element"
value="{{ realm_message_retention_days }}"
data-setting-widget-type="number"
{{#unless zulip_plan_is_not_limited}}disabled{{/unless}}/>
</div>
</div>
</div>
</div>
<div id="org-other-settings" class="org-subsection-parent">
<div class="subsection-header">
<h3>{{t "Other settings" }}</h3>
@ -209,18 +242,6 @@
is_checked=realm_message_content_allowed_in_email_notifications
label=admin_settings_label.realm_message_content_allowed_in_email_notifications}}
{{#if false}}
<div class="input-group">
<label for="realm_message_retention_days"
id="id_realm_message_retention_days_label">
{{t 'Messages retention period in days (blank means messages are retained forever)' }}
</label>
<input type="text" id="id_realm_message_retention_days"
name="realm_message_retention_days"
class="admin-realm-message-retention-days prop-element"
value="{{ realm_message_retention_days }}"/>
</div>
{{/if}}
{{> settings_checkbox
setting_name="realm_mandatory_topics"
prefix="id_"

8
zerver/decorator.py
View File

@ -26,7 +26,6 @@ from zerver.lib.exceptions import JsonableError, ErrorCode, \
InvalidJSONError, InvalidAPIKeyError, InvalidAPIKeyFormatError, \
OrganizationAdministratorRequired
from zerver.lib.types import ViewFuncT
from zerver.lib.validator import to_non_negative_int
from zerver.lib.rate_limiter import RateLimitedUser
from zerver.lib.request import REQ, has_request_variables
@ -716,13 +715,6 @@ def internal_notify_view(is_tornado_view: bool) -> Callable[[ViewFuncT], ViewFun
return _wrapped_func_arguments
return _wrapped_view_func
def to_not_negative_int_or_none(s: str) -> Optional[int]:
if s:
return to_non_negative_int(s)
return None
def to_utc_datetime(timestamp: str) -> datetime.datetime:
return timestamp_to_datetime(float(timestamp))

11
zerver/lib/validator.py
View File

@ -418,6 +418,17 @@ def to_non_negative_int(s: str, max_int_size: int=2**32-1) -> int:
raise ValueError('%s is too large (max %s)' % (x, max_int_size))
return x
def to_positive_or_allowed_int(allowed_integer: int) -> Callable[[str], int]:
@set_type_structure('int')
def convertor(s: str) -> int:
x = int(s)
if x == allowed_integer:
return x
if x == 0:
raise ValueError("argument is 0")
return to_non_negative_int(s)
return convertor
@set_type_structure('any(List[int], str)]')
def check_string_or_int_list(var_name: str, val: object) -> Optional[str]:
if isinstance(val, str):

1
zerver/models.py
View File

@ -264,6 +264,7 @@ class Realm(models.Model):
"Stream", related_name="+", null=True, blank=True, on_delete=CASCADE
)
RETAIN_MESSAGE_FOREVER = -1
# For old messages being automatically deleted
message_retention_days: Optional[int] = models.IntegerField(null=True)

15
zerver/tests/test_decorators.py
View File

@ -37,7 +37,7 @@ from zerver.decorator import (
authenticate_notify, cachify,
get_client_name, internal_notify_view, is_local_addr,
rate_limit, validate_api_key,
return_success_on_head_request, to_not_negative_int_or_none,
return_success_on_head_request,
zulip_login_required
)
from zerver.lib.cache import ignore_unhashable_lru_cache, dict_to_items_tuple, items_tuple_to_dict
@ -45,7 +45,8 @@ from zerver.lib.validator import (
check_string, check_dict, check_dict_only, check_bool, check_float, check_int, check_list, Validator,
check_variable_type, equals, check_none_or, check_url, check_short_string,
check_string_fixed_length, check_capped_string, check_color, to_non_negative_int,
check_string_or_int_list, check_string_or_int, check_int_in, check_string_in
check_string_or_int_list, check_string_or_int, check_int_in, check_string_in,
to_positive_or_allowed_int
)
from zerver.models import \
get_realm, get_user, UserProfile, Realm
@ -773,11 +774,13 @@ class ValidatorTestCase(TestCase):
with self.assertRaisesRegex(ValueError, re.escape('%s is too large (max %s)' % (2**32, 2**32-1))):
self.assertEqual(to_non_negative_int(str(2**32)))
def test_check_to_not_negative_int_or_none(self) -> None:
self.assertEqual(to_not_negative_int_or_none('5'), 5)
self.assertEqual(to_not_negative_int_or_none(None), None)
def test_to_positive_or_allowed_int(self) -> None:
self.assertEqual(to_positive_or_allowed_int(-1)('5'), 5)
self.assertEqual(to_positive_or_allowed_int(-1)('-1'), -1)
with self.assertRaisesRegex(ValueError, 'argument is negative'):
to_positive_or_allowed_int(-1)('-5')
with self.assertRaises(ValueError):
to_not_negative_int_or_none('-5')
to_positive_or_allowed_int(-1)('0')
def test_check_float(self) -> None:
x: Any = 5.5

7
zerver/tornado/views.py
View File

@ -5,10 +5,11 @@ import ujson
from django.http import HttpRequest, HttpResponse
from django.utils.translation import ugettext as _
from zerver.decorator import REQ, to_non_negative_int, \
has_request_variables, internal_notify_view, process_client
from zerver.decorator import REQ, has_request_variables, internal_notify_view, \
process_client
from zerver.lib.response import json_error, json_success
from zerver.lib.validator import check_bool, check_int, check_list, check_string
from zerver.lib.validator import check_bool, check_int, check_list, check_string, \
to_non_negative_int
from zerver.models import Client, UserProfile, get_client, get_user_profile_by_id
from zerver.tornado.event_queue import fetch_events, \
get_client_descriptor, process_notification

5
zerver/views/messages.py
View File

@ -9,8 +9,7 @@ from typing import Dict, List, Set, Any, Iterable, \
Optional, Tuple, Union, Sequence, cast
from zerver.lib.exceptions import JsonableError, ErrorCode
from zerver.lib.html_diff import highlight_html_differences
from zerver.decorator import has_request_variables, \
REQ, to_non_negative_int
from zerver.decorator import has_request_variables, REQ
from django.utils.html import escape as escape_html
from zerver.lib import bugdown
from zerver.lib.zcommand import process_zcommands
@ -50,7 +49,7 @@ from zerver.lib.utils import statsd
from zerver.lib.validator import \
check_list, check_int, check_dict, check_string, check_bool, \
check_string_or_int_list, check_string_or_int, check_string_in, \
check_required_string
check_required_string, to_non_negative_int
from zerver.lib.zephyr import compute_mit_user_fullname
from zerver.models import Message, UserProfile, Stream, Subscription, Client,\
Realm, RealmDomain, Recipient, UserMessage, \

2
zerver/views/pointer.py
View File

@ -1,10 +1,10 @@
from django.http import HttpRequest, HttpResponse
from django.utils.translation import ugettext as _
from zerver.decorator import to_non_negative_int
from zerver.lib.actions import do_update_pointer
from zerver.lib.request import has_request_variables, JsonableError, REQ
from zerver.lib.response import json_success
from zerver.lib.validator import to_non_negative_int
from zerver.models import UserProfile, get_usermessage_by_message_id
def get_pointer_backend(request: HttpRequest, user_profile: UserProfile) -> HttpResponse:

8
zerver/views/realm.py
View File

@ -5,7 +5,7 @@ from django.utils.translation import ugettext as _
from django.core.exceptions import ValidationError
from django.views.decorators.http import require_safe
from zerver.decorator import require_realm_admin, to_non_negative_int, to_not_negative_int_or_none
from zerver.decorator import require_realm_admin
from zerver.lib.actions import (
do_set_realm_message_editing,
do_set_realm_message_deleting,
@ -19,7 +19,8 @@ from zerver.lib.actions import (
from zerver.lib.i18n import get_available_language_codes
from zerver.lib.request import has_request_variables, REQ, JsonableError
from zerver.lib.response import json_success, json_error
from zerver.lib.validator import check_string, check_dict, check_bool, check_int, check_int_in
from zerver.lib.validator import check_string, check_dict, check_bool, check_int, \
check_int_in, to_positive_or_allowed_int, to_non_negative_int
from zerver.lib.streams import access_stream_by_id
from zerver.lib.domains import validate_domain
from zerver.lib.video_calls import request_zoom_video_call_url
@ -55,7 +56,8 @@ def update_realm(
authentication_methods: Optional[Dict[Any, Any]]=REQ(validator=check_dict([]), default=None),
notifications_stream_id: Optional[int]=REQ(validator=check_int, default=None),
signup_notifications_stream_id: Optional[int]=REQ(validator=check_int, default=None),
message_retention_days: Optional[int]=REQ(converter=to_not_negative_int_or_none, default=None),
message_retention_days: Optional[int] = REQ(converter=to_positive_or_allowed_int(
Realm.RETAIN_MESSAGE_FOREVER), default=None),
send_welcome_emails: Optional[bool]=REQ(validator=check_bool, default=None),
digest_emails_enabled: Optional[bool]=REQ(validator=check_bool, default=None),
message_content_allowed_in_email_notifications: Optional[bool]=REQ(

5
zerver/views/report.py
View File

@ -6,8 +6,7 @@ from django.conf import settings
from django.http import HttpRequest, HttpResponse
from django.views.decorators.csrf import csrf_exempt
from django.views.decorators.http import require_POST
from zerver.decorator import human_users_only, \
to_non_negative_int
from zerver.decorator import human_users_only
from zerver.lib.bugdown import privacy_clean_markdown
from zerver.lib.request import has_request_variables, REQ
from zerver.lib.response import json_success
@ -15,7 +14,7 @@ from zerver.lib.queue import queue_json_publish
from zerver.lib.storage import static_path
from zerver.lib.unminify import SourceMap
from zerver.lib.utils import statsd, statsd_key
from zerver.lib.validator import check_bool, check_dict
from zerver.lib.validator import check_bool, check_dict, to_non_negative_int
from zerver.models import UserProfile
import subprocess

4
zerver/views/streams.py
View File

@ -9,7 +9,7 @@ from django.http import HttpRequest, HttpResponse
from zerver.lib.exceptions import JsonableError, ErrorCode
from zerver.lib.request import REQ, has_request_variables
from zerver.decorator import authenticated_json_post_view, \
require_realm_admin, to_non_negative_int, require_non_guest_user
require_realm_admin, require_non_guest_user
from zerver.lib.actions import bulk_remove_subscriptions, \
do_change_subscription_property, internal_prep_private_message, \
internal_prep_stream_message, \
@ -28,7 +28,7 @@ from zerver.lib.streams import access_stream_by_id, access_stream_by_name, \
from zerver.lib.topic import get_topic_history_for_stream, messages_for_topic
from zerver.lib.validator import check_string, check_int, check_list, check_dict, \
check_bool, check_variable_type, check_capped_string, check_color, check_dict_only, \
check_int_in
check_int_in, to_non_negative_int
from zerver.models import UserProfile, Stream, Realm, UserMessage, \
get_system_bot, get_active_user