diff options
Diffstat (limited to 'packages')
3 files changed, 284 insertions, 1 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js index 6b9c82a..7d13260 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js @@ -96,6 +96,30 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, // every registered app when a node predates the setting (or hasn't answered // yet), so nothing disappears for an existing group. const [enabledApps, setEnabledApps] = useState(null); + // Declared here rather than beside the render, because the effect below + // depends on it and a `const` further down would be in its temporal dead + // zone — the hook-ordering trap this codebase has already paid for. + const apps = visibleApps(enabledApps); + + // The landing tab is chosen before the node has said which applications this + // group has, and a preference is a preference — not a promise that the app + // exists here. Two ways to land on a tab that renders nothing at all, with no + // tab shown active and no way to tell what went wrong: the group has Chat + // disabled while 'chat' is the default, or the reader's preferred app is one + // this group does not run. The first app the group *does* offer is the + // answer to both. + // + // Also covers an operator disabling the app someone is currently looking at: + // `enabledApps` changes live over `apps_enabled`, and being moved to a + // working tab beats being left staring at an empty panel. + // + // Settings is exempt: it is not an application, it is never in `apps`, and + // the create-group wizard lands on it deliberately. + useEffect(() => { + if (tab === 'settings') return; + if (!apps.length || apps.some(a => a.key === tab)) return; + setTab(apps[0].key); + }, [enabledApps, tab]); // Reconcile interval / debounce currently in effect on the node — shown // to the operator in Settings, not enforced from here (indexer.py owns // that). Null until the handshake ack arrives. @@ -532,7 +556,6 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, setPreviewEntry(entry); }, [entries, onPlayQueue, onStopMusic]); - const apps = visibleApps(enabledApps); const commonProps = { groupId, transportRef, gekRef, status, username, entries, nodeDirs, nodeRoots, setEntries, setNodeDirs, setNodeRoots, applyIndex, diff --git a/packages/meshbay-hub/tests/harness/group_tab_probe.py b/packages/meshbay-hub/tests/harness/group_tab_probe.py new file mode 100644 index 0000000..5e4e452 --- /dev/null +++ b/packages/meshbay-hub/tests/harness/group_tab_probe.py @@ -0,0 +1,185 @@ +#!/usr/bin/env python3 +""" +Which tab does a group actually open on? + +The landing tab is chosen from a preference before the node has said which +applications the group runs, so the two can disagree — the group has Chat +disabled while 'chat' is the default, or the reader's preferred app is one this +group does not offer. Both used to land on a tab that rendered *nothing*: no +panel, no tab shown active, and nothing on screen to say why. + +That is not visible in the source, where the tab is a plain string and the app +list arrives from a handshake several `await`s later. So this renders the real +`GroupPage` — the shipped module — against a stub node that answers with a +chosen `enabled_apps`, and reports the tab that ended up active. + + group_tab_probe.py + +Prints JSON: one entry per case, with the tab bar as rendered and whether a +panel was drawn under it. +""" +import http.server +import json +import socketserver +import subprocess +import sys +import tempfile +import threading +import time +from pathlib import Path + +STATIC = Path(__file__).resolve().parents[2] / "src" / "meshbay_hub" / "static" +PORT = 8748 +RECORDS = [] +socketserver.TCPServer.allow_reuse_address = True + +# name -> (the reader's default_tab preference, what the group runs) +CASES = [ + ("every app, default chat", "chat", None), + ("chat disabled", "chat", ["files", "video"]), + ("preferred app absent", "music", ["chat", "files"]), + ("preferred app present", "video", ["chat", "files", "video"]), + ("preference names nothing real", "nonsense", ["files", "music"]), +] + +FRAME = r"""<!doctype html><html><head><meta charset=utf-8> +<link rel="stylesheet" href="/style.css"></head><body> +<div id="root"></div> +<script> +// GroupPage reaches for this global; transport.js is a classic script and this +// stands in for it. `connect()` is where a node says what the group runs. +window.MeshBayTransport = class { + constructor() { this.connected = false; this.memberRole = 'member'; } + async connect() { + this.connected = true; + return { is_node_admin: false, member_upload: true, + enabled_apps: %(enabled)s, + tmdb_enabled: false, musicbrainz_enabled: false, + video_root: '', audio_root: '', photo_roots: [] }; + } + async fetchIndex() { return { entries: [], dirs: [], roots: [] }; } + async fetchChatHistory() { return { messages: [], hasMore: false }; } + async fetchLinkPreview() { return { ok: false }; } + close() {} +}; +</script> +<script type="module"> +import { html, render } from '/vendor/htm-preact.js'; +import { initLocale } from '/i18n.js'; +import { GroupPage } from '/group-page.js'; + +await initLocale(); +render(html`<${GroupPage} groupId="g1" token="t" username="me" userId="u1" + group=${{ id: 'g1', name: 'a group', owner_username: 'me', is_admin: true }} + userPrefs=${{ default_tab: %(pref)s }} />`, document.getElementById('root')); + +setTimeout(() => { + const tabs = [...document.querySelectorAll('.group-tabs .group-tab')]; + const bar = document.querySelector('.group-tabs'); + parent.postMessage({ + case: %(index)d, + // Titles are translated labels, which is what a person reads off the bar. + tabs: tabs.map(b => b.getAttribute('title')), + active: tabs.findIndex(b => b.classList.contains('active')), + activeTitle: (tabs.find(b => b.classList.contains('active')) || {}) + .getAttribute?.('title') ?? null, + // A tab can be active with nothing under it; that is the whole bug. + panelDrawn: !!(bar && bar.nextElementSibling), + }, '*'); +}, 1500); +</script></body></html>""" + +PAGE = r"""<!doctype html><html><head><meta charset=utf-8></head> +<body style="margin:0"><div id="frames"></div><script> +const N = %(count)d; +const seen = []; +addEventListener('message', (e) => { + seen.push(e.data); + if (seen.length === N) { + fetch('/log', { method: 'POST', body: JSON.stringify(seen) }); + } +}); +for (let i = 0; i < N; i++) { + const f = document.createElement('iframe'); + f.src = '/case?n=' + i; + f.style.cssText = 'width:1100px;height:700px;border:0;display:block'; + document.getElementById('frames').appendChild(f); +} +</script></body></html>""" + + +class H(http.server.BaseHTTPRequestHandler): + def log_message(self, *a): + pass + + def do_POST(self): + length = int(self.headers.get("Content-Length") or 0) + if self.path == "/log": + RECORDS.append(json.loads(self.rfile.read(length).decode())) + else: + self.rfile.read(length) + self.send_response(204) + self.end_headers() + + def _send(self, body: bytes, ctype: str) -> None: + self.send_response(200) + self.send_header("Content-Type", ctype) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def do_GET(self): + path = self.path.split("?")[0] + if path == "/": + self._send((PAGE % {"count": len(CASES)}).encode(), "text/html; charset=utf-8") + elif path == "/case": + index = int(self.path.split("n=")[1]) + _, pref, enabled = CASES[index] + self._send((FRAME % {"index": index, + "pref": json.dumps(pref), + "enabled": json.dumps(enabled)}).encode(), + "text/html; charset=utf-8") + elif path == "/v1/groups/g1/nodes": + self._send(b'{"nodes": [{"node_id": "n1"}]}', "application/json") + else: + asset = (STATIC / path.lstrip("/")).resolve() + if not str(asset).startswith(str(STATIC)) or not asset.is_file(): + self.send_response(404) + self.end_headers() + return + self._send(asset.read_bytes(), + "text/css" if asset.suffix == ".css" + else "text/javascript" if asset.suffix == ".js" + else "application/octet-stream") + + +def main() -> int: + with socketserver.TCPServer(("127.0.0.1", PORT), H) as srv: + threading.Thread(target=srv.serve_forever, daemon=True).start() + with tempfile.TemporaryDirectory() as profile: + proc = subprocess.Popen( + ["google-chrome", "--headless=new", "--disable-gpu", "--no-sandbox", + f"--user-data-dir={profile}", "--window-size=1100,900", + f"http://127.0.0.1:{PORT}/"], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + for _ in range(300): + if RECORDS: + break + time.sleep(0.1) + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + if not RECORDS: + print(json.dumps({"error": "no measurement"}), file=sys.stderr) + return 1 + by_case = {r["case"]: r for r in RECORDS[0]} + print(json.dumps( + [dict(name=CASES[i][0], preference=CASES[i][1], enabled=CASES[i][2], + **by_case[i]) for i in sorted(by_case)], indent=1)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/packages/meshbay-hub/tests/test_group_tab_fallback.py b/packages/meshbay-hub/tests/test_group_tab_fallback.py new file mode 100644 index 0000000..6310b9c --- /dev/null +++ b/packages/meshbay-hub/tests/test_group_tab_fallback.py @@ -0,0 +1,75 @@ +""" +A group always opens on a tab that exists. + +The landing tab comes from a preference and is chosen at mount; which +applications the group runs comes from the node, several awaits later. When the +two disagree — Chat disabled here while 'chat' is the default, or a preferred +app this group does not offer — the page used to render no panel at all, with +no tab shown active and nothing on screen to explain it. + +A string comparison against a list that arrives later is not something reading +`group-page.js` makes obvious, so this renders the real `GroupPage` against a +stub node and reads the tab bar back. +""" +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +HARNESS = Path(__file__).parent / "harness" / "group_tab_probe.py" +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" + +pytestmark = pytest.mark.skipif( + shutil.which("google-chrome") is None or not (STATIC / "group-page.js").exists(), + reason="Chrome or the SPA sources are not available") + + +@pytest.fixture(scope="module") +def cases(): + run = subprocess.run(["python3", str(HARNESS)], capture_output=True, timeout=180) + assert run.returncode == 0, run.stderr.decode()[-2000:] + return {c["name"]: c for c in json.loads(run.stdout.decode())} + + +# Asserted by position, not by label: the harness renders in whatever language +# the browser resolves to. + +def test_every_case_lands_somewhere(cases): + """The bug, stated once: a tab bar with nothing selected and nothing under + it.""" + for name, case in cases.items(): + assert case["active"] >= 0, f"{name}: no tab is active" + assert case["panelDrawn"], f"{name}: a tab is active but no panel was drawn" + + +def test_the_default_is_the_first_app(cases): + """Nothing changes for a group running everything.""" + case = cases["every app, default chat"] + assert case["active"] == 0 + + +def test_a_disabled_default_falls_back(cases): + """'chat' is the default for everyone, and a group may not run it.""" + case = cases["chat disabled"] + assert case["active"] == 0, "should have fallen back to the group's first app" + assert len(case["tabs"]) == 3, "Files, Videos and Settings, with no Chat tab" + + +def test_a_preference_for_an_absent_app_falls_back(cases): + """The preference is global; the app list is per group.""" + assert cases["preferred app absent"]["active"] == 0 + + +def test_a_preference_that_names_nothing_falls_back(cases): + """A stale or hand-edited preference must not strand anyone either.""" + assert cases["preference names nothing real"]["active"] == 0 + + +def test_a_preference_the_group_offers_is_kept(cases): + """The fallback must not become 'always the first app'.""" + case = cases["preferred app present"] + assert case["active"] == 2, ( + "Videos was enabled and preferred; the reader should have landed on it, " + f"not on tab {case['active']}") |