From 7fcb226cd8be1a78ea859c42da17b9362ff4c469 Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Fri, 29 May 2026 08:17:16 -0700 Subject: [PATCH] fix(#1486): collapse chevron no longer reopens closed detail panel (#1492) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #1486 — clicking the collapse chevron on a grouped packet row in the packets table no longer reopens the detail panel that the operator just closed. ## Root cause In the `#pktBody` row click handler the `toggle-select` action ran **both** `pktToggleGroup(value)` and `pktSelectHash(value)` on every chevron click. `pktToggleGroup()` already opens the detail panel itself (via `selectPacket()`) when it expands a row, so the trailing `pktSelectHash()` was: - redundant on **expand** (the panel was already opening), and - harmful on **collapse** — after the operator closed the detail panel via the ✕ in `#pktRight`, clicking the same chevron a second time to collapse the tree re-fetched `/packets/` and re-populated the panel with the same packet, exactly the behavior the issue describes. ## Fix Drop the unconditional `pktSelectHash(value)` call inside the `toggle-select` branch. `pktToggleGroup()` already handles the expand-side panel open; the collapse branch does no panel work, so a closed panel stays closed. ```js else if (action === 'toggle-select') { // #1486: pktToggleGroup() already opens the detail panel on EXPAND // (via selectPacket()), and must NOT open it on COLLAPSE. pktToggleGroup(value); } ``` ## Tests - New Playwright E2E `test-issue-1486-collapse-reopens-detail-e2e.js` walks the operator-visible repro: expand → assert panel open → click ✕ → assert panel empty → click chevron again → assert row collapsed AND panel STILL empty. - Committed red-first: the test was added in its own commit and FAILS on the unpatched code (3 passed / 1 failed), then GREEN on the fix commit (4 passed / 0 failed). - CI workflow seeds two extra observations onto the newest fixture transmission so a grouped (`toggle-select`) row exists; without this the fixture renders only flat rows and the chevron can't be exercised. ## Reproduction (manual, against staging or local) 1. Open `/#/packets` on desktop. 2. Click a grouped row's `▶` chevron — the tree expands and the detail panel opens on the right. 3. Click the `✕` in the top-right of the detail panel — panel goes back to "Select a packet to view details". 4. Click the same chevron (now `▼`) again — **before:** detail panel reopens with the same packet. **After:** the row collapses and the panel stays empty. --------- Co-authored-by: mc-bot --- .github/workflows/deploy.yml | 49 +++++++ public/packets.js | 9 +- ...-issue-1486-collapse-reopens-detail-e2e.js | 130 ++++++++++++++++++ 3 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 test-issue-1486-collapse-reopens-detail-e2e.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 452d5705..3f9a7b33 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -241,6 +241,54 @@ jobs: - name: Freshen fixture timestamps run: bash tools/freshen-fixture.sh test-fixtures/e2e-fixture.db + - name: Seed grouped-packet row for #1486 collapse test + # The committed fixture has 499 packets, each with exactly ONE + # observation, so the packets-page renders only flat + # (select-hash) rows. The #1486 repro needs at least one grouped + # (toggle-select) row. Insert a NEW transmission with 3 + # observations. + # + # The server's async hash-migrate (cmd/server/hash_migrate.go) + # recomputes `transmissions.hash` from `raw_hex` via + # ComputeContentHash(), so the inserted hash MUST equal that + # function's output for the chosen raw_hex — otherwise the row + # gets relabelled and the E2E can't find it. + # + # raw_hex 15000102030405060708090a0b0c0d0e0f + # → header=0x15 (route_type=1, payload_type=5) + # → ComputeContentHash(...) = fae0c9e6d357a814 + # + # The first_seen / observation timestamps are pinned to a date + # within retentionHours but outside the default 15-min UI + # window so the row is hidden in the default view (keeping + # test-e2e-playwright's first-10-rows hex-pane test + # unaffected) and reachable via the explicit ?timeWindow=0 + # deep-link the #1486 test uses. + run: | + sqlite3 test-fixtures/e2e-fixture.db <<'SQL' + -- Sort the seeded row LAST in BOTH default packets views: + -- • flat view sorts by transmissions.id DESC → id=0 puts it last + -- • grouped view (#default for the packets page) sorts by + -- MAX(observations.timestamp) DESC → we must keep our obs + -- timestamps OLDER than every other fixture observation. + -- Fixture (after freshen) has obs timestamps spanning + -- 2026-05-17 16:01:39Z .. 2026-05-28 00:00:00Z (max). + -- Note: freshen only shifts transmissions.first_seen forward + -- to ~now; observation.timestamp is left alone except for + -- the timestamp=0 case. + -- Use 2026-05-15 (~2 days older than the oldest fixture obs) + -- so our row sorts LAST in the grouped view too, keeping + -- test-e2e-playwright's first-10-rows hex-pane test + -- unaffected. The #1486 test still reaches the row via the + -- explicit hash + ?timeWindow=0 deep-link. + INSERT INTO transmissions(id,raw_hex,hash,first_seen,route_type,payload_type,payload_version,decoded_json,channel_hash,from_pubkey) + VALUES (0,'15000102030405060708090a0b0c0d0e0f','fae0c9e6d357a814','2026-05-15T00:00:00Z',1,5,0,'{"type":"CHAN","channel":"#test","text":"#1486 fixture"}',NULL,NULL); + INSERT INTO observations(transmission_id,observer_idx,direction,snr,rssi,score,path_json,timestamp,resolved_path) VALUES + (0,1,'rx',5.0,-95,0,'["AA"]',CAST(strftime('%s','2026-05-15T00:00:00Z') AS INTEGER),'["aa00000000000000000000000000000000000000000000000000000000000000"]'), + (0,2,'rx',5.5,-92,0,'["BB"]',CAST(strftime('%s','2026-05-15T00:00:00Z') AS INTEGER),'["bb00000000000000000000000000000000000000000000000000000000000000"]'), + (0,3,'rx',6.0,-90,0,'["CC"]',CAST(strftime('%s','2026-05-15T00:00:00Z') AS INTEGER),'["cc00000000000000000000000000000000000000000000000000000000000000"]'); + SQL + - name: Migrate fixture DB to current schema (#1287) # Server now ASSERTs schema is migrated and refuses to start # otherwise (cmd/server/main.go: dbschema.AssertReady). In prod @@ -302,6 +350,7 @@ jobs: BASE_URL=http://localhost:13581 node test-issue-1146-path-link-contrast-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1147-section-order-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1151-orphan-separators-e2e.js 2>&1 | tee -a e2e-output.txt + BASE_URL=http://localhost:13581 node test-issue-1486-collapse-reopens-detail-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-rebrand-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-theme-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-default-sage-teal-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/public/packets.js b/public/packets.js index 52e8f2c8..ff9270ca 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1939,7 +1939,14 @@ } } else if (action === 'select-hash') pktSelectHash(value); - else if (action === 'toggle-select') { pktToggleGroup(value); pktSelectHash(value); } + else if (action === 'toggle-select') { + // #1486: pktToggleGroup() already opens the detail panel on EXPAND + // (via selectPacket()), and must NOT open it on COLLAPSE. The + // previously-unconditional pktSelectHash() trailing call was both + // redundant on expand AND reopened the panel the operator had just + // closed when they clicked the chevron to collapse — drop it. + pktToggleGroup(value); + } }; pktBody.addEventListener('click', handler); pktBody.addEventListener('keydown', handler); diff --git a/test-issue-1486-collapse-reopens-detail-e2e.js b/test-issue-1486-collapse-reopens-detail-e2e.js new file mode 100644 index 00000000..5e754fe0 --- /dev/null +++ b/test-issue-1486-collapse-reopens-detail-e2e.js @@ -0,0 +1,130 @@ +/** + * E2E (#1486): Packets-page collapse chevron must not reopen the detail + * panel that the operator just closed. + * + * Repro: + * 1. Open /#/packets on desktop. + * 2. Click a group-header row's expand chevron → tree expands, detail + * panel opens on the right. + * 3. Close the detail panel via the ✕ button in its top-right. + * 4. Click the SAME chevron again, intending to collapse the tree. + * + * Bug: the second click reopens the detail panel (because the + * `toggle-select` action handler unconditionally calls both + * `pktToggleGroup(value)` AND `pktSelectHash(value)` — the latter + * re-fires selectPacket even when the row was being collapsed). + * + * Fix expectation: after the second chevron click the tree row must be + * collapsed AND the #pktRight panel must remain in its "empty" / + * collapsed state. + * + * The CI workflow's "Seed grouped-packet row for #1486" step inserts a + * transmission with hash SEED_HASH that carries 3 observations so the + * page renders a grouped (toggle-select) row. When running locally, + * seed the same row (see .github/workflows/deploy.yml for the SQL). + * + * Usage: BASE_URL=http://localhost:13581 node test-issue-1486-collapse-reopens-detail-e2e.js + */ +'use strict'; +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +const SEED_HASH = 'fae0c9e6d357a814'; + +let passed = 0, failed = 0; +async function step(name, fn) { + try { await fn(); passed++; console.log(' ✓ ' + name); } + catch (e) { failed++; console.error(' ✗ ' + name + ': ' + e.message); } +} +function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } + +(async () => { + const browser = await chromium.launch({ + headless: true, + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'], + }); + const ctx = await browser.newContext({ viewport: { width: 1400, height: 900 } }); + const page = await ctx.newPage(); + page.setDefaultTimeout(10000); + page.on('pageerror', (e) => console.error('[pageerror]', e.message)); + + console.log(`\n=== #1486 packets collapse-reopens-detail E2E against ${BASE} ===`); + + await step('navigate to /packets filtered to seeded grouped row', async () => { + // Deep-link with the seeded hash filter + unbounded time window so + // the seeded row is the only thing in the table and is visible. + await page.goto(BASE + '/#/packets?hash=' + SEED_HASH + '&timeWindow=0', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#pktBody', { timeout: 8000 }); + await page.waitForFunction( + (h) => !!document.querySelector(`#pktBody tr[data-hash="${h}"][data-action="toggle-select"]`), + SEED_HASH, + { timeout: 12000 } + ); + }); + + await step('1st chevron click expands row AND opens detail panel', async () => { + await page.evaluate((h) => { + const row = document.querySelector(`#pktBody tr[data-hash="${h}"][data-action="toggle-select"]`); + row.click(); + }, SEED_HASH); + + await page.waitForFunction( + () => { + const p = document.getElementById('pktRight'); + return p && !p.classList.contains('empty'); + }, + { timeout: 8000 } + ); + + const state = await page.evaluate((h) => { + const row = document.querySelector(`#pktBody tr[data-hash="${h}"][data-action="toggle-select"]`); + const panel = document.getElementById('pktRight'); + return { + expanded: !!row && row.classList.contains('expanded'), + panelEmpty: !!panel && panel.classList.contains('empty'), + }; + }, SEED_HASH); + assert(state.expanded, 'row should be expanded after 1st click'); + assert(!state.panelEmpty, 'detail panel should be open after 1st click'); + }); + + await step('Close detail panel via ✕ button — panel goes back to empty', async () => { + await page.evaluate(() => { + const btn = document.querySelector('#pktRight .panel-close-btn'); + if (btn) btn.click(); + }); + await page.waitForFunction( + () => { + const p = document.getElementById('pktRight'); + return p && p.classList.contains('empty'); + }, + { timeout: 5000 } + ); + }); + + await step('2nd chevron click COLLAPSES the row WITHOUT reopening the detail panel', async () => { + await page.evaluate((h) => { + const row = document.querySelector(`#pktBody tr[data-hash="${h}"][data-action="toggle-select"]`); + row.click(); + }, SEED_HASH); + + // Give any (incorrect) async pktSelectHash time to re-populate. + await page.waitForTimeout(1000); + + const state = await page.evaluate((h) => { + const row = document.querySelector(`#pktBody tr[data-hash="${h}"][data-action="toggle-select"]`); + const panel = document.getElementById('pktRight'); + return { + expanded: !!row && row.classList.contains('expanded'), + panelEmpty: !!panel && panel.classList.contains('empty'), + }; + }, SEED_HASH); + assert(!state.expanded, 'row must be collapsed after 2nd chevron click'); + assert(state.panelEmpty, 'detail panel must NOT reopen after collapse'); + }); + + await browser.close(); + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed === 0 ? 0 : 1); +})().catch(e => { console.error(e); process.exit(1); });