mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-03 04:38:48 +00:00
Merge pull request #2178 from rcourtman/maintainer/20260923T002652Z
Keep the resource drawer's selected tab across data refreshes
This commit is contained in:
pulse-triage[bot]
GitHub
commit
08175af524
7 changed files with 308 additions and 17 deletions
|
|
@ -5266,3 +5266,19 @@ separate conflicting IDs, devices, hosts and controller members; the linked-disk
|
|||
ambiguity test rejects multiple opposite-source candidates in both directions.
|
||||
These are synthetic source proofs, not appliance acceptance or a claim about
|
||||
which collector produced a reporter's row.
|
||||
|
||||
### Drawer tab selection survives a transient snapshot change (#1723)
|
||||
|
||||
The shared resource detail drawer keeps the operator's selected tab while a
|
||||
live snapshot is being replaced. `useResourceDetailDrawerState` falls back to
|
||||
Overview only when the drawer starts showing a different resource; it must not
|
||||
reset merely because the current tab is momentarily absent from the derived tab
|
||||
list. A merged host's metrics target can be omitted for one snapshot during a
|
||||
refresh, and treating that as a permanent loss of the History tab made the
|
||||
Proxmox -> Backups drawer silently jump back to Overview. Each tab body already
|
||||
renders its own availability notice while its tab is unavailable, so the
|
||||
selection can be retained and recovered without manufacturing a target. The
|
||||
`ResourceDetailDrawer.history.test.tsx` guard pins the single-resource reset
|
||||
boundary, and `scripts/check-drawer-tab-retention.cjs` exercises the rendered
|
||||
drawer at desktop and narrow widths: selecting History, dropping the merged
|
||||
metrics target, and restoring it must keep the selection and recover the chart.
|
||||
|
|
|
|||
|
|
@ -107,6 +107,9 @@ const resources =
|
|||
const Fixture = () => {
|
||||
const [servers, setServers] = createSignal(structuredClone(resources));
|
||||
const [snapshot, setSnapshot] = createSignal(0);
|
||||
// Simulates a snapshot in which the merged host correlation target is
|
||||
// transiently absent, which is what gates the drawer's History tab.
|
||||
const [dropMetricsTarget, setDropMetricsTarget] = createSignal(false);
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
onCleanup(() => clearTimeout(timer));
|
||||
const refresh = () => {
|
||||
|
|
@ -120,6 +123,9 @@ const Fixture = () => {
|
|||
if (query.get('order') !== 'stable' && count % 2 === 1) {
|
||||
next[0].pbs!.datastores!.reverse();
|
||||
}
|
||||
if (dropMetricsTarget()) {
|
||||
for (const resource of next) delete resource.metricsTarget;
|
||||
}
|
||||
setServers(next);
|
||||
setSnapshot(count);
|
||||
};
|
||||
|
|
@ -134,6 +140,22 @@ const Fixture = () => {
|
|||
>
|
||||
Schedule automatic snapshot
|
||||
</button>
|
||||
<button
|
||||
onClick={() => {
|
||||
setDropMetricsTarget(true);
|
||||
refresh();
|
||||
}}
|
||||
>
|
||||
Drop metrics target
|
||||
</button>
|
||||
<button
|
||||
onClick={() => {
|
||||
setDropMetricsTarget(false);
|
||||
refresh();
|
||||
}}
|
||||
>
|
||||
Restore metrics target
|
||||
</button>
|
||||
<output aria-label="Snapshot number">{snapshot()}</output>
|
||||
<ProxmoxBackupServersTable servers={servers()} />
|
||||
</>
|
||||
|
|
|
|||
|
|
@ -1,36 +1,38 @@
|
|||
{
|
||||
"version": 1,
|
||||
"base_sha": "702278cd8cca6bfbd65747c72fbb7afdc7f7a48e",
|
||||
"verified_at": "2026-09-22T16:26:06.944232Z",
|
||||
"base_sha": "79cf2d0058c374adc7fcc2e7268ab2261e10fa24",
|
||||
"verified_at": "2026-09-22T23:54:27Z",
|
||||
"result": "passed",
|
||||
"changed_paths": [
|
||||
"frontend-modern/src/utils/agentInstallCommand.ts"
|
||||
"frontend-modern/src/components/Infrastructure/useResourceDetailDrawerState.ts"
|
||||
],
|
||||
"content_sha256": {
|
||||
"frontend-modern/src/utils/agentInstallCommand.ts": "5c456b9b8cf2d114d5be62bb46c27b265d30fc49c3df88d2ec42631414f65c64"
|
||||
"frontend-modern/src/components/Infrastructure/useResourceDetailDrawerState.ts": "24f77d42e2542dd0e8fcf9c55f25a8373c563069c5a0dc112afa6a4456d5a93a"
|
||||
},
|
||||
"routes": [
|
||||
"Production buildUnixAgentInstallCommand in synthetic clipboard/text-input fixture"
|
||||
"/browser-tests/pbs-host-history-correlation.html?topology=guest&order=stable (Proxmox Backups server drawer)"
|
||||
],
|
||||
"viewports": [
|
||||
{
|
||||
"width": 1280,
|
||||
"height": 850
|
||||
"height": 900
|
||||
},
|
||||
{
|
||||
"width": 390,
|
||||
"height": 850
|
||||
"height": 844
|
||||
}
|
||||
],
|
||||
"states": [
|
||||
"Tokenless HTTPS, token HTTP, quoted URL/token, insecure HTTPS, custom CA with extra flags"
|
||||
"Drawer History tab selected with a populated host history target",
|
||||
"Snapshot with the merged host metrics target transiently absent (availability notice)",
|
||||
"Snapshot with the metrics target restored on the same selection"
|
||||
],
|
||||
"interactions": [
|
||||
"Copy generated command with browser clipboard API and paste with Control+V into input type=text",
|
||||
"Assert five variants preserve exact bytes at both viewports; old source changes all five",
|
||||
"Check pasted output syntax with sh and bash: old five fail, repaired five pass",
|
||||
"Inspect desktop-before and narrow-after screenshots"
|
||||
"Expand the Proxmox Backups server row and select the History tab; assert the chart paths render",
|
||||
"Apply a snapshot that removes the merged host metrics target; assert the drawer is not remounted and the History selection is retained, showing the in-tab availability notice",
|
||||
"Apply a snapshot that restores the metrics target; assert the same History selection recovers the chart paths",
|
||||
"Repeat at 1280x900 and 390x844; the same check fails on the pre-fix source (no notice, selection reset)"
|
||||
],
|
||||
"command": "pulse-worker-browser .proof-install/browser.cjs",
|
||||
"notes": "Chromium 141.0.7390.37 / Playwright 1.56.1. TypeScript-transpiled production utility, synthetic pulse.invalid and invented tokens. Generic text field, not pfSense DOM or installation. No backend/network/service proof. Retained proof was rebound after the test-only assertion correction and exact candidate integration; production utility bytes are unchanged from the browser run."
|
||||
"command": "pulse-worker-browser scripts/check-drawer-tab-retention.cjs",
|
||||
"notes": "Chromium 141.0.7390.37 / Playwright 1.56.1. Production ResourceDetailDrawer mounted through the real ProxmoxBackupServersTable fixture with synthetic API responses. Proves the drawer tab-retention behaviour, not backend persistence or installed release behaviour. The identical check timed out on the pre-fix source because the selected tab reset to Overview."
|
||||
}
|
||||
|
|
|
|||
|
|
@ -216,6 +216,11 @@ describe('ResourceDetailDrawer change history section', () => {
|
|||
);
|
||||
expect(resourceDetailDrawerStateSource).not.toContain('createResource(');
|
||||
expect(resourceDetailDrawerStateSource).not.toContain('MonitoringAPI.');
|
||||
// #1723: a transient snapshot change (for example a momentarily absent
|
||||
// merged metrics target) must not discard the selected drawer tab. The
|
||||
// guard resets only when the drawer starts showing a different resource.
|
||||
expect(resourceDetailDrawerStateSource).toContain('lastResourceId');
|
||||
expect(resourceDetailDrawerStateSource).toContain('resourceId !== lastResourceId');
|
||||
expect(resourceDetailDrawerHistoryStateSource).toContain(
|
||||
"from '@/hooks/createNonSuspendingQuery'",
|
||||
);
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { cleanup, fireEvent, render, screen, within } from '@solidjs/testing-library';
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import { createStore, reconcile } from 'solid-js/store';
|
||||
import type { Resource } from '@/types/resource';
|
||||
import { ResourceDetailDrawer } from '../ResourceDetailDrawer';
|
||||
import { resetAIRuntimeState, syncAIRuntimeSettings } from '@/stores/aiRuntimeState';
|
||||
|
|
@ -314,4 +315,43 @@ describe('ResourceDetailDrawer machine metrics history', () => {
|
|||
expect(within(hostSection).queryByRole('button', { name: 'Hide host' })).toBeNull();
|
||||
expect(within(hostSection).queryByText('Hardware')).toBeNull();
|
||||
});
|
||||
|
||||
it('keeps the selected History tab across a snapshot refresh that briefly drops the metrics target', async () => {
|
||||
const initial = resource({
|
||||
id: 'pbs-1',
|
||||
type: 'pbs',
|
||||
metricsTarget: { resourceType: 'agent', resourceId: 'agent-pbs-1' },
|
||||
cpu: { current: 12 },
|
||||
});
|
||||
const [pbs, setPbs] = createStore<Resource>(initial);
|
||||
render(() => <ResourceDetailDrawer resource={pbs} />);
|
||||
|
||||
await fireEvent.click(screen.getByRole('tab', { name: 'History' }));
|
||||
expect(screen.getByTestId('machine-history')).toHaveAttribute(
|
||||
'data-resource-id',
|
||||
'agent-pbs-1',
|
||||
);
|
||||
|
||||
// A snapshot refresh arrives without the merged metrics target. The drawer
|
||||
// must not silently discard the user's History selection: it should show
|
||||
// the in-tab notice and recover when the target returns on the next poll.
|
||||
setPbs(reconcile({ ...initial, metricsTarget: undefined }, { merge: false }));
|
||||
expect(screen.getByText('Metrics history is unavailable.')).toBeInTheDocument();
|
||||
|
||||
setPbs(
|
||||
reconcile(
|
||||
resource({
|
||||
id: 'pbs-1',
|
||||
type: 'pbs',
|
||||
metricsTarget: { resourceType: 'agent', resourceId: 'agent-pbs-1' },
|
||||
cpu: { current: 12 },
|
||||
}),
|
||||
{ merge: false },
|
||||
),
|
||||
);
|
||||
expect(screen.getByTestId('machine-history')).toHaveAttribute(
|
||||
'data-resource-id',
|
||||
'agent-pbs-1',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -87,11 +87,21 @@ export const useResourceDetailDrawerState = (options: UseResourceDetailDrawerSta
|
|||
}
|
||||
});
|
||||
|
||||
// Fall back to Overview only when the drawer starts showing a different
|
||||
// resource. A live snapshot can transiently omit a field that gates a tab
|
||||
// (for example a merged metrics target during a refresh), and discarding the
|
||||
// user's selection on that change made the History tab silently jump back to
|
||||
// Overview. Each tab body already renders an availability notice while its
|
||||
// tab is unavailable, so a same-resource change must not reset the selection.
|
||||
let lastResourceId: string | undefined;
|
||||
createEffect(() => {
|
||||
const current = activeTab();
|
||||
const resourceId = resource.id;
|
||||
const available = new Set(derived.tabs().map((tab) => tab.id));
|
||||
if (!available.has(current)) {
|
||||
setActiveTab('overview');
|
||||
if (resourceId !== lastResourceId) {
|
||||
if (!available.has(activeTab())) {
|
||||
setActiveTab('overview');
|
||||
}
|
||||
lastResourceId = resourceId;
|
||||
}
|
||||
});
|
||||
|
||||
|
|
|
|||
196
scripts/check-drawer-tab-retention.cjs
Normal file
196
scripts/check-drawer-tab-retention.cjs
Normal file
|
|
@ -0,0 +1,196 @@
|
|||
// Offline real-browser proof for the #1723 drawer tab-retention repair.
|
||||
//
|
||||
// Opens the production Proxmox Backups server drawer, selects History, then
|
||||
// drives a snapshot that transiently removes the merged host metrics target
|
||||
// (the field that gates the History tab). The drawer must keep the user's
|
||||
// History selection and show the in-tab availability notice instead of
|
||||
// silently falling back to Overview; restoring the target must bring the chart
|
||||
// back on the same selection. Synthetic API responses prove UI behaviour, not
|
||||
// backend persistence or installed release behaviour.
|
||||
// Run: pulse-worker-browser scripts/check-drawer-tab-retention.cjs
|
||||
const path = require('node:path');
|
||||
const assert = require('node:assert/strict');
|
||||
const { chromium } = require('playwright');
|
||||
|
||||
const ROOT = path.resolve(process.cwd(), 'frontend-modern');
|
||||
const launchOptions = {
|
||||
headless: true,
|
||||
channel: 'chromium',
|
||||
args: ['--no-sandbox'],
|
||||
};
|
||||
|
||||
(async () => {
|
||||
process.chdir(ROOT);
|
||||
const { createServer } = await import(
|
||||
path.join(ROOT, 'node_modules', 'vite', 'dist', 'node', 'index.js')
|
||||
);
|
||||
const server = await createServer({
|
||||
root: ROOT,
|
||||
configFile: path.join(ROOT, 'vite.config.ts'),
|
||||
server: { host: '127.0.0.1', port: 5199, strictPort: true },
|
||||
});
|
||||
let browser;
|
||||
const results = [];
|
||||
const pageErrors = [];
|
||||
const failures = [];
|
||||
try {
|
||||
await server.listen();
|
||||
browser = await chromium.launch(launchOptions);
|
||||
for (const viewport of [
|
||||
{ width: 1280, height: 900 },
|
||||
{ width: 390, height: 844 },
|
||||
]) {
|
||||
const caseName = `${viewport.width}`;
|
||||
const page = await browser.newPage({ viewport });
|
||||
page.on('pageerror', (error) =>
|
||||
pageErrors.push(`${viewport.width}px: ${error.message}`),
|
||||
);
|
||||
await page.route('**/api/license/runtime-capabilities', (route) =>
|
||||
route.fulfill({
|
||||
json: {
|
||||
capabilities: [],
|
||||
limits: [],
|
||||
hosted_mode: false,
|
||||
max_history_days: 7,
|
||||
runtime: { build: 'community', label: 'Pulse Community runtime' },
|
||||
blocked_capabilities: [],
|
||||
},
|
||||
}),
|
||||
);
|
||||
await page.route('**/api/settings/ai', (route) =>
|
||||
route.fulfill({ json: { enabled: false } }),
|
||||
);
|
||||
await page.route('**/api/metrics-store/history?**', async (route) => {
|
||||
const query = new URL(route.request().url()).searchParams;
|
||||
await route.fulfill({
|
||||
json: {
|
||||
resourceType: query.get('resourceType'),
|
||||
resourceId: query.get('resourceId'),
|
||||
range: query.get('range'),
|
||||
start: Date.now() - 3600000,
|
||||
end: Date.now(),
|
||||
source: 'store',
|
||||
metrics: Object.fromEntries(
|
||||
[
|
||||
'cpu',
|
||||
'memory',
|
||||
'disk',
|
||||
'netin',
|
||||
'netout',
|
||||
'diskread',
|
||||
'diskwrite',
|
||||
].map((metric) => [
|
||||
metric,
|
||||
[30, 20, 10].map((minutes, i) => ({
|
||||
timestamp: Date.now() - minutes * 60000,
|
||||
value: 10 + i * 5,
|
||||
min: 10 + i * 5,
|
||||
max: 10 + i * 5,
|
||||
})),
|
||||
]),
|
||||
),
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
try {
|
||||
await page.goto(
|
||||
'http://127.0.0.1:5199/browser-tests/pbs-host-history-correlation.html?topology=guest&order=stable',
|
||||
{ timeout: 60000 },
|
||||
);
|
||||
const serverRow = page
|
||||
.locator('tr')
|
||||
.filter({
|
||||
has: page.getByRole('button', {
|
||||
name: 'Expand details for proxback',
|
||||
exact: true,
|
||||
}),
|
||||
})
|
||||
.first();
|
||||
await serverRow.waitFor({ timeout: 20000 });
|
||||
await serverRow.getByRole('button').focus();
|
||||
await page.keyboard.press('Enter');
|
||||
await page
|
||||
.getByRole('tab', { name: 'History', exact: true })
|
||||
.click();
|
||||
await page
|
||||
.locator('[data-testid="guest-history-plot"] path')
|
||||
.first()
|
||||
.waitFor({ timeout: 20000 });
|
||||
const detail = page.locator(
|
||||
'[data-inline-platform-resource-detail-for="pbs-1"]',
|
||||
);
|
||||
await detail.evaluate((node) => {
|
||||
node.dataset.proofIdentity = 'retained';
|
||||
});
|
||||
const pathsBefore = await detail
|
||||
.locator('[data-testid="guest-history-plot"] path')
|
||||
.count();
|
||||
assert.ok(pathsBefore > 0, 'history plot did not render before the drop');
|
||||
|
||||
// Snapshot arrives without the merged host metrics target.
|
||||
await page.getByRole('button', { name: 'Drop metrics target' }).click();
|
||||
await page
|
||||
.getByText('Metrics history is unavailable.', { exact: true })
|
||||
.waitFor({ timeout: 20000 });
|
||||
assert.equal(
|
||||
await detail.getAttribute('data-proof-identity'),
|
||||
'retained',
|
||||
'drawer remounted on the transient target loss',
|
||||
);
|
||||
|
||||
// Next snapshot restores it; the same History selection must recover.
|
||||
await page.getByRole('button', { name: 'Restore metrics target' }).click();
|
||||
await page
|
||||
.locator('[data-testid="guest-history-plot"] path')
|
||||
.first()
|
||||
.waitFor({ timeout: 20000 });
|
||||
assert.equal(
|
||||
await page
|
||||
.getByRole('tab', { name: 'History', exact: true })
|
||||
.getAttribute('aria-selected'),
|
||||
'true',
|
||||
'history selection was discarded across the transient target loss',
|
||||
);
|
||||
assert.equal(
|
||||
await detail
|
||||
.locator('[data-testid="guest-history-plot"] path')
|
||||
.count(),
|
||||
pathsBefore,
|
||||
);
|
||||
results.push({ viewport, paths: pathsBefore, result: 'passed' });
|
||||
} catch (error) {
|
||||
failures.push(`${caseName}: ${error.message}`);
|
||||
results.push({ viewport, result: 'failed', error: error.message });
|
||||
} finally {
|
||||
await page.screenshot({
|
||||
path: path.join(ROOT, 'browser-tests', `drawer-tab-retention-${caseName}.png`),
|
||||
});
|
||||
await page.close();
|
||||
}
|
||||
}
|
||||
|
||||
if (pageErrors.length > 0) {
|
||||
throw new Error(`page errors: ${pageErrors.join(' | ')}`);
|
||||
}
|
||||
console.log(
|
||||
JSON.stringify(
|
||||
{
|
||||
result: failures.length ? 'failed' : 'passed',
|
||||
browser: browser.version(),
|
||||
results,
|
||||
failures,
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
assert.equal(failures.length, 0, failures.join(' | '));
|
||||
} finally {
|
||||
if (browser) await browser.close();
|
||||
await server.close();
|
||||
}
|
||||
})().catch((error) => {
|
||||
console.error('FAILED:', error && error.stack ? error.stack : error);
|
||||
process.exitCode = 1;
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue