diff --git a/public/packets.js b/public/packets.js index 191ebed2..e4549d12 100644 --- a/public/packets.js +++ b/public/packets.js @@ -774,7 +774,11 @@ if (m && m[1]) subpath = m[1]; // Don't double-encode filters.hash when it's already the path segment. var skipHash = !!(filters.hash && subpath === '/' + filters.hash); - history.replaceState(null, '', '#/packets' + subpath + buildPacketsQuery(savedTimeWindowMin, RegionFilter.getRegionParam(), skipHash)); + var query = buildPacketsQuery(savedTimeWindowMin, RegionFilter.getRegionParam(), skipHash); + // Observation selection belongs to the current detail route, not filters. + var obs = subpath ? getHashParams().get('obs') : null; + if (obs) query += (query ? '&' : '?') + 'obs=' + encodeURIComponent(obs); + history.replaceState(null, '', '#/packets' + subpath + query); // Update clear-filters button visibility var cb = document.getElementById('clearFiltersBtn'); if (cb) { @@ -1174,6 +1178,7 @@ // Read URL params (router strips query from routeParam; read from location.hash) var _initUrlParams = getHashParams(); + directObsId = _initUrlParams.get('obs'); var _urlTimeWindow = Number(_initUrlParams.get('timeWindow')); if (Number.isFinite(_urlTimeWindow) && _urlTimeWindow > 0) { savedTimeWindowMin = _urlTimeWindow; @@ -1277,10 +1282,13 @@ // If linked directly to a packet by ID, load its detail and filter list if (directPacketId) { const pktId = Number(directPacketId); + const obsTarget = directObsId; directPacketId = null; + directObsId = null; try { const data = await api(`/packets/${pktId}`); if (gen !== initGeneration) return; + selectedObservationId = obsTarget; if (data.packet?.hash) { filters.hash = data.packet.hash; const hashInput = document.getElementById('fHash'); @@ -1300,7 +1308,7 @@ const newHops = hops.filter(h => !(h in hopNameCache)); if (newHops.length) await resolveHops(newHops); } catch {} - await renderDetail(content, data); + await renderDetail(content, data, obsTarget); initPanelResize(); } } catch {} diff --git a/tests/e2e/test-filter-ux-e2e.js b/tests/e2e/test-filter-ux-e2e.js index dc008667..d8a42713 100644 --- a/tests/e2e/test-filter-ux-e2e.js +++ b/tests/e2e/test-filter-ux-e2e.js @@ -173,6 +173,66 @@ function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } await page.evaluate(() => localStorage.removeItem('corescope_saved_filters_v1')); }); + // Reuse the real three-observation transmission seeded by deploy.yml for + // #1486. Missing fixture data must fail, never silently skip this regression. + for (const routeKind of ['hash', 'id']) { + await step('#2091: ' + routeKind + ' observation deep link survives filters and refresh', async () => { + const response = await ctx.request.get(BASE + '/api/packets/fae0c9e6d357a814'); + assert(response.ok(), '#1486 grouped packet fixture is required'); + const detail = await response.json(); + assert(detail.packet && detail.observations && detail.observations.length > 1, + 'fixture must expose a packet with at least two observations'); + const observation = detail.observations[1]; + assert(String(observation.id) !== String(detail.observations[0].id), 'observation IDs must differ'); + + const detailContext = await browser.newContext({ viewport: { width: 1600, height: 1000 } }); + try { + const detailPage = await detailContext.newPage(); + const route = routeKind === 'hash' ? detail.packet.hash : 'id/' + detail.packet.id; + await detailPage.goto(BASE + '/#/packets/' + route + '?obs=' + observation.id, + { waitUntil: 'domcontentloaded' }); + + async function assertSelection(stage) { + await detailPage.waitForSelector('#pktRight .detail-obs-row.observation-current', { state: 'attached' }); + const selected = await detailPage.locator('#pktRight .observation-current').getAttribute('data-obs-id'); + assert(selected === String(observation.id), stage + ': selected observation changed to ' + selected); + const hash = new URL(detailPage.url()).hash; + const params = new URLSearchParams(hash.split('?')[1] || ''); + assert(params.get('obs') === String(observation.id), stage + ': obs missing from ' + hash); + assert(params.getAll('obs').length === 1, stage + ': duplicated observation parameter'); + if (routeKind === 'hash') assert(!params.has('hash'), stage + ': hash duplicated in query'); + } + + await assertSelection('initial load'); + await detailPage.click('#typeTrigger'); + await detailPage.locator('#typeMenu input[data-type-id="' + detail.packet.payload_type + '"]').check(); + assert(await detailPage.evaluate(() => localStorage.getItem('meshcore-type-filter')) === String(detail.packet.payload_type), + 'type filter must apply the requested selection'); + await assertSelection('type filter'); + await detailPage.click('#observerTrigger'); + await detailPage.locator('#observerMenu input[data-obs-id=' + JSON.stringify(observation.observer_id) + ']').check(); + assert(new URLSearchParams(new URL(detailPage.url()).hash.split('?')[1]).get('observer') === String(observation.observer_id), + 'observer filter must update the URL to the requested observer'); + await assertSelection('observer filter'); + await detailPage.click('#observerTrigger'); + await detailPage.selectOption('#fTimeWindow', '60'); + assert(new URLSearchParams(new URL(detailPage.url()).hash.split('?')[1]).get('timeWindow') === '60', + 'time-window filter must update the URL to 60 minutes'); + await assertSelection('time-window filter'); + await detailPage.reload({ waitUntil: 'load' }); + await assertSelection('refresh'); + await detailPage.click('#clearFiltersBtn'); + await assertSelection('Clear filters'); + const params = new URLSearchParams(new URL(detailPage.url()).hash.split('?')[1]); + assert(!params.has('observer') && !params.has('timeWindow'), 'Clear must still remove filters'); + await detailPage.reload({ waitUntil: 'load' }); + await assertSelection('refresh after Clear'); + } finally { + await detailContext.close(); + } + }); + } + await browser.close(); console.log(`\n=== Results: passed ${passed} failed ${failed} ===`); diff --git a/tests/unit/test-issue-2012-clear-filters-selection.js b/tests/unit/test-issue-2012-clear-filters-selection.js index 19bc00d2..8d12db8d 100644 --- a/tests/unit/test-issue-2012-clear-filters-selection.js +++ b/tests/unit/test-issue-2012-clear-filters-selection.js @@ -39,6 +39,9 @@ const multiSelectSrc = slice('// --- Observer multi-select ---', '// --- Channel const clearSrc = slice('// --- Clear filters button ---', '// Show clear button if page loaded'); // The real updatePacketsUrl(), which shows or hides the Clear button. const urlSrc = slice('function buildPacketsQuery(', 'let filtersBuilt = false;'); +const appSrc = fs.readFileSync(REPO_ROOT + '/public/app.js', 'utf-8'); +const hashParamsSrc = appSrc.match(/function getHashParams\(\) \{[\s\S]*?\n\}/)[0]; +const initParamsSrc = slice('// Parse ?obs=OBSERVER_ID from routeParam', 'app.innerHTML ='); function makeEl(id) { const listeners = {}; @@ -69,7 +72,7 @@ function makeEl(id) { return el; } -function setup() { +function setup(hash = '#/packets', initialFilters = {}) { const elements = {}; // #observerList and #observerSearchInput are not in packets.js yet. The // observer search PR (#1884) renders the observer rows into #observerList @@ -93,11 +96,11 @@ function setup() { const observers = [{ id: 'obsA', name: 'Observer A' }, { id: 'obsB', name: 'Observer B' }]; const observerMap = new Map(observers.map((o) => [o.id, o])); const SHORT_BY_ID = { 4: 'ADVERT', 5: 'GRP_TXT' }; - const filters = { myNodes: false }; + const filters = { myNodes: false, ...initialFilters }; const escapeHtml = (s) => String(s); const RegionFilter = { setSelected() {}, getRegionParam: () => '' }; - const location = { hash: '#/packets' }; - const history = { replaceState() {} }; + const location = { hash }; + const history = { replaceState(_state, _title, url) { location.hash = url; } }; const noop = () => {}; const run = new Function( @@ -105,9 +108,10 @@ function setup() { 'RegionFilter', 'renderTableRows', 'loadPackets', '_rebuildObserverMenu', '_observerFilterSet', 'savedTimeWindowMin', 'DEFAULT_TIME_WINDOW', 'location', 'history', 'window', '_packetSortColumn', '_packetSortDirection', - urlSrc + '\n' + multiSelectSrc + '\n' + clearSrc + hashParamsSrc + '\n' + urlSrc + '\n' + multiSelectSrc + '\n' + clearSrc + '\n' + + 'updatePacketsUrl(); return { updatePacketsUrl, buildPacketsQuery };' ); - run(filters, observers, observerMap, SHORT_BY_ID, escapeHtml, document, localStorage, + const actions = run(filters, observers, observerMap, SHORT_BY_ID, escapeHtml, document, localStorage, RegionFilter, noop, noop, null, null, 15, 15, location, history, {}, null, null); @@ -128,7 +132,7 @@ function setup() { return boxes(menuId).find((c) => c.dataset[attr] === '__all__'); } return { - filters, elements, storage, boxes, + filters, elements, storage, boxes, location, ...actions, clear: () => elements.clearFiltersBtn.fire('click'), pickObserver: (id) => toggle('observerMenu', 'obsId', id, true), pickType: (id) => toggle('typeMenu', 'typeId', id, true), @@ -204,6 +208,79 @@ test('observer only: Clear button is shown once an observer is picked, hidden af assert.strictEqual(s.elements.clearFiltersBtn.style.display, 'none', 'Clear button still shown after Clear'); }); +for (const route of ['#/packets/aabbccddeeff0011', '#/packets/id/42']) { + test('#2091: initial URL rewrite preserves observation on ' + route, () => { + const s = setup(route + '?obs=123'); + assert.strictEqual(s.location.hash, route + '?obs=123'); + assert.strictEqual(s.elements.clearFiltersBtn.style.display, 'none', 'observation is not a filter'); + }); +} + +test('#2091: type and observer changes preserve detail selection and canonical hash', () => { + const route = '#/packets/aabbccddeeff0011'; + const s = setup(route + '?obs=123', { hash: 'aabbccddeeff0011' }); + s.pickType('4'); + s.pickObserver('obsA'); + s.pickObserver('obsB'); + const params = new URLSearchParams(s.location.hash.split('?')[1]); + assert.strictEqual(params.get('obs'), '123'); + assert.strictEqual(params.getAll('obs').length, 1); + assert.strictEqual(params.get('observer'), 'obsA,obsB'); + assert.strictEqual(params.has('hash'), false, 'path hash must not be duplicated as a filter'); + assert.strictEqual(s.location.hash.split('?')[0], route); +}); + +test('#2091: Clear removes filters but keeps the current observation detail', () => { + const route = '#/packets/aabbccddeeff0011'; + const s = setup(route + '?obs=123', { hash: 'aabbccddeeff0011' }); + s.pickObserver('obsA'); + s.pickType('4'); + s.clear(); + assert.strictEqual(s.location.hash, route + '?obs=123'); + assert.strictEqual(s.elements.clearFiltersBtn.style.display, 'none'); + assert.strictEqual(s.observerAllRow().checked, true); + assert.strictEqual(s.typeAllRow().checked, true); +}); + +test('#2091: encoded values survive repeated filter rewrites without duplicate obs', () => { + const s = setup('#/packets/id/42?obs=row%2B%26%3D%20%3F&obs=discarded', { + node: 'node +&=', channel: 'channel +&=', _filterExpr: 'name == "A & B"', + }); + s.pickObserver('obsA'); + s.updatePacketsUrl(); + const params = new URLSearchParams(s.location.hash.split('?')[1]); + assert.deepStrictEqual(params.getAll('obs'), ['row+&= ?']); + assert.strictEqual(params.get('node'), s.filters.node); + assert.strictEqual(params.get('channel'), s.filters.channel); + assert.strictEqual(params.get('filter'), s.filters._filterExpr); + assert.strictEqual(params.get('observer'), 'obsA'); +}); + +test('#2091: returning to list or selecting another packet does not resurrect stale obs', () => { + const s = setup('#/packets/aabbccddeeff0011?obs=123'); + for (const route of ['#/packets', '#/packets?obs=123', '#/packets/1122334455667788']) { + s.location.hash = route; + s.pickType('4'); + assert.strictEqual(new URLSearchParams(s.location.hash.split('?')[1]).has('obs'), false, route); + } +}); + +test('#2091: list query builder never inherits detail observation', () => { + const s = setup('#/packets/aabbccddeeff0011?obs=123'); + assert.strictEqual(s.buildPacketsQuery(60, 'region +&=', false), '?timeWindow=60®ion=region%20%2B%26%3D'); +}); + +test('#2091: init restores obs from the hash after router strips the query', () => { + const readInitialObservation = new Function('location', 'routeParam', + 'let directObsId = "stale", directPacketId = null, directPacketHash = null; ' + + 'let savedTimeWindowMin = 15, _pendingUrlRegion = null; const filters = {}, window = {}; ' + + hashParamsSrc + '\n' + initParamsSrc + '\nreturn directObsId;'); + for (const route of ['aabbccddeeff0011', 'id/42']) { + assert.strictEqual(readInitialObservation({ hash: '#/packets/' + route + '?obs=123' }, route), '123'); + assert.strictEqual(readInitialObservation({ hash: '#/packets/' + route }, route), null); + } +}); + console.log(`\n${passed} passed, ${failed} failed`); if (failed > 0) process.exit(1); console.log('All tests passed ✅');