fix(analytics): Prefix Tool hash-sizes query (#179) and stale tab on return (#183)

public/analytics.js

@@ -253,15 +253,14 @@
     // Deep-link: #/analytics?tab=collisions&window=7d
     const hashParams = location.hash.split('?')[1] || '';
     const _ap = new URLSearchParams(hashParams);
+    // Every mount starts from the URL, falling back to Overview: the tab
+    // selected before leaving the page must not survive into this mount (#183).
     const urlTab = _ap.get('tab');
-    if (urlTab) {
-      const tabBtn = analyticsTabs.querySelector(`[data-tab="${urlTab}"]`);
-      if (tabBtn) {
-        analyticsTabs.querySelectorAll('.tab-btn').forEach(b => b.classList.remove('active'));
-        tabBtn.classList.add('active');
-        _currentTab = urlTab;
-      }
-    }
+    const urlTabBtn = urlTab && analyticsTabs.querySelector(`[data-tab="${urlTab}"]`);
+    _currentTab = urlTabBtn ? urlTab : 'overview';
+    const activeBtn = urlTabBtn || analyticsTabs.querySelector('[data-tab="overview"]');
+    analyticsTabs.querySelectorAll('.tab-btn').forEach(b => b.classList.remove('active'));
+    if (activeBtn) activeBtn.classList.add('active');
     // #749 — restore time window from URL.
     const urlWindow = _ap.get('window');
     if (urlWindow) {
@@ -328,6 +327,12 @@
   var _themeRefreshHandler = null;
   let _currentTab = 'overview';
 
+  // Filter fragments ("&region=…", "&area=…", "&window=…") all start with
+  // '&'; turn their concatenation into the path's query string (#179).
+  function withQuery(path, frag) {
+    return frag ? path + '?' + frag.slice(1) : path;
+  }
+
   async function loadAnalytics(startedAt) {
     _cancelLoadRetry();
     const gen = _loadGen;
@@ -344,23 +349,20 @@
       const twVal = twEl ? twEl.value : '';
       const tws = twVal ? '&window=' + encodeURIComponent(twVal) : '';
       // hash-sizes / hash-collisions: region + area, no window
-      const baseQS = (rqs + aqs).slice(1);
-      const sepBase = baseQS ? '?' + baseQS : '';
+      const baseQ = rqs + aqs;
       // rf / topology: region + area + window
-      const windowedQS = (rqs + aqs + tws).slice(1);
-      const sepWin = windowedQS ? '?' + windowedQS : '';
+      const windowedQ = rqs + aqs + tws;
       // channels: region + window (no area per original PR intent)
-      const chanQS = (rqs + tws).slice(1);
-      const sepChan = chanQS ? '?' + chanQS : '';
+      const chanQ = rqs + tws;
       // This load retries 503s itself (retry503:false), see _loadGen.
       const opts = { ttl: CLIENT_TTL.analyticsRF, retry503: false };
       const [hashData, rfData, topoData, chanData, collisionData, airtimeData] = await Promise.all([
-        api('/analytics/hash-sizes' + sepBase, opts),
-        api('/analytics/rf' + sepWin, opts),
-        api('/analytics/topology' + sepWin, opts),
-        api('/analytics/channels' + sepChan, opts),
-        api('/analytics/hash-collisions' + sepBase, opts),
-        api('/analytics/relay-airtime-share' + sepWin, { ttl: CLIENT_TTL.analyticsRF }).catch(() => ({ rows: [] })),
+        api(withQuery('/analytics/hash-sizes', baseQ), opts),
+        api(withQuery('/analytics/rf', windowedQ), opts),
+        api(withQuery('/analytics/topology', windowedQ), opts),
+        api(withQuery('/analytics/channels', chanQ), opts),
+        api(withQuery('/analytics/hash-collisions', baseQ), opts),
+        api(withQuery('/analytics/relay-airtime-share', windowedQ), { ttl: CLIENT_TTL.analyticsRF }).catch(() => ({ rows: [] })),
       ]);
       if (gen !== _loadGen) return;
       _analyticsData = { hashData, rfData, topoData, chanData, collisionData, airtimeData };
@@ -3050,8 +3052,7 @@
     const gen = _distanceGen;
     try {
       const rqs = RegionFilter.regionQueryString();
-      const sep = rqs ? '?' + rqs.slice(1) : '';
-      const data = await api('/analytics/distance' + sep, { ttl: CLIENT_TTL.analyticsRF });
+      const data = await api(withQuery('/analytics/distance', rqs), { ttl: CLIENT_TTL.analyticsRF });
       if (gen !== _distanceGen) return;   // re-rendered, switched tab or left meanwhile
       if (_distanceIsBuilding(data)) {
         const ms = _retryAfterDelayMs(data);
@@ -3232,10 +3233,9 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _stopForeignTraf
 
     // Load data
     const rqs = RegionFilter.regionQueryString();
-    const sep = rqs ? '?' + rqs.slice(1) : '';
     let graphData;
     try {
-      graphData = await api('/analytics/neighbor-graph' + sep + (sep ? '&' : '?') + 'min_count=1&min_score=0', { ttl: CLIENT_TTL.analyticsRF });
+      graphData = await api(withQuery('/analytics/neighbor-graph', rqs + '&min_count=1&min_score=0'), { ttl: CLIENT_TTL.analyticsRF });
     } catch (e) {
       el.innerHTML = `<div class="analytics-card"><p class="text-muted">Failed to load neighbor graph: ${esc(e.message)}</p></div>`;
       return;
@@ -3714,7 +3714,7 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _stopForeignTraf
         // #1270: fetch CONFIGURED-hash-size counts so the Network Overview
         // tells the operational story (matching Hash Stats "By Repeaters"),
         // not just a math-only count of unique pubkey slices.
-        api('/analytics/hash-sizes' + rq, { ttl: CLIENT_TTL.analyticsRF }).catch(() => null),
+        api(withQuery('/analytics/hash-sizes', rq), { ttl: CLIENT_TTL.analyticsRF }).catch(() => null),
       ]);
     } catch (e) {
       el.innerHTML = `<div class="text-muted" role="alert" style="padding:40px">Failed to load: ${esc(e.message)}</div>`;

test-all.sh

@@ -144,6 +144,7 @@ run test-issue-111-drawer-version.js
 run test-packet-filter-time.js
 run test-confidence-indicator.js
 run test-1659-analytics-warmup.js
+run test-analytics-tab-state-and-query.js
 run test-channels-merge-1498-unit.js
 run test-issue-1518-home-url.js
 run test-live-region-filter.js

test-analytics-tab-state-and-query.js

@@ -0,0 +1,252 @@
+/* test-analytics-tab-state-and-query.js
+ *
+ * #179: the Prefix Tool fetched `/analytics/hash-sizes&region=…` (no `?`)
+ *       whenever a region or area filter was active, and got the SPA HTML.
+ * #183: leaving #/analytics and coming back without ?tab= marked Overview
+ *       active but rendered the tab selected before leaving.
+ *
+ * Runs the REAL app.js api() and the REAL analytics page in one vm with a
+ * fake fetch and a small fake DOM whose tab buttons come from the markup
+ * init() writes, so the active button is whatever the page set.
+ */
+'use strict';
+
+const vm = require('vm');
+const fs = require('fs');
+const assert = require('assert');
+
+let passed = 0, failed = 0;
+function test(name, fn) {
+  return Promise.resolve()
+    .then(fn)
+    .then(() => { passed++; console.log('  ✅ ' + name); })
+    .catch((e) => { failed++; console.log('  ❌ ' + name + ': ' + (e && e.message || e)); });
+}
+
+const flush = async () => { for (let i = 0; i < 30; i++) await new Promise((r) => setImmediate(r)); };
+
+// Real responses of the CI fixture server for the shared endpoints (#172).
+const REAL = JSON.parse(fs.readFileSync('test-fixtures/analytics-tabs-172.json', 'utf8'));
+const MARKERS = { overview: /Total Transmissions/, topology: /Per-Observer Reachability/ };
+
+function fakeClassList(initial) {
+  const s = new Set(initial);
+  return { add: (c) => s.add(c), remove: (c) => s.delete(c), contains: (c) => s.has(c), toggle() {} };
+}
+
+function fakeEl(id) {
+  let html = '';
+  const listeners = {};
+  return {
+    id, value: '', style: {}, dataset: {}, parentElement: null,
+    get innerHTML() { return html; },
+    set innerHTML(v) { html = String(v); },
+    classList: fakeClassList([]),
+    addEventListener(type, fn) { (listeners[type] = listeners[type] || []).push(fn); }, removeEventListener() {},
+    dispatch(type, ev) { for (const fn of listeners[type] || []) fn(ev); },
+    contains: () => true,
+    querySelector: () => null, querySelectorAll: () => [],
+    appendChild: (c) => c, insertBefore: (c) => c, setAttribute() {}, getAttribute: () => null,
+  };
+}
+
+function pageEnv() {
+  const fetchLog = [];
+  let region = '', area = '';
+  const respond = (body) => ({
+    ok: true, status: 200, headers: { get: () => null },
+    json: async () => JSON.parse(JSON.stringify(body)),
+  });
+  const fetchImpl = async (url) => {
+    fetchLog.push(url);
+    const m = /\/api\/analytics\/(rf|topology|channels|hash-sizes|hash-collisions)(\?|$)/.exec(url);
+    const key = m && { rf: 'rfData', topology: 'topoData', channels: 'chanData', 'hash-sizes': 'hashData', 'hash-collisions': 'collisionData' }[m[1]];
+    if (key) return respond(REAL[key]);
+    if (/\/api\/nodes\?/.test(url)) return respond({ nodes: [{ public_key: 'aabbccddeeff', name: 'R1', role: 'repeater' }] });
+    return respond({});
+  };
+
+  // A new app.innerHTML is a new page: every element below it is new, so
+  // listeners of the previous mount are gone, as in a browser.
+  let els = {};
+  const app = fakeEl('app');
+  let appHtml = '';
+  Object.defineProperty(app, 'innerHTML', {
+    get: () => appHtml,
+    set: (v) => { appHtml = String(v); els = {}; buttons = null; },
+  });
+  let buttons = null;
+  const tabButtons = () => {
+    if (!buttons) {
+      buttons = [];
+      const re = /<button class="tab-btn( active)?" data-tab="([^"]+)"/g;
+      let m;
+      while ((m = re.exec(appHtml))) {
+        buttons.push({ dataset: { tab: m[2] }, classList: fakeClassList(m[1] ? ['active'] : []), setAttribute() {} });
+      }
+    }
+    return buttons;
+  };
+  const el = (id) => {
+    if (id === 'app') return app;
+    if (!els[id]) {
+      els[id] = fakeEl(id);
+      if (id === 'analyticsTabs') {
+        els[id].querySelectorAll = (sel) => (sel === '.tab-btn' ? tabButtons() : []);
+        els[id].querySelector = (sel) => {
+          const m = /^\[data-tab="([^"]+)"\]$/.exec(sel);
+          return m ? tabButtons().find((b) => b.dataset.tab === m[1]) || null : null;
+        };
+      }
+    }
+    return els[id];
+  };
+
+  const pages = {};
+  const ctx = {
+    window: { addEventListener() {}, removeEventListener() {}, dispatchEvent() {} },
+    document: {
+      readyState: 'complete', body: { appendChild() {} }, head: { appendChild() {} },
+      createElement: () => fakeEl(''), getElementById: el, addEventListener() {},
+      querySelectorAll: (sel) => (sel === '.tab-btn' ? tabButtons() : []), querySelector: () => null,
+      documentElement: fakeEl('html'),
+    },
+    console: { log() {}, warn() {}, error: console.error }, Date, Infinity, Math, Array, Object, String, Number,
+    JSON, RegExp, Error, TypeError, parseInt, parseFloat, isNaN, isFinite, encodeURIComponent, decodeURIComponent,
+    setTimeout: () => 0, clearTimeout() {}, setInterval: () => 0, clearInterval() {},
+    requestAnimationFrame: () => 0, cancelAnimationFrame() {},
+    fetch: fetchImpl, performance: { now: () => 0 },
+    localStorage: { getItem: () => null, setItem() {}, removeItem() {} },
+    location: { hash: '#/analytics' }, history: { replaceState() {} },
+    CustomEvent: class CustomEvent {}, Map, Set, Promise, URLSearchParams,
+    getComputedStyle: () => ({ getPropertyValue: () => '' }),
+    timeAgo: () => 'x ago', initTabBar() {}, makeColumnsResizable() {},
+    onWS() {}, offWS() {}, connectWS() {}, invalidateApiCache() {}, IATA_COORDS_GEO: {},
+    RegionFilter: { init() {}, onChange() {}, regionQueryString: () => region },
+    AreaFilter: { init() {}, onChange() {}, areaQueryString: () => area },
+  };
+  vm.createContext(ctx);
+  const load = (f) => { vm.runInContext(fs.readFileSync(f, 'utf8'), ctx); for (const k of Object.keys(ctx.window)) ctx[k] = ctx.window[k]; };
+  load('public/payload-labels.js');
+  load('public/roles.js');
+  try { load('public/app.js'); } catch (e) { /* DOM-only tail */ }
+  ctx.registerPage = (name, obj) => { pages[name] = obj; };   // app.js defines its own
+  load('public/analytics.js');
+  const page = pages.analytics;
+
+  return {
+    fetchLog,
+    setFilters: (r, a) => { region = r ? '&region=' + r : ''; area = a ? '&area=' + a : ''; },
+    async mount(hash) { ctx.location.hash = hash; page.init(app); await flush(); },
+    destroy: () => page.destroy(),
+    clickTab: async (tab) => {
+      const btn = tabButtons().find((b) => b.dataset.tab === tab);
+      el('analyticsTabs').dispatch('click', { target: { closest: () => btn } });
+      await flush();
+    },
+    activeTabs: () => tabButtons().filter((b) => b.classList.contains('active')).map((b) => b.dataset.tab),
+    content: () => el('analyticsContent').innerHTML,
+  };
+}
+
+(async () => {
+  console.log('\n=== #179: Prefix Tool hash-sizes query ===');
+
+  const hashSizesUrls = (env) => env.fetchLog.filter((u) => /\/api\/analytics\/hash-sizes/.test(u));
+
+  await test('with a region and an area, the Prefix Tool asks for hash-sizes?region=…&area=…', async () => {
+    const env = pageEnv();
+    env.setFilters('SJC', 'north');
+    await env.mount('#/analytics?tab=prefix-tool');
+    const urls = hashSizesUrls(env);
+    const bad = urls.filter((u) => !/\/api\/analytics\/hash-sizes(\?|$)/.test(u));
+    assert.deepStrictEqual(bad, [], 'malformed hash-sizes URL(s)');
+    assert.ok(urls.some((u) => /\/api\/analytics\/hash-sizes\?region=SJC&area=north$/.test(u)), 'no hash-sizes?region=SJC&area=north: ' + urls.join(', '));
+  });
+
+  await test('with only an area, the query still starts with ?', async () => {
+    const env = pageEnv();
+    env.setFilters('', 'north');
+    await env.mount('#/analytics?tab=prefix-tool');
+    const bad = hashSizesUrls(env).filter((u) => !/\/api\/analytics\/hash-sizes(\?|$)/.test(u));
+    assert.deepStrictEqual(bad, [], 'malformed hash-sizes URL(s)');
+  });
+
+  await test('without filters, the Prefix Tool asks for plain /analytics/hash-sizes', async () => {
+    const env = pageEnv();
+    await env.mount('#/analytics?tab=prefix-tool');
+    const urls = hashSizesUrls(env);
+    assert.ok(urls.length > 0, 'no hash-sizes fetch');
+    assert.ok(urls.every((u) => /\/api\/analytics\/hash-sizes$/.test(u)), 'unexpected hash-sizes URL(s): ' + urls.join(', '));
+  });
+
+  // The same helper now builds every analytics query; these pin the URLs
+  // the other callers sent before it, so the refactor changes none of them.
+  await test('the shared loads keep their query strings (region + area + window)', async () => {
+    const env = pageEnv();
+    env.setFilters('SJC', 'north');
+    await env.mount('#/analytics?window=7d');
+    for (const want of [
+      '/api/analytics/hash-sizes?region=SJC&area=north',
+      '/api/analytics/hash-collisions?region=SJC&area=north',
+      '/api/analytics/rf?region=SJC&area=north&window=7d',
+      '/api/analytics/topology?region=SJC&area=north&window=7d',
+      '/api/analytics/channels?region=SJC&window=7d',
+      '/api/analytics/relay-airtime-share?region=SJC&area=north&window=7d',
+    ]) assert.ok(env.fetchLog.includes(want), 'missing ' + want + ' in ' + env.fetchLog.join(', '));
+  });
+
+  await test('the shared loads without filters have no query string', async () => {
+    const env = pageEnv();
+    await env.mount('#/analytics');
+    for (const ep of ['hash-sizes', 'hash-collisions', 'rf', 'topology', 'channels', 'relay-airtime-share']) {
+      assert.ok(env.fetchLog.includes('/api/analytics/' + ep), 'missing plain /api/analytics/' + ep);
+    }
+  });
+
+  await test('the distance tab asks for distance?region=… and plain distance', async () => {
+    const env = pageEnv();
+    env.setFilters('SJC');
+    await env.mount('#/analytics?tab=distance');
+    assert.ok(env.fetchLog.includes('/api/analytics/distance?region=SJC'), env.fetchLog.join(', '));
+    const env2 = pageEnv();
+    await env2.mount('#/analytics?tab=distance');
+    assert.ok(env2.fetchLog.includes('/api/analytics/distance'), env2.fetchLog.join(', '));
+  });
+
+  console.log('\n=== #183: tab state across leave and return ===');
+
+  await test('select Topology, leave, return without ?tab=: button and content are Overview', async () => {
+    const env = pageEnv();
+    await env.mount('#/analytics');
+    await env.clickTab('topology');
+    assert.ok(MARKERS.topology.test(env.content()), 'Topology did not render on click');
+    env.destroy();
+    await env.mount('#/analytics');
+    assert.deepStrictEqual(env.activeTabs(), ['overview'], 'active button');
+    assert.ok(MARKERS.overview.test(env.content()), 'content is not Overview: ' + env.content().slice(0, 200));
+    assert.ok(!MARKERS.topology.test(env.content()), 'content is still Topology');
+  });
+
+  await test('select Topology, leave, return with ?tab=topology: button and content are Topology', async () => {
+    const env = pageEnv();
+    await env.mount('#/analytics');
+    await env.clickTab('topology');
+    env.destroy();
+    await env.mount('#/analytics?tab=topology');
+    assert.deepStrictEqual(env.activeTabs(), ['topology'], 'active button');
+    assert.ok(MARKERS.topology.test(env.content()), 'content is not Topology: ' + env.content().slice(0, 200));
+  });
+
+  await test('select Topology, then (without destroy) mount with an unknown ?tab=: Overview', async () => {
+    const env = pageEnv();
+    await env.mount('#/analytics');
+    await env.clickTab('topology');
+    await env.mount('#/analytics?tab=no-such-tab');
+    assert.deepStrictEqual(env.activeTabs(), ['overview'], 'active button');
+    assert.ok(MARKERS.overview.test(env.content()), 'content is not Overview: ' + env.content().slice(0, 200));
+  });
+
+  console.log('\n' + passed + ' passed, ' + failed + ' failed');
+  if (failed > 0) process.exit(1);
+})();