mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 12:15:42 +00:00
Unify the update apply flow behind one store action with a post-update toast
Both the update banner and the Settings update panel duplicated the confirm/apply POST with divergent error handling; the banner's failure path was a native alert(). updateStore.applyUpdate() now owns the POST, the shared error toast, and a persisted pending-apply marker recording the pre-update version. On the next version fetch after the post-update reload, the store confirms the running version moved and shows a one-time 'Updated to vX.Y.Z' toast, clearing the marker either way.
This commit is contained in:
@@ -162,7 +162,6 @@ describe('useSystemSettingsState', () => {
|
||||
UpdatesAPI: {
|
||||
getVersion: getVersionMock,
|
||||
getUpdatePlan: vi.fn(),
|
||||
applyUpdate: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -195,6 +194,7 @@ describe('useSystemSettingsState', () => {
|
||||
vi.doMock('@/stores/updates', () => ({
|
||||
updateStore: {
|
||||
checkForUpdates: vi.fn().mockResolvedValue(undefined),
|
||||
applyUpdate: vi.fn().mockResolvedValue(true),
|
||||
updateInfo: vi.fn().mockReturnValue(null),
|
||||
versionInfo: updateStoreVersionInfoMock,
|
||||
isDismissed: vi.fn().mockReturnValue(false),
|
||||
|
||||
@@ -14,7 +14,6 @@ import {
|
||||
getBackupIntervalSummary,
|
||||
getDockerUpdateActionsUpdateErrorMessage,
|
||||
getHideLocalLoginUpdateErrorMessage,
|
||||
getStartUpdateErrorMessage,
|
||||
getSystemSettingsSaveErrorMessage,
|
||||
getTelemetryUpdateErrorMessage,
|
||||
getTemperatureMonitoringUpdateErrorMessage,
|
||||
@@ -461,18 +460,13 @@ export function useSystemSettingsState({
|
||||
};
|
||||
|
||||
const handleConfirmUpdate = async () => {
|
||||
const info = updateInfo();
|
||||
if (!info?.downloadUrl) {
|
||||
return;
|
||||
}
|
||||
|
||||
setIsInstallingUpdate(true);
|
||||
try {
|
||||
await UpdatesAPI.applyUpdate(info.downloadUrl);
|
||||
setShowUpdateConfirmation(false);
|
||||
} catch (error) {
|
||||
logger.error('Failed to start update', error);
|
||||
notificationStore.error(getStartUpdateErrorMessage());
|
||||
// The shared store action owns the POST and error toast.
|
||||
const started = await updateStore.applyUpdate();
|
||||
if (started) {
|
||||
setShowUpdateConfirmation(false);
|
||||
}
|
||||
} finally {
|
||||
setIsInstallingUpdate(false);
|
||||
}
|
||||
|
||||
@@ -36,13 +36,13 @@ vi.mock('@/stores/updates', () => ({
|
||||
versionInfo: () => ({ version: '6.0.0', deploymentType: 'systemd' }),
|
||||
isUpdateVisible: () => true,
|
||||
dismissUpdate: vi.fn(),
|
||||
applyUpdate: vi.fn(() => Promise.resolve(true)),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('@/api/updates', () => ({
|
||||
UpdatesAPI: {
|
||||
getUpdatePlan: vi.fn(() => Promise.resolve(mock.plan)),
|
||||
applyUpdate: vi.fn(() => Promise.resolve()),
|
||||
},
|
||||
}));
|
||||
|
||||
|
||||
@@ -68,17 +68,14 @@ export function UpdateBanner() {
|
||||
};
|
||||
|
||||
const handleConfirmUpdate = async () => {
|
||||
const info = updateStore.updateInfo();
|
||||
if (!info?.downloadUrl) return;
|
||||
|
||||
setIsApplying(true);
|
||||
try {
|
||||
await UpdatesAPI.applyUpdate(info.downloadUrl);
|
||||
// Close confirmation - GlobalUpdateProgressWatcher will auto-open the progress modal
|
||||
setShowConfirmModal(false);
|
||||
} catch (error) {
|
||||
logger.error('Failed to start update', error);
|
||||
alert('Failed to start update. Please try again.');
|
||||
// The shared store action owns the POST and error toast.
|
||||
const started = await updateStore.applyUpdate();
|
||||
if (started) {
|
||||
// Close confirmation - GlobalUpdateProgressWatcher will auto-open the progress modal
|
||||
setShowConfirmModal(false);
|
||||
}
|
||||
} finally {
|
||||
setIsApplying(false);
|
||||
}
|
||||
|
||||
@@ -3,11 +3,22 @@ import { STORAGE_KEYS } from '@/utils/localStorage';
|
||||
|
||||
const mockGetVersion = vi.fn();
|
||||
const mockCheckForUpdates = vi.fn();
|
||||
const mockApplyUpdate = vi.fn();
|
||||
const mockNotifySuccess = vi.fn();
|
||||
const mockNotifyError = vi.fn();
|
||||
|
||||
vi.mock('@/api/updates', () => ({
|
||||
UpdatesAPI: {
|
||||
getVersion: (...args: unknown[]) => mockGetVersion(...args),
|
||||
checkForUpdates: (...args: unknown[]) => mockCheckForUpdates(...args),
|
||||
applyUpdate: (...args: unknown[]) => mockApplyUpdate(...args),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('@/stores/notifications', () => ({
|
||||
notificationStore: {
|
||||
success: (...args: unknown[]) => mockNotifySuccess(...args),
|
||||
error: (...args: unknown[]) => mockNotifyError(...args),
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -43,6 +54,9 @@ describe('updateStore', () => {
|
||||
localStorage.clear();
|
||||
mockGetVersion.mockReset();
|
||||
mockCheckForUpdates.mockReset();
|
||||
mockApplyUpdate.mockReset();
|
||||
mockNotifySuccess.mockReset();
|
||||
mockNotifyError.mockReset();
|
||||
});
|
||||
|
||||
it('retries transient update-check failures before succeeding', async () => {
|
||||
@@ -112,4 +126,128 @@ describe('updateStore', () => {
|
||||
expect(updateStore.isDismissed()).toBe(false);
|
||||
expect(updateStore.updateAvailable()).toBe(true);
|
||||
});
|
||||
|
||||
describe('applyUpdate', () => {
|
||||
it('posts the apply and records the pending-apply marker before it', async () => {
|
||||
mockApplyUpdate.mockResolvedValue({ status: 'ok', message: '' });
|
||||
|
||||
const updateStore = await loadUpdateStore();
|
||||
updateStore.simulateUpdate('v1.1.0');
|
||||
|
||||
const started = await updateStore.applyUpdate();
|
||||
|
||||
expect(started).toBe(true);
|
||||
expect(mockApplyUpdate).toHaveBeenCalledWith('#');
|
||||
const persisted = JSON.parse(localStorage.getItem(STORAGE_KEYS.UPDATES) ?? '{}');
|
||||
expect(persisted.pendingApply).toMatchObject({
|
||||
fromVersion: 'v6.0.0',
|
||||
toVersion: 'v1.1.0',
|
||||
});
|
||||
expect(mockNotifyError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('clears the marker and toasts the shared error message when the apply fails', async () => {
|
||||
mockApplyUpdate.mockRejectedValue(new Error('boom'));
|
||||
|
||||
const updateStore = await loadUpdateStore();
|
||||
updateStore.simulateUpdate('v1.1.0');
|
||||
|
||||
const started = await updateStore.applyUpdate();
|
||||
|
||||
expect(started).toBe(false);
|
||||
expect(mockNotifyError).toHaveBeenCalledWith('Unable to start the update. Please try again.');
|
||||
const persisted = JSON.parse(localStorage.getItem(STORAGE_KEYS.UPDATES) ?? '{}');
|
||||
expect(persisted.pendingApply).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does nothing without a download URL', async () => {
|
||||
const updateStore = await loadUpdateStore();
|
||||
|
||||
const started = await updateStore.applyUpdate();
|
||||
|
||||
expect(started).toBe(false);
|
||||
expect(mockApplyUpdate).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('post-update confirmation', () => {
|
||||
const seedPendingApply = (fromVersion: string, toVersion: string) => {
|
||||
localStorage.setItem(
|
||||
STORAGE_KEYS.UPDATES,
|
||||
JSON.stringify({
|
||||
lastCheck: 0,
|
||||
pendingApply: { fromVersion, toVersion, startedAt: Date.now() },
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
it('toasts once when the running version moved off the pre-update version', async () => {
|
||||
seedPendingApply('v1.0.0', 'v1.1.0');
|
||||
mockGetVersion.mockResolvedValue({ ...baseVersionInfo, version: 'v1.1.0' });
|
||||
mockCheckForUpdates.mockResolvedValue({
|
||||
...baseUpdateInfo,
|
||||
available: false,
|
||||
currentVersion: 'v1.1.0',
|
||||
});
|
||||
|
||||
const updateStore = await loadUpdateStore();
|
||||
await updateStore.checkForUpdates(true);
|
||||
|
||||
expect(mockNotifySuccess).toHaveBeenCalledWith('Updated to v1.1.0');
|
||||
const persisted = JSON.parse(localStorage.getItem(STORAGE_KEYS.UPDATES) ?? '{}');
|
||||
expect(persisted.pendingApply).toBeUndefined();
|
||||
|
||||
// The marker is consumed: a later check must not toast again.
|
||||
mockNotifySuccess.mockClear();
|
||||
await updateStore.checkForUpdates(true);
|
||||
expect(mockNotifySuccess).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('confirms even for dev builds that skip the update check itself', async () => {
|
||||
seedPendingApply('v1.0.0', 'v1.1.0');
|
||||
mockGetVersion.mockResolvedValue({
|
||||
...baseVersionInfo,
|
||||
version: 'v1.1.0',
|
||||
isDevelopment: true,
|
||||
});
|
||||
|
||||
const updateStore = await loadUpdateStore();
|
||||
await updateStore.checkForUpdates(true);
|
||||
|
||||
expect(mockCheckForUpdates).not.toHaveBeenCalled();
|
||||
expect(mockNotifySuccess).toHaveBeenCalledWith('Updated to v1.1.0');
|
||||
});
|
||||
|
||||
it('clears the marker silently when the version did not change', async () => {
|
||||
seedPendingApply('v1.0.0', 'v1.1.0');
|
||||
mockGetVersion.mockResolvedValue({ ...baseVersionInfo, version: 'v1.0.0' });
|
||||
mockCheckForUpdates.mockResolvedValue({ ...baseUpdateInfo, available: false });
|
||||
|
||||
const updateStore = await loadUpdateStore();
|
||||
await updateStore.checkForUpdates(true);
|
||||
|
||||
expect(mockNotifySuccess).not.toHaveBeenCalled();
|
||||
const persisted = JSON.parse(localStorage.getItem(STORAGE_KEYS.UPDATES) ?? '{}');
|
||||
expect(persisted.pendingApply).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not resurrect a consumed marker when the check later saves state', async () => {
|
||||
// Fresh-path save at the end of checkForUpdates spreads the in-memory
|
||||
// state; a stale copy would re-persist the consumed marker.
|
||||
seedPendingApply('v1.0.0', 'v1.1.0');
|
||||
mockGetVersion.mockResolvedValue({ ...baseVersionInfo, version: 'v1.1.0' });
|
||||
mockCheckForUpdates.mockResolvedValue({
|
||||
...baseUpdateInfo,
|
||||
currentVersion: 'v1.1.0',
|
||||
latestVersion: 'v1.2.0',
|
||||
});
|
||||
|
||||
const updateStore = await loadUpdateStore();
|
||||
await updateStore.checkForUpdates(true);
|
||||
|
||||
const persisted = JSON.parse(localStorage.getItem(STORAGE_KEYS.UPDATES) ?? '{}');
|
||||
expect(persisted.updateInfo?.latestVersion).toBe('v1.2.0');
|
||||
expect(persisted.pendingApply).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,18 +1,29 @@
|
||||
import { createSignal } from 'solid-js';
|
||||
import { UpdatesAPI } from '@/api/updates';
|
||||
import type { UpdateInfo, VersionInfo } from '@/api/updates';
|
||||
import { notificationStore } from '@/stores/notifications';
|
||||
import { getPulseHostname } from '@/utils/url';
|
||||
import { logger } from '@/utils/logger';
|
||||
import { STORAGE_KEYS } from '@/utils/localStorage';
|
||||
import { getStartUpdateErrorMessage } from '@/utils/systemSettingsPresentation';
|
||||
|
||||
const CHECK_INTERVAL = 24 * 60 * 60 * 1000; // 24 hours
|
||||
const TRANSIENT_RETRY_MAX_ATTEMPTS = 3;
|
||||
const TRANSIENT_RETRY_BASE_DELAY_MS = import.meta.env.MODE === 'test' ? 0 : 400;
|
||||
|
||||
// Persisted before POSTing an apply so the boot after the post-update page
|
||||
// reload can confirm the update landed (in-memory state does not survive it).
|
||||
interface PendingApply {
|
||||
fromVersion: string;
|
||||
toVersion: string;
|
||||
startedAt: number;
|
||||
}
|
||||
|
||||
interface UpdateState {
|
||||
lastCheck: number;
|
||||
dismissedVersion?: string;
|
||||
updateInfo?: UpdateInfo;
|
||||
pendingApply?: PendingApply;
|
||||
}
|
||||
|
||||
const isRecord = (value: unknown): value is Record<string, unknown> =>
|
||||
@@ -65,6 +76,21 @@ const normalizeUpdateInfo = (value: unknown): UpdateInfo | undefined => {
|
||||
};
|
||||
};
|
||||
|
||||
const normalizePendingApply = (value: unknown): PendingApply | undefined => {
|
||||
if (!isRecord(value)) return undefined;
|
||||
|
||||
const { fromVersion, toVersion, startedAt } = value;
|
||||
if (
|
||||
typeof fromVersion !== 'string' ||
|
||||
typeof toVersion !== 'string' ||
|
||||
!isFiniteNumber(startedAt)
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return { fromVersion, toVersion, startedAt };
|
||||
};
|
||||
|
||||
const normalizeUpdateState = (value: unknown): UpdateState => {
|
||||
if (!isRecord(value)) {
|
||||
return { lastCheck: 0 };
|
||||
@@ -83,6 +109,11 @@ const normalizeUpdateState = (value: unknown): UpdateState => {
|
||||
normalized.updateInfo = normalizedUpdateInfo;
|
||||
}
|
||||
|
||||
const normalizedPendingApply = normalizePendingApply(value.pendingApply);
|
||||
if (normalizedPendingApply) {
|
||||
normalized.pendingApply = normalizedPendingApply;
|
||||
}
|
||||
|
||||
return normalized;
|
||||
};
|
||||
|
||||
@@ -164,6 +195,59 @@ export const withTransientRetry = async <T>(
|
||||
}
|
||||
};
|
||||
|
||||
const markApplyStarted = (fromVersion: string, toVersion: string) => {
|
||||
const state = loadState();
|
||||
saveState({ ...state, pendingApply: { fromVersion, toVersion, startedAt: Date.now() } });
|
||||
};
|
||||
|
||||
const clearPendingApply = () => {
|
||||
const state = loadState();
|
||||
if (!state.pendingApply) return;
|
||||
delete state.pendingApply;
|
||||
saveState(state);
|
||||
};
|
||||
|
||||
const formatVersionLabel = (version: string) =>
|
||||
version.startsWith('v') ? version : `v${version}`;
|
||||
|
||||
// One-time post-update confirmation: consume the pending-apply marker on the
|
||||
// first version fetch after an apply. When the running version moved off the
|
||||
// recorded pre-update version, the update landed; toast it once. When it did
|
||||
// not (apply failed or rolled back), clear the marker silently. Mutates the
|
||||
// caller's state object so later saveState(state) calls in the same check
|
||||
// cannot resurrect the consumed marker.
|
||||
const confirmPendingApply = (currentVersion: string, state: UpdateState) => {
|
||||
const pending = state.pendingApply;
|
||||
if (!pending) return;
|
||||
|
||||
delete state.pendingApply;
|
||||
saveState(state);
|
||||
|
||||
if (currentVersion && currentVersion !== pending.fromVersion) {
|
||||
notificationStore.success(`Updated to ${formatVersionLabel(currentVersion)}`);
|
||||
}
|
||||
};
|
||||
|
||||
// Shared confirm/apply action for both update surfaces (banner and Settings).
|
||||
// Persists the pre-update version so the post-restart boot can confirm the
|
||||
// update, and owns the single error-handling path. Returns true when the
|
||||
// backend accepted the apply request.
|
||||
const applyUpdate = async (): Promise<boolean> => {
|
||||
const info = updateInfo();
|
||||
if (!info?.downloadUrl) return false;
|
||||
|
||||
markApplyStarted(versionInfo()?.version || info.currentVersion, info.latestVersion);
|
||||
try {
|
||||
await UpdatesAPI.applyUpdate(info.downloadUrl);
|
||||
return true;
|
||||
} catch (error) {
|
||||
clearPendingApply();
|
||||
logger.error('Failed to start update', error);
|
||||
notificationStore.error(getStartUpdateErrorMessage());
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
// Check for updates
|
||||
const checkForUpdates = async (force = false): Promise<void> => {
|
||||
// Don't check if already checking
|
||||
@@ -181,6 +265,7 @@ const checkForUpdates = async (force = false): Promise<void> => {
|
||||
const currentVersion = await withTransientRetry('version check', () =>
|
||||
UpdatesAPI.getVersion(),
|
||||
);
|
||||
confirmPendingApply(currentVersion.version, state);
|
||||
if (state.updateInfo.currentVersion !== currentVersion.version) {
|
||||
// Version changed, invalidate cache and check again
|
||||
state.updateInfo = undefined;
|
||||
@@ -215,6 +300,7 @@ const checkForUpdates = async (force = false): Promise<void> => {
|
||||
// First get version info to check deployment type
|
||||
const version = await withTransientRetry('version check', () => UpdatesAPI.getVersion());
|
||||
setVersionInfo(version);
|
||||
confirmPendingApply(version.version, state);
|
||||
|
||||
// Clear cache if version has changed (user updated)
|
||||
if (state.updateInfo && state.updateInfo.currentVersion !== version.version) {
|
||||
@@ -319,6 +405,7 @@ export const updateStore = {
|
||||
|
||||
// Actions
|
||||
checkForUpdates,
|
||||
applyUpdate,
|
||||
dismissUpdate,
|
||||
clearDismissed,
|
||||
|
||||
|
||||
Reference in New Issue
Block a user