diff --git a/frontend_tests/node_tests/message_edit.js b/frontend_tests/node_tests/message_edit.js index 16607df47b..b0beecea70 100644 --- a/frontend_tests/node_tests/message_edit.js +++ b/frontend_tests/node_tests/message_edit.js @@ -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; diff --git a/frontend_tests/node_tests/popovers.js b/frontend_tests/node_tests/popovers.js index 823bd92a5b..a77829221d 100644 --- a/frontend_tests/node_tests/popovers.js +++ b/frontend_tests/node_tests/popovers.js @@ -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) => { diff --git a/static/js/message_edit.js b/static/js/message_edit.js index dfdf734320..b1feefb173 100644 --- a/static/js/message_edit.js +++ b/static/js/message_edit.js @@ -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) ) { diff --git a/static/js/settings_config.ts b/static/js/settings_config.ts index 0c5877703e..1b2d4cbc0e 100644 --- a/static/js/settings_config.ts +++ b/static/js/settings_config.ts @@ -275,7 +275,7 @@ const time_limit_dropdown_values = new Map([ "any_time", { text: $t({defaultMessage: "Any time"}), - seconds: 0, + seconds: null, }, ], [ diff --git a/static/js/settings_org.js b/static/js/settings_org.js index 60b08c82c3..00f2033f76 100644 --- a/static/js/settings_org.js +++ b/static/js/settings_org.js @@ -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) { diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index ce1e7b522f..6a0504e704 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -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): diff --git a/version.py b/version.py index bd2d484eb1..bef5f2408d 100644 --- a/version.py +++ b/version.py @@ -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 diff --git a/zerver/actions/message_edit.py b/zerver/actions/message_edit.py index 73877065f6..026b030ac6 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -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")) diff --git a/zerver/actions/realm_settings.py b/zerver/actions/realm_settings.py index f035de6ca2..c5a97ed925 100644 --- a/zerver/actions/realm_settings.py +++ b/zerver/actions/realm_settings.py @@ -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], diff --git a/zerver/lib/message.py b/zerver/lib/message.py index 21b7a376f0..d8bbc10439 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -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 diff --git a/zerver/migrations/0406_alter_realm_message_content_edit_limit_seconds.py b/zerver/migrations/0406_alter_realm_message_content_edit_limit_seconds.py new file mode 100644 index 0000000000..ac674f8e38 --- /dev/null +++ b/zerver/migrations/0406_alter_realm_message_content_edit_limit_seconds.py @@ -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), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index dd92bde2ed..ffff63ada3 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -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 diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 3c45f740bb..0678d3a8e9 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -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: | diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index f625cce72d..20cff113e8 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -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( diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index f5feb895dc..70585dd2a1 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -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) diff --git a/zerver/views/realm.py b/zerver/views/realm.py index dd4ba3cb37..a743a5e872 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -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(