From 91197fa4f1497c89c3c65bf81103bcf1dd961fdd Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 2 Dec 2017 15:56:17 -0800 Subject: [PATCH] org settings: Add logic for applying allow_community_topic_editing. Applies the logic to allow community members to edit topics of others' messages if this setting is True. Otherwise, only administrators can update the topic of others' messages. This logic includes a 24-hour time limit for community topic editing. --- static/js/message_edit.js | 17 +++++-- static/js/message_list_view.js | 2 +- static/js/ui_init.js | 2 +- zerver/models.py | 1 + zerver/tests/test_messages.py | 86 ++++++++++++++++++++++++++++++---- zerver/views/messages.py | 15 +++++- 6 files changed, 107 insertions(+), 16 deletions(-) diff --git a/static/js/message_edit.js b/static/js/message_edit.js index 4dcabd0691..8375acad18 100644 --- a/static/js/message_edit.js +++ b/static/js/message_edit.js @@ -18,7 +18,10 @@ exports.editability_types = editability_types; function get_editability(message, edit_limit_seconds_buffer) { edit_limit_seconds_buffer = edit_limit_seconds_buffer || 0; - if (!(message && message.sent_by_me)) { + if (!message) { + return editability_types.NO; + } + if (!(message.sent_by_me || page_params.realm_allow_community_topic_editing)) { return editability_types.NO; } if (message.failed_request) { @@ -40,15 +43,21 @@ function get_editability(message, edit_limit_seconds_buffer) { return editability_types.NO; } - if (page_params.realm_message_content_edit_limit_seconds === 0) { + if (page_params.realm_message_content_edit_limit_seconds === 0 && message.sent_by_me) { return editability_types.FULL; } var now = new XDate(); - if (page_params.realm_message_content_edit_limit_seconds + edit_limit_seconds_buffer + - now.diffSeconds(message.timestamp * 1000) > 0) { + if ((page_params.realm_message_content_edit_limit_seconds + edit_limit_seconds_buffer + + now.diffSeconds(message.timestamp * 1000) > 0) && message.sent_by_me) { return editability_types.FULL; } + + // TODO: Change hardcoded value (24 hrs) to be realm setting + if (!message.sent_by_me && ( + 86400 + edit_limit_seconds_buffer + now.diffSeconds(message.timestamp * 1000) <= 0)) { + return editability_types.NO; + } // time's up! if (message.type === 'stream') { return editability_types.TOPIC_ONLY; diff --git a/static/js/message_list_view.js b/static/js/message_list_view.js index 1c4104cd62..91edfecf61 100644 --- a/static/js/message_list_view.js +++ b/static/js/message_list_view.js @@ -87,7 +87,7 @@ function set_topic_edit_properties(group, message) { // to encourage updating them. Admins can also edit any topic. if (message.subject === compose.empty_topic_placeholder()) { group.always_visible_topic_edit = true; - } else if (page_params.is_admin) { + } else if (page_params.is_admin || page_params.realm_allow_community_topic_editing) { group.on_hover_topic_edit = true; } } diff --git a/static/js/ui_init.js b/static/js/ui_init.js index 53120c3c83..c7227f79f7 100644 --- a/static/js/ui_init.js +++ b/static/js/ui_init.js @@ -34,7 +34,7 @@ function message_hover(message_row) { message_row.addClass('message_hovered'); current_message_hover = message_row; - if (!message.sent_by_me) { + if (!message.sent_by_me && !page_params.realm_allow_community_topic_editing) { // The actions and reactions icon hover logic is handled entirely by CSS return; } diff --git a/zerver/models.py b/zerver/models.py index 753dc82ede..c7098efaea 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -164,6 +164,7 @@ class Realm(models.Model): message_content_edit_limit_seconds = models.IntegerField(default=DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS) # type: int message_retention_days = models.IntegerField(null=True) # type: Optional[int] allow_edit_history = models.BooleanField(default=True) # type: bool + DEFAULT_COMMUNITY_TOPIC_EDITING_LIMIT_SECONDS = 86400 allow_community_topic_editing = models.BooleanField(default=False) # type: bool # Valid org_types are {CORPORATE, COMMUNITY} diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index 61e5e49c8c..25f50c1cee 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -1909,10 +1909,12 @@ class EditMessageTest(ZulipTestCase): def test_edit_message_content_limit(self) -> None: def set_message_editing_params(allow_message_editing: bool, - message_content_edit_limit_seconds: int) -> None: + message_content_edit_limit_seconds: int, + allow_community_topic_editing: bool) -> None: result = self.client_patch("/json/realm", { 'allow_message_editing': ujson.dumps(allow_message_editing), - 'message_content_edit_limit_seconds': message_content_edit_limit_seconds + 'message_content_edit_limit_seconds': message_content_edit_limit_seconds, + 'allow_community_topic_editing': ujson.dumps(allow_community_topic_editing), }) self.assert_json_success(result) @@ -1954,26 +1956,94 @@ class EditMessageTest(ZulipTestCase): # test the various possible message editing settings # high enough time limit, all edits allowed - set_message_editing_params(True, 240) + set_message_editing_params(True, 240, False) do_edit_message_assert_success(id_, 'A') # out of time, only topic editing allowed - set_message_editing_params(True, 120) + set_message_editing_params(True, 120, False) do_edit_message_assert_success(id_, 'B', True) do_edit_message_assert_error(id_, 'C', "The time limit for editing this message has past") # infinite time, all edits allowed - set_message_editing_params(True, 0) + set_message_editing_params(True, 0, False) do_edit_message_assert_success(id_, 'D') # without allow_message_editing, nothing is allowed - set_message_editing_params(False, 240) + set_message_editing_params(False, 240, False) do_edit_message_assert_error(id_, 'E', "Your organization has turned off message editing", True) - set_message_editing_params(False, 120) + set_message_editing_params(False, 120, False) do_edit_message_assert_error(id_, 'F', "Your organization has turned off message editing", True) - set_message_editing_params(False, 0) + set_message_editing_params(False, 0, False) do_edit_message_assert_error(id_, 'G', "Your organization has turned off message editing", True) + def test_allow_community_topic_editing(self) -> None: + def set_message_editing_params(allow_message_editing, + message_content_edit_limit_seconds, + allow_community_topic_editing): + # type: (bool, int, bool) -> None + result = self.client_patch("/json/realm", { + 'allow_message_editing': ujson.dumps(allow_message_editing), + 'message_content_edit_limit_seconds': message_content_edit_limit_seconds, + 'allow_community_topic_editing': ujson.dumps(allow_community_topic_editing), + }) + self.assert_json_success(result) + + def do_edit_message_assert_success(id_, unique_str): + # type: (int, Text) -> None + new_subject = 'subject' + unique_str + params_dict = {'message_id': id_, 'subject': new_subject} + result = self.client_patch("/json/messages/" + str(id_), params_dict) + self.assert_json_success(result) + self.check_message(id_, subject=new_subject) + + def do_edit_message_assert_error(id_, unique_str, error): + # type: (int, Text, Text) -> None + message = Message.objects.get(id=id_) + old_subject = message.topic_name() + old_content = message.content + new_subject = 'subject' + unique_str + params_dict = {'message_id': id_, 'subject': new_subject} + result = self.client_patch("/json/messages/" + str(id_), params_dict) + message = Message.objects.get(id=id_) + self.assert_json_error(result, error) + self.check_message(id_, subject=old_subject, content=old_content) + + self.login(self.example_email("iago")) + # send a message in the past + id_ = self.send_stream_message(self.example_email("hamlet"), "Scotland", + content="content", topic_name="subject") + message = Message.objects.get(id=id_) + message.pub_date = message.pub_date - datetime.timedelta(seconds=180) + message.save() + + # any user can edit the topic of a message + set_message_editing_params(True, 0, True) + # log in as a new user + self.login(self.example_email("cordelia")) + do_edit_message_assert_success(id_, 'A') + + # only admins can edit the topics of messages + self.login(self.example_email("iago")) + set_message_editing_params(True, 0, False) + do_edit_message_assert_success(id_, 'B') + self.login(self.example_email("cordelia")) + do_edit_message_assert_error(id_, 'C', "You don't have permission to edit this message") + + # users cannot edit topics if allow_message_editing is False + self.login(self.example_email("iago")) + set_message_editing_params(False, 0, True) + self.login(self.example_email("cordelia")) + do_edit_message_assert_error(id_, 'D', "Your organization has turned off message editing") + + # non-admin users cannot edit topics sent > 24 hrs ago + message.pub_date = message.pub_date - datetime.timedelta(seconds=90000) + message.save() + self.login(self.example_email("iago")) + set_message_editing_params(True, 0, True) + do_edit_message_assert_success(id_, 'E') + self.login(self.example_email("cordelia")) + do_edit_message_assert_error(id_, 'F', "The time limit for editing this message has past") + def test_propagate_topic_forward(self) -> None: self.login(self.example_email("hamlet")) id1 = self.send_stream_message(self.example_email("hamlet"), "Scotland", diff --git a/zerver/views/messages.py b/zerver/views/messages.py index 87dcefd42e..3ed02d8846 100644 --- a/zerver/views/messages.py +++ b/zerver/views/messages.py @@ -1213,11 +1213,13 @@ def update_message_backend(request: HttpRequest, user_profile: UserMessage, # you change this value also change those two parameters in message_edit.js. # 1. You sent it, OR: # 2. This is a topic-only edit for a (no topic) message, OR: - # 3. This is a topic-only edit and you are an admin. + # 3. This is a topic-only edit and you are an admin, OR: + # 4. This is a topic-only edit and your realm allows users to edit topics. if message.sender == user_profile: pass elif (content is None) and ((message.topic_name() == "(no topic)") or - user_profile.is_realm_admin): + user_profile.is_realm_admin or + user_profile.realm.allow_community_topic_editing): pass else: raise JsonableError(_("You don't have permission to edit this message")) @@ -1233,6 +1235,15 @@ def update_message_backend(request: HttpRequest, user_profile: UserMessage, if (timezone_now() - message.pub_date) > datetime.timedelta(seconds=deadline_seconds): raise JsonableError(_("The time limit for editing this message has past")) + # If there is a change to the topic, check that the user is allowed to + # edit it and that it has not been too long. If this is not the user who + # sent the message, they are not the admin, and the time limit for editing + # topics is passed, raise an error. + if content is None and message.sender != user_profile and not user_profile.is_realm_admin: + deadline_seconds = Realm.DEFAULT_COMMUNITY_TOPIC_EDITING_LIMIT_SECONDS + edit_limit_buffer + if (timezone_now() - message.pub_date) > datetime.timedelta(deadline_seconds): + raise JsonableError(_("The time limit for editing this message has past")) + if subject is None and content is None: return json_error(_("Nothing to change")) if subject is not None: