message_edit: Apply topic edit restrictions to "(no topic)" messages.

Previously, editing topic of "(no topic)" messages was allowed
irrespective of time limit or the "edit_topic_policy" setting.
Since we are working in the direction of having "no topic" messages
feel reasonable, this commit changes the code to not consider them
as a special case and topic editing restrictions apply to them as
well now like all other messages.

We still highlight the topic edit icon in recipient bar without
hovering for "no topic" messages, but it is only shown when user
has permission to edit topics.

api_docs/changelog.md

@@ -20,6 +20,11 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 7.0
 
+**Feature level 172**
+
+* [`PATCH /messages/{message_id}`](/api/update-message): Topic editing
+  restrictions now apply to messages without a topic as well.
+
 **Feature level 171**:
 
 * [`POST /fetch_api_key`](/api/fetch-api-key),

web/src/message_edit.js

@@ -54,10 +54,6 @@ export function is_topic_editable(message, edit_limit_seconds_buffer = 0) {
         return false;
     }
 
-    if (message.topic === compose.empty_topic_placeholder()) {
-        return true;
-    }
-
     if (!settings_data.user_can_move_messages_to_another_topic()) {
         return false;
     }

web/src/message_list_view.js

@@ -156,14 +156,21 @@ function set_timestr(message_container) {
 function set_topic_edit_properties(group, message) {
     group.always_visible_topic_edit = false;
     group.on_hover_topic_edit = false;
+
+    const is_topic_editable = message_edit.is_topic_editable(message);
+
     // if a user who can edit a topic, can resolve it as well
-    group.user_can_resolve_topic = message_edit.is_topic_editable(message);
+    group.user_can_resolve_topic = is_topic_editable;
+
+    if (!is_topic_editable) {
+        return;
+    }
 
     // Messages with no topics should always have an edit icon visible
     // to encourage updating them. Admins can also edit any topic.
     if (message.topic === compose.empty_topic_placeholder()) {
         group.always_visible_topic_edit = true;
-    } else if (message_edit.is_topic_editable(message)) {
+    } else {
         group.on_hover_topic_edit = true;
     }
 }

web/tests/message_edit.test.js

@@ -112,7 +112,7 @@ run_test("is_topic_editable", ({override}) => {
     assert.equal(message_edit.is_topic_editable(message), false);
 
     message.topic = "translated: (no topic)";
-    assert.equal(message_edit.is_topic_editable(message), true);
+    assert.equal(message_edit.is_topic_editable(message), false);
 
     message.topic = "test topic";
     override(settings_data, "user_can_move_messages_to_another_topic", () => false);

zerver/actions/message_edit.py

@@ -106,22 +106,6 @@ def validate_message_edit_payload(
         raise JsonableError(_("Widgets cannot be edited."))
 
 
-def can_edit_topic(
-    user_profile: UserProfile,
-    is_no_topic_msg: bool,
-) -> bool:
-    # We allow anyone to edit (no topic) messages to help tend them.
-    if is_no_topic_msg:
-        return True
-
-    # 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
-
-
 def maybe_send_resolve_topic_notifications(
     *,
     user_profile: UserProfile,
@@ -995,9 +979,7 @@ def check_update_message(
     ):
         raise JsonableError(_("You don't have permission to edit this message"))
 
-    is_no_topic_msg = message.topic_name() == "(no topic)"
+    if topic_name is not None and not user_profile.can_move_messages_to_another_topic():
-
-    if topic_name is not None and not can_edit_topic(user_profile, is_no_topic_msg):
         raise JsonableError(_("You don't have permission to edit this message"))
 
     # If there is a change to the content, check that it hasn't been too long
@@ -1019,7 +1001,6 @@ def check_update_message(
         and user_profile.realm.move_messages_within_stream_limit_seconds is not None
         and not user_profile.is_realm_admin
         and not user_profile.is_moderator
-        and not is_no_topic_msg
     ):
         deadline_seconds = (
             user_profile.realm.move_messages_within_stream_limit_seconds + edit_limit_buffer

zerver/openapi/zulip.yaml

@@ -6520,6 +6520,11 @@ paths:
         Before Zulip 7.0 (feature level 159), message senders were allowed
         to edit the topic indefinitely.
 
+        **Changes**: Before Zulip 7.0 (feature level 172), anyone could add
+        a topic to messages without topic regardless of topic edit
+        permissions. Now topic editing restrictions apply to messages without a
+        topic as well.
+
         [config-message-editing]: /help/restrict-message-editing-and-deletion
       x-curl-examples-parameters:
         oneOf:

zerver/tests/test_message_edit.py

@@ -1195,6 +1195,13 @@ class EditMessageTest(EditMessageTestCase):
             id_, "G", "The time limit for editing this message's topic has passed", "hamlet"
         )
 
+        # topic edit permissions apply on "no topic" messages as well
+        message.set_topic_name("(no topic)")
+        message.save()
+        do_edit_message_assert_error(
+            id_, "G", "The time limit for editing this message's topic has passed", "cordelia"
+        )
+
         # set the topic edit limit to two weeks
         do_set_realm_property(
             hamlet.realm,
@@ -1205,11 +1212,6 @@ class EditMessageTest(EditMessageTestCase):
         do_edit_message_assert_success(id_, "G", "cordelia")
         do_edit_message_assert_success(id_, "H", "hamlet")
 
-        # anyone should be able to edit "no topic" indefinitely
-        message.set_topic_name("(no topic)")
-        message.save()
-        do_edit_message_assert_success(id_, "D", "cordelia")
-
     @mock.patch("zerver.actions.message_edit.send_event")
     def test_edit_topic_public_history_stream(self, mock_send_event: mock.MagicMock) -> None:
         stream_name = "Macbeth"