From 939a6edf0f3636333b4e2b49bd84c7aaebbcb7d5 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Tue, 18 Oct 2022 17:18:30 +0530 Subject: [PATCH] settings: Rename helper function to check who can edit topics. This commit renames "can_edit_topic_of_any_message" function in models.py to "can_move_messages_to_another_topic" and "user_can_edit_topic_of_any_message" function in settings_data.js to "user_can_move_messages_to_another_topic". This change is done since topic editing permission does not depend on message sender now and messages are considered same irrespective of whether the user who is editing the topic had sent the message or not. This also makes the naming consistent with what we use for the label of this setting in webapp and how we describe this action in help documentation. --- frontend_tests/node_tests/compose_validate.js | 2 +- frontend_tests/node_tests/message_edit.js | 10 +++++----- frontend_tests/node_tests/settings_data.js | 8 ++++---- static/js/compose_validate.js | 2 +- static/js/message_edit.js | 2 +- static/js/settings_data.ts | 2 +- zerver/actions/message_edit.py | 6 +++--- zerver/models.py | 2 +- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/frontend_tests/node_tests/compose_validate.js b/frontend_tests/node_tests/compose_validate.js index 90ebffc417..bc59de2d4e 100644 --- a/frontend_tests/node_tests/compose_validate.js +++ b/frontend_tests/node_tests/compose_validate.js @@ -742,7 +742,7 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => { test_ui("test warn_if_topic_resolved", ({override, mock_template}) => { mock_banners(); $("#compose_banners .topic_resolved").length = 0; - override(settings_data, "user_can_edit_topic_of_any_message", () => true); + override(settings_data, "user_can_move_messages_to_another_topic", () => true); let error_shown = false; mock_template("compose_banner/compose_banner.hbs", false, (data) => { diff --git a/frontend_tests/node_tests/message_edit.js b/frontend_tests/node_tests/message_edit.js index 7049d33af2..b372042b99 100644 --- a/frontend_tests/node_tests/message_edit.js +++ b/frontend_tests/node_tests/message_edit.js @@ -17,7 +17,7 @@ const editability_types = message_edit.editability_types; const settings_data = mock_esm("../../static/js/settings_data"); run_test("get_editability", ({override}) => { - override(settings_data, "user_can_edit_topic_of_any_message", () => true); + override(settings_data, "user_can_move_messages_to_another_topic", () => true); // You can't edit a null message assert.equal(get_editability(null), editability_types.NO); // You can't edit a message you didn't send @@ -119,7 +119,7 @@ run_test("is_topic_editable", ({override}) => { type: "stream", }; page_params.realm_allow_message_editing = true; - override(settings_data, "user_can_edit_topic_of_any_message", () => true); + override(settings_data, "user_can_move_messages_to_another_topic", () => true); page_params.is_admin = true; assert.equal(message_edit.is_topic_editable(message), false); @@ -134,7 +134,7 @@ run_test("is_topic_editable", ({override}) => { page_params.sent_by_me = false; assert.equal(message_edit.is_topic_editable(message), true); - override(settings_data, "user_can_edit_topic_of_any_message", () => false); + override(settings_data, "user_can_move_messages_to_another_topic", () => false); assert.equal(message_edit.is_topic_editable(message), false); page_params.is_admin = false; @@ -144,13 +144,13 @@ run_test("is_topic_editable", ({override}) => { assert.equal(message_edit.is_topic_editable(message), true); message.topic = "test topic"; - override(settings_data, "user_can_edit_topic_of_any_message", () => false); + override(settings_data, "user_can_move_messages_to_another_topic", () => false); assert.equal(message_edit.is_topic_editable(message), false); page_params.realm_community_topic_editing_limit_seconds = 259200; message.timestamp = current_timestamp - 60; - override(settings_data, "user_can_edit_topic_of_any_message", () => true); + override(settings_data, "user_can_move_messages_to_another_topic", () => true); assert.equal(message_edit.is_topic_editable(message), true); message.timestamp = current_timestamp - 600000; diff --git a/frontend_tests/node_tests/settings_data.js b/frontend_tests/node_tests/settings_data.js index 1e72dcd1e8..3267f375f7 100644 --- a/frontend_tests/node_tests/settings_data.js +++ b/frontend_tests/node_tests/settings_data.js @@ -243,16 +243,16 @@ function test_message_policy(label, policy, validation_func) { } test_message_policy( - "user_can_edit_topic_of_any_message", + "user_can_move_messages_to_another_topic", "realm_edit_topic_policy", - settings_data.user_can_edit_topic_of_any_message, + settings_data.user_can_move_messages_to_another_topic, ); -run_test("user_can_edit_topic_of_any_message_nobody_case", () => { +run_test("user_can_move_messages_to_another_topic_nobody_case", () => { page_params.is_admin = true; page_params.is_guest = false; page_params.realm_edit_topic_policy = settings_config.edit_topic_policy_values.nobody.code; - assert.equal(settings_data.user_can_edit_topic_of_any_message(), false); + assert.equal(settings_data.user_can_move_messages_to_another_topic(), false); }); run_test("user_can_move_messages_between_streams_nobody_case", () => { diff --git a/static/js/compose_validate.js b/static/js/compose_validate.js index dad81e2115..f85ccb509d 100644 --- a/static/js/compose_validate.js +++ b/static/js/compose_validate.js @@ -201,7 +201,7 @@ export function warn_if_topic_resolved(topic_changed) { return; } - const button_text = settings_data.user_can_edit_topic_of_any_message() + const button_text = settings_data.user_can_move_messages_to_another_topic() ? $t({defaultMessage: "Unresolve topic"}) : null; diff --git a/static/js/message_edit.js b/static/js/message_edit.js index ceb1d7a853..8263a6a2dd 100644 --- a/static/js/message_edit.js +++ b/static/js/message_edit.js @@ -70,7 +70,7 @@ export function is_topic_editable(message, edit_limit_seconds_buffer = 0) { return true; } - if (!settings_data.user_can_edit_topic_of_any_message()) { + if (!settings_data.user_can_move_messages_to_another_topic()) { return false; } diff --git a/static/js/settings_data.ts b/static/js/settings_data.ts index 970e1d0b4f..cfb4b59151 100644 --- a/static/js/settings_data.ts +++ b/static/js/settings_data.ts @@ -222,7 +222,7 @@ export function user_can_add_custom_emoji(): boolean { return user_has_permission(page_params.realm_add_custom_emoji_policy); } -export function user_can_edit_topic_of_any_message(): boolean { +export function user_can_move_messages_to_another_topic(): boolean { return user_has_permission(page_params.realm_edit_topic_policy); } diff --git a/zerver/actions/message_edit.py b/zerver/actions/message_edit.py index 29c31640e2..91b65e5121 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -112,9 +112,9 @@ def can_edit_topic( if is_no_topic_msg: return True - # The can_edit_topic_of_any_message helper returns whether the user can edit the topic - # or not based on edit_topic_policy setting and the user's role. - if user_profile.can_edit_topic_of_any_message(): + # The can_move_messages_to_another_topic helper returns whether the user can edit + # the topic or not based on edit_topic_policy setting and the user's role. + if user_profile.can_move_messages_to_another_topic(): return True return False diff --git a/zerver/models.py b/zerver/models.py index bd7234180c..9d900924f3 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -2121,7 +2121,7 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings): # type def can_edit_user_groups(self) -> bool: return self.has_permission("user_group_edit_policy") - def can_edit_topic_of_any_message(self) -> bool: + def can_move_messages_to_another_topic(self) -> bool: return self.has_permission("edit_topic_policy") def can_add_custom_emoji(self) -> bool: