message_edit: Make zero invalid value for message_content_edit_time_limit_seconds.

This commit changes the code to consider zero as an invalid value for
message_content_edit_time_limit_seconds. Now to represent the setting that
user can edit the message anytime, the setting value will be "None" in
database and "unlimited" will be passed to API from clients.
This commit is contained in:
Sahil Batra 2022-04-12 16:43:02 +05:30 committed by Tim Abbott
parent 1747ab8482
commit a1f40ccda5
16 changed files with 170 additions and 51 deletions

View File

@ -59,7 +59,7 @@ run_test("get_editability", ({override}) => {
page_params.realm_allow_message_editing = true;
// Limit of 0 means no time limit on editing messages
page_params.realm_message_content_edit_limit_seconds = 0;
page_params.realm_message_content_edit_limit_seconds = null;
assert.equal(get_editability(message), editability_types.FULL);
page_params.realm_message_content_edit_limit_seconds = 10;
@ -88,7 +88,7 @@ run_test("get_editability", ({override}) => {
page_params.realm_edit_topic_policy =
settings_config.common_message_policy_values.by_everyone.code;
page_params.realm_allow_message_editing = true;
page_params.realm_message_content_edit_limit_seconds = 0;
page_params.realm_message_content_edit_limit_seconds = null;
page_params.realm_community_topic_editing_limit_seconds = 259200;
page_params.is_admin = false;
message.timestamp = current_timestamp - 60;

View File

@ -244,7 +244,7 @@ test_ui("actions_popover", ({override, mock_template}) => {
};
override(page_params, "realm_allow_message_editing", true);
override(page_params, "realm_message_content_edit_limit_seconds", 0);
override(page_params, "realm_message_content_edit_limit_seconds", null);
assert.equal(message_edit.get_editability(message), message_edit.editability_types.FULL);
$target.closest = (sel) => {

View File

@ -127,7 +127,7 @@ export function get_editability(message, edit_limit_seconds_buffer = 0) {
}
if (
page_params.realm_message_content_edit_limit_seconds === 0 &&
page_params.realm_message_content_edit_limit_seconds === null &&
message.sent_by_me &&
!is_widget_message(message)
) {

View File

@ -275,7 +275,7 @@ const time_limit_dropdown_values = new Map([
"any_time",
{
text: $t({defaultMessage: "Any time"}),
seconds: 0,
seconds: null,
},
],
[

View File

@ -169,6 +169,11 @@ function get_property_value(property_name, for_realm_default_settings) {
if (!page_params.realm_allow_message_editing) {
return "never";
}
if (page_params.realm_message_content_edit_limit_seconds === null) {
return "any_time";
}
for (const [value, elem] of settings_config.msg_edit_limit_dropdown_values) {
if (elem.seconds === page_params.realm_message_content_edit_limit_seconds) {
return value;
@ -899,20 +904,33 @@ export function register_save_discard_widget_handlers(
switch (subsection) {
case "msg_editing": {
const edit_limit_setting_value = $("#id_realm_msg_edit_limit_setting").val();
if (edit_limit_setting_value === "never") {
data.allow_message_editing = false;
} else if (edit_limit_setting_value === "custom_limit") {
data.message_content_edit_limit_seconds = parse_time_limit(
$("#id_realm_message_content_edit_limit_minutes"),
);
// Disable editing if the parsed time limit is 0 seconds
data.allow_message_editing = Boolean(data.message_content_edit_limit_seconds);
} else {
data.allow_message_editing = true;
data.message_content_edit_limit_seconds =
settings_config.msg_edit_limit_dropdown_values.get(
edit_limit_setting_value,
).seconds;
switch (edit_limit_setting_value) {
case "never": {
data.allow_message_editing = false;
break;
}
case "custom_limit": {
data.message_content_edit_limit_seconds = parse_time_limit(
$("#id_realm_message_content_edit_limit_minutes"),
);
data.allow_message_editing = true;
break;
}
case "any_time": {
data.allow_message_editing = true;
data.message_content_edit_limit_seconds = JSON.stringify("unlimited");
break;
}
default: {
data.allow_message_editing = true;
data.message_content_edit_limit_seconds =
settings_config.msg_edit_limit_dropdown_values.get(
edit_limit_setting_value,
).seconds;
}
}
const delete_limit_setting_value = $("#id_realm_msg_delete_limit_setting").val();
switch (delete_limit_setting_value) {

View File

@ -20,6 +20,15 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 6.0
**Feature level 138**
* [`POST /register`](/api/register-queue), [`GET
/events`](/api/get-events): `message_content_edit_limit_seconds`
now represents no limit using `null`, instead of the integer 0.
* `PATCH /realm`: One now sets `message_content_edit_limit_seconds`
to no limit by passing the string `unlimited`, rather than the
integer 0.
**Feature level 137**
* [`GET /messages/{message_id}/read_receipts`](/api/get-read-receipts):

View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
# Changes should be accompanied by documentation explaining what the
# new level means in templates/zerver/api/changelog.md, as well as
# "**Changes**" entries in the endpoint's documentation in `zulip.yaml`.
API_FEATURE_LEVEL = 137
API_FEATURE_LEVEL = 138
# Bump the minor PROVISION_VERSION to indicate that folks should provision
# only when going from an old version of the code to a newer version. Bump

View File

@ -930,7 +930,7 @@ def check_update_message(
# from (min_seconds_to_edit + seconds_left_buffer) in message_edit.js; if
# you change this value also change those two parameters in message_edit.js.
edit_limit_buffer = 20
if content is not None and user_profile.realm.message_content_edit_limit_seconds > 0:
if content is not None and user_profile.realm.message_content_edit_limit_seconds is not None:
deadline_seconds = user_profile.realm.message_content_edit_limit_seconds + edit_limit_buffer
if (timezone_now() - message.date_sent) > datetime.timedelta(seconds=deadline_seconds):
raise JsonableError(_("The time limit for editing this message has passed"))

View File

@ -138,7 +138,7 @@ def do_set_realm_authentication_methods(
def do_set_realm_message_editing(
realm: Realm,
allow_message_editing: bool,
message_content_edit_limit_seconds: int,
message_content_edit_limit_seconds: Optional[int],
edit_topic_policy: int,
*,
acting_user: Optional[UserProfile],

View File

@ -1615,14 +1615,16 @@ def wildcard_mention_allowed(sender: UserProfile, stream: Stream) -> bool:
raise AssertionError("Invalid wildcard mention policy")
def parse_message_content_delete_limit(
def parse_message_content_edit_or_delete_limit(
value: Union[int, str],
special_values_map: Mapping[str, Optional[int]],
*,
setting_name: str,
) -> Optional[int]:
if isinstance(value, str) and value in special_values_map.keys():
return special_values_map[value]
if isinstance(value, str) or value <= 0:
raise RequestVariableConversionError("message_content_delete_limit_seconds", value)
raise RequestVariableConversionError(setting_name, value)
assert isinstance(value, int)
return value

View File

@ -0,0 +1,57 @@
# Generated by Django 3.2.12 on 2022-04-12 14:51
from django.db import migrations, models
from django.db.backends.postgresql.schema import BaseDatabaseSchemaEditor
from django.db.migrations.state import StateApps
def make_zero_invalid_for_message_content_edit_limit(
apps: StateApps, schema_editor: BaseDatabaseSchemaEditor
) -> None:
Realm = apps.get_model("zerver", "Realm")
Realm.DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS = 600
Realm.objects.filter(allow_message_editing=True, message_content_edit_limit_seconds=0).update(
message_content_edit_limit_seconds=None
)
Realm.objects.filter(allow_message_editing=False, message_content_edit_limit_seconds=0).update(
message_content_edit_limit_seconds=Realm.DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS
)
def reverse_make_zero_invalid_for_message_content_edit_limit(
apps: StateApps, schema_editor: BaseDatabaseSchemaEditor
) -> None:
Realm = apps.get_model("zerver", "Realm")
Realm.DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS = 600
Realm.objects.filter(
allow_message_editing=True, message_content_edit_limit_seconds=None
).update(message_content_edit_limit_seconds=0)
class Migration(migrations.Migration):
atomic = False
dependencies = [
("zerver", "0405_set_default_for_enable_read_receipts"),
]
operations = [
migrations.AlterField(
model_name="realm",
name="message_content_edit_limit_seconds",
field=models.IntegerField(default=600, null=True),
),
migrations.RunPython(
make_zero_invalid_for_message_content_edit_limit,
reverse_code=reverse_make_zero_invalid_for_message_content_edit_limit,
elidable=True,
),
migrations.AlterField(
model_name="realm",
name="message_content_edit_limit_seconds",
field=models.PositiveIntegerField(default=600, null=True),
),
]

View File

@ -457,7 +457,7 @@ class Realm(models.Model):
DEFAULT_MESSAGE_CONTENT_DELETE_LIMIT_SECONDS = (
600 # if changed, also change in admin.js, setting_org.js
)
MESSAGE_CONTENT_DELETE_LIMIT_SPECIAL_VALUES_MAP = {
MESSAGE_CONTENT_EDIT_OR_DELETE_LIMIT_SPECIAL_VALUES_MAP = {
"unlimited": None,
}
message_content_delete_limit_seconds: Optional[int] = models.PositiveIntegerField(
@ -468,8 +468,8 @@ class Realm(models.Model):
DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS = (
600 # if changed, also change in admin.js, setting_org.js
)
message_content_edit_limit_seconds: int = models.IntegerField(
default=DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS,
message_content_edit_limit_seconds: Optional[int] = models.PositiveIntegerField(
default=DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS, null=True
)
# Whether users have access to message edit history

View File

@ -3993,10 +3993,14 @@ paths:
special value `0` before Zulip 5.0 (feature level 100).
message_content_edit_limit_seconds:
type: integer
nullable: true
description: |
Messages sent more than this many seconds ago cannot be edited
with this organization's
[message edit policy](/help/configure-message-editing-and-deletion).
**Changes**: No limit was represented using the
special value `0` before Zulip 6.0 (feature level 138).
move_messages_between_streams_policy:
type: integer
description: |
@ -4197,7 +4201,7 @@ paths:
"data":
{
"allow_message_editing": false,
"message_content_edit_limit_seconds": 0,
"message_content_edit_limit_seconds": 600,
"edit_topic_policy": 2,
},
"id": 0,
@ -11555,12 +11559,19 @@ paths:
[calc-full-member]: /api/roles-and-permissions#determining-if-a-user-is-a-full-member
realm_message_content_edit_limit_seconds:
type: integer
nullable: true
description: |
Present if `realm` is present in `fetch_event_types`.
Messages sent more than this many seconds ago cannot be edited
with this organization's
[message edit policy](/help/configure-message-editing-and-deletion).
Will not be 0. A 'null' value means no limit: messages can be edited
regardless of how long ago they were sent.
**Changes**: No limit was represented using the
special value `0` before Zulip 6.0 (feature level 138).
realm_community_topic_editing_limit_seconds:
type: integer
description: |

View File

@ -964,14 +964,16 @@ class EditMessageTest(EditMessageTestCase):
def test_edit_message_content_limit(self) -> None:
def set_message_editing_params(
allow_message_editing: bool,
message_content_edit_limit_seconds: int,
message_content_edit_limit_seconds: Union[int, str],
edit_topic_policy: int,
) -> None:
result = self.client_patch(
"/json/realm",
{
"allow_message_editing": orjson.dumps(allow_message_editing).decode(),
"message_content_edit_limit_seconds": message_content_edit_limit_seconds,
"message_content_edit_limit_seconds": orjson.dumps(
message_content_edit_limit_seconds
).decode(),
"edit_topic_policy": edit_topic_policy,
},
)
@ -1031,7 +1033,7 @@ class EditMessageTest(EditMessageTestCase):
do_edit_message_assert_error(id_, "C", "The time limit for editing this message has passed")
# infinite time, all edits allowed
set_message_editing_params(True, 0, Realm.POLICY_ADMINS_ONLY)
set_message_editing_params(True, "unlimited", Realm.POLICY_ADMINS_ONLY)
do_edit_message_assert_success(id_, "D")
# without allow_message_editing, nothing is allowed
@ -1043,7 +1045,7 @@ class EditMessageTest(EditMessageTestCase):
do_edit_message_assert_error(
id_, "F", "Your organization has turned off message editing", True
)
set_message_editing_params(False, 0, Realm.POLICY_ADMINS_ONLY)
set_message_editing_params(False, "unlimited", Realm.POLICY_ADMINS_ONLY)
do_edit_message_assert_error(
id_, "G", "Your organization has turned off message editing", True
)
@ -1051,7 +1053,7 @@ class EditMessageTest(EditMessageTestCase):
def test_edit_topic_policy(self) -> None:
def set_message_editing_params(
allow_message_editing: bool,
message_content_edit_limit_seconds: int,
message_content_edit_limit_seconds: Union[int, str],
edit_topic_policy: int,
) -> None:
self.login("iago")
@ -1059,7 +1061,9 @@ class EditMessageTest(EditMessageTestCase):
"/json/realm",
{
"allow_message_editing": orjson.dumps(allow_message_editing).decode(),
"message_content_edit_limit_seconds": message_content_edit_limit_seconds,
"message_content_edit_limit_seconds": orjson.dumps(
message_content_edit_limit_seconds
).decode(),
"edit_topic_policy": edit_topic_policy,
},
)
@ -1102,18 +1106,18 @@ class EditMessageTest(EditMessageTestCase):
self.subscribe(polonius, "Denmark")
# any user can edit the topic of a message
set_message_editing_params(True, 0, Realm.POLICY_EVERYONE)
set_message_editing_params(True, "unlimited", Realm.POLICY_EVERYONE)
do_edit_message_assert_success(id_, "A", "polonius")
# only members can edit topic of a message
set_message_editing_params(True, 0, Realm.POLICY_MEMBERS_ONLY)
set_message_editing_params(True, "unlimited", Realm.POLICY_MEMBERS_ONLY)
do_edit_message_assert_error(
id_, "B", "You don't have permission to edit this message", "polonius"
)
do_edit_message_assert_success(id_, "B", "cordelia")
# only full members can edit topic of a message
set_message_editing_params(True, 0, Realm.POLICY_FULL_MEMBERS_ONLY)
set_message_editing_params(True, "unlimited", Realm.POLICY_FULL_MEMBERS_ONLY)
cordelia = self.example_user("cordelia")
do_set_realm_property(cordelia.realm, "waiting_period_threshold", 10, acting_user=None)
@ -1129,21 +1133,21 @@ class EditMessageTest(EditMessageTestCase):
do_edit_message_assert_success(id_, "C", "cordelia")
# only moderators can edit topic of a message
set_message_editing_params(True, 0, Realm.POLICY_MODERATORS_ONLY)
set_message_editing_params(True, "unlimited", Realm.POLICY_MODERATORS_ONLY)
do_edit_message_assert_error(
id_, "D", "You don't have permission to edit this message", "cordelia"
)
do_edit_message_assert_success(id_, "D", "shiva")
# only admins can edit the topics of messages
set_message_editing_params(True, 0, Realm.POLICY_ADMINS_ONLY)
set_message_editing_params(True, "unlimited", Realm.POLICY_ADMINS_ONLY)
do_edit_message_assert_error(
id_, "E", "You don't have permission to edit this message", "shiva"
)
do_edit_message_assert_success(id_, "E", "iago")
# users cannot edit topics if allow_message_editing is False
set_message_editing_params(False, 0, Realm.POLICY_EVERYONE)
set_message_editing_params(False, "unlimited", Realm.POLICY_EVERYONE)
do_edit_message_assert_error(
id_, "D", "Your organization has turned off message editing", "cordelia"
)
@ -1151,7 +1155,7 @@ class EditMessageTest(EditMessageTestCase):
# non-admin users cannot edit topics sent > 72 hrs ago
message.date_sent = message.date_sent - datetime.timedelta(seconds=290000)
message.save()
set_message_editing_params(True, 0, Realm.POLICY_EVERYONE)
set_message_editing_params(True, "unlimited", Realm.POLICY_EVERYONE)
do_edit_message_assert_success(id_, "E", "iago")
do_edit_message_assert_success(id_, "F", "shiva")
do_edit_message_assert_error(

View File

@ -1283,7 +1283,7 @@ class RealmAPITest(ZulipTestCase):
def test_update_realm_allow_message_editing(self) -> None:
"""Tests updating the realm property 'allow_message_editing'."""
self.set_up_db("allow_message_editing", False)
self.set_up_db("message_content_edit_limit_seconds", 0)
self.set_up_db("message_content_edit_limit_seconds", None)
self.set_up_db("edit_topic_policy", Realm.POLICY_ADMINS_ONLY)
realm = self.update_with_api("allow_message_editing", True)
realm = self.update_with_api("message_content_edit_limit_seconds", 100)
@ -1325,6 +1325,16 @@ class RealmAPITest(ZulipTestCase):
result = self.client_patch("/json/realm", req)
self.assert_json_error(result, "Invalid edit_topic_policy")
# Test 0 is invalid value of message_content_edit_limit_seconds
invalid_message_content_edit_limit_seconds_value = 0
req = {
"message_content_edit_limit_seconds": orjson.dumps(
invalid_message_content_edit_limit_seconds_value
).decode()
}
result = self.client_patch("/json/realm", req)
self.assert_json_error(result, "Bad value for 'message_content_edit_limit_seconds': 0")
def test_update_realm_delete_own_message_policy(self) -> None:
"""Tests updating the realm property 'delete_own_message_policy'."""
self.set_up_db("delete_own_message_policy", Realm.POLICY_EVERYONE)

View File

@ -23,7 +23,7 @@ from zerver.decorator import require_realm_admin, require_realm_owner
from zerver.forms import check_subdomain_available as check_subdomain
from zerver.lib.exceptions import JsonableError, OrganizationOwnerRequired
from zerver.lib.i18n import get_available_language_codes
from zerver.lib.message import parse_message_content_delete_limit
from zerver.lib.message import parse_message_content_edit_or_delete_limit
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_success
from zerver.lib.retention import parse_message_retention_days
@ -82,8 +82,8 @@ def update_realm(
json_validator=check_int_in(Realm.COMMON_MESSAGE_POLICY_TYPES), default=None
),
mandatory_topics: Optional[bool] = REQ(json_validator=check_bool, default=None),
message_content_edit_limit_seconds: Optional[int] = REQ(
converter=to_non_negative_int, default=None
message_content_edit_limit_seconds_raw: Optional[Union[int, str]] = REQ(
"message_content_edit_limit_seconds", json_validator=check_string_or_int, default=None
),
allow_edit_history: Optional[bool] = REQ(json_validator=check_bool, default=None),
default_language: Optional[str] = REQ(default=None),
@ -187,9 +187,10 @@ def update_realm(
message_content_delete_limit_seconds: Optional[int] = None
if message_content_delete_limit_seconds_raw is not None:
message_content_delete_limit_seconds = parse_message_content_delete_limit(
message_content_delete_limit_seconds = parse_message_content_edit_or_delete_limit(
message_content_delete_limit_seconds_raw,
Realm.MESSAGE_CONTENT_DELETE_LIMIT_SPECIAL_VALUES_MAP,
Realm.MESSAGE_CONTENT_EDIT_OR_DELETE_LIMIT_SPECIAL_VALUES_MAP,
setting_name="message_content_delete_limit_seconds",
)
if (
message_content_delete_limit_seconds is None
@ -232,18 +233,25 @@ def update_realm(
data["authentication_methods"] = authentication_methods
# The message_editing settings are coupled to each other, and thus don't fit
# into the do_set_realm_property framework.
message_content_edit_limit_seconds = realm.message_content_edit_limit_seconds
if message_content_edit_limit_seconds_raw is not None:
message_content_edit_limit_seconds = parse_message_content_edit_or_delete_limit(
message_content_edit_limit_seconds_raw,
Realm.MESSAGE_CONTENT_EDIT_OR_DELETE_LIMIT_SPECIAL_VALUES_MAP,
setting_name="message_content_edit_limit_seconds",
)
if (
(allow_message_editing is not None and realm.allow_message_editing != allow_message_editing)
or (
message_content_edit_limit_seconds is not None
and realm.message_content_edit_limit_seconds != message_content_edit_limit_seconds
message_content_edit_limit_seconds_raw is not None
and message_content_edit_limit_seconds != realm.message_content_edit_limit_seconds
)
or (edit_topic_policy is not None and realm.edit_topic_policy != edit_topic_policy)
):
if allow_message_editing is None:
allow_message_editing = realm.allow_message_editing
if message_content_edit_limit_seconds is None:
message_content_edit_limit_seconds = realm.message_content_edit_limit_seconds
if edit_topic_policy is None:
edit_topic_policy = realm.edit_topic_policy
do_set_realm_message_editing(