From 891f83601d4f5491544618c3b5c4f24c41f8e26f Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Thu, 22 Dec 2022 12:55:29 +0530 Subject: [PATCH] message_edit: Use move_messages_between_streams_limit_seconds setting. This commit adds time restriction on moving messages between streams using the move_messages_between_streams_limit_seconds setting in the backend. There is no time limit for admins and moderators. --- api_docs/changelog.md | 3 ++ version.py | 2 +- zerver/actions/message_edit.py | 13 +++++ zerver/openapi/zulip.yaml | 11 ++-- zerver/tests/test_message_edit.py | 86 +++++++++++++++++++++++++++++++ 5 files changed, 109 insertions(+), 6 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 227af50592..decdb197a6 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -29,6 +29,9 @@ format used by the Zulip server that they are interacting with. * [`PATCH /messages/{message_id}`](/api/update-message): Time limit to edit topics, for users other than administrators and moderators, can now be configured using `move_messages_within_stream_limit_seconds` setting. +* [`PATCH /messages/{message_id}`](/api/update-message): Time limit to move + messages between streams, for users other than administrators and moderators, + can now be configured using `move_messages_between_streams_limit_seconds` setting. **Feature level 161** diff --git a/version.py b/version.py index 6b1d93b42c..188d651b6c 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 api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 161 +API_FEATURE_LEVEL = 162 # 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 a086e74e26..7a8602a3c1 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -1019,6 +1019,19 @@ def check_update_message( new_stream = access_stream_by_id(user_profile, stream_id, require_active=True)[0] check_stream_access_based_on_stream_post_policy(user_profile, new_stream) + if ( + user_profile.realm.move_messages_between_streams_limit_seconds is not None + and not user_profile.is_realm_admin + and not user_profile.is_moderator + ): + deadline_seconds = ( + user_profile.realm.move_messages_between_streams_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's stream has passed") + ) + number_changed = do_update_message( user_profile, message, diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 8eb27a1ddb..2b9ec32b5c 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -6489,11 +6489,12 @@ paths: **Note**: See [configuring message editing][config-message-editing] for detailed documentation on when users are allowed to edit topics. - **Changes**: Before Zulip 7.0 (feature level 162), users who - were not administrators or moderators could only edit topics - if the target message was sent within the last 3 days. That - time limit is now controlled by the - `move_messages_within_stream_limit_seconds` setting. + **Changes**: Before Zulip 7.0 (feature level 162), users who were not + administrators or moderators could only edit topics if the target + message was sent within the last 3 days. That time limit is now + controlled by the `move_messages_within_stream_limit_seconds` setting. A + similar time limit for moving topics to different streams was added, + controlled by the `move_messages_between_streams_limit_seconds` setting. **Changes**: Before Zulip 7.0 (feature level 159), editing streams and topics of messages was forbidden if diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index 7b80b3b1d4..e299d2dc58 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -2148,6 +2148,92 @@ class EditMessageTest(EditMessageTestCase): check_move_message_according_to_policy(UserProfile.ROLE_GUEST, expect_fail=True) check_move_message_according_to_policy(UserProfile.ROLE_MEMBER) + def test_move_message_to_stream_time_limit(self) -> None: + shiva = self.example_user("shiva") + iago = self.example_user("iago") + cordelia = self.example_user("cordelia") + + test_stream_1 = self.make_stream("test_stream_1") + test_stream_2 = self.make_stream("test_stream_2") + + self.subscribe(shiva, test_stream_1.name) + self.subscribe(iago, test_stream_1.name) + self.subscribe(cordelia, test_stream_1.name) + self.subscribe(shiva, test_stream_2.name) + self.subscribe(iago, test_stream_2.name) + self.subscribe(cordelia, test_stream_2.name) + + msg_id = self.send_stream_message( + cordelia, test_stream_1.name, topic_name="test", content="First" + ) + self.send_stream_message(cordelia, test_stream_1.name, topic_name="test", content="Second") + + self.send_stream_message(cordelia, test_stream_1.name, topic_name="test", content="third") + + do_set_realm_property( + cordelia.realm, + "move_messages_between_streams_policy", + Realm.POLICY_MEMBERS_ONLY, + acting_user=None, + ) + + def check_move_message_to_stream( + user: UserProfile, + old_stream: Stream, + new_stream: Stream, + *, + expect_error_message: Optional[str] = None, + ) -> None: + self.login_user(user) + result = self.client_patch( + "/json/messages/" + str(msg_id), + { + "stream_id": new_stream.id, + "propagate_mode": "change_all", + "send_notification_to_new_thread": orjson.dumps(False).decode(), + }, + ) + + if expect_error_message is not None: + self.assert_json_error(result, expect_error_message) + messages = get_topic_messages(user, old_stream, "test") + self.assert_length(messages, 3) + messages = get_topic_messages(user, new_stream, "test") + self.assert_length(messages, 0) + else: + self.assert_json_success(result) + messages = get_topic_messages(user, old_stream, "test") + self.assert_length(messages, 0) + messages = get_topic_messages(user, new_stream, "test") + self.assert_length(messages, 3) + + # non-admin and non-moderator users cannot move messages sent > 1 week ago + # including sender of the message. + message = Message.objects.get(id=msg_id) + message.date_sent = message.date_sent - datetime.timedelta(seconds=604900) + message.save() + check_move_message_to_stream( + cordelia, + test_stream_1, + test_stream_2, + expect_error_message="The time limit for editing this message's stream has passed", + ) + + # admins and moderators can move messages irrespective of time limit. + check_move_message_to_stream(shiva, test_stream_1, test_stream_2, expect_error_message=None) + check_move_message_to_stream(iago, test_stream_2, test_stream_1, expect_error_message=None) + + # set the topic edit limit to two weeks + do_set_realm_property( + cordelia.realm, + "move_messages_between_streams_limit_seconds", + 604800 * 2, + acting_user=None, + ) + check_move_message_to_stream( + cordelia, test_stream_1, test_stream_2, expect_error_message=None + ) + def test_move_message_to_stream_based_on_stream_post_policy(self) -> None: (user_profile, old_stream, new_stream, msg_id, msg_id_later) = self.prepare_move_topics( "othello", "old_stream_1", "new_stream_1", "test"