diff --git a/frontend_tests/node_tests/stream_topic_history.js b/frontend_tests/node_tests/stream_topic_history.js index 8cadbb2c16..b9f8a8c1dc 100644 --- a/frontend_tests/node_tests/stream_topic_history.js +++ b/frontend_tests/node_tests/stream_topic_history.js @@ -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"]); diff --git a/static/js/stream_topic_history.js b/static/js/stream_topic_history.js index 715c50160c..03a4987310 100644 --- a/static/js/stream_topic_history.js +++ b/static/js/stream_topic_history.js @@ -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, diff --git a/static/js/unread.js b/static/js/unread.js index c218641ec3..08a98f5ba5 100644 --- a/static/js/unread.js +++ b/static/js/unread.js @@ -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) => {