Skip to content

Commit 6897f7e

Browse files
fix(onboarding): reload after server rename (#1981)
## Summary This PR improves the onboarding flow when a user changes the Unraid server name. A successful hostname change can interrupt the current browser session and force the user to refresh before continuing. The app cannot bypass the browser trust / reload interruption itself, but it can choose when that interruption happens. Today, the interruption shows up too late in the flow. Users apply settings successfully, move forward, and only run into the break when leaving onboarding for the dashboard. This change moves that interruption to the correct point in the flow. After a successful server-name change, the Summary step now advances the wizard to `NEXT_STEPS`, persists that step state, and then reloads the page. Once the page is back, onboarding can reopen at the persisted next step instead of forcing the user back through apply. ## Problem When onboarding applies a server-name change: - the current browser session can be interrupted - the current flow waits until the user exits onboarding to surface that interruption - this makes the flow feel broken and can lead to confusing re-entry behavior after refresh The app cannot automatically bypass the browser interruption, so the best UX is to make it happen immediately after the rename and before final completion. ## What Changed In `OnboardingSummaryStep.vue`: - widened the `onComplete` prop to allow an async completion path - added a one-shot `shouldReloadAfterApplyResult` flag - detect successful server-name changes - on results dialog confirmation, await the parent completion callback first - after step advancement settles, reload the page This preserves the intended flow: 1. Apply settings 2. Show success dialog 3. User clicks `OK` 4. Advance to `NEXT_STEPS` 5. Reload the page if the successful apply included a server-name change 6. Onboarding resumes from the persisted next step ## Why This Shape We intentionally reload only after: - the server-name change succeeded - the parent `onComplete()` callback has already advanced the step That keeps the refresh tied to the actual rename, while avoiding a regression where users would come back to the Summary step and re-run apply. ## Testing Added and updated a focused regression test in `web/__test__/components/Onboarding/OnboardingSummaryStep.test.ts` that verifies: - a successful server rename arms the reload behavior - the Summary step still waits for the user to confirm the result dialog - `onComplete()` is invoked before the reload happens - `window.location.reload()` is called exactly once in the rename case Validated with: ```bash pnpm test __test__/components/Onboarding/OnboardingSummaryStep.test.ts ``` ## Notes This PR does not attempt to eliminate the browser interruption itself. That is outside the control of the web app. It only makes the interruption happen at the right moment and preserves onboarding progress across that reload.
1 parent a78b895 commit 6897f7e

2 files changed

Lines changed: 42 additions & 6 deletions

File tree

web/__test__/components/Onboarding/OnboardingSummaryStep.test.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ interface SummaryVm {
295295
applyResultSeverity: 'success' | 'warning' | 'error';
296296
handleBootDriveWarningConfirm: () => Promise<void>;
297297
handleBootDriveWarningCancel: () => void;
298-
handleApplyResultConfirm: () => void;
298+
handleApplyResultConfirm: () => Promise<void>;
299299
}
300300

301301
const getSummaryVm = (wrapper: ReturnType<typeof mountComponent>['wrapper']) =>
@@ -332,7 +332,7 @@ const clickButtonByText = async (
332332
} else if (normalizedTarget === 'cancel') {
333333
vm.handleBootDriveWarningCancel();
334334
} else if (normalizedTarget === 'ok') {
335-
vm.handleApplyResultConfirm();
335+
await vm.handleApplyResultConfirm();
336336
} else {
337337
expect(button).toBeTruthy();
338338
}
@@ -1015,6 +1015,28 @@ describe('OnboardingSummaryStep', () => {
10151015
expect(onComplete).not.toHaveBeenCalled();
10161016
});
10171017

1018+
it('advances to next steps before reloading after a successful server rename', async () => {
1019+
draftStore.serverName = 'Newtower';
1020+
const reloadSpy = vi.spyOn(window.location, 'reload').mockImplementation(() => undefined);
1021+
const { wrapper, onComplete } = mountComponent();
1022+
1023+
await clickApply(wrapper);
1024+
1025+
expect(updateServerIdentityMock).toHaveBeenCalledWith({
1026+
name: 'Newtower',
1027+
comment: '',
1028+
sysModel: undefined,
1029+
});
1030+
expect(onComplete).not.toHaveBeenCalled();
1031+
1032+
await clickButtonByText(wrapper, 'OK');
1033+
1034+
expect(onComplete).toHaveBeenCalledTimes(1);
1035+
expect(reloadSpy).toHaveBeenCalledTimes(1);
1036+
1037+
reloadSpy.mockRestore();
1038+
});
1039+
10181040
it('retries final identity update after transient network errors when SSH changed', async () => {
10191041
draftStore.useSsh = true;
10201042
draftStore.serverDescription = 'Primary host';

web/src/components/Onboarding/steps/OnboardingSummaryStep.vue

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<script lang="ts" setup>
2-
import { computed, onBeforeUnmount, onMounted, ref } from 'vue';
2+
import { computed, nextTick, onBeforeUnmount, onMounted, ref } from 'vue';
33
import { useI18n } from 'vue-i18n';
44
import { storeToRefs } from 'pinia';
55
import { useMutation, useQuery } from '@vue/apollo-composable';
@@ -57,7 +57,7 @@ import {
5757
} from '~/composables/gql/graphql';
5858
5959
export interface Props {
60-
onComplete: () => void;
60+
onComplete: () => void | Promise<void>;
6161
onBack?: () => void;
6262
showBack?: boolean;
6363
}
@@ -227,6 +227,7 @@ const showBootDriveWarningDialog = ref(false);
227227
const applyResultTitle = ref('');
228228
const applyResultMessage = ref('');
229229
const applyResultSeverity = ref<'success' | 'warning' | 'error'>('success');
230+
const shouldReloadAfterApplyResult = ref(false);
230231
const summaryT = (key: string, values?: Record<string, unknown>) =>
231232
t(`onboarding.summaryStep.${key}`, values ?? {});
232233
@@ -579,6 +580,7 @@ const handleComplete = async () => {
579580
isProcessing.value = true;
580581
error.value = null;
581582
logs.value = []; // Clear logs
583+
shouldReloadAfterApplyResult.value = false;
582584
583585
addLog(summaryT('logs.startingConfiguration'), 'info');
584586
draftStore.setInternalBootApplySucceeded(false);
@@ -617,6 +619,7 @@ const handleComplete = async () => {
617619
? Boolean(coreSettingsResult.value?.vars?.useSsh || false)
618620
: TRUSTED_DEFAULT_PROFILE.useSsh;
619621
const currentSysModel = baselineLoaded ? coreSettingsResult.value?.vars?.sysModel || '' : '';
622+
const serverNameChanged = baselineLoaded ? targetCoreSettings.serverName !== currentName : false;
620623
const shouldApplyPartnerSysModel = Boolean(
621624
isFreshInstall.value &&
622625
activationSystemModel.value &&
@@ -657,6 +660,9 @@ const handleComplete = async () => {
657660
}),
658661
shouldRetryNetworkMutations
659662
);
663+
if (serverNameChanged) {
664+
shouldReloadAfterApplyResult.value = true;
665+
}
660666
addLog(summaryT('logs.serverIdentityUpdated'), 'success');
661667
} catch (caughtError: unknown) {
662668
hadNonOptimisticFailures = true;
@@ -1032,9 +1038,17 @@ const handleComplete = async () => {
10321038
}
10331039
};
10341040
1035-
const handleApplyResultConfirm = () => {
1041+
const handleApplyResultConfirm = async () => {
10361042
showApplyResultDialog.value = false;
1037-
props.onComplete();
1043+
await Promise.resolve(props.onComplete());
1044+
1045+
if (!shouldReloadAfterApplyResult.value) {
1046+
return;
1047+
}
1048+
1049+
shouldReloadAfterApplyResult.value = false;
1050+
await nextTick();
1051+
window.location.reload();
10381052
};
10391053
10401054
const handleApplyClick = async () => {

0 commit comments

Comments
 (0)