refactor: Move all_topics_in_cache.

This is a pure code move.

The diff is bit cluttered by the necessity
to add with_field when we moved the test.

frontend_tests/node_tests/stream_data.js

@@ -13,7 +13,6 @@ const color_data = zrequire("color_data");
 const stream_topic_history = zrequire("stream_topic_history");
 const people = zrequire("people");
 const stream_data = zrequire("stream_data");
-const message_list = zrequire("message_list");
 const settings_config = zrequire("settings_config");
 
 const me = {
@@ -809,37 +808,3 @@ test("get_invite_stream_data", () => {
     });
     assert.deepEqual(stream_data.get_invite_stream_data(), expected_list);
 });
-
-test("all_topics_in_cache", (override) => {
-    // Add a new stream with first_message_id set.
-    const general = {
-        name: "general",
-        stream_id: 21,
-        first_message_id: null,
-    };
-    const messages = [
-        {id: 1, stream_id: 21},
-        {id: 2, stream_id: 21},
-        {id: 3, stream_id: 21},
-    ];
-    const sub = stream_data.create_sub_from_server_data(general);
-
-    assert.equal(stream_data.all_topics_in_cache(sub), false);
-
-    message_list.all.data.clear();
-    message_list.all.data.add_messages(messages);
-
-    let has_found_newest = false;
-
-    override(message_list.all.data.fetch_status, "has_found_newest", () => has_found_newest);
-
-    assert.equal(stream_data.all_topics_in_cache(sub), false);
-    has_found_newest = true;
-    assert.equal(stream_data.all_topics_in_cache(sub), true);
-
-    sub.first_message_id = 0;
-    assert.equal(stream_data.all_topics_in_cache(sub), false);
-
-    sub.first_message_id = 2;
-    assert.equal(stream_data.all_topics_in_cache(sub), true);
-});

frontend_tests/node_tests/stream_topic_history.js

@@ -2,13 +2,13 @@
 
 const {strict: assert} = require("assert");
 
-const {mock_esm, zrequire} = require("../zjsunit/namespace");
+const {mock_esm, with_field, zrequire} = require("../zjsunit/namespace");
 const {run_test} = require("../zjsunit/test");
 
 const channel = mock_esm("../../static/js/channel");
-const message_list = mock_esm("../../static/js/message_list");
 const message_util = mock_esm("../../static/js/message_util");
 
+const message_list = zrequire("message_list");
 const unread = zrequire("unread");
 const stream_data = zrequire("stream_data");
 const stream_topic_history = zrequire("stream_topic_history");
@@ -128,7 +128,7 @@ test("is_complete_for_stream_id", () => {
     };
     stream_data.add_sub(sub);
 
-    message_list.all = {
+    const message_list_all = {
         empty: () => false,
         data: {
             fetch_status: {
@@ -138,20 +138,22 @@ test("is_complete_for_stream_id", () => {
         first: () => ({id: 5}),
     };
 
-    assert.equal(stream_topic_history.is_complete_for_stream_id(sub.stream_id), true);
+    with_field(message_list, "all", message_list_all, () => {
+        assert.equal(stream_topic_history.is_complete_for_stream_id(sub.stream_id), true);
 
-    // Now simulate a more recent message id.
-    message_list.all.first = () => ({id: sub.first_message_id + 1});
+        // Now simulate a more recent message id.
+        message_list.all.first = () => ({id: sub.first_message_id + 1});
 
-    // Note that we'll return `true` here due to
-    // fetched_stream_ids having the stream_id now.
-    assert.equal(stream_topic_history.is_complete_for_stream_id(sub.stream_id), true);
+        // Note that we'll return `true` here due to
+        // fetched_stream_ids having the stream_id now.
+        assert.equal(stream_topic_history.is_complete_for_stream_id(sub.stream_id), true);
 
-    // But now clear the data to see what we'd have without
-    // the previous call.
-    stream_topic_history.reset();
+        // But now clear the data to see what we'd have without
+        // the previous call.
+        stream_topic_history.reset();
 
-    assert.equal(stream_topic_history.is_complete_for_stream_id(sub.stream_id), false);
+        assert.equal(stream_topic_history.is_complete_for_stream_id(sub.stream_id), false);
+    });
 });
 
 test("server_history", () => {
@@ -336,3 +338,37 @@ test("server_history_end_to_end", () => {
     });
     assert(on_success_called);
 });
+
+test("all_topics_in_cache", (override) => {
+    // Add a new stream with first_message_id set.
+    const general = {
+        name: "general",
+        stream_id: 21,
+        first_message_id: null,
+    };
+    const messages = [
+        {id: 1, stream_id: 21},
+        {id: 2, stream_id: 21},
+        {id: 3, stream_id: 21},
+    ];
+    const sub = stream_data.create_sub_from_server_data(general);
+
+    assert.equal(stream_topic_history.all_topics_in_cache(sub), false);
+
+    message_list.all.data.clear();
+    message_list.all.data.add_messages(messages);
+
+    let has_found_newest = false;
+
+    override(message_list.all.data.fetch_status, "has_found_newest", () => has_found_newest);
+
+    assert.equal(stream_topic_history.all_topics_in_cache(sub), false);
+    has_found_newest = true;
+    assert.equal(stream_topic_history.all_topics_in_cache(sub), true);
+
+    sub.first_message_id = 0;
+    assert.equal(stream_topic_history.all_topics_in_cache(sub), false);
+
+    sub.first_message_id = 2;
+    assert.equal(stream_topic_history.all_topics_in_cache(sub), true);
+});

static/js/stream_data.js

@@ -3,7 +3,6 @@ import * as color_data from "./color_data";
 import {FoldDict} from "./fold_dict";
 import * as hash_util from "./hash_util";
 import {i18n} from "./i18n";
-import * as message_list from "./message_list";
 import {page_params} from "./page_params";
 import * as peer_data from "./peer_data";
 import * as people from "./people";
@@ -602,39 +601,6 @@ export function get_invite_only(stream_name) {
     return sub.invite_only;
 }
 
-export function all_topics_in_cache(sub) {
-    // Checks whether this browser's cache of contiguous messages
-    // (used to locally render narrows) in message_list.all has all
-    // messages from a given stream, and thus all historical topics
-    // for it.  Because message_list.all is a range, we just need to
-    // compare it to the range of history on the stream.
-
-    // If the cache isn't initialized, it's a clear false.
-    if (message_list.all === undefined || message_list.all.empty()) {
-        return false;
-    }
-
-    // If the cache doesn't have the latest messages, we can't be sure
-    // we have all topics.
-    if (!message_list.all.data.fetch_status.has_found_newest()) {
-        return false;
-    }
-
-    if (sub.first_message_id === null) {
-        // If the stream has no message history, we have it all
-        // vacuously.  This should be a very rare condition, since
-        // stream creation sends a message.
-        return true;
-    }
-
-    // Now, we can just compare the first cached message to the first
-    // message ID in the stream; if it's older, we're good, otherwise,
-    // we might be missing the oldest topics in this stream in our
-    // cache.
-    const first_cached_message = message_list.all.first();
-    return first_cached_message.id <= sub.first_message_id;
-}
-
 export function set_realm_default_streams(realm_default_streams) {
     default_stream_ids.clear();
 

static/js/stream_topic_history.js

@@ -1,5 +1,6 @@
 import * as channel from "./channel";
 import {FoldDict} from "./fold_dict";
+import * as message_list from "./message_list";
 import * as message_util from "./message_util";
 import * as stream_data from "./stream_data";
 import * as unread from "./unread";
@@ -7,20 +8,46 @@ import * as unread from "./unread";
 const stream_dict = new Map(); // stream_id -> PerStreamHistory object
 const fetched_stream_ids = new Set();
 
+export function all_topics_in_cache(sub) {
+    // Checks whether this browser's cache of contiguous messages
+    // (used to locally render narrows) in message_list.all has all
+    // messages from a given stream, and thus all historical topics
+    // for it.  Because message_list.all is a range, we just need to
+    // compare it to the range of history on the stream.
+
+    // If the cache isn't initialized, it's a clear false.
+    if (message_list.all === undefined || message_list.all.empty()) {
+        return false;
+    }
+
+    // If the cache doesn't have the latest messages, we can't be sure
+    // we have all topics.
+    if (!message_list.all.data.fetch_status.has_found_newest()) {
+        return false;
+    }
+
+    if (sub.first_message_id === null) {
+        // If the stream has no message history, we have it all
+        // vacuously.  This should be a very rare condition, since
+        // stream creation sends a message.
+        return true;
+    }
+
+    // Now, we can just compare the first cached message to the first
+    // message ID in the stream; if it's older, we're good, otherwise,
+    // we might be missing the oldest topics in this stream in our
+    // cache.
+    const first_cached_message = message_list.all.first();
+    return first_cached_message.id <= sub.first_message_id;
+}
+
 export function is_complete_for_stream_id(stream_id) {
     if (fetched_stream_ids.has(stream_id)) {
         return true;
     }
 
-    /*
-        TODO: We should possibly move all_topics_in_cache
-        from stream_data to here, since the function
-        mostly looks at message_list.all and has little
-        to do with typical stream_data stuff.  (We just
-        need sub.first_message_id.)
-    */
     const sub = stream_data.get_sub_by_id(stream_id);
-    const in_cache = stream_data.all_topics_in_cache(sub);
+    const in_cache = all_topics_in_cache(sub);
 
     if (in_cache) {
         /*