realm: Replace allow_message_deleting with delete_own_message_policy.

This commit replaces 'allow_message_deleting' boolean setting
with an integer setting 'delete_own_message_policy'. We have a
separate dropdown now for deciding which user-roles can delete
messages sent by themselves and the time-limit setting droddown
is different.

This new setting has two options - everyone and admins only. Other
options including moderators will be added further.

We also remove the "Never" option from the original time-limit
dropdown, as admins are always allowed to delete message. This
never option resembled the case of only admins being allowed to
delete but this state is now resembled by setting the dropdown
to "admins only" and we also disable the time-limit dropdown in
this case as admins are allowed to delete irrespective of limit.

Note, this setting is only for deleting messages sent by the
deleting user themselves, and only admins are allowed to delete
messages sent by others as before.
This commit is contained in:
sahil839 2021-06-08 17:15:14 +05:30 committed by Tim Abbott
parent b13bfa09c5
commit 909a3cde76
23 changed files with 252 additions and 51 deletions

View File

@ -124,7 +124,8 @@ run_test("get_editability", ({override}) => {
run_test("get_deletability", () => {
page_params.is_admin = true;
page_params.realm_allow_message_deleting = false;
page_params.realm_delete_own_message_policy =
settings_config.delete_own_message_policy_values.by_admins_only.code;
page_params.realm_message_content_delete_limit_seconds = null;
const message = {
sent_by_me: false,
@ -145,7 +146,8 @@ run_test("get_deletability", () => {
message.locally_echoed = false;
assert.equal(message_edit.get_deletability(message), false);
page_params.realm_allow_message_deleting = true;
page_params.realm_delete_own_message_policy =
settings_config.delete_own_message_policy_values.by_everyone.code;
assert.equal(message_edit.get_deletability(message), true);
const now = new Date();

View File

@ -575,7 +575,8 @@ function test_discard_changes_button(discard_changes) {
settings_config.common_message_policy_values.by_everyone.code;
page_params.realm_allow_message_editing = true;
page_params.realm_message_content_edit_limit_seconds = 3600;
page_params.realm_allow_message_deleting = true;
page_params.realm_delete_own_message_policy =
settings_config.delete_own_message_policy_values.by_everyone.code;
page_params.realm_message_content_delete_limit_seconds = 120;
const allow_edit_history = $("#id_realm_allow_edit_history").prop("checked", false);

View File

@ -28,6 +28,7 @@ import * as overlays from "./overlays";
import {page_params} from "./page_params";
import * as resize from "./resize";
import * as rows from "./rows";
import * as settings_config from "./settings_config";
import * as settings_data from "./settings_data";
import * as stream_bar from "./stream_bar";
import * as stream_data from "./stream_data";
@ -167,7 +168,10 @@ export function get_deletability(message) {
if (message.locally_echoed) {
return false;
}
if (!page_params.realm_allow_message_deleting) {
if (
page_params.realm_delete_own_message_policy ===
settings_config.delete_own_message_policy_values.by_admins_only.code
) {
return false;
}

View File

@ -183,7 +183,6 @@ export function dispatch_normal_event(event) {
const realm_settings = {
add_custom_emoji_policy: settings_emoji.update_custom_emoji_ui,
allow_edit_history: noop,
allow_message_deleting: noop,
allow_message_editing: noop,
edit_topic_policy: noop,
user_group_edit_policy: noop,
@ -193,6 +192,7 @@ export function dispatch_normal_event(event) {
invite_to_stream_policy: noop,
default_code_block_language: noop,
default_language: noop,
delete_own_message_policy: noop,
description: noop,
digest_emails_enabled: noop,
digest_weekday: noop,

View File

@ -255,6 +255,19 @@ export const common_message_policy_values = {
},
};
export const delete_own_message_policy_values = {
by_everyone: {
order: 1,
code: 5,
description: $t({defaultMessage: "Admins, members and guests"}),
},
by_admins_only: {
order: 2,
code: 2,
description: $t({defaultMessage: "Admins only"}),
},
};
const time_limit_dropdown_values = new Map([
[
"any_time",
@ -327,7 +340,71 @@ const time_limit_dropdown_values = new Map([
],
]);
export const msg_edit_limit_dropdown_values = time_limit_dropdown_values;
export const msg_delete_limit_dropdown_values = time_limit_dropdown_values;
export const msg_delete_limit_dropdown_values = new Map([
[
"any_time",
{
text: $t({defaultMessage: "Any time"}),
seconds: 0,
},
],
[
"upto_two_min",
{
text: $t(
{defaultMessage: "Up to {time_limit} after posting"},
{time_limit: $t({defaultMessage: "2 minutes"})},
),
seconds: 2 * 60,
},
],
[
"upto_ten_min",
{
text: $t(
{defaultMessage: "Up to {time_limit} after posting"},
{time_limit: $t({defaultMessage: "10 minutes"})},
),
seconds: 10 * 60,
},
],
[
"upto_one_hour",
{
text: $t(
{defaultMessage: "Up to {time_limit} after posting"},
{time_limit: $t({defaultMessage: "1 hour"})},
),
seconds: 60 * 60,
},
],
[
"upto_one_day",
{
text: $t(
{defaultMessage: "Up to {time_limit} after posting"},
{time_limit: $t({defaultMessage: "1 day"})},
),
seconds: 24 * 60 * 60,
},
],
[
"upto_one_week",
{
text: $t(
{defaultMessage: "Up to {time_limit} after posting"},
{time_limit: $t({defaultMessage: "1 week"})},
),
seconds: 7 * 24 * 60 * 60,
},
],
[
"custom_limit",
{
text: $t({defaultMessage: "Up to N minutes after posting"}),
},
],
]);
export const retain_message_forever = -1;
export const user_role_values = {

View File

@ -109,6 +109,9 @@ export function get_organization_settings_options() {
options.invite_to_realm_policy_values = get_sorted_options_list(
settings_config.invite_to_realm_policy_values,
);
options.delete_own_message_policy_values = get_sorted_options_list(
settings_config.delete_own_message_policy_values,
);
return options;
}
@ -159,9 +162,6 @@ function get_property_value(property_name) {
}
if (property_name === "realm_msg_delete_limit_setting") {
if (!page_params.realm_allow_message_deleting) {
return "never";
}
if (page_params.realm_message_content_delete_limit_seconds === null) {
return "any_time";
}
@ -249,6 +249,24 @@ function set_msg_edit_limit_dropdown() {
settings_ui.disable_sub_setting_onchange(value !== "never", "id_realm_edit_topic_policy", true);
}
function set_delete_own_message_policy_dropdown() {
const value = get_property_value("realm_delete_own_message_policy");
$("#id_realm_delete_own_message_policy").val(value);
settings_ui.disable_sub_setting_onchange(
value !== settings_config.delete_own_message_policy_values.by_admins_only.code,
"id_realm_msg_delete_limit_setting",
true,
);
const limit_value = get_property_value("realm_msg_delete_limit_setting");
if (limit_value === "custom_limit") {
settings_ui.disable_sub_setting_onchange(
value !== settings_config.delete_own_message_policy_values.by_admins_only.code,
"id_realm_message_content_delete_limit_minutes",
true,
);
}
}
function set_msg_delete_limit_dropdown() {
const value = get_property_value("realm_msg_delete_limit_setting");
$("#id_realm_msg_delete_limit_setting").val(value);
@ -374,6 +392,9 @@ function update_dependent_subsettings(property_name) {
case "realm_message_content_delete_limit_minutes":
set_msg_delete_limit_dropdown();
break;
case "realm_delete_own_message_policy":
set_delete_own_message_policy_dropdown();
break;
case "realm_org_join_restrictions":
set_org_join_restrictions_dropdown();
break;
@ -457,9 +478,6 @@ export function sync_realm_settings(property) {
case "message_content_delete_limit_seconds":
property = "message_content_delete_limit_minutes";
break;
case "allow_message_deleting":
property = "msg_delete_limit_setting";
break;
}
const element = $(`#id_realm_${CSS.escape(property)}`);
if (element.length) {
@ -689,6 +707,7 @@ export function build_page() {
set_giphy_rating_dropdown();
set_msg_edit_limit_dropdown();
set_msg_delete_limit_dropdown();
set_delete_own_message_policy_dropdown();
set_message_retention_setting_dropdown();
set_org_join_restrictions_dropdown();
set_message_content_in_email_notifications_visiblity();
@ -774,13 +793,7 @@ export function build_page() {
}
const delete_limit_setting_value = $("#id_realm_msg_delete_limit_setting").val();
switch (delete_limit_setting_value) {
case "never": {
data.allow_message_deleting = false;
break;
}
case "any_time": {
data.allow_message_deleting = true;
data.message_content_delete_limit_seconds = JSON.stringify("unlimited");
break;
@ -789,18 +802,17 @@ export function build_page() {
data.message_content_delete_limit_seconds = parse_time_limit(
$("#id_realm_message_content_delete_limit_minutes"),
);
data.allow_message_deleting = true;
break;
}
default: {
data.allow_message_deleting = true;
data.message_content_delete_limit_seconds =
settings_config.msg_delete_limit_dropdown_values.get(
delete_limit_setting_value,
).seconds;
}
}
data.delete_own_message_policy = $("#id_realm_delete_own_message_policy").val();
break;
}
case "notifications":

View File

@ -1570,6 +1570,7 @@ input[type="checkbox"] {
#id_realm_invite_to_realm_policy,
#id_realm_edit_topic_policy,
#id_realm_msg_edit_limit_setting,
#id_realm_delete_own_message_policy,
#id_realm_msg_delete_limit_setting {
width: 325px;
}

View File

@ -158,11 +158,21 @@
is_checked=realm_allow_edit_history
label=admin_settings_label.realm_allow_edit_history}}
<div class="input-group">
<label for="realm_delete_own_message_policy" class="dropdown-title">{{t "Who can delete their own messages" }}
<i class="fa fa-info-circle settings-info-icon tippy-zulip-tooltip"
aria-hidden="true" data-tippy-content="{{t 'Administrators can delete any message.' }}"></i>
</label>
<select name="realm_delete_own_message_policy" id="id_realm_delete_own_message_policy" class="prop-element" data-setting-widget-type="number">
{{> dropdown_options_widget option_values=delete_own_message_policy_values}}
</select>
</div>
<div class="input-group">
<label for="realm_msg_delete_limit_setting" class="dropdown-title">
{{t "Allow message deleting" }}
<i class="fa fa-info-circle settings-info-icon realm_allow_message_deleting_tooltip tippy-zulip-tooltip"
aria-hidden="true" data-tippy-content="{{t 'Administrators can always delete any message.' }}"></i>
{{t "Time limit for deleting messages" }}
<i class="fa fa-info-circle settings-info-icon tippy-zulip-tooltip"
aria-hidden="true" data-tippy-content="{{t 'Administrators can delete any message.' }}"></i>
</label>
<select name="realm_msg_delete_limit_setting" id="id_realm_msg_delete_limit_setting" class="prop-element">
{{#each msg_delete_limit_dropdown_values}}

View File

@ -11,6 +11,13 @@ below features are supported.
## Changes in Zulip 5.0
**Feature level 101**
* [`POST /register`](/api/register-queue), `PATCH /realm`: Replaced
the `allow_message_deleting` boolean field with an integer field
`delete_own_message_policy` defining which roles can delete messages
they had sent.
**Feature level 100**
* [`POST /register`](/api/register-queue), [`GET

View File

@ -29,7 +29,7 @@ is highly configurable. Two things are true under any configuration:
[2] Controlled by **Who can edit topic of any message**.
[3] Controlled by **Allow message deleting**.
[3] Controlled by **Who can delete their own messages**.
[4] Controlled by **Who can move messages between streams**, in
addition to other restrictions on editing topics.
@ -58,7 +58,7 @@ You can access the message editing and deletion settings as follows.
{settings_tab|organization-permissions}
4. Under **Message editing**, configure **Allow message editing**,
**Who can edit topic of any message**, and **Allow message deleting**.
**Who can edit topic of any message**, and **Who can delete their own messages**.
{!save-changes.md!}

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 = 100
API_FEATURE_LEVEL = 101
# 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

@ -238,8 +238,8 @@ def fetch_initial_state_data(
state["realm_edit_topic_policy"] = (
Realm.POLICY_ADMINS_ONLY if user_profile is None else realm.edit_topic_policy
)
state["realm_allow_message_deleting"] = (
False if user_profile is None else realm.allow_message_deleting
state["realm_delete_own_message_policy"] = (
Realm.POLICY_ADMINS_ONLY if user_profile is None else realm.delete_own_message_policy
)
# TODO: Can we delete these lines? They seem to be in property_types...

View File

@ -0,0 +1,18 @@
# Generated by Django 3.2.4 on 2021-06-09 10:00
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
("zerver", "0354_alter_realm_message_content_delete_limit_seconds"),
]
operations = [
migrations.AddField(
model_name="realm",
name="delete_own_message_policy",
field=models.PositiveSmallIntegerField(default=2),
),
]

View File

@ -0,0 +1,34 @@
# Generated by Django 3.2.4 on 2021-06-09 10:02
from django.db import migrations
from django.db.backends.postgresql.schema import DatabaseSchemaEditor
from django.db.migrations.state import StateApps
def migrate_to_delete_own_message_policy(
apps: StateApps, schema_editor: DatabaseSchemaEditor
) -> None:
Realm = apps.get_model("zerver", "Realm")
Realm.POLICY_EVERYONE = 5
Realm.POLICY_ADMINS_ONLY = 2
Realm.objects.filter(allow_message_deleting=False).update(
delete_own_message_policy=Realm.POLICY_ADMINS_ONLY
)
Realm.objects.filter(allow_message_deleting=True).update(
delete_own_message_policy=Realm.POLICY_EVERYONE
)
class Migration(migrations.Migration):
dependencies = [
("zerver", "0355_realm_delete_own_message_policy"),
]
operations = [
migrations.RunPython(
migrate_to_delete_own_message_policy,
reverse_code=migrations.RunPython.noop,
elidable=True,
),
]

View File

@ -0,0 +1,17 @@
# Generated by Django 3.2.4 on 2021-06-09 13:17
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
("zerver", "0356_migrate_to_delete_own_message_policy"),
]
operations = [
migrations.RemoveField(
model_name="realm",
name="allow_message_deleting",
),
]

View File

@ -367,7 +367,13 @@ class Realm(models.Model):
# some other actions.
waiting_period_threshold: int = models.PositiveIntegerField(default=0)
allow_message_deleting: bool = models.BooleanField(default=False)
DELETE_OWN_MESSAGE_POLICY_TYPES = [
POLICY_EVERYONE,
POLICY_ADMINS_ONLY,
]
delete_own_message_policy: bool = models.PositiveSmallIntegerField(default=POLICY_ADMINS_ONLY)
DEFAULT_MESSAGE_CONTENT_DELETE_LIMIT_SECONDS = (
600 # if changed, also change in admin.js, setting_org.js
)
@ -602,7 +608,6 @@ class Realm(models.Model):
property_types: Dict[str, Union[type, Tuple[type, ...]]] = dict(
add_custom_emoji_policy=int,
allow_edit_history=bool,
allow_message_deleting=bool,
bot_creation_policy=int,
create_stream_policy=int,
invite_to_stream_policy=int,
@ -634,6 +639,7 @@ class Realm(models.Model):
default_code_block_language=(str, type(None)),
message_content_delete_limit_seconds=(int, type(None)),
wildcard_mention_policy=int,
delete_own_message_policy=int,
)
DIGEST_WEEKDAY_VALUES = [0, 1, 2, 3, 4, 5, 6]

View File

@ -10336,12 +10336,20 @@ paths:
Whether this organization is configured to allow users to access
[message edit history](/help/view-a-messages-edit-history).
realm_allow_message_deleting:
type: boolean
realm_delete_own_message_policy:
type: integer
description: |
Present if `realm` is present in `fetch_event_types`.
Whether messages can be deleted in this Zulip organization.
The policy defining which users can delete
messages that they had sent.
- 2 = admins only
- 5 = everyone
**Changes**: New in Zulip 5.0 (feature level 101), replacing the
previous `allow_message_deleting` boolean;
`true` corresponded to `everyone`, and `false` to `admins only`.
realm_bot_creation_policy:
type: integer
description: |

View File

@ -2145,6 +2145,7 @@ class RealmPropertyActionTest(BaseAction):
invite_to_realm_policy=[6, 4, 3, 2, 1],
move_messages_between_streams_policy=[4, 3, 2, 1],
add_custom_emoji_policy=[4, 3, 2, 1],
delete_own_message_policy=[5, 2],
)
vals = test_values.get(name)

View File

@ -104,7 +104,6 @@ class HomeTest(ZulipTestCase):
"queue_id",
"realm_add_custom_emoji_policy",
"realm_allow_edit_history",
"realm_allow_message_deleting",
"realm_allow_message_editing",
"realm_authentication_methods",
"realm_available_video_chat_providers",
@ -119,6 +118,7 @@ class HomeTest(ZulipTestCase):
"realm_default_language",
"realm_default_stream_groups",
"realm_default_streams",
"realm_delete_own_message_policy",
"realm_description",
"realm_digest_emails_enabled",
"realm_digest_weekday",

View File

@ -2142,13 +2142,13 @@ class DeleteMessageTest(ZulipTestCase):
def test_delete_message_by_user(self) -> None:
def set_message_deleting_params(
allow_message_deleting: bool, message_content_delete_limit_seconds: Union[int, str]
delete_own_message_policy: int, message_content_delete_limit_seconds: Union[int, str]
) -> None:
self.login("iago")
result = self.client_patch(
"/json/realm",
{
"allow_message_deleting": orjson.dumps(allow_message_deleting).decode(),
"delete_own_message_policy": delete_own_message_policy,
"message_content_delete_limit_seconds": orjson.dumps(
message_content_delete_limit_seconds
).decode(),
@ -2172,7 +2172,7 @@ class DeleteMessageTest(ZulipTestCase):
return result
# Test if message deleting is not allowed(default).
set_message_deleting_params(False, "unlimited")
set_message_deleting_params(Realm.POLICY_ADMINS_ONLY, "unlimited")
hamlet = self.example_user("hamlet")
self.login_user(hamlet)
msg_id = self.send_stream_message(hamlet, "Scotland")
@ -2188,7 +2188,7 @@ class DeleteMessageTest(ZulipTestCase):
# Test if message deleting is allowed.
# Test if time limit is None(no limit).
set_message_deleting_params(True, "unlimited")
set_message_deleting_params(Realm.POLICY_EVERYONE, "unlimited")
msg_id = self.send_stream_message(hamlet, "Scotland")
message = Message.objects.get(id=msg_id)
message.date_sent = message.date_sent - datetime.timedelta(seconds=600)
@ -2201,7 +2201,7 @@ class DeleteMessageTest(ZulipTestCase):
self.assert_json_success(result)
# Test if time limit is non-zero.
set_message_deleting_params(True, 240)
set_message_deleting_params(Realm.POLICY_EVERYONE, 240)
msg_id_1 = self.send_stream_message(hamlet, "Scotland")
message = Message.objects.get(id=msg_id_1)
message.date_sent = message.date_sent - datetime.timedelta(seconds=120)

View File

@ -482,6 +482,7 @@ class RealmTest(ZulipTestCase):
invite_to_realm_policy=10,
move_messages_between_streams_policy=10,
add_custom_emoji_policy=10,
delete_own_message_policy=10,
)
# We need an admin user.
@ -838,6 +839,7 @@ class RealmAPITest(ZulipTestCase):
invite_to_realm_policy=Realm.INVITE_TO_REALM_POLICY_TYPES,
move_messages_between_streams_policy=Realm.COMMON_POLICY_TYPES,
add_custom_emoji_policy=Realm.COMMON_POLICY_TYPES,
delete_own_message_policy=Realm.DELETE_OWN_MESSAGE_POLICY_TYPES,
)
vals = test_values.get(name)
@ -1002,23 +1004,22 @@ class RealmAPITest(ZulipTestCase):
result = self.client_patch("/json/realm", req)
self.assert_json_error(result, "Invalid edit_topic_policy")
def test_update_realm_allow_message_deleting(self) -> None:
"""Tests updating the realm property 'allow_message_deleting'."""
self.set_up_db("allow_message_deleting", True)
realm = self.update_with_api("allow_message_deleting", False)
self.assertEqual(realm.allow_message_deleting, False)
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)
realm = self.update_with_api("delete_own_message_policy", Realm.POLICY_ADMINS_ONLY)
self.assertEqual(realm.delete_own_message_policy, Realm.POLICY_ADMINS_ONLY)
self.assertEqual(realm.message_content_delete_limit_seconds, 600)
realm = self.update_with_api("allow_message_deleting", True)
realm = self.update_with_api("delete_own_message_policy", Realm.POLICY_EVERYONE)
realm = self.update_with_api("message_content_delete_limit_seconds", 100)
self.assertEqual(realm.allow_message_deleting, True)
self.assertEqual(realm.delete_own_message_policy, Realm.POLICY_EVERYONE)
self.assertEqual(realm.message_content_delete_limit_seconds, 100)
realm = self.update_with_api(
"message_content_delete_limit_seconds", orjson.dumps("unlimited").decode()
)
self.assertEqual(realm.allow_message_deleting, True)
self.assertEqual(realm.message_content_delete_limit_seconds, None)
realm = self.update_with_api("message_content_delete_limit_seconds", 600)
self.assertEqual(realm.allow_message_deleting, True)
self.assertEqual(realm.delete_own_message_policy, Realm.POLICY_EVERYONE)
self.assertEqual(realm.message_content_delete_limit_seconds, 600)
# Test that 0 is invalid value.

View File

@ -18,7 +18,7 @@ from zerver.lib.response import json_success
from zerver.lib.timestamp import datetime_to_timestamp
from zerver.lib.topic import LEGACY_PREV_TOPIC, REQ_topic
from zerver.lib.validator import check_bool, check_string_in, to_non_negative_int
from zerver.models import Message, UserProfile
from zerver.models import Message, Realm, UserProfile
def fill_edit_history_entries(message_history: List[Dict[str, Any]], message: Message) -> None:
@ -133,7 +133,7 @@ def validate_can_delete_message(user_profile: UserProfile, message: Message) ->
if message.sender != user_profile:
# Users can only delete messages sent by them.
raise JsonableError(_("You don't have permission to delete this message"))
if not user_profile.realm.allow_message_deleting:
if user_profile.realm.delete_own_message_policy == Realm.POLICY_ADMINS_ONLY:
# User can not delete message, if message deleting is not allowed in realm.
raise JsonableError(_("You don't have permission to delete this message"))

View File

@ -67,7 +67,9 @@ def update_realm(
add_custom_emoji_policy: Optional[int] = REQ(
json_validator=check_int_in(Realm.COMMON_POLICY_TYPES), default=None
),
allow_message_deleting: Optional[bool] = REQ(json_validator=check_bool, default=None),
delete_own_message_policy: Optional[int] = REQ(
json_validator=check_int_in(Realm.DELETE_OWN_MESSAGE_POLICY_TYPES), default=None
),
message_content_delete_limit_seconds_raw: Optional[Union[int, str]] = REQ(
"message_content_delete_limit_seconds", json_validator=check_string_or_int, default=None
),