fix(analytics): sort the Hash Stats adopters table by the clicked column, with direction (#226)

.github/workflows/deploy.yml

@@ -505,6 +505,7 @@ jobs:
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1567-corner-clears-drag-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1306-collisions-terminology-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-205-analytics-subtab-deeplinks-e2e.js 2>&1 | tee -a e2e-output.txt
+          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-226-hash-stats-sort-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1374-route-map-a11y-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-channels-list-render-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-channels-selection-flow-e2e.js 2>&1 | tee -a e2e-output.txt

public/analytics.js

@@ -351,6 +351,10 @@
   // #208 — Hash Stats' multi-byte adopters filter. URL only (no storageKey):
   // it had no stored state before, so a plain visit still opens on All.
   var HASHSTATS_MB_FILTER = { param: 'mbf', allowed: ['all', 'confirmed', 'suspected', 'unknown'], dflt: 'all' };
+  // #226 — the adopters table's sort column and direction, URL only like mbf=.
+  // 'none' keeps the server's order.
+  var HASHSTATS_MB_SORT = { param: 'mbsort', allowed: ['none', 'name', 'role', 'status', 'hashSize', 'packets', 'lastSeen'], dflt: 'none' };
+  var HASHSTATS_MB_DIR = { param: 'mbdir', allowed: ['asc', 'desc'], dflt: 'asc' };
 
   // The hash keys each tab owns; _updateAnalyticsUrl drops them when
   // another tab is selected. Hash Issues' bytes= is deliberately not listed:
@@ -360,7 +364,7 @@
   var TAB_URL_PARAMS = {
     'rf-health': ['range', 'observer', 'from', 'to'],
     collisions: ['section'],
-    hashsizes: [HASHSTATS_MB_FILTER.param],
+    hashsizes: [HASHSTATS_MB_FILTER.param, HASHSTATS_MB_SORT.param, HASHSTATS_MB_DIR.param],
     scopes: [SCOPES_SUBTAB.param, SCOPES_WINDOW.param],
     wardriving: [WARDRIVING_WINDOW.param],
   };
@@ -1540,7 +1544,10 @@
 
   // ===================== HASH SIZES (original) =====================
   function renderHashSizes(el, data) {
-    const mbFilter = restoreViewParams([HASHSTATS_MB_FILTER])[0];   // ?mbf= (#208)
+    // ?mbf= (#208), ?mbsort= and ?mbdir= (#226)
+    const [mbFilter, mbSortCol, mbSortDir] = restoreViewParams([HASHSTATS_MB_FILTER, HASHSTATS_MB_SORT, HASHSTATS_MB_DIR]);
+    const mbSort = { col: mbSortCol, dir: mbSortCol === HASHSTATS_MB_SORT.dflt ? HASHSTATS_MB_DIR.dflt : mbSortDir };
+    if (mbSort.dir !== mbSortDir) setViewParam(HASHSTATS_MB_DIR, mbSort.dir);   // no direction without a column
     const d = data.distribution;
     const total = data.total;
     const pct = (n) => total ? (n / total * 100).toFixed(1) : '0';
@@ -1590,7 +1597,7 @@
         </div>
       </div>
 
-      ${renderMultiByteAdopters(data.multiByteNodes, data.multiByteCapability || [], mbFilter)}
+      ${renderMultiByteAdopters(data.multiByteNodes, data.multiByteCapability || [], mbFilter, mbSort)}
 
       <div class="analytics-row">
         <div class="analytics-card flex-1">
@@ -1614,9 +1621,53 @@
     `;
   }
 
+  // #226 — the value an adopter row sorts by in a column: a lower-case string
+  // for Node and Role, a number otherwise. Last Seen is the timestamp (NaN when
+  // missing or unparseable), not the "5m ago" text the cell shows.
+  var MB_STATUS_WEIGHT = { confirmed: 0, suspected: 1, unknown: 2 };
+  function mbAdopterSortValue(r, col) {
+    switch (col) {
+      case 'name': return String(r.name || '').toLowerCase();
+      case 'role': return String(r.role || 'unknown').toLowerCase();
+      case 'status': return Object.prototype.hasOwnProperty.call(MB_STATUS_WEIGHT, r.status) ? MB_STATUS_WEIGHT[r.status] : MB_STATUS_WEIGHT.unknown;
+      case 'hashSize': return Number(r.hashSize);
+      case 'packets': return Number(r.packets);
+      case 'lastSeen': return r.lastSeen ? Date.parse(r.lastSeen) : NaN;
+    }
+    return 0;
+  }
+
+  // The rows in sort order; without a known column, as given. A missing value
+  // (NaN) is last in both directions, and ties keep the given order.
+  function sortMbAdopterRows(rows, sort) {
+    var col = sort && sort.col;
+    if (col === HASHSTATS_MB_SORT.dflt || HASHSTATS_MB_SORT.allowed.indexOf(col) < 0) return rows;
+    var sign = sort.dir === 'desc' ? -1 : 1;
+    return rows.map(function (r, i) { return { r: r, i: i, v: mbAdopterSortValue(r, col) }; })
+      .sort(function (a, b) {
+        var an = typeof a.v === 'number' && isNaN(a.v), bn = typeof b.v === 'number' && isNaN(b.v);
+        if (an || bn) return an === bn ? a.i - b.i : (an ? 1 : -1);
+        if (a.v < b.v) return -sign;
+        if (a.v > b.v) return sign;
+        return a.i - b.i;
+      })
+      .map(function (x) { return x.r; });
+  }
+
+  // Clicking the sorted column flips its direction; another column starts
+  // ascending.
+  function nextMbSort(sort, col) {
+    if (sort && sort.col === col) return { col: col, dir: sort.dir === 'asc' ? 'desc' : 'asc' };
+    return { col: col, dir: 'asc' };
+  }
+
   // filter: the initially selected filter (All when missing or unknown).
-  function renderMultiByteAdopters(nodes, caps, filter) {
+  // sort: the initial { col, dir } (the server's order when missing or unknown).
+  function renderMultiByteAdopters(nodes, caps, filter, sort) {
     var initialFilter = HASHSTATS_MB_FILTER.allowed.indexOf(filter) >= 0 ? filter : HASHSTATS_MB_FILTER.dflt;
+    var initialSort = sort && HASHSTATS_MB_SORT.allowed.indexOf(sort.col) >= 0
+      ? { col: sort.col, dir: sort.dir === 'desc' ? 'desc' : 'asc' }
+      : { col: HASHSTATS_MB_SORT.dflt, dir: HASHSTATS_MB_DIR.dflt };
     var mbBtnClass = function (f) { return f === initialFilter ? 'tab-btn active' : 'tab-btn'; };
     // Merge capability status into adopter nodes
     var capByPubkey = {};
@@ -1640,17 +1691,24 @@
     var counts = { confirmed: 0, suspected: 0, unknown: 0 };
     rows.forEach(function(r) { counts[r.status] = (counts[r.status] || 0) + 1; });
 
-    function buildTableContent(rows, filter) {
-      var filtered = filter === 'all' ? rows : rows.filter(function(r) { return r.status === filter; });
+    var sortCols = [
+      { key: 'name', label: 'Node' }, { key: 'role', label: 'Role' }, { key: 'status', label: 'Status' },
+      { key: 'hashSize', label: 'Hash Size' }, { key: 'packets', label: 'Adverts' }, { key: 'lastSeen', label: 'Last Seen' },
+    ];
+    // The keys are the fixed ones above, never a value from the URL.
+    function theadHtml(sort) {
+      return '<thead><tr>' + sortCols.map(function (c) {
+        var active = c.key === sort.col;
+        return '<th scope="col" class="sortable' + (active ? ' sort-active' : '') + '" data-sort="' + c.key + '"' +
+          (active ? ' aria-sort="' + (sort.dir === 'asc' ? 'ascending' : 'descending') + '"' : '') + '>' +
+          c.label + channelSortArrow(c.key, sort.col, sort.dir) + '</th>';
+      }).join('') + '</tr></thead>';
+    }
+
+    function buildTableContent(rows, filter, sort) {
+      var filtered = sortMbAdopterRows(filter === 'all' ? rows : rows.filter(function(r) { return r.status === filter; }), sort);
       return (filtered.length ? '<table class="analytics-table" id="mbAdoptersTable" style="margin-top:12px">' +
-          '<thead><tr>' +
-            '<th scope="col" data-sort="name">Node</th>' +
-            '<th scope="col" data-sort="role">Role</th>' +
-            '<th scope="col" data-sort="status">Status</th>' +
-            '<th scope="col" data-sort="hashSize">Hash Size</th>' +
-            '<th scope="col" data-sort="packets">Adverts</th>' +
-            '<th scope="col" data-sort="lastSeen">Last Seen</th>' +
-          '</tr></thead>' +
+          theadHtml(sort) +
           '<tbody>' +
             filtered.map(function(r) {
               var roleColor = (window.ROLE_COLORS || {})[r.role] || '#6b7280';
@@ -1688,14 +1746,15 @@
           '<button class="' + mbBtnClass('unknown') + '" data-mb-filter="unknown" style="--filter-color:var(--text-muted, #888)"><svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-question"/></svg> Unknown (' + counts.unknown + ')</button>' +
         '</div>' +
       '</div>' +
-      '<div id="mbAdoptersTableWrap">' + buildTableContent(rows, initialFilter) + '</div>' +
+      '<div id="mbAdoptersTableWrap">' + buildTableContent(rows, initialFilter, initialSort) + '</div>' +
     '</div></div>';
 
     // Use setTimeout for event delegation on the stable section container
     setTimeout(function() {
       var section = document.getElementById('mbAdoptersSection');
       if (!section) return;
       var currentFilter = initialFilter;
+      var currentSort = initialSort;
 
       section.addEventListener('click', function handler(e) {
         var btn = e.target.closest('[data-mb-filter]');
@@ -1706,31 +1765,21 @@
           buttons.forEach(function(b) { b.classList.toggle('active', b.dataset.mbFilter === currentFilter); });
           // Replace only the table content, not the whole section
           var wrap = section.querySelector('#mbAdoptersTableWrap');
-          if (wrap) wrap.innerHTML = buildTableContent(rows, currentFilter);
+          if (wrap) wrap.innerHTML = buildTableContent(rows, currentFilter, currentSort);
           setViewParam(HASHSTATS_MB_FILTER, currentFilter);
           return;
         }
+        // #226: a header click re-renders the table (the same path as a
+        // filter click) in the new order, so the sort survives the next
+        // filter click and the header shows it.
         var th = e.target.closest('[data-sort]');
         if (th) {
-          var tbody = section.querySelector('tbody');
-          if (!tbody) return;
-          var sortRows = Array.from(tbody.querySelectorAll('tr'));
           var col = th.dataset.sort;
-          var colIdx = { name: 0, status: 1, hashSize: 2, packets: 3, lastSeen: 4 };
-          var statusWeight = { 'confirmed': 0, 'suspected': 1, 'unknown': 2 };
-          sortRows.sort(function(a, b) {
-            var va = a.children[colIdx[col]] ? a.children[colIdx[col]].textContent.trim() : '';
-            var vb = b.children[colIdx[col]] ? b.children[colIdx[col]].textContent.trim() : '';
-            if (col === 'status') {
-              va = statusWeight[va.toLowerCase().split(' ').pop()] !== undefined ? statusWeight[va.toLowerCase().split(' ').pop()] : 2;
-              vb = statusWeight[vb.toLowerCase().split(' ').pop()] !== undefined ? statusWeight[vb.toLowerCase().split(' ').pop()] : 2;
-            }
-            if (col === 'hashSize' || col === 'packets') { va = parseInt(va) || 0; vb = parseInt(vb) || 0; }
-            if (va < vb) return -1;
-            if (va > vb) return 1;
-            return 0;
-          });
-          sortRows.forEach(function(r) { tbody.appendChild(r); });
+          if (col === HASHSTATS_MB_SORT.dflt || HASHSTATS_MB_SORT.allowed.indexOf(col) < 0) return;
+          currentSort = nextMbSort(currentSort, col);
+          var sortWrap = section.querySelector('#mbAdoptersTableWrap');
+          if (sortWrap) sortWrap.innerHTML = buildTableContent(rows, currentFilter, currentSort);
+          _writeViewParams([HASHSTATS_MB_SORT, HASHSTATS_MB_DIR], [currentSort.col, currentSort.dir]);
         }
       });
     }, 100);

test-all.sh

@@ -147,6 +147,7 @@ run test-confidence-indicator.js
 run test-1659-analytics-warmup.js
 run test-analytics-tab-state-and-query.js
 run test-analytics-subtab-deeplinks-205.js
+run test-hash-stats-sort-226.js
 run test-channels-merge-1498-unit.js
 run test-issue-1518-home-url.js
 run test-live-region-filter.js

test-analytics-subtab-deeplinks-205.js

@@ -731,6 +731,90 @@ function pageEnv(opts) {
     assert.deepStrictEqual(active(env.renderMultiByteAdopters(nodes, caps)), ['all']);
   });
 
+  // #226: the adopters table's sort is deep-linked as mbsort= (column) and
+  // mbdir= (asc|desc), URL only like mbf=. Without mbsort= the table keeps the
+  // server's order; an unknown value is the default, never markup.
+  console.log('\n=== #226: Hash Stats multi-byte adopters sort (mbsort=, mbdir=) ===');
+
+  // The adopter names in the order the table shows them.
+  const adopterOrder = (env) => {
+    const html = env.content();
+    const start = html.indexOf('id="mbAdoptersTable"');
+    const out = [];
+    const re = /<td><strong>([^<]*)<\/strong><\/td>/g;
+    re.lastIndex = start < 0 ? html.length : start;
+    let m;
+    while ((m = re.exec(html)) && m.index < html.indexOf('</table>', start)) out.push(m[1]);
+    return out;
+  };
+  const byPackets = REAL.hashData.multiByteNodes.slice().sort((a, b) => a.packets - b.packets).map((n) => n.name);
+  const byLastSeen = REAL.hashData.multiByteNodes.slice().sort((a, b) => Date.parse(a.lastSeen) - Date.parse(b.lastSeen)).map((n) => n.name);
+  const serverOrder = REAL.hashData.multiByteNodes.map((n) => n.name);
+
+  await test('precondition: the fixture adopters differ in Adverts and Last Seen, and the server order is not ascending by Adverts', async () => {
+    assert.ok(serverOrder.length >= 2, 'fewer than two adopters');
+    assert.notDeepStrictEqual(byPackets, serverOrder, 'server order already ascending by Adverts');
+  });
+
+  for (const [query, want] of [
+    ['mbsort=packets', byPackets],
+    ['mbsort=packets&mbdir=desc', byPackets.slice().reverse()],
+    ['mbsort=lastSeen', byLastSeen],
+    ['mbsort=lastSeen&mbdir=desc', byLastSeen.slice().reverse()],
+  ]) {
+    await test('#/analytics?tab=hashsizes&' + query + ' sorts the table, URL unchanged', async () => {
+      const env = pageEnv();
+      await env.mount('#/analytics?tab=hashsizes&' + query);
+      assert.strictEqual(env.initError(), null, 'init() threw');
+      assert.deepStrictEqual(adopterOrder(env), want);
+      assert.strictEqual(env.hash(), '#/analytics?tab=hashsizes&' + query, 'URL not canonical');
+    });
+  }
+
+  await test('mbsort= and mbf= together: filtered and sorted, both kept', async () => {
+    const env = pageEnv();
+    await env.mount('#/analytics?tab=hashsizes&mbf=confirmed&mbsort=packets&mbdir=desc');
+    assert.deepStrictEqual(env.activeMbFilters(), ['confirmed']);
+    assert.strictEqual(env.hash(), '#/analytics?tab=hashsizes&mbf=confirmed&mbsort=packets&mbdir=desc');
+  });
+
+  for (const value of ['', 'Packets', 'x"]', 'x"],[data-sort="name', '__proto__', 'constructor', 'none', '<img src=x onerror=alert(1)>']) {
+    await test('?mbsort=' + JSON.stringify(value) + ' keeps the server order, canonical URL, no exception', async () => {
+      const env = pageEnv();
+      await env.mount('#/analytics?tab=hashsizes&mbsort=' + encodeURIComponent(value) + '&mbdir=desc');
+      assert.strictEqual(env.initError(), null, 'init() threw');
+      assert.deepStrictEqual(adopterOrder(env), serverOrder);
+      assert.strictEqual(env.hash(), '#/analytics?tab=hashsizes', 'URL not canonical');
+      assert.ok(env.content().indexOf('onerror') < 0, 'URL value reached the markup');
+    });
+  }
+
+  for (const value of ['', 'DESC', 'down', '__proto__', '<img src=x onerror=alert(1)>']) {
+    await test('?mbdir=' + JSON.stringify(value) + ' is ascending, dropped from the URL', async () => {
+      const env = pageEnv();
+      await env.mount('#/analytics?tab=hashsizes&mbsort=packets&mbdir=' + encodeURIComponent(value));
+      assert.deepStrictEqual(adopterOrder(env), byPackets);
+      assert.strictEqual(env.hash(), '#/analytics?tab=hashsizes&mbsort=packets');
+      assert.ok(env.content().indexOf('onerror') < 0, 'URL value reached the markup');
+    });
+  }
+
+  await test('mbsort=/mbdir= are URL only: nothing stored, a later plain visit keeps the server order', async () => {
+    const env = pageEnv();
+    await env.mount('#/analytics?tab=hashsizes&mbsort=packets&mbdir=desc');
+    assert.deepStrictEqual(Object.keys(env.session), [], 'sessionStorage written: ' + JSON.stringify(env.session));
+    env.destroy();
+    await env.mount('#/analytics?tab=hashsizes');
+    assert.deepStrictEqual(adopterOrder(env), serverOrder);
+  });
+
+  await test('switching from Hash Stats to another tab drops mbsort= and mbdir=', async () => {
+    const env = pageEnv();
+    await env.mount('#/analytics?tab=hashsizes&mbf=confirmed&mbsort=packets&mbdir=desc&window=24h');
+    await env.clickTab('topology');
+    assert.strictEqual(env.hash(), '#/analytics?tab=topology&window=24h');
+  });
+
   console.log('\n' + passed + ' passed, ' + failed + ' failed');
   if (failed > 0) process.exit(1);
 })();