From d529a94e4dea25e861b98aca6866decd69e8b6e7 Mon Sep 17 00:00:00 2001 From: Rishi Gupta Date: Thu, 7 Jul 2016 17:25:55 -0700 Subject: [PATCH] Add realm setting to time-limit editing of message content. This is controlled through the admin tab and a new field in the Realms table. Notes: * The admin tab setting takes a value in minutes, whereas the backend stores it in seconds. * This setting is unused when allow_message_editing is false. * There is some generosity in how the limit is enforced. For instance, if the user sees the hovering edit button, we ensure they have at least 5 seconds to click it, and if the user gets to the message edit form, we ensure they have at least 10 seconds to make the edit, by relaxing the limit. * This commit also includes a countdown timer in the message edit form. Resolves #903. --- frontend_tests/casper_tests/10-admin.js | 81 ++++++++++++++++++- static/js/admin.js | 43 +++++++++- static/js/message_edit.js | 78 +++++++++++++++++- static/js/ui.js | 7 +- static/styles/settings.css | 5 ++ static/styles/zulip.css | 25 ++++++ static/templates/admin_tab.handlebars | 15 ++++ static/templates/message_edit_form.handlebars | 7 ++ zerver/lib/actions.py | 27 +++++-- .../0025_realm_message_content_edit_limit.py | 19 +++++ zerver/models.py | 3 + zerver/tests/test_events.py | 12 ++- zerver/tests/test_messages.py | 67 ++++++++++++++- zerver/views/__init__.py | 16 +++- 14 files changed, 384 insertions(+), 21 deletions(-) create mode 100644 zerver/migrations/0025_realm_message_content_edit_limit.py diff --git a/frontend_tests/casper_tests/10-admin.js b/frontend_tests/casper_tests/10-admin.js index 2d4d4d460b..c0a0eb28eb 100644 --- a/frontend_tests/casper_tests/10-admin.js +++ b/frontend_tests/casper_tests/10-admin.js @@ -312,7 +312,7 @@ casper.waitForSelector('input[type="checkbox"][id="id_realm_allow_message_editin casper.click('input[type="checkbox"][id="id_realm_allow_message_editing"]'); casper.click('form.admin-realm-form input.btn'); casper.waitUntilVisible('#admin-realm-message-editing-status', function () { - casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can now edit the content and topics of all their past messages!'); + casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can now edit topics for all their messages, and the content of messages which are less than 10 minutes old.'); casper.test.assertEval(function () { return document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked; }, 'Allow message editing Setting re-activated'); @@ -353,6 +353,85 @@ casper.then(function () { }); }); +// go back to admin page +casper.then(function () { + casper.test.info('Administration page'); + casper.click('a[href^="#administration"]'); + casper.test.assertUrlMatch(/^http:\/\/[^\/]+\/#administration/, 'URL suggests we are on administration page'); + casper.test.assertExists('#administration.tab-pane.active', 'Administration page is active'); +}); + +casper.waitForSelector('form.admin-realm-form input.btn', function () { + // deactivate message editing + casper.waitForSelector('input[type="checkbox"][id="id_realm_allow_message_editing"]', function () { + casper.evaluate(function () { + $('input[type="text"][id="id_realm_message_content_edit_limit_minutes"]').val('4'); + }); + casper.click('input[type="checkbox"][id="id_realm_allow_message_editing"]'); + casper.click('form.admin-realm-form input.btn'); + casper.waitUntilVisible('#admin-realm-message-editing-status', function () { + casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can no longer edit their past messages!'); + casper.test.assertEval(function () { + return !(document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked); + }, 'Allow message editing Setting de-activated'); + casper.test.assertEval(function () { + return $('input[type="text"][id="id_realm_message_content_edit_limit_minutes"]').val() === '4'; + }, 'Message content edit limit now 4'); + }); + }); + + // allow message editing again, and check that the old edit limit is still there + casper.waitForSelector('input[type="checkbox"][id="id_realm_allow_message_editing"]', function () { + casper.click('input[type="checkbox"][id="id_realm_allow_message_editing"]'); + casper.click('form.admin-realm-form input.btn'); + casper.waitUntilVisible('#admin-realm-message-editing-status', function () { + casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can now edit topics for all their messages, and the content of messages which are less than 4 minutes old.'); + casper.test.assertEval(function () { + return document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked; + }, 'Allow message editing Setting activated'); + casper.test.assertEval(function () { + return $('input[type="text"][id="id_realm_message_content_edit_limit_minutes"]').val() === '4'; + }, 'Message content edit limit still 4'); + }); + }); + + // allow arbitrary message editing + casper.waitForSelector('input[type="checkbox"][id="id_realm_allow_message_editing"]', function () { + casper.evaluate(function () { + $('input[type="text"][id="id_realm_message_content_edit_limit_minutes"]').val('0'); + }); + casper.click('form.admin-realm-form input.btn'); + casper.waitUntilVisible('#admin-realm-message-editing-status', function () { + casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can now edit the content and topics of all their past messages!'); + casper.test.assertEval(function () { + return document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked; + }, 'Allow message editing Setting still activated'); + casper.test.assertEval(function () { + return $('input[type="text"][id="id_realm_message_content_edit_limit_minutes"]').val() === '0'; + }, 'Message content edit limit is 0'); + }); + }); + + // disallow message editing, with illegal edit limit value. should be fixed by admin.js + casper.waitForSelector('input[type="checkbox"][id="id_realm_allow_message_editing"]', function () { + casper.evaluate(function () { + $('input[type="text"][id="id_realm_message_content_edit_limit_minutes"]').val('moo'); + }); + casper.click('input[type="checkbox"][id="id_realm_allow_message_editing"]'); + casper.click('form.admin-realm-form input.btn'); + casper.waitUntilVisible('#admin-realm-message-editing-status', function () { + casper.test.assertSelectorHasText('#admin-realm-message-editing-status', 'Users can no longer edit their past messages!'); + casper.test.assertEval(function () { + return !(document.querySelector('input[type="checkbox"][id="id_realm_allow_message_editing"]').checked); + }, 'Allow message editing Setting de-activated'); + casper.test.assertEval(function () { + return $('input[type="text"][id="id_realm_message_content_edit_limit_minutes"]').val() === '10'; + }, 'Message content edit limit has been reset to its default'); + }); + }); +}); + + common.then_log_out(); casper.run(function () { diff --git a/static/js/admin.js b/static/js/admin.js index 2ca9d7c89d..395964d20e 100644 --- a/static/js/admin.js +++ b/static/js/admin.js @@ -143,7 +143,8 @@ function _setup_page() { realm_invite_required: page_params.realm_invite_required, realm_invite_by_admins_only: page_params.realm_invite_by_admins_only, realm_create_stream_by_admins_only: page_params.realm_create_stream_by_admins_only, - realm_allow_message_editing: page_params.realm_allow_message_editing + realm_allow_message_editing: page_params.realm_allow_message_editing, + realm_message_content_edit_limit_minutes: Math.ceil(page_params.realm_message_content_edit_limit_seconds / 60) }; var admin_tab = templates.render('admin_tab', options); $("#administration").html(admin_tab); @@ -340,6 +341,16 @@ function _setup_page() { } }); + $("#id_realm_allow_message_editing").change(function () { + if (this.checked) { + $("#id_realm_message_content_edit_limit_minutes").removeAttr("disabled"); + $("#id_realm_message_content_edit_limit_minutes_label").removeClass("control-label-disabled"); + } else { + $("#id_realm_message_content_edit_limit_minutes").attr("disabled", true); + $("#id_realm_message_content_edit_limit_minutes_label").addClass("control-label-disabled"); + } + }); + $(".administration").on("submit", "form.admin-realm-form", function (e) { var name_status = $("#admin-realm-name-status").expectOne(); var restricted_to_domain_status = $("#admin-realm-restricted-to-domain-status").expectOne(); @@ -363,6 +374,19 @@ function _setup_page() { var new_invite_by_admins_only = $("#id_realm_invite_by_admins_only").prop("checked"); var new_create_stream_by_admins_only = $("#id_realm_create_stream_by_admins_only").prop("checked"); var new_allow_message_editing = $("#id_realm_allow_message_editing").prop("checked"); + var new_message_content_edit_limit_minutes = $("#id_realm_message_content_edit_limit_minutes").val(); + + // If allow_message_editing is unchecked, message_content_edit_limit_minutes + // is irrelevant. Hence if allow_message_editing is unchecked, and + // message_content_edit_limit_minutes is poorly formed, we set the latter to + // a default value to prevent the server from returning an error. + if (!new_allow_message_editing) { + if ((parseInt(new_message_content_edit_limit_minutes, 10).toString() !== + new_message_content_edit_limit_minutes) || + new_message_content_edit_limit_minutes < 0) { + new_message_content_edit_limit_minutes = 10; // Realm.DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS / 60 + } + } var url = "/json/realm"; var data = { @@ -371,7 +395,8 @@ function _setup_page() { invite_required: JSON.stringify(new_invite), invite_by_admins_only: JSON.stringify(new_invite_by_admins_only), create_stream_by_admins_only: JSON.stringify(new_create_stream_by_admins_only), - allow_message_editing: JSON.stringify(new_allow_message_editing) + allow_message_editing: JSON.stringify(new_allow_message_editing), + message_content_edit_limit_seconds: JSON.stringify(parseInt(new_message_content_edit_limit_minutes, 10) * 60) }; channel.patch({ @@ -410,11 +435,23 @@ function _setup_page() { } } if (response_data.allow_message_editing !== undefined) { + // We expect message_content_edit_limit_seconds was sent in the + // response as well + var data_message_content_edit_limit_minutes = Math.ceil(response_data.message_content_edit_limit_seconds / 60); if (response_data.allow_message_editing) { - ui.report_success(i18n.t("Users can now edit the content and topics of all their past messages!"), message_editing_status); + if (response_data.message_content_edit_limit_seconds > 0) { + ui.report_success(i18n.t("Users can now edit topics for all their messages, and the content of messages which are less than __num_minutes__ minutes old.", + {'num_minutes' : data_message_content_edit_limit_minutes}), + message_editing_status); + } else { + ui.report_success(i18n.t("Users can now edit the content and topics of all their past messages!"), message_editing_status); + } } else { ui.report_success(i18n.t("Users can no longer edit their past messages!"), message_editing_status); } + // message_content_edit_limit_seconds could have been changed earlier + // in this function, so update the field just in case + $("#id_realm_message_content_edit_limit_minutes").val(data_message_content_edit_limit_minutes); } }, error: function (xhr, error) { diff --git a/static/js/message_edit.js b/static/js/message_edit.js index 83c7646e39..ff28e6d941 100644 --- a/static/js/message_edit.js +++ b/static/js/message_edit.js @@ -86,6 +86,17 @@ function handle_edit_keydown(from_topic_edited_only, e) { } } +function timer_text(seconds_left) { + var minutes = Math.floor(seconds_left / 60); + var seconds = seconds_left % 60; + if (minutes >= 1) { + return i18n.t("__minutes__ min to edit", {'minutes': minutes.toString()}); + } else if (seconds_left >= 10) { + return i18n.t("__seconds__ sec to edit", {'seconds': (seconds - seconds % 5).toString()}); + } + return i18n.t("__seconds__ sec to edit", {'seconds': seconds.toString()}); +} + function edit_message (row, raw_content) { var content_top = row.find('.message_content')[0] .getBoundingClientRect().top; @@ -95,7 +106,8 @@ function edit_message (row, raw_content) { var form = $(templates.render('message_edit_form', {is_stream: message.is_stream, topic: message.subject, - content: raw_content})); + content: raw_content, + minutes_to_edit: Math.floor(page_params.realm_message_content_edit_limit_seconds / 60)})); var edit_obj = {form: form, raw_content: raw_content}; var original_topic = message.subject; @@ -104,8 +116,69 @@ function edit_message (row, raw_content) { form.keydown(_.partial(handle_edit_keydown, false)); + // We potentially got to this function by clicking a button that implied the + // user would be able to edit their message. Give a little bit of buffer in + // case the button has been around for a bit, e.g. we show the + // edit_content_button (hovering pencil icon) as long as the user would have + // been able to click it at the time the mouse entered the message_row. Also + // a buffer in case their computer is slow, or stalled for a second, etc + // If you change this number also change edit_limit_buffer in + // zerver.views.messages.update_message_backend + var seconds_left_buffer = 5; + + var now = new XDate(); + var seconds_left = page_params.realm_message_content_edit_limit_seconds + + now.diffSeconds(message.timestamp * 1000); + var can_edit_content = (page_params.realm_message_content_edit_limit_seconds === 0) || + (seconds_left + seconds_left_buffer > 0); + if (!can_edit_content) { + row.find('textarea.message_edit_content').attr("disabled","disabled"); + } + + // If we allow editing at all, give them at least 10 seconds to do it. + // If you change this number also change edit_limit_buffer in + // zerver.views.messages.update_message_backend + var min_seconds_to_edit = 10; + seconds_left = Math.floor(Math.max(seconds_left, min_seconds_to_edit)); + + // Add a visual timer if appropriate + if (can_edit_content && page_params.realm_message_content_edit_limit_seconds > 0) { + row.find('.message-edit-timer-control-group').show(); + $('#message_edit_tooltip').tooltip({ animation: false, placement: 'left', + template: ''}); + // I believe these need to be defined outside the countdown_timer, since + // row just refers to something like the currently selected message, and + // can change out from under us + var message_content_row = row.find('textarea.message_edit_content'); + var message_topic_row, message_topic_propagate_row; + if (message.is_stream) { + message_topic_row = row.find('input.message_edit_topic'); + message_topic_propagate_row = row.find('select.message_edit_topic_propagate'); + } + var message_save_row = row.find('button.message_edit_save'); + var timer_row = row.find('.message_edit_countdown_timer'); + // Do this right away, rather than waiting for the timer to do its first update, + // since otherwise there is a noticeable lag + timer_row.text(timer_text(seconds_left)); + var countdown_timer = setInterval(function () { + if (--seconds_left <= 0) { + clearInterval(countdown_timer); + message_content_row.attr("disabled","disabled"); + if (message.is_stream) { + message_topic_row.attr("disabled","disabled"); + message_topic_propagate_row.hide(); + } + message_save_row.addClass("disabled"); + timer_row.text(i18n.t("Time's up!")); + } else { + timer_row.text(timer_text(seconds_left)); + } + }, 1000); + } + currently_editing_messages[message.id] = edit_obj; - if (message.type === 'stream' && message.subject === compose.empty_subject_placeholder()) { + if ((message.type === 'stream' && message.subject === compose.empty_subject_placeholder()) || + !can_edit_content) { edit_row.find(".message_edit_topic").focus(); } else { edit_row.find(".message_edit_content").focus(); @@ -128,6 +201,7 @@ function edit_message (row, raw_content) { } composebox_typeahead.initialize_compose_typeahead("#message_edit_content", {emoji: true}); + } function start_edit_maintaining_scroll(row, content) { diff --git a/static/js/ui.js b/static/js/ui.js index ec44788244..870719566b 100644 --- a/static/js/ui.js +++ b/static/js/ui.js @@ -118,7 +118,12 @@ function message_hover(message_row) { message = current_msg_list.get(rows.id(message_row)); message_unhover(); message_row.addClass('message_hovered'); - if (message && message.sent_by_me && !message.status_message && page_params.realm_allow_message_editing) { + var now = new XDate(); + if (message && message.sent_by_me && !message.status_message && + page_params.realm_allow_message_editing && + (page_params.realm_message_content_edit_limit_seconds === 0 || + page_params.realm_message_content_edit_limit_seconds + now.diffSeconds(message.timestamp * 1000) > 0)) + { message_row.find('.message_content').find('p:last').append(edit_content_button); } current_message_hover = message_row; diff --git a/static/styles/settings.css b/static/styles/settings.css index cd724a1d40..ecc6d631c5 100644 --- a/static/styles/settings.css +++ b/static/styles/settings.css @@ -345,6 +345,11 @@ form.admin-realm .control-label { color: #d3d3d3; } +.admin-realm-message-content-edit-limit-minutes { + width: 5ch; + text-align: right; +} + #settings-status { text-align: center; width: 50%; diff --git a/static/styles/zulip.css b/static/styles/zulip.css index a04a6eb7f8..d441ba0348 100644 --- a/static/styles/zulip.css +++ b/static/styles/zulip.css @@ -321,6 +321,7 @@ a:hover code { display: inline; } +#message_edit_tooltip, #streams_inline_cog, #streams_filter_icon { float: right; @@ -331,11 +332,19 @@ a:hover code { opacity: 0.5; } +#message_edit_tooltip:hover, #streams_inline_cog:hover, #streams_filter_icon:hover { opacity: 1.0; } +.message-edit-tooltip-inner { + width: 200px; + position: absolute; + right: 7px; + top: -18px; +} + #streams_header a { color: inherit; text-decoration: none; @@ -1406,6 +1415,22 @@ div.focused_table { line-height: 18px; } +.message_edit_countdown_timer { + text-align: right; + display: inline; + color: #a1a1a1; +} + +.message_edit_tooltip { + display: inline; + color: #a1a1a1; +} + +.message-edit-timer-control-group { + float: right; + display: none; +} + .topic_edit { display: none; line-height: 22px; diff --git a/static/templates/admin_tab.handlebars b/static/templates/admin_tab.handlebars index 1cdadc3038..09364239e0 100644 --- a/static/templates/admin_tab.handlebars +++ b/static/templates/admin_tab.handlebars @@ -101,6 +101,21 @@ {{#if realm_allow_message_editing}}checked="checked"{{/if}} /> +
+ +
+ +
+
diff --git a/static/templates/message_edit_form.handlebars b/static/templates/message_edit_form.handlebars index 16e483910f..61cf5f37fd 100644 --- a/static/templates/message_edit_form.handlebars +++ b/static/templates/message_edit_form.handlebars @@ -14,6 +14,7 @@ {{/if}} +
@@ -23,6 +24,12 @@
+
+ + + +
diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 26d17813ea..77975fc537 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -412,15 +412,17 @@ def do_set_realm_create_stream_by_admins_only(realm, create_stream_by_admins_onl ) send_event(event, active_user_ids(realm)) -def do_set_realm_message_editing(realm, allow_message_editing): - # type: (Realm, bool) -> None +def do_set_realm_message_editing(realm, allow_message_editing, message_content_edit_limit_seconds): + # type: (Realm, bool, int) -> None realm.allow_message_editing = allow_message_editing - realm.save(update_fields=['allow_message_editing']) + realm.message_content_edit_limit_seconds = message_content_edit_limit_seconds + realm.save(update_fields=['allow_message_editing', 'message_content_edit_limit_seconds']) event = dict( type="realm", op="update_dict", property="default", - data=dict(allow_message_editing=allow_message_editing), + data=dict(allow_message_editing=allow_message_editing, + message_content_edit_limit_seconds=message_content_edit_limit_seconds), ) send_event(event, active_user_ids(realm)) @@ -2388,7 +2390,7 @@ def do_update_message(user_profile, message_id, subject, propagate_mode, content edit_history_event = {} changed_messages = [message] - # You can only edit a message if: + # You only have permission to edit a message if: # 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. @@ -2400,6 +2402,20 @@ def do_update_message(user_profile, message_id, subject, propagate_mode, content else: raise JsonableError(_("You don't have permission to edit this message")) + # We already check for realm.allow_message_editing in + # zerver.views.messages.update_message_backend + # If there is a change to the content, we also need to check it hasn't been + # too long + + # Allow an extra 20 seconds since we potentially allow editing 15 seconds + # past the limit, and in case there are network issues, etc. The 15 comes + # from (min_seconds_to_edit + seconds_left_buffer) in message_edit.js; if + # you change this value also change those two parameters in message_edit.js. + edit_limit_buffer = 20 + if content is not None and user_profile.realm.message_content_edit_limit_seconds > 0 and \ + (now() - message.pub_date) > datetime.timedelta(seconds=user_profile.realm.message_content_edit_limit_seconds + edit_limit_buffer): + raise JsonableError(_("The time limit for editing this message has past")) + # Set first_rendered_content to be the oldest version of the # rendered content recorded; which is the current version if the # content hasn't been edited before. Note that because one could @@ -2708,6 +2724,7 @@ def fetch_initial_state_data(user_profile, event_types, queue_id): state['realm_invite_by_admins_only'] = user_profile.realm.invite_by_admins_only state['realm_create_stream_by_admins_only'] = user_profile.realm.create_stream_by_admins_only state['realm_allow_message_editing'] = user_profile.realm.allow_message_editing + state['realm_message_content_edit_limit_seconds'] = user_profile.realm.message_content_edit_limit_seconds if want('realm_domain'): state['realm_domain'] = user_profile.realm.domain diff --git a/zerver/migrations/0025_realm_message_content_edit_limit.py b/zerver/migrations/0025_realm_message_content_edit_limit.py new file mode 100644 index 0000000000..4a330f7da5 --- /dev/null +++ b/zerver/migrations/0025_realm_message_content_edit_limit.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('zerver', '0024_realm_allow_message_editing'), + ] + + operations = [ + migrations.AddField( + model_name='realm', + name='message_content_edit_limit_seconds', + field=models.IntegerField(default=600), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index cc8cc527d9..b71f628b92 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -143,7 +143,10 @@ class Realm(ModelReprMixin, models.Model): mandatory_topics = models.BooleanField(default=False) # type: bool show_digest_email = models.BooleanField(default=True) # type: bool name_changes_disabled = models.BooleanField(default=False) # type: bool + allow_message_editing = models.BooleanField(default=True) # type: bool + DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS = 600 # if changed, also change in admin.js + message_content_edit_limit_seconds = models.IntegerField(default=DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS) # type: int date_created = models.DateTimeField(default=timezone.now) # type: datetime.datetime notifications_stream = models.ForeignKey('Stream', related_name='+', null=True, blank=True) # type: Optional[Stream] diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 2451ba770c..b4819c7e07 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -512,12 +512,16 @@ class EventsRegisterTest(AuthedTestCase): ('type', equals('realm')), ('op', equals('update_dict')), ('property', equals('default')), - ('data', check_dict([('allow_message_editing', check_bool)])), + ('data', check_dict([('allow_message_editing', check_bool), + ('message_content_edit_limit_seconds', check_int)])), ]) - # The first False is probably a noop, then we get transitions in both directions. - for allow_message_editing in [False, True, False]: + # Test every transition among the four possibilities {T,F} x {0, non-0} + for (allow_message_editing, message_content_edit_limit_seconds) in \ + ((True, 0), (False, 0), (True, 0), (False, 1234), (True, 0), (True, 1234), (True, 0), + (False, 0), (False, 1234), (False, 0), (True, 1234), (False, 0), + (True, 1234), (True, 600), (False, 600), (False, 1234), (True, 600)): events = self.do_test(lambda: do_set_realm_message_editing(self.user_profile.realm, - allow_message_editing)) + allow_message_editing, message_content_edit_limit_seconds)) error = schema_checker('events[0]', events[0]) self.assert_on_error(error) diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index 11a773a8de..a58e5a86ce 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -779,6 +779,72 @@ class EditMessageTest(AuthedTestCase): }) self.assert_json_error(result, "Content can't be empty") + def test_edit_message_content_limit(self): + def set_message_editing_params(allow_message_editing, + message_content_edit_limit_seconds): + result = self.client_patch("/json/realm", { + 'allow_message_editing': ujson.dumps(allow_message_editing), + 'message_content_edit_limit_seconds': message_content_edit_limit_seconds + }) + self.assert_json_success(result) + + def do_edit_message_assert_success(id_, unique_str, topic_only = False): + new_subject = 'subject' + unique_str + new_content = 'content' + unique_str + params_dict = { 'message_id': id_, 'subject': new_subject } + if not topic_only: + params_dict['content'] = new_content + result = self.client.post("/json/update_message", params_dict) + self.assert_json_success(result) + if topic_only: + self.check_message(id_, subject=new_subject) + else: + self.check_message(id_, subject=new_subject, content=new_content) + + def do_edit_message_assert_error(id_, unique_str, error, topic_only = False): + message = Message.objects.get(id=id_) + old_subject = message.subject + old_content = message.content + new_subject = 'subject' + unique_str + new_content = 'content' + unique_str + params_dict = { 'message_id': id_, 'subject': new_subject } + if not topic_only: + params_dict['content'] = new_content + result = self.client.post("/json/update_message", 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("iago@zulip.com") + # send a message in the past + id_ = self.send_message("iago@zulip.com", "Scotland", Recipient.STREAM, + content="content", subject="subject") + message = Message.objects.get(id=id_) + message.pub_date = message.pub_date - datetime.timedelta(seconds=180) + message.save() + + # test the various possible message editing settings + # high enough time limit, all edits allowed + set_message_editing_params(True, 240) + do_edit_message_assert_success(id_, 'A') + + # out of time, only topic editing allowed + set_message_editing_params(True, 120) + 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) + do_edit_message_assert_success(id_, 'D') + + # without allow_message_editing, nothing is allowed + set_message_editing_params(False, 240) + do_edit_message_assert_error(id_, 'E', "Your organization has turned off message editing.", True) + set_message_editing_params(False, 120) + do_edit_message_assert_error(id_, 'F', "Your organization has turned off message editing.", True) + set_message_editing_params(False, 0) + do_edit_message_assert_error(id_, 'G', "Your organization has turned off message editing.", True) + def test_propagate_topic_forward(self): self.login("hamlet@zulip.com") id1 = self.send_message("hamlet@zulip.com", "Scotland", Recipient.STREAM, @@ -982,4 +1048,3 @@ class CheckMessageTest(AuthedTestCase): new_count = message_stream_count(parent) self.assertEqual(new_count, old_count + 1) self.assertEqual(ret['message'].sender.email, 'othello-bot@zulip.com') - diff --git a/zerver/views/__init__.py b/zerver/views/__init__.py index 26187c7fd7..2a4e6746ad 100644 --- a/zerver/views/__init__.py +++ b/zerver/views/__init__.py @@ -946,6 +946,7 @@ def home(request): realm_invite_by_admins_only = register_ret['realm_invite_by_admins_only'], realm_create_stream_by_admins_only = register_ret['realm_create_stream_by_admins_only'], realm_allow_message_editing = register_ret['realm_allow_message_editing'], + realm_message_content_edit_limit_seconds = register_ret['realm_message_content_edit_limit_seconds'], realm_restricted_to_domain = register_ret['realm_restricted_to_domain'], enter_sends = user_profile.enter_sends, left_side_userlist = register_ret['left_side_userlist'], @@ -1103,8 +1104,9 @@ def update_realm(request, user_profile, name=REQ(validator=check_string, default invite_required=REQ(validator=check_bool, default=None), invite_by_admins_only=REQ(validator=check_bool, default=None), create_stream_by_admins_only=REQ(validator=check_bool, default=None), - allow_message_editing=REQ(validator=check_bool, default=None)): - # type: (HttpRequest, UserProfile, Optional[str], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool]) -> HttpResponse + allow_message_editing=REQ(validator=check_bool, default=None), + message_content_edit_limit_seconds=REQ(converter=to_non_negative_int, default=None)): + # type: (HttpRequest, UserProfile, Optional[str], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[int]) -> HttpResponse realm = user_profile.realm data = {} # type: Dict[str, Any] if name is not None and realm.name != name: @@ -1122,9 +1124,15 @@ def update_realm(request, user_profile, name=REQ(validator=check_string, default if create_stream_by_admins_only is not None and realm.create_stream_by_admins_only != create_stream_by_admins_only: do_set_realm_create_stream_by_admins_only(realm, create_stream_by_admins_only) data['create_stream_by_admins_only'] = create_stream_by_admins_only - if allow_message_editing is not None and realm.allow_message_editing != allow_message_editing: - do_set_realm_message_editing(realm, allow_message_editing) + if (allow_message_editing is not None and realm.allow_message_editing != allow_message_editing) or \ + (message_content_edit_limit_seconds is not None and realm.message_content_edit_limit_seconds != message_content_edit_limit_seconds): + if allow_message_editing is None: + allow_message_editing = realm.allow_message_editing + if message_content_edit_limit_seconds is None: + message_content_edit_limit_seconds = realm.message_content_edit_limit_seconds + do_set_realm_message_editing(realm, allow_message_editing, message_content_edit_limit_seconds) data['allow_message_editing'] = allow_message_editing + data['message_content_edit_limit_seconds'] = message_content_edit_limit_seconds return json_success(data) @csrf_exempt