stream_topic_history: Update topic last msg_id after msg deletion.

Fixes #15992.
If the last message of the topic was deleted, we update the stored
message_id in the topic history so that the topic order in topic_list
is updated correctly.
This commit is contained in:
Aman Agrawal 2020-08-04 14:42:42 +05:30 committed by Tim Abbott
parent dc2b9fd4f6
commit b32dc5e58f
6 changed files with 56 additions and 1 deletions

View File

@ -813,6 +813,7 @@ run_test("delete_message", (override) => {
assert_same(args.opts.stream_id, 99);
assert_same(args.opts.topic_name, "topic1");
assert_same(args.opts.num_messages, 1);
assert_same(args.opts.max_removed_msg_id, 1337);
});
});

View File

@ -41,11 +41,40 @@ run_test("basics", () => {
assert.deepEqual(history, ["topic2", "Topic1"]);
assert.deepEqual(max_message_id, 103);
stream_topic_history.add_message({
stream_id,
message_id: 104,
topic_name: "Topic1",
});
history = stream_topic_history.get_recent_topic_names(stream_id);
max_message_id = stream_topic_history.get_max_message_id(stream_id);
assert.deepEqual(history, ["Topic1", "topic2"]);
assert.deepEqual(max_message_id, 104);
set_global("message_util", {
get_messages_in_topic: () => [{id: 101}, {id: 102}],
});
// Removing the last msg in topic1 changes the order
stream_topic_history.remove_messages({
stream_id,
topic_name: "Topic1",
num_messages: 1,
max_removed_msg_id: 104,
});
history = stream_topic_history.get_recent_topic_names(stream_id);
assert.deepEqual(history, ["topic2", "Topic1"]);
max_message_id = stream_topic_history.get_max_message_id(stream_id);
assert.deepEqual(max_message_id, 104);
set_global("message_util", {
get_messages_in_topic: () => [{id: 102}],
});
// Removing first topic1 message has no effect.
stream_topic_history.remove_messages({
stream_id,
topic_name: "toPic1",
num_messages: 1,
max_removed_msg_id: 101,
});
history = stream_topic_history.get_recent_topic_names(stream_id);
assert.deepEqual(history, ["topic2", "Topic1"]);
@ -58,6 +87,7 @@ run_test("basics", () => {
stream_id,
topic_name: "Topic1",
num_messages: 1,
max_removed_msg_id: 102,
});
history = stream_topic_history.get_recent_topic_names(stream_id);
assert.deepEqual(history, ["topic2"]);
@ -67,6 +97,7 @@ run_test("basics", () => {
stream_id,
topic_name: "Topic1",
num_messages: 1,
max_removed_msg_id: 0,
});
history = stream_topic_history.get_recent_topic_names(stream_id);
assert.deepEqual(history, ["topic2"]);

View File

@ -202,6 +202,7 @@ exports.edit_locally = function (message, request) {
stream_id: message.stream_id,
topic_name: message.topic,
num_messages: 1,
max_removed_msg_id: message.id,
});
if (new_stream_id !== undefined) {

View File

@ -196,6 +196,7 @@ exports.update_messages = function update_messages(events) {
stream_id: msg.stream_id,
topic_name: msg.topic,
num_messages: 1,
max_removed_msg_id: msg.id,
});
// Update the unread counts; again, this must be called

View File

@ -33,17 +33,20 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) {
// which returns all the unread messages out of a given list.
// So double marking something as read would not occur
unread_ops.process_read_messages_event(msg_ids);
// This methods updates message_list too and since stream_topic_history relies on it
// this method should be called first.
ui.remove_messages(msg_ids);
if (event.message_type === "stream") {
stream_topic_history.remove_messages({
stream_id: event.stream_id,
topic_name: event.topic,
num_messages: msg_ids.length,
max_removed_msg_id: Math.max(...msg_ids),
});
stream_list.update_streams_sidebar();
}
ui.remove_messages(msg_ids);
break;
}

View File

@ -202,6 +202,7 @@ exports.remove_messages = function (opts) {
const stream_id = opts.stream_id;
const topic_name = opts.topic_name;
const num_messages = opts.num_messages;
const max_removed_msg_id = opts.max_removed_msg_id;
const history = stream_dict.get(stream_id);
// This is the special case of "removing" a message from
@ -213,6 +214,23 @@ exports.remove_messages = function (opts) {
// This is the normal case of an incoming message.
history.maybe_remove(topic_name, num_messages);
const existing_topic = history.topics.get(topic_name);
if (!existing_topic) {
return;
}
// Update max_message_id in topic
if (existing_topic.message_id <= max_removed_msg_id) {
const msgs_in_topic = message_util.get_messages_in_topic(stream_id, topic_name);
let max_message_id = 0;
for (const msg of msgs_in_topic) {
if (msg.id > max_message_id) {
max_message_id = msg.id;
}
}
existing_topic.message_id = max_message_id;
}
};
exports.find_or_create = function (stream_id) {