diff --git a/static/js/message_edit.js b/static/js/message_edit.js index 5df0ad5f75..150503433e 100644 --- a/static/js/message_edit.js +++ b/static/js/message_edit.js @@ -6,6 +6,12 @@ const currently_editing_messages = new Map(); let currently_deleting_messages = []; const currently_echoing_messages = new Map(); +// These variables are designed to preserve the user's most recent +// choices when editing a group of messages, to make it convenient to +// move several topics in a row with the same settings. +exports.notify_old_thread_default = true; +exports.notify_new_thread_default = true; + const editability_types = { NO: 1, NO_LONGER: 2, @@ -283,6 +289,8 @@ function edit_message(row, raw_content) { available_streams: available_streams, stream_id: message.stream_id, stream_name: message.stream, + notify_new_thread: exports.notify_new_thread_default, + notify_old_thread: exports.notify_old_thread_default, })); const edit_obj = {form: form, raw_content: raw_content}; @@ -298,6 +306,7 @@ function edit_message(row, raw_content) { const message_edit_content = row.find('textarea.message_edit_content'); const message_edit_topic = row.find('input.message_edit_topic'); const message_edit_topic_propagate = row.find('select.message_edit_topic_propagate'); + const message_edit_breadcrumb_messages = row.find('div.message_edit_breadcrumb_messages'); const message_edit_countdown_timer = row.find('.message_edit_countdown_timer'); const copy_message = row.find('.copy_message'); @@ -380,6 +389,7 @@ function edit_message(row, raw_content) { if (message.type === 'stream') { message_edit_topic.prop("readonly", "readonly"); message_edit_topic_propagate.hide(); + message_edit_breadcrumb_messages.hide(); } // We don't go directly to a "TOPIC_ONLY" type state (with an active Save button), // since it isn't clear what to do with the half-finished edit. It's nice to keep @@ -424,6 +434,7 @@ function edit_message(row, raw_content) { const is_topic_edited = new_topic !== original_topic && new_topic !== ""; const is_stream_edited = new_stream_id !== original_stream_id; message_edit_topic_propagate.toggle(is_topic_edited || is_stream_edited); + message_edit_breadcrumb_messages.toggle(is_stream_edited); } if (!message.locally_echoed) { @@ -628,7 +639,13 @@ exports.save_message_row_edit = function (row) { if (topic_changed || stream_changed) { const selected_topic_propagation = row.find("select.message_edit_topic_propagate").val() || "change_later"; + const send_notification_to_old_thread = row.find('.send_notification_to_old_thread').is(':checked'); + const send_notification_to_new_thread = row.find('.send_notification_to_new_thread').is(':checked'); request.propagate_mode = selected_topic_propagation; + request.send_notification_to_old_thread = send_notification_to_old_thread; + request.send_notification_to_new_thread = send_notification_to_new_thread; + exports.notify_old_thread_default = send_notification_to_old_thread; + exports.notify_new_thread_default = send_notification_to_new_thread; changed = true; } @@ -904,12 +921,17 @@ exports.handle_narrow_deactivated = function () { }; exports.move_topic_containing_message_to_stream = - function (message_id, new_stream_id, new_topic_name) { + function (message_id, new_stream_id, new_topic_name, send_notification_to_new_thread, + send_notification_to_old_thread) { const request = { stream_id: new_stream_id, propagate_mode: 'change_all', topic: new_topic_name, + send_notification_to_old_thread: send_notification_to_old_thread, + send_notification_to_new_thread: send_notification_to_new_thread, }; + exports.notify_old_thread_default = send_notification_to_old_thread; + exports.notify_new_thread_default = send_notification_to_new_thread; channel.patch({ url: '/json/messages/' + message_id, data: request, diff --git a/static/js/stream_popover.js b/static/js/stream_popover.js index 1454092496..d95a4562bc 100644 --- a/static/js/stream_popover.js +++ b/static/js/stream_popover.js @@ -273,7 +273,11 @@ function build_move_topic_to_stream_popover(e, current_stream_id, topic_name) { // streams in the future. const available_streams = stream_data.subscribed_subs() .filter(s => s.stream_id !== current_stream_id); - const args = { available_streams, topic_name, current_stream_id }; + const args = { + available_streams, topic_name, current_stream_id, + notify_new_thread: message_edit.notify_new_thread_default, + notify_old_thread: message_edit.notify_old_thread_default, + }; exports.hide_topic_popover(); @@ -551,8 +555,11 @@ exports.register_topic_handlers = function () { }, {}); const {old_topic_name, select_stream_id} = params; - let {current_stream_id, new_topic_name} = params; + let {current_stream_id, new_topic_name, + send_notification_to_new_thread, send_notification_to_old_thread} = params; new_topic_name = new_topic_name.trim(); + send_notification_to_new_thread = send_notification_to_new_thread === 'on'; + send_notification_to_old_thread = send_notification_to_old_thread === 'on'; current_stream_id = parseInt(current_stream_id, 10); // The API endpoint for editing messages to change their @@ -594,7 +601,9 @@ exports.register_topic_handlers = function () { if (old_topic_name && select_stream_id) { message_edit.move_topic_containing_message_to_stream( - message_id, select_stream_id, new_topic_name); + message_id, select_stream_id, new_topic_name, + send_notification_to_new_thread, + send_notification_to_old_thread); $('#move_topic_modal').modal('hide'); } }, diff --git a/static/styles/zulip.scss b/static/styles/zulip.scss index 7cbef11bd5..b22e0c224b 100644 --- a/static/styles/zulip.scss +++ b/static/styles/zulip.scss @@ -1358,6 +1358,31 @@ div.focused_table { max-width: 100%; } +.topic_move_breadcrumb_messages, +.message_edit_breadcrumb_messages { + display: inline-block; + margin: 0 5px 5px 0 !important; + align-self: center; + + label.checkbox { + margin-right: 0 !important; + + input { + margin: 0; + vertical-align: baseline; + } + } + + label { + display: inline-block; + margin-right: 10px; + } +} + +.topic_move_breadcrumb_messages { + margin-top: 10px !important; +} + .message_length_controller { display: none; text-align: center; diff --git a/static/templates/message_edit_form.hbs b/static/templates/message_edit_form.hbs index ef3bf33fd6..d8e689e6af 100644 --- a/static/templates/message_edit_form.hbs +++ b/static/templates/message_edit_form.hbs @@ -18,6 +18,18 @@ + +
+ + + + +
diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 80677e36d4..da3850956f 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -10,6 +10,8 @@ below features are supported. ## Changes in Zulip 2.2 +**Feature level 10** + **Feature level 9** * [`POST users/me/subscriptions`](/api/add-subscriptions), [`DELETE @@ -17,6 +19,9 @@ below features are supported. subscribe/unsubscribe, declared in the `principals` parameter, can now be referenced by user_id, rather than Zulip display email address. +* [PATCH /messages/{message_id}](/api/update-message): Added + `send_notification_to_old_thread` and + `send_notification_to_new_thread` optional parameters. **Feature level 8** diff --git a/version.py b/version.py index 9296cc36c1..fe2531737d 100644 --- a/version.py +++ b/version.py @@ -29,7 +29,7 @@ DESKTOP_WARNING_VERSION = "5.2.0" # # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md. -API_FEATURE_LEVEL = 8 +API_FEATURE_LEVEL = 9 # 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/lib/actions.py b/zerver/lib/actions.py index e75369a697..86f350e1ac 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -4188,7 +4188,9 @@ class MessageUpdateUserInfoResult(TypedDict): def notify_topic_moved_streams(user_profile: UserProfile, old_stream: Stream, old_topic: str, - new_stream: Stream, new_topic: Optional[str]) -> None: + new_stream: Stream, new_topic: Optional[str], + send_notification_to_old_thread: bool, + send_notification_to_new_thread: bool) -> None: # Since moving content between streams is highly disruptive, # it's worth adding a couple tombstone messages showing what # happened. @@ -4200,17 +4202,18 @@ def notify_topic_moved_streams(user_profile: UserProfile, user_mention = "@_**%s|%s**" % (user_profile.full_name, user_profile.id) old_topic_link = "#**%s>%s**" % (old_stream.name, old_topic) new_topic_link = "#**%s>%s**" % (new_stream.name, new_topic) + if send_notification_to_new_thread: + internal_send_stream_message( + new_stream.realm, sender, new_stream, new_topic, + _("This topic was moved here from %(old_location)s by %(user)s") + % dict(old_location=old_topic_link, user=user_mention)) - internal_send_stream_message( - new_stream.realm, sender, new_stream, new_topic, - _("This topic was moved here from %(old_location)s by %(user)s") - % dict(old_location=old_topic_link, user=user_mention)) - - # Send a notification to the old stream that the topic was moved. - internal_send_stream_message( - old_stream.realm, sender, old_stream, old_topic, - _("This topic was moved by %(user)s to %(new_location)s") - % dict(user=user_mention, new_location=new_topic_link)) + if send_notification_to_old_thread: + # Send a notification to the old stream that the topic was moved. + internal_send_stream_message( + old_stream.realm, sender, old_stream, old_topic, + _("This topic was moved by %(user)s to %(new_location)s") + % dict(user=user_mention, new_location=new_topic_link)) def get_user_info_for_message_updates(message_id: int) -> MessageUpdateUserInfoResult: @@ -4322,7 +4325,8 @@ def do_update_embedded_data(user_profile: UserProfile, @transaction.atomic def do_update_message(user_profile: UserProfile, message: Message, new_stream: Optional[Stream], topic_name: Optional[str], - propagate_mode: str, content: Optional[str], + propagate_mode: str, send_notification_to_old_thread: bool, + send_notification_to_new_thread: bool, content: Optional[str], rendered_content: Optional[str], prior_mention_user_ids: Set[int], mention_user_ids: Set[int], mention_data: Optional[bugdown.MentionData]=None) -> int: """ @@ -4527,7 +4531,8 @@ def do_update_message(user_profile: UserProfile, message: Message, stream_being_edited is not None): # Notify users that the topic was moved. notify_topic_moved_streams(user_profile, stream_being_edited, orig_topic_name, - new_stream, topic_name) + new_stream, topic_name, send_notification_to_old_thread, + send_notification_to_new_thread) return len(changed_messages) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 22d080cd51..2587f3f943 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -898,9 +898,9 @@ paths: - name: propagate_mode in: query description: | - "Which message(s) should be edited: just the one indicated in + Which message(s) should be edited: just the one indicated in `message_id`, messages in the same topic that had been sent after this - one, or all of them." + one, or all of them. schema: type: string enum: @@ -909,6 +909,28 @@ paths: - change_all default: change_one example: change_all + - name: send_notification_to_old_thread + in: query + description: | + Whether to send breadcrumb message to the old thread to + notify users where the messages were moved to. + + **Changes**: New in Zulip 2.2 (feature level 9). + schema: + type: boolean + default: true + example: true + - name: send_notification_to_new_thread + in: query + description: | + Whether to send a notification message to the new thread to + notify users where the messages came from. + + **Changes**: New in Zulip 2.2 (feature level 9). + schema: + type: boolean + default: true + example: true - $ref: '#/components/parameters/Content' - $ref: '#/components/parameters/StreamIdInQuery' responses: diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 003f5c3968..757b51cce0 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -745,7 +745,7 @@ class EventsRegisterTest(ZulipTestCase): events = self.do_test( lambda: do_update_message(self.user_profile, message, None, topic, - propagate_mode, content, rendered_content, + propagate_mode, False, False, content, rendered_content, prior_mention_user_ids, mentioned_user_ids, mention_data), state_change_expected=True, diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index 5ae5760a21..0a480a7d8d 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -3264,6 +3264,8 @@ class EditMessageTest(ZulipTestCase): new_stream=None, topic_name=topic_name, propagate_mode="change_later", + send_notification_to_old_thread=False, + send_notification_to_new_thread=False, content=None, rendered_content=None, prior_mention_user_ids=set(), @@ -3540,6 +3542,68 @@ class EditMessageTest(ZulipTestCase): self.assertEqual(messages[3].content, "This topic was moved here from #**test move stream>test** by @_**Iago|%s**" % (user_profile.id,)) self.assert_json_success(result) + def test_no_notify_move_message_to_stream(self) -> None: + (user_profile, old_stream, new_stream, msg_id, msg_id_lt) = self.prepare_move_topics( + "iago", "test move stream", "new stream", "test") + + result = self.client_patch("/json/messages/" + str(msg_id), { + 'message_id': msg_id, + 'stream_id': new_stream.id, + 'propagate_mode': 'change_all', + 'send_notification_to_old_thread': 'false', + 'send_notification_to_new_thread': 'false', + }) + + self.assert_json_success(result) + + messages = get_topic_messages(user_profile, old_stream, "test") + self.assertEqual(len(messages), 0) + + messages = get_topic_messages(user_profile, new_stream, "test") + self.assertEqual(len(messages), 3) + + def test_notify_new_thread_move_message_to_stream(self) -> None: + (user_profile, old_stream, new_stream, msg_id, msg_id_lt) = self.prepare_move_topics( + "iago", "test move stream", "new stream", "test") + + result = self.client_patch("/json/messages/" + str(msg_id), { + 'message_id': msg_id, + 'stream_id': new_stream.id, + 'propagate_mode': 'change_all', + 'send_notification_to_old_thread': 'false', + 'send_notification_to_new_thread': 'true', + }) + + self.assert_json_success(result) + + messages = get_topic_messages(user_profile, old_stream, "test") + self.assertEqual(len(messages), 0) + + messages = get_topic_messages(user_profile, new_stream, "test") + self.assertEqual(len(messages), 4) + self.assertEqual(messages[3].content, "This topic was moved here from #**test move stream>test** by @_**Iago|%d**" % (user_profile.id,)) + + def test_notify_old_thread_move_message_to_stream(self) -> None: + (user_profile, old_stream, new_stream, msg_id, msg_id_lt) = self.prepare_move_topics( + "iago", "test move stream", "new stream", "test") + + result = self.client_patch("/json/messages/" + str(msg_id), { + 'message_id': msg_id, + 'stream_id': new_stream.id, + 'propagate_mode': 'change_all', + 'send_notification_to_old_thread': 'true', + 'send_notification_to_new_thread': 'false', + }) + + self.assert_json_success(result) + + messages = get_topic_messages(user_profile, old_stream, "test") + self.assertEqual(len(messages), 1) + self.assertEqual(messages[0].content, "This topic was moved by @_**Iago|%s** to #**new stream>test**" % (user_profile.id,)) + + messages = get_topic_messages(user_profile, new_stream, "test") + self.assertEqual(len(messages), 3) + def test_move_message_to_stream_to_private_stream(self) -> None: user_profile = self.example_user("iago") self.login("iago") @@ -4152,7 +4216,7 @@ class MessageHasKeywordsTest(ZulipTestCase): realm_id = hamlet.realm.id rendered_content = render_markdown(msg, content) mention_data = bugdown.MentionData(realm_id, content) - do_update_message(hamlet, msg, None, None, "change_one", content, + do_update_message(hamlet, msg, None, None, "change_one", False, False, content, rendered_content, set(), set(), mention_data=mention_data) def test_finds_link_after_edit(self) -> None: diff --git a/zerver/views/messages.py b/zerver/views/messages.py index 14979772c1..58620413b9 100644 --- a/zerver/views/messages.py +++ b/zerver/views/messages.py @@ -1503,6 +1503,8 @@ def update_message_backend(request: HttpRequest, user_profile: UserMessage, propagate_mode: Optional[str]=REQ( default="change_one", str_validator=check_string_in(PROPAGATE_MODE_VALUES)), + send_notification_to_old_thread: bool=REQ(default=True, validator=check_bool), + send_notification_to_new_thread: bool=REQ(default=True, validator=check_bool), content: Optional[str]=REQ(default=None)) -> HttpResponse: if not user_profile.realm.allow_message_editing: return json_error(_("Your organization has turned off message editing")) @@ -1607,6 +1609,8 @@ def update_message_backend(request: HttpRequest, user_profile: UserMessage, number_changed = do_update_message(user_profile, message, new_stream, topic_name, propagate_mode, + send_notification_to_old_thread, + send_notification_to_new_thread, content, rendered_content, prior_mention_user_ids, mention_user_ids, mention_data)