paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-23 16:14:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

message_edit: Fix live update bug in left sidebar.

Browse Source 
We've had for years a subtle bug, where after editing a topic in the
left sidebar that had previously had unread messages (but doesn't
anymore), the old topic might still appear in the sidebar.

The bug was hard to notice except for new organizations or in the
development environment, because the pre-edit topic appeared with a
sort key of -Infinity (that being the max ID in an empty list of
message IDs). But this is an important onboarding bug in reducing
faith in Zulip's topic editing just working, so I'm glad to have it
fixed.

Fixes #11901.
This commit is contained in:
Tim Abbott
2021-06-29 09:43:52 -07:00
parent 303bde6c55
commit 9173ed0fb9
3 changed files with 24 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
frontend_tests/node_tests/stream_topic_history.js
View File

@@ -256,10 +256,14 @@ test("test_unread_logic", () => {
assert.deepEqual(history, ["toPic1", "topic2"]);
const msgs = [
{id: 150, topic: "TOPIC2"}, // will be ignored
// This will be ignored as a case variant of `topic2` above.
{id: 150, topic: "TOPIC2"},
{id: 61, topic: "unread1"},
{id: 60, topic: "unread1"},
{id: 20, topic: "UNREAD2"},
// We're going to mark this as read; this will verify the logic
// in unreads.js for only including topics with nonzero unreads.
{id: 79, topic: "to_mark_as_read"},
];
for (const msg of msgs) {
@@ -269,6 +273,7 @@ test("test_unread_logic", () => {
}
unread.process_loaded_messages(msgs);
unread.mark_as_read(79);
history = stream_topic_history.get_recent_topic_names(stream_id);
assert.deepEqual(history, ["toPic1", "unread1", "topic2", "UNREAD2"]);

2
static/js/stream_topic_history.js
View File

@@ -201,6 +201,8 @@ export class PerStreamHistory {
get_recent_topic_names() {
const my_recents = Array.from(this.topics.values());
/* Add any older topics with unreads that may not be present
* in our local cache. */
const missing_topics = unread.get_missing_topics({
stream_id: this.stream_id,
topic_dict: this.topics,

16
static/js/unread.js
View File

@@ -265,6 +265,15 @@ class UnreadTopicCounter {
}
get_missing_topics(opts) {
/* Clients have essentially complete unread data, but
* stream_topic_history.is_complete_for_stream_id() can be
* false. In that situation, this function helps ensure that
* we include all topics with unread messages in data that.
*
* It will return all topics in the provided stream with a
* nonzero unread count that are not already present in the
* topic_dict parameter.
*/
const stream_id = opts.stream_id;
const topic_dict = opts.topic_dict;
@@ -275,6 +284,13 @@ class UnreadTopicCounter {
let topic_names = Array.from(per_stream_bucketer.keys());
/* Include topics that have at least one unread. It would likely
* be better design for buckets to be deleted when emptied. */
topic_names = topic_names.filter((topic_name) => {
const messages = Array.from(per_stream_bucketer.get_bucket(topic_name));
return messages.length > 0;
});
/* And aren't already present in topic_dict. */
topic_names = topic_names.filter((topic_name) => !topic_dict.has(topic_name));
const result = topic_names.map((topic_name) => {