"Online only" takes the offline bots off the sidebar (#948)

docs/UPDATES-PUBLIC.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- **Fixed: "Online only" in the List Browser now takes the offline bots off the list at the side.** It used to work only while you had typed something to search for - with the search box empty, ticking it changed nothing. Now ticking it hides every bot that is not in a channel right now (a bot still joining is kept, and so are your own lists and the bot you have open), and unticking it brings them back.
 - **Changed: a bot's free slots, queue and speed now show under it when you click it in the List Browser**, instead of on every row at once - where they crowded the name out on a narrow sidebar. Click a bot to see its line, click another to see that one instead, click the open one to close it. Any bot you can see there can be clicked, including one whose list you have not fetched yet.
 
 ## v1.13.1 — The Fetch Queue Looks After Itself

docs/UPDATES.md

@@ -4,6 +4,31 @@ All version changes, optimizations, and bug fixes made over time in the DCCore p
 
 ## 🟨 Unreleased
 
+### 🟢 "Online only" takes the offline bots off the sidebar (#948)
+
+#931 added the box beside the List Browser's filter, but it only reached `/api/filelists/search?online=1`, and that
+request is made only while a search term is typed. With the filter box empty, ticking it changed the box and
+nothing else on screen: the operator ticked it and the offline (red) bots were still listed.
+
+Now the sidebar leaves out a bot that is known to be away and redraws the moment the box changes, without waiting
+for the four-second poll (`state.filelistsBotRows` is the rows it was last built from).
+
+- **Only a bot known to be away goes** (`online === false`). `null` is a bot that has not finished joining, where the
+  membership mirror is empty and every nick would read as gone - it stays.
+- **Never removed:** our own lists, and the bot whose list is open (the table would be showing a list the sidebar no
+  longer has a row for).
+- **Only the row is left out.** `state.filelistsBots` still holds every bot; the fetch box, the tabs and
+  `entriesForNick()` look bots up there.
+- A soft reload can hand the box back ticked, so the state now starts from what is on screen instead of `false`.
+- Web file only (`web/app.js`); no daemon change, no new setting.
+
+`tests/test_online_only_filters_the_sidebar.py` (9): the state is declared and starts from the box, ticking redraws the
+sidebar before it searches, every bot is registered before any row is left out; and under node (skipped where it is
+not installed) the real `hiddenByOnlineOnly()` with the real `primaryEntry()`, `isOwnSource()` and `nickOfSource()`:
+nothing goes while the box is off, a bot that is here stays, one known to be away goes, one still joining stays, our
+own lists stay, the open bot stays while another away bot goes. Mutation-checked five ways: unknown presence hidden
+too, own lists hidden, the open bot hidden, the box ignored, and ticking not redrawing each fail a test.
+
 ### 👆 A bot's slots, queue and speed show under it when you click it (#943)
 
 #932 wrote each online bot's advertised free slots, queue and speed on its own sidebar row. Beside the name, in the

tests/test_online_only_filters_the_sidebar.py

@@ -0,0 +1,162 @@
+"""Ticking "Online only" takes the offline bots off the sidebar (#948).
+
+#931 added the box beside the List Browser's filter, but it only reached
+/api/filelists/search?online=1 - and that request is made only while a search
+term is typed. With the filter box empty the box changed state and nothing else
+on screen, which is what the operator saw: ticked, and the offline (red) bots
+still listed.
+
+Now the sidebar itself leaves out a bot that is KNOWN to be away, and redraws
+the moment the box changes. Left in: our own lists, the bot whose list is open
+(the table would otherwise show a list the sidebar has no row for), and a bot
+whose presence is unknown - `null` is "has not finished joining", where the
+membership mirror is empty and every nick would read as gone.
+
+The source guards read app.js, as this suite's other sidebar guards do. The
+behaviour test runs the real hiddenByOnlineOnly() - with the real
+primaryEntry(), isOwnSource() and nickOfSource() beside it - under node, and is
+skipped where node is not installed.
+"""
+
+import io
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
+import unittest
+
+REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+if REPO_ROOT not in sys.path:
+    sys.path.insert(0, REPO_ROOT)
+
+
+def app_js():
+    with io.open(os.path.join(REPO_ROOT, "web", "app.js"), encoding="utf-8") as handle:
+        return handle.read()
+
+
+def body_of(code, signature):
+    """From `signature` to the end of its braces, by counting them - enough for
+    the small helpers read here, none of which has a brace in a string."""
+    start = code.index(signature)
+    open_at = code.index("{", start)
+    depth = 0
+    for at in range(open_at, len(code)):
+        if code[at] == "{":
+            depth += 1
+        elif code[at] == "}":
+            depth -= 1
+            if depth == 0:
+                return code[start:at + 1]
+    raise AssertionError("unbalanced braces after " + signature)
+
+
+class TheSidebarAnswersTheBox(unittest.TestCase):
+
+    def setUp(self):
+        self.js = app_js()
+
+    def test_the_box_is_state_and_starts_from_what_is_on_screen(self):
+        self.assertIn("filelistsOnlineOnly: false, filelistsBotRows: null", self.js)
+        at = self.js.index("if (el.filelistsOnlineOnly) {")
+        self.assertIn("state.filelistsOnlineOnly = el.filelistsOnlineOnly.checked;",
+                      self.js[at:at + 300])
+
+    def test_ticking_it_redraws_the_sidebar_before_the_search(self):
+        handler = self.js.index('el.filelistsOnlineOnly.addEventListener("change"')
+        end = self.js.index("});", handler)
+        body = self.js[handler:end]
+        self.assertIn("renderFilelistsSwitcher(state.filelistsBotRows)", body)
+        self.assertLess(body.index("renderFilelistsSwitcher("), body.index("runFilelistsFilter()"))
+
+    def test_the_sidebar_leaves_the_row_out_but_keeps_the_bot(self):
+        """state.filelistsBots is filled for EVERY bot before any row is left
+        out: the rest of the page - the fetch box, the tabs, entriesForNick() -
+        looks bots up there, and a bot that is merely not shown is still a bot."""
+        body = body_of(self.js, "function renderFilelistsSwitcher(rows)")
+        self.assertLess(body.index("state.filelistsBots[row.bot] = row;"),
+                        body.index("hiddenByOnlineOnly("))
+        self.assertIn("state.filelistsBotRows = rows;", body)
+        self.assertIn("if (hiddenByOnlineOnly(groupsByNick[nickKey])) { return; }", body)
+
+
+HARNESS = r"""
+const fs = require("fs");
+const src = fs.readFileSync(process.argv[2], "utf8");
+function fn(signature) {
+  const start = src.indexOf(signature);
+  if (start < 0) { throw new Error("missing: " + signature); }
+  let depth = 0, i = src.indexOf("{", start);
+  for (; i < src.length; i++) {
+    if (src[i] === "{") { depth++; }
+    else if (src[i] === "}") { depth--; if (depth === 0) { break; } }
+  }
+  return src.slice(start, i + 1);
+}
+const code = ["function splitFetchedSource(", "function nickOfSource(", "function isOwnSource(",
+  "function primaryEntry(", "function hiddenByOnlineOnly("].map(fn).join("\n");
+const make = new Function("state", code + "\nreturn hiddenByOnlineOnly;");
+
+function group(nick, online, list) {
+  return { nick: nick, entries: [{ bot: list || nick, nick: nick, online: online }] };
+}
+const out = [];
+function ask(label, state, g) { out.push(label + "=" + make(state)(g)); }
+
+ask("off_offline", { filelistsOnlineOnly: false, filelistsSource: "__own__" }, group("Gone", false));
+const on = { filelistsOnlineOnly: true, filelistsSource: "__own__" };
+ask("on_here", on, group("Here", true));
+ask("on_gone", on, group("Gone", false));
+ask("on_unknown", on, group("Joining", null));
+ask("on_own_list", on, group("__own__", false));
+ask("on_own_second_list", on, group("__own__:video", false, "__own__:video"));
+ask("on_open_bot_gone", { filelistsOnlineOnly: true, filelistsSource: "GoneBot/rar" }, group("GoneBot", false));
+ask("on_other_bot_gone", { filelistsOnlineOnly: true, filelistsSource: "SomeoneElse" }, group("GoneBot", false));
+console.log(out.join("\n"));
+"""
+
+
+@unittest.skipUnless(shutil.which("node"), "node is not installed; CI's runners have it")
+class TheRealHelperDecidesWhichRowsGo(unittest.TestCase):
+
+    def seen(self):
+        handle, path = tempfile.mkstemp(suffix=".js")
+        try:
+            with os.fdopen(handle, "w", encoding="utf-8") as out:
+                out.write(HARNESS)
+            done = subprocess.run(["node", path, os.path.join(REPO_ROOT, "web", "app.js")],
+                                  capture_output=True, timeout=60)
+        finally:
+            os.unlink(path)
+        self.assertEqual(done.returncode, 0, done.stderr.decode("utf-8", "replace"))
+        return dict(line.split("=", 1) for line in done.stdout.decode("utf-8").splitlines())
+
+    def test_nothing_goes_while_the_box_is_off(self):
+        self.assertEqual(self.seen()["off_offline"], "false")
+
+    def test_a_bot_that_is_here_stays(self):
+        self.assertEqual(self.seen()["on_here"], "false")
+
+    def test_a_bot_known_to_be_away_goes(self):
+        self.assertEqual(self.seen()["on_gone"], "true")
+
+    def test_a_bot_that_has_not_finished_joining_stays(self):
+        """`null` is not "offline": nothing is known yet."""
+        self.assertEqual(self.seen()["on_unknown"], "false")
+
+    def test_our_own_lists_always_stay(self):
+        seen = self.seen()
+
+        self.assertEqual(seen["on_own_list"], "false")
+        self.assertEqual(seen["on_own_second_list"], "false")
+
+    def test_the_open_bot_stays_even_when_it_is_away_but_another_one_does_not(self):
+        seen = self.seen()
+
+        self.assertEqual(seen["on_open_bot_gone"], "false")
+        self.assertEqual(seen["on_other_bot_gone"], "true")
+
+
+if __name__ == "__main__":
+    unittest.main()

web/app.js

@@ -113,6 +113,10 @@
     // than asking again: the rows are already here, and a round trip per
     // click would be slower than the search that produced them.
     filelistsExcluded: {}, filelistsFilterPayload: null, filelistsMatchTerms: [],
+    // #948: the "Online only" box, and the rows the sidebar was last built
+    // from - kept so ticking the box redraws the sidebar at once instead of
+    // waiting for the next poll to hand it the same rows again.
+    filelistsOnlineOnly: false, filelistsBotRows: null,
     // #943: the one bot (lowercased nick) whose slots/queue/speed line is
     // open under its row. Set by a click on any sidebar row, so it is NOT the
     // open list - a bot we only saw advertising can be clicked for its line
@@ -1572,8 +1576,15 @@
 
   // #926: search only the lists of bots that are in a channel right now.
   if (el.filelistsOnlineOnly) {
+    // A soft reload can hand the box back ticked; the state must start from
+    // what is on screen, not from false.
+    state.filelistsOnlineOnly = el.filelistsOnlineOnly.checked;
     el.filelistsOnlineOnly.addEventListener("change", function () {
       state.filelistsOnlineOnly = el.filelistsOnlineOnly.checked;
+      // #948: the sidebar too, and now - not only the search. With no term
+      // typed the search has nothing to ask, so this used to change nothing
+      // at all on screen.
+      if (state.filelistsBotRows) { renderFilelistsSwitcher(state.filelistsBotRows); }
       runFilelistsFilter();
     });
   }
@@ -1617,6 +1628,23 @@
     }).catch(function () { markConnection(false); });
   }
 
+  // #948: whether "Online only" keeps this bot's row off the sidebar. Only a
+  // bot KNOWN to be away goes (`online === false`); `null` is a bot that has
+  // not finished joining, where the membership mirror is empty and every nick
+  // would read as gone (see presenceClass). Our own lists never go, and
+  // neither does the bot whose list is open - the table would be showing a
+  // list the sidebar no longer has a row for. Only the ROW is left out:
+  // state.filelistsBots still holds every bot, since the rest of the page
+  // looks bots up there.
+  function hiddenByOnlineOnly(group) {
+    if (!state.filelistsOnlineOnly) { return false; }
+    var primary = primaryEntry(group);
+    if (isOwnSource(primary.bot)) { return false; }
+    var open = nickOfSource(state.filelistsSource || "__own__").toLowerCase();
+    if (String(group.nick || "").toLowerCase() === open) { return false; }
+    return primary.online === false;
+  }
+
   // BUILT WITH DOM APIs, not concatenated markup. A bot nick is remote input
   // - it is whatever that bot called itself in a channel - and escapeHtml()
   // does not encode a quote, so a nick in an attribute is the break-out this
@@ -1670,7 +1698,9 @@
       }
       group.entries.push(row);
     });
+    state.filelistsBotRows = rows;
     groupOrder.forEach(function (nickKey) {
+      if (hiddenByOnlineOnly(groupsByNick[nickKey])) { return; }
       list.appendChild(botRow(groupsByNick[nickKey]));
     });