move_topic_to_stream: Add option to disable breadcrumb messages.

Option to disable breadcrumb messages were given in both message edit
form and topic edit stream popover.

User now has the option to select which stream to send the notification
of stream edit of a topic via checkboxes in the UI.
This commit is contained in:
Aman Agrawal 2020-06-03 20:14:57 +05:30 committed by Tim Abbott
parent 5f50e15ec5
commit a096f34cab
12 changed files with 202 additions and 22 deletions

View File

@ -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,

View File

@ -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');
}
},

View File

@ -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;

View File

@ -18,6 +18,18 @@
</select>
<i class="fa fa-angle-right" aria-hidden="true" {{#unless show_edit_stream}}style="display:none"{{/unless}}></i>
<input type="text" placeholder="{{topic}}" value="{{topic}}" class="message_edit_topic" id="message_edit_topic" />
<div class="message_edit_breadcrumb_messages" style='display:none;'>
<label class="checkbox">
<input class="send_notification_to_new_thread" name="send_notification_to_new_thread" type="checkbox" {{#if notify_new_thread}}checked="checked"{{/if}}>
<span></span>
</label>
<label for="send_notification_to_new_thread">{{t "Send notification to new thread" }}</label>
<label class="checkbox">
<input class="send_notification_to_old_thread" name="send_notification_to_old_thread" type="checkbox" {{#if notify_old_thread}}checked="checked"{{/if}}>
<span></span>
</label>
<label for="send_notification_to_old_thread">{{t "Send notification to old thread" }}</label>
</div>
<select class='message_edit_topic_propagate' style='display:none;'>
<option selected="selected" value="change_later"> {{t "Change later messages to this topic" }}</option>
<option value="change_one"> {{t "Change only this message topic" }}</option>

View File

@ -22,6 +22,18 @@
<input name="new_topic_name" type="text" class="inline_topic_edit" value="{{topic_name}}">
<input name="old_topic_name" type="hidden" class="inline_topic_edit" value="{{topic_name}}">
<input name="current_stream_id" type="hidden" value="{{current_stream_id}}">
<div class="topic_move_breadcrumb_messages new-style">
<label class="checkbox">
<input class="send_notification_to_new_thread" name="send_notification_to_new_thread" type="checkbox" {{#if notify_new_thread}}checked="checked"{{/if}}>
<span></span>
</label>
<label for="send_notification_to_new_thread">{{t "Send notification to new topic" }}</label>
<label class="checkbox">
<input class="send_notification_to_old_thread" name="send_notification_to_old_thread" type="checkbox" {{#if notify_old_thread}}checked="checked"{{/if}}>
<span></span>
</label>
<label for="send_notification_to_old_thread">{{t "Send notification to old topic" }}</label>
</div>
</div>
</form>
<div class="topic_edit_spinner"></div>

View File

@ -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**

View File

@ -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

View File

@ -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)

View File

@ -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:

View File

@ -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,

View File

@ -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:

View File

@ -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)