From 1420797dddf0e5ab9968f25c80b890cbcc99d66f Mon Sep 17 00:00:00 2001 From: Ajit Mehrotra Date: Sun, 15 Mar 2026 16:13:37 -0400 Subject: [PATCH 01/16] fix(onboarding): preserve resumable flow on API errors - Purpose: tighten onboarding error handling so real API/tracker failures hide the modal, while missing tracker state and saved drafts can still resume the flow. - Before: onboarding tracker reads flattened missing and failed states together, and the modal gate did not preserve resumable drafts during restart windows. - Problem: summary-step API restarts could strand users mid-flow, while tracker failures could be misinterpreted as normal onboarding state. - Change: model tracker reads with explicit ok/missing/error results, surface true tracker failures as GraphQL errors, and allow the web modal to stay displayable when a resumable draft exists. - How: add tracker/result tests, update the customization resolver and generated GraphQL artifacts, and compute resumable draft visibility in the onboarding stores. --- api/generated-schema.graphql | 120 ++++++------- .../config/onboarding-tracker.service.spec.ts | 81 +++++++++ .../config/onboarding-tracker.service.ts | 60 ++++++- .../customization.resolver.spec.ts | 75 ++++++-- .../customization/customization.resolver.ts | 8 +- web/__test__/store/onboardingStatus.test.ts | 162 ++++++++++++++++++ .../Onboarding/store/onboardingDraft.ts | 11 +- .../Onboarding/store/onboardingStatus.ts | 9 +- web/src/composables/gql/graphql.ts | 2 +- 9 files changed, 439 insertions(+), 89 deletions(-) create mode 100644 web/__test__/store/onboardingStatus.test.ts diff --git a/api/generated-schema.graphql b/api/generated-schema.graphql index 66bcc8231c..d6dd77cc91 100644 --- a/api/generated-schema.graphql +++ b/api/generated-schema.graphql @@ -2031,6 +2031,65 @@ type Info implements Node { primaryNetwork: InfoNetworkInterface } +type NotificationCounts { + info: Int! + warning: Int! + alert: Int! + total: Int! +} + +type NotificationOverview { + unread: NotificationCounts! + archive: NotificationCounts! +} + +type Notification implements Node { + id: PrefixedID! + + """Also known as 'event'""" + title: String! + subject: String! + description: String! + importance: NotificationImportance! + link: String + type: NotificationType! + + """ISO Timestamp for when the notification occurred""" + timestamp: String + formattedTimestamp: String +} + +enum NotificationImportance { + ALERT + INFO + WARNING +} + +enum NotificationType { + UNREAD + ARCHIVE +} + +type Notifications implements Node { + id: PrefixedID! + + """A cached overview of the notifications in the system & their severity.""" + overview: NotificationOverview! + list(filter: NotificationFilter!): [Notification!]! + + """ + Deduplicated list of unread warning and alert notifications, sorted latest first. + """ + warningsAndAlerts: [Notification!]! +} + +input NotificationFilter { + importance: NotificationImportance + type: NotificationType! + offset: Int! + limit: Int! +} + type ExplicitStatusItem { name: String! updateStatus: UpdateStatus! @@ -2328,65 +2387,6 @@ type FlatOrganizerEntry { meta: DockerContainer } -type NotificationCounts { - info: Int! - warning: Int! - alert: Int! - total: Int! -} - -type NotificationOverview { - unread: NotificationCounts! - archive: NotificationCounts! -} - -type Notification implements Node { - id: PrefixedID! - - """Also known as 'event'""" - title: String! - subject: String! - description: String! - importance: NotificationImportance! - link: String - type: NotificationType! - - """ISO Timestamp for when the notification occurred""" - timestamp: String - formattedTimestamp: String -} - -enum NotificationImportance { - ALERT - INFO - WARNING -} - -enum NotificationType { - UNREAD - ARCHIVE -} - -type Notifications implements Node { - id: PrefixedID! - - """A cached overview of the notifications in the system & their severity.""" - overview: NotificationOverview! - list(filter: NotificationFilter!): [Notification!]! - - """ - Deduplicated list of unread warning and alert notifications, sorted latest first. - """ - warningsAndAlerts: [Notification!]! -} - -input NotificationFilter { - importance: NotificationImportance - type: NotificationType! - offset: Int! - limit: Int! -} - type FlashBackupStatus { """Status message indicating the outcome of the backup initiation.""" status: String! @@ -3569,4 +3569,4 @@ type Subscription { systemMetricsTemperature: TemperatureMetrics upsUpdates: UPSDevice! pluginInstallUpdates(operationId: ID!): PluginInstallEvent! -} +} \ No newline at end of file diff --git a/api/src/unraid-api/config/onboarding-tracker.service.spec.ts b/api/src/unraid-api/config/onboarding-tracker.service.spec.ts index 4a17584954..1ec6c538aa 100644 --- a/api/src/unraid-api/config/onboarding-tracker.service.spec.ts +++ b/api/src/unraid-api/config/onboarding-tracker.service.spec.ts @@ -87,3 +87,84 @@ describe('OnboardingTrackerService write retries', () => { expect(mockAtomicWriteFile).toHaveBeenCalledTimes(3); }); }); + +describe('OnboardingTrackerService tracker state availability', () => { + beforeEach(() => { + mockReadFile.mockReset(); + mockAtomicWriteFile.mockReset(); + }); + + it('treats a missing tracker file as a valid empty onboarding state', async () => { + const config = createConfigService(); + const overrides = new OnboardingOverrideService(); + + mockReadFile.mockImplementation(async (filePath) => { + if (String(filePath).includes('unraid-version')) { + return 'version="7.2.0"\n'; + } + throw Object.assign(new Error('Not found'), { code: 'ENOENT' }); + }); + + const tracker = new OnboardingTrackerService(config, overrides); + await tracker.onApplicationBootstrap(); + + await expect(tracker.getStateResult()).resolves.toEqual({ + kind: 'missing', + state: { + completed: false, + completedAtVersion: undefined, + }, + }); + }); + + it('captures tracker read failures when reading the tracker file fails for a non-ENOENT reason', async () => { + const config = createConfigService(); + const overrides = new OnboardingOverrideService(); + + mockReadFile.mockImplementation(async (filePath) => { + if (String(filePath).includes('unraid-version')) { + return 'version="7.2.0"\n'; + } + throw new Error('permission denied'); + }); + + const tracker = new OnboardingTrackerService(config, overrides); + await tracker.onApplicationBootstrap(); + + const stateResult = await tracker.getStateResult(); + expect(stateResult.kind).toBe('error'); + if (stateResult.kind === 'error') { + expect(stateResult.error).toBeInstanceOf(Error); + } + }); + + it('returns override-backed onboarding state as a successful read result', async () => { + const config = createConfigService(); + const overrides = new OnboardingOverrideService(); + + overrides.setState({ + onboarding: { + completed: true, + completedAtVersion: '7.2.0', + }, + }); + + mockReadFile.mockImplementation(async (filePath) => { + if (String(filePath).includes('unraid-version')) { + return 'version="7.2.0"\n'; + } + throw Object.assign(new Error('Not found'), { code: 'ENOENT' }); + }); + + const tracker = new OnboardingTrackerService(config, overrides); + await tracker.onApplicationBootstrap(); + + await expect(tracker.getStateResult()).resolves.toEqual({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.2.0', + }, + }); + }); +}); diff --git a/api/src/unraid-api/config/onboarding-tracker.service.ts b/api/src/unraid-api/config/onboarding-tracker.service.ts index a7fb9d785f..02d25d1c55 100644 --- a/api/src/unraid-api/config/onboarding-tracker.service.ts +++ b/api/src/unraid-api/config/onboarding-tracker.service.ts @@ -15,6 +15,13 @@ const DEFAULT_OS_VERSION_FILE_PATH = '/etc/unraid-version'; const WRITE_RETRY_ATTEMPTS = 3; const WRITE_RETRY_DELAY_MS = 100; +type PublicTrackerState = { completed: boolean; completedAtVersion?: string }; + +export type OnboardingTrackerStateResult = + | { kind: 'ok'; state: PublicTrackerState } + | { kind: 'missing'; state: PublicTrackerState } + | { kind: 'error'; error: Error }; + /** * Simplified onboarding tracker service. * Tracks whether onboarding has been completed and at which version. @@ -39,15 +46,15 @@ export class OnboardingTrackerService implements OnApplicationBootstrap { async onApplicationBootstrap() { this.currentVersion = await this.readCurrentVersion(); - const previousState = await this.readTrackerState(); - this.state = previousState ?? {}; + const previousState = await this.readTrackerStateResult(); + this.state = previousState.kind === 'error' ? {} : previousState.state; this.syncConfig(); } /** * Get the current onboarding state. */ - getState(): { completed: boolean; completedAtVersion?: string } { + getState(): PublicTrackerState { // Check for override first (for testing) const overrideState = this.onboardingOverrides.getState(); if (overrideState?.onboarding !== undefined) { @@ -84,6 +91,23 @@ export class OnboardingTrackerService implements OnApplicationBootstrap { return this.currentVersion; } + async getStateResult(): Promise { + const overrideState = this.onboardingOverrides.getState(); + if (overrideState?.onboarding !== undefined) { + return { + kind: 'ok', + state: this.getState(), + }; + } + + const trackerStateResult = await this.readTrackerStateResult(); + if (trackerStateResult.kind !== 'error') { + this.state = trackerStateResult.state; + } + + return trackerStateResult; + } + /** * Mark onboarding as completed for the current OS version. */ @@ -164,15 +188,39 @@ export class OnboardingTrackerService implements OnApplicationBootstrap { } } - private async readTrackerState(): Promise { + private async readTrackerStateResult(): Promise { try { const content = await readFile(this.trackerPath, 'utf8'); - return JSON.parse(content) as TrackerState; + const state = JSON.parse(content) as TrackerState; + return { + kind: 'ok', + state: { + completed: state.completed ?? false, + completedAtVersion: state.completedAtVersion, + }, + }; } catch (error) { + if (error instanceof Error && 'code' in error && error.code === 'ENOENT') { + this.logger.debug(`Onboarding tracker state does not exist yet at ${this.trackerPath}.`); + return { + kind: 'missing', + state: { + completed: false, + completedAtVersion: undefined, + }, + }; + } + this.logger.debug( `Unable to read onboarding tracker state at ${this.trackerPath}: ${error}` ); - return undefined; + return { + kind: 'error', + error: + error instanceof Error + ? error + : new Error('Unable to read onboarding tracker state'), + }; } } diff --git a/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts b/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts index a2faeddab1..2b168296f6 100644 --- a/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts +++ b/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { OnboardingTrackerStateResult } from '@app/unraid-api/config/onboarding-tracker.service.js'; import { OnboardingTrackerService } from '@app/unraid-api/config/onboarding-tracker.module.js'; import { OnboardingStatus } from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js'; import { CustomizationResolver } from '@app/unraid-api/graph/resolvers/customization/customization.resolver.js'; @@ -16,7 +17,7 @@ describe('CustomizationResolver', () => { getOnboardingState: vi.fn(), } as unknown as OnboardingService; const onboardingTracker = { - getState: vi.fn(), + getStateResult: vi.fn(), getCurrentVersion: vi.fn(), } as unknown as OnboardingTrackerService; const displayService = { @@ -27,6 +28,13 @@ describe('CustomizationResolver', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ + kind: 'ok', + state: { + completed: false, + completedAtVersion: undefined, + }, + } satisfies OnboardingTrackerStateResult); vi.mocked(onboardingTracker.getCurrentVersion).mockReturnValue('7.2.0'); vi.mocked(onboardingService.getPublicPartnerInfo).mockResolvedValue(null); vi.mocked(onboardingService.getActivationDataForPublic).mockResolvedValue(null); @@ -39,10 +47,24 @@ describe('CustomizationResolver', () => { }); }); + it('throws when tracker state could not be read', async () => { + vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ + kind: 'error', + error: new Error('permission denied'), + }); + + await expect(resolver.resolveOnboarding()).rejects.toThrow( + 'Onboarding tracker state is unavailable.' + ); + }); + it('returns INCOMPLETE status when not completed', async () => { - vi.mocked(onboardingTracker.getState).mockReturnValue({ - completed: false, - completedAtVersion: undefined, + vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ + kind: 'ok', + state: { + completed: false, + completedAtVersion: undefined, + }, }); const result = await resolver.resolveOnboarding(); @@ -63,9 +85,12 @@ describe('CustomizationResolver', () => { }); it('returns COMPLETED status when completed on current version', async () => { - vi.mocked(onboardingTracker.getState).mockReturnValue({ - completed: true, - completedAtVersion: '7.2.0', + vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.2.0', + }, }); const result = await resolver.resolveOnboarding(); @@ -86,9 +111,12 @@ describe('CustomizationResolver', () => { }); it('returns COMPLETED status when completed on a prior patch of current minor', async () => { - vi.mocked(onboardingTracker.getState).mockReturnValue({ - completed: true, - completedAtVersion: '7.2.1', + vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.2.1', + }, }); vi.mocked(onboardingTracker.getCurrentVersion).mockReturnValue('7.2.3'); @@ -110,9 +138,12 @@ describe('CustomizationResolver', () => { }); it('returns UPGRADE status when completed on older version', async () => { - vi.mocked(onboardingTracker.getState).mockReturnValue({ - completed: true, - completedAtVersion: '7.1.0', + vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.1.0', + }, }); const result = await resolver.resolveOnboarding(); @@ -133,9 +164,12 @@ describe('CustomizationResolver', () => { }); it('returns DOWNGRADE status when completed on newer version', async () => { - vi.mocked(onboardingTracker.getState).mockReturnValue({ - completed: true, - completedAtVersion: '7.3.0', + vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.3.0', + }, }); const result = await resolver.resolveOnboarding(); @@ -156,9 +190,12 @@ describe('CustomizationResolver', () => { }); it('returns isPartnerBuild true when partner info exists', async () => { - vi.mocked(onboardingTracker.getState).mockReturnValue({ - completed: false, - completedAtVersion: undefined, + vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ + kind: 'ok', + state: { + completed: false, + completedAtVersion: undefined, + }, }); vi.mocked(onboardingService.getPublicPartnerInfo).mockResolvedValue({ partner: { diff --git a/api/src/unraid-api/graph/resolvers/customization/customization.resolver.ts b/api/src/unraid-api/graph/resolvers/customization/customization.resolver.ts index 1f4c770495..94a95f7bb3 100644 --- a/api/src/unraid-api/graph/resolvers/customization/customization.resolver.ts +++ b/api/src/unraid-api/graph/resolvers/customization/customization.resolver.ts @@ -2,6 +2,7 @@ import { Query, ResolveField, Resolver } from '@nestjs/graphql'; import { AuthAction, Resource } from '@unraid/shared/graphql.model.js'; import { UsePermissions } from '@unraid/shared/use-permissions.directive.js'; +import { GraphQLError } from 'graphql'; import { Public } from '@app/unraid-api/auth/public.decorator.js'; import { OnboardingTrackerService } from '@app/unraid-api/config/onboarding-tracker.module.js'; @@ -59,7 +60,12 @@ export class CustomizationResolver { resource: Resource.CUSTOMIZATIONS, }) async resolveOnboarding(): Promise { - const state = this.onboardingTracker.getState(); + const trackerStateResult = await this.onboardingTracker.getStateResult(); + if (trackerStateResult.kind === 'error') { + throw new GraphQLError('Onboarding tracker state is unavailable.'); + } + + const state = trackerStateResult.state; const currentVersion = this.onboardingTracker.getCurrentVersion() ?? 'unknown'; const partnerInfo = await this.onboardingService.getPublicPartnerInfo(); const activationData = await this.onboardingService.getActivationData(); diff --git a/web/__test__/store/onboardingStatus.test.ts b/web/__test__/store/onboardingStatus.test.ts new file mode 100644 index 0000000000..f4f2f761ee --- /dev/null +++ b/web/__test__/store/onboardingStatus.test.ts @@ -0,0 +1,162 @@ +import { ref } from 'vue'; +import { createPinia, setActivePinia } from 'pinia'; + +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { Ref } from 'vue'; + +import { useOnboardingDraftStore } from '~/components/Onboarding/store/onboardingDraft'; +import { useOnboardingStore } from '~/components/Onboarding/store/onboardingStatus'; +import { useServerStore } from '~/store/server'; + +type OnboardingQueryResult = { + customization?: { + onboarding?: { + status?: 'INCOMPLETE' | 'UPGRADE' | 'DOWNGRADE' | 'COMPLETED'; + isPartnerBuild?: boolean; + completed?: boolean; + completedAtVersion?: string | null; + } | null; + }; +}; + +const { state, refetchMock, useQueryMock } = vi.hoisted(() => ({ + state: { + onboardingResult: null as unknown as Ref, + onboardingLoading: null as unknown as Ref, + onboardingError: null as unknown as Ref, + osVersionRef: null as unknown as Ref, + hasResumableDraftRef: null as unknown as Ref, + }, + refetchMock: vi.fn(), + useQueryMock: vi.fn(), +})); + +const createOnboardingResult = (): OnboardingQueryResult => ({ + customization: { + onboarding: { + status: 'INCOMPLETE', + isPartnerBuild: false, + completed: false, + completedAtVersion: null, + }, + }, +}); + +describe('onboardingStatus store', () => { + beforeEach(() => { + vi.clearAllMocks(); + setActivePinia(createPinia()); + + state.onboardingResult = ref(createOnboardingResult()); + state.onboardingLoading = ref(false); + state.onboardingError = ref(null); + state.osVersionRef = ref('7.3.0'); + state.hasResumableDraftRef = ref(false); + refetchMock.mockResolvedValue(undefined); + + useQueryMock.mockReturnValue({ + result: state.onboardingResult, + loading: state.onboardingLoading, + error: state.onboardingError, + refetch: refetchMock, + }); + + vi.mocked(useServerStore).mockReturnValue({ + osVersion: state.osVersionRef, + } as unknown as ReturnType); + + vi.mocked(useOnboardingDraftStore).mockReturnValue({ + hasResumableDraft: state.hasResumableDraftRef, + } as unknown as ReturnType); + }); + + it('blocks onboarding modal while the onboarding query is still loading', () => { + state.onboardingResult.value = null; + state.onboardingLoading.value = true; + + const store = useOnboardingStore(); + + expect(store.canDisplayOnboardingModal).toBe(true); + expect(store.shouldShowOnboarding).toBe(false); + }); + + it('blocks onboarding modal when the onboarding query errors and no draft exists', () => { + state.onboardingResult.value = null; + state.onboardingError.value = new Error('Network error'); + + const store = useOnboardingStore(); + + expect(store.hasOnboardingQueryError).toBe(true); + expect(store.canDisplayOnboardingModal).toBe(false); + expect(store.shouldShowOnboarding).toBe(false); + }); + + it('keeps onboarding modal display enabled when a resumable draft exists during an error', () => { + state.onboardingResult.value = null; + state.onboardingError.value = new Error('Network error'); + state.hasResumableDraftRef.value = true; + + const store = useOnboardingStore(); + + expect(store.hasOnboardingQueryError).toBe(true); + expect(store.canDisplayOnboardingModal).toBe(true); + expect(store.shouldShowOnboarding).toBe(false); + }); + + it('allows onboarding modal when onboarding state is absent but there is no query error', () => { + state.onboardingResult.value = { + customization: { + onboarding: null, + }, + }; + + const store = useOnboardingStore(); + + expect(store.canDisplayOnboardingModal).toBe(true); + expect(store.shouldShowOnboarding).toBe(false); + }); + + it('keeps onboarding modal display enabled when tracker state is missing but a draft exists', () => { + state.onboardingResult.value = { + customization: { + onboarding: null, + }, + }; + state.hasResumableDraftRef.value = true; + + const store = useOnboardingStore(); + + expect(store.canDisplayOnboardingModal).toBe(true); + expect(store.shouldShowOnboarding).toBe(false); + }); + + it('allows onboarding modal when the onboarding query succeeds', () => { + const store = useOnboardingStore(); + + expect(store.hasOnboardingQueryError).toBe(false); + expect(store.canDisplayOnboardingModal).toBe(true); + expect(store.shouldShowOnboarding).toBe(true); + }); + + it('keeps onboarding modal display enabled during refetch when onboarding data already exists', () => { + state.onboardingLoading.value = true; + + const store = useOnboardingStore(); + + expect(store.canDisplayOnboardingModal).toBe(true); + expect(store.shouldShowOnboarding).toBe(true); + }); +}); + +vi.mock('@vue/apollo-composable', () => ({ + useQuery: () => useQueryMock(), +})); + +vi.mock('~/components/Onboarding/store/onboardingDraft', () => ({ + useOnboardingDraftStore: vi.fn(), +})); + +vi.mock('~/store/server', () => ({ + useServerStore: vi.fn(), +})); diff --git a/web/src/components/Onboarding/store/onboardingDraft.ts b/web/src/components/Onboarding/store/onboardingDraft.ts index df9676999b..78bc37d0f9 100644 --- a/web/src/components/Onboarding/store/onboardingDraft.ts +++ b/web/src/components/Onboarding/store/onboardingDraft.ts @@ -1,4 +1,4 @@ -import { ref } from 'vue'; +import { computed, ref } from 'vue'; import { defineStore } from 'pinia'; export interface OnboardingInternalBootSelection { @@ -118,6 +118,14 @@ export const useOnboardingDraftStore = defineStore( // Navigation const currentStepIndex = ref(0); + const hasResumableDraft = computed( + () => + currentStepIndex.value > 0 || + coreSettingsInitialized.value || + pluginSelectionInitialized.value || + internalBootInitialized.value || + internalBootApplySucceeded.value + ); // Actions function setCoreSettings(settings: { @@ -198,6 +206,7 @@ export const useOnboardingDraftStore = defineStore( internalBootSkipped, internalBootApplySucceeded, currentStepIndex, + hasResumableDraft, setCoreSettings, setPlugins, setInternalBootSelection, diff --git a/web/src/components/Onboarding/store/onboardingStatus.ts b/web/src/components/Onboarding/store/onboardingStatus.ts index 1f876da0e0..7292d68b4d 100644 --- a/web/src/components/Onboarding/store/onboardingStatus.ts +++ b/web/src/components/Onboarding/store/onboardingStatus.ts @@ -8,6 +8,7 @@ import gte from 'semver/functions/gte'; import type { OnboardingStatus } from '~/composables/gql/graphql'; +import { useOnboardingDraftStore } from '~/components/Onboarding/store/onboardingDraft'; import { useServerStore } from '~/store/server'; const MIN_ONBOARDING_VERSION = '7.3.0'; @@ -47,9 +48,11 @@ const isVersionAtLeast = (version: string | null | undefined, minVersion: string export const useOnboardingStore = defineStore('onboarding', () => { const { osVersion } = storeToRefs(useServerStore()); + const { hasResumableDraft } = storeToRefs(useOnboardingDraftStore()); const { result: onboardingResult, loading: onboardingLoading, + error: onboardingError, refetch, } = useQuery(ONBOARDING_QUERY, {}, { errorPolicy: 'all' }); @@ -84,7 +87,10 @@ export const useOnboardingStore = defineStore('onboarding', () => { } }; - const canDisplayOnboardingModal = computed(() => isVersionSupported.value); + const hasOnboardingQueryError = computed(() => Boolean(onboardingError.value)); + const canDisplayOnboardingModal = computed( + () => isVersionSupported.value && (hasResumableDraft.value || !hasOnboardingQueryError.value) + ); // Automatic onboarding should only run for initial setup. const shouldShowOnboarding = computed(() => { @@ -112,6 +118,7 @@ export const useOnboardingStore = defineStore('onboarding', () => { effectiveOsVersion, isVersionSupported, mockOsVersion, + hasOnboardingQueryError, canDisplayOnboardingModal, shouldShowOnboarding, // Actions diff --git a/web/src/composables/gql/graphql.ts b/web/src/composables/gql/graphql.ts index b424688a9a..a4e4d00de5 100644 --- a/web/src/composables/gql/graphql.ts +++ b/web/src/composables/gql/graphql.ts @@ -4117,4 +4117,4 @@ export const SignOutDocument = {"kind":"Document","definitions":[{"kind":"Operat export const IsSsoEnabledDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"query","name":{"kind":"Name","value":"IsSSOEnabled"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"isSSOEnabled"}}]}}]} as unknown as DocumentNode; export const CloudStateDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"query","name":{"kind":"Name","value":"cloudState"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"cloud"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"FragmentSpread","name":{"kind":"Name","value":"PartialCloud"}}]}}]}},{"kind":"FragmentDefinition","name":{"kind":"Name","value":"PartialCloud"},"typeCondition":{"kind":"NamedType","name":{"kind":"Name","value":"Cloud"}},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"error"}},{"kind":"Field","name":{"kind":"Name","value":"apiKey"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"valid"}},{"kind":"Field","name":{"kind":"Name","value":"error"}}]}},{"kind":"Field","name":{"kind":"Name","value":"cloud"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"status"}},{"kind":"Field","name":{"kind":"Name","value":"error"}}]}},{"kind":"Field","name":{"kind":"Name","value":"minigraphql"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"status"}},{"kind":"Field","name":{"kind":"Name","value":"error"}}]}},{"kind":"Field","name":{"kind":"Name","value":"relay"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"status"}},{"kind":"Field","name":{"kind":"Name","value":"error"}}]}}]}}]} as unknown as DocumentNode; export const ServerStateDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"query","name":{"kind":"Name","value":"serverState"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"config"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"error"}},{"kind":"Field","name":{"kind":"Name","value":"valid"}}]}},{"kind":"Field","name":{"kind":"Name","value":"info"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"os"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"hostname"}}]}}]}},{"kind":"Field","name":{"kind":"Name","value":"owner"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"avatar"}},{"kind":"Field","name":{"kind":"Name","value":"username"}}]}},{"kind":"Field","name":{"kind":"Name","value":"registration"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"type"}},{"kind":"Field","name":{"kind":"Name","value":"state"}},{"kind":"Field","name":{"kind":"Name","value":"expiration"}},{"kind":"Field","name":{"kind":"Name","value":"keyFile"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"contents"}}]}},{"kind":"Field","name":{"kind":"Name","value":"updateExpiration"}}]}},{"kind":"Field","name":{"kind":"Name","value":"vars"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"bootedFromFlashWithInternalBootSetup"}},{"kind":"Field","name":{"kind":"Name","value":"flashGuid"}},{"kind":"Field","name":{"kind":"Name","value":"tpmGuid"}},{"kind":"Field","name":{"kind":"Name","value":"mdState"}},{"kind":"Field","name":{"kind":"Name","value":"regGen"}},{"kind":"Field","name":{"kind":"Name","value":"regState"}},{"kind":"Field","name":{"kind":"Name","value":"configError"}},{"kind":"Field","name":{"kind":"Name","value":"configValid"}}]}}]}}]} as unknown as DocumentNode; -export const GetThemeDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"query","name":{"kind":"Name","value":"getTheme"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"publicTheme"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"name"}},{"kind":"Field","name":{"kind":"Name","value":"showBannerImage"}},{"kind":"Field","name":{"kind":"Name","value":"showBannerGradient"}},{"kind":"Field","name":{"kind":"Name","value":"headerBackgroundColor"}},{"kind":"Field","name":{"kind":"Name","value":"showHeaderDescription"}},{"kind":"Field","name":{"kind":"Name","value":"headerPrimaryTextColor"}},{"kind":"Field","name":{"kind":"Name","value":"headerSecondaryTextColor"}}]}}]}}]} as unknown as DocumentNode; \ No newline at end of file +export const GetThemeDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"query","name":{"kind":"Name","value":"getTheme"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"publicTheme"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"name"}},{"kind":"Field","name":{"kind":"Name","value":"showBannerImage"}},{"kind":"Field","name":{"kind":"Name","value":"showBannerGradient"}},{"kind":"Field","name":{"kind":"Name","value":"headerBackgroundColor"}},{"kind":"Field","name":{"kind":"Name","value":"showHeaderDescription"}},{"kind":"Field","name":{"kind":"Name","value":"headerPrimaryTextColor"}},{"kind":"Field","name":{"kind":"Name","value":"headerSecondaryTextColor"}}]}}]}}]} as unknown as DocumentNode; From 4964be4059575c7f7ea880261521aed56dec270a Mon Sep 17 00:00:00 2001 From: Ajit Mehrotra Date: Sun, 15 Mar 2026 16:14:03 -0400 Subject: [PATCH 02/16] fix(plugin): add corepack install symlink - Purpose: make the packaged plugin expose the expected package-manager entrypoints after install. - Before: the install script only recreated npm and npx links, leaving corepack unavailable from the installed environment. - Problem: package-manager workflows that rely on corepack could fail even though Node tooling was otherwise installed. - Change: create the corepack symlink during plugin install alongside the npm and npx links. - How: extend doinst.sh to link usr/local/bin/corepack to the bundled corepack entrypoint while preserving the existing npm and npx setup. --- plugin/source/dynamix.unraid.net/install/doinst.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/plugin/source/dynamix.unraid.net/install/doinst.sh b/plugin/source/dynamix.unraid.net/install/doinst.sh index 710522e626..49d650686e 100644 --- a/plugin/source/dynamix.unraid.net/install/doinst.sh +++ b/plugin/source/dynamix.unraid.net/install/doinst.sh @@ -37,3 +37,9 @@ cp usr/local/unraid-api/.env.production usr/local/unraid-api/.env ( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npm-cli.js npm ) ( cd usr/local/bin ; rm -rf npx ) ( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx ) +( cd usr/local/bin ; rm -rf corepack ) +( cd usr/local/bin ; ln -sf ../lib/node_modules/corepack/dist/corepack.js corepack ) +( cd usr/local/bin ; rm -rf npm ) +( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npm-cli.js npm ) +( cd usr/local/bin ; rm -rf npx ) +( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx ) From 31570be2fb3ab20c4923cc7c9e224a8def384df6 Mon Sep 17 00:00:00 2001 From: Ajit Mehrotra Date: Sun, 15 Mar 2026 16:39:45 -0400 Subject: [PATCH 03/16] feat: add tpmGuid field to the System type in the GraphQL schema. --- api/src/unraid-api/cli/generated/graphql.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/api/src/unraid-api/cli/generated/graphql.ts b/api/src/unraid-api/cli/generated/graphql.ts index 1f03d6b58f..a12769f4ec 100644 --- a/api/src/unraid-api/cli/generated/graphql.ts +++ b/api/src/unraid-api/cli/generated/graphql.ts @@ -3361,6 +3361,7 @@ export type Vars = Node & { sysFlashSlots?: Maybe; sysModel?: Maybe; timeZone?: Maybe; + tpmGuid?: Maybe; /** Should a NTP server be used for time sync? */ useNtp?: Maybe; useSsh?: Maybe; From 99bb90b5af1afa0842ab22c3790277fe9fd8e77f Mon Sep 17 00:00:00 2001 From: Ajit Mehrotra Date: Sun, 15 Mar 2026 17:12:57 -0400 Subject: [PATCH 04/16] fix(review): address CodeRabbit feedback - Purpose: resolve the outstanding CodeRabbit findings on PR #1917 after rebasing onto the latest mainline. - Before: the onboarding store over-blocked on partial Apollo errors, tracker read failures could still be flattened through mutation/runtime helper paths, one resolver spec asserted exact error text, and the plugin install script repeated the symlink block. - Problem: review comments highlighted behavior drift, brittle tests, and redundant install steps that could leave the PR confusing or incomplete. - Change: tighten the onboarding error gate for partial data, route runtime tracker readers through the error-propagating result path, relax the brittle spec assertion, rename the mismatched test, and collapse the duplicated install block. - How: update the API/web logic and focused tests for onboarding state handling, mutation responses, and tracker reads, then validate with targeted Vitest runs plus a shell syntax check for the install script. --- api/src/unraid-api/config/api-config.test.ts | 30 +++++-- .../config/onboarding-tracker.service.spec.ts | 34 ++++++++ .../config/onboarding-tracker.service.ts | 33 +++++--- .../customization.resolver.spec.ts | 4 +- .../customization/onboarding.service.ts | 2 +- .../onboarding/onboarding.mutation.spec.ts | 84 +++++++++++++------ .../onboarding/onboarding.mutation.ts | 7 +- .../dynamix.unraid.net/install/doinst.sh | 12 --- web/__test__/store/onboardingStatus.test.ts | 12 ++- .../Onboarding/store/onboardingStatus.ts | 4 +- 10 files changed, 156 insertions(+), 66 deletions(-) diff --git a/api/src/unraid-api/config/api-config.test.ts b/api/src/unraid-api/config/api-config.test.ts index 231306488d..4b7d830def 100644 --- a/api/src/unraid-api/config/api-config.test.ts +++ b/api/src/unraid-api/config/api-config.test.ts @@ -163,9 +163,13 @@ describe('OnboardingTracker', () => { const tracker = createOnboardingTracker(configService); await tracker.onApplicationBootstrap(); - const state = tracker.getState(); - expect(state.completed).toBe(false); - expect(state.completedAtVersion).toBeUndefined(); + await expect(tracker.getStateResult()).resolves.toEqual({ + kind: 'missing', + state: { + completed: false, + completedAtVersion: undefined, + }, + }); }); it('returns completed state when previously marked', async () => { @@ -185,9 +189,13 @@ describe('OnboardingTracker', () => { const tracker = createOnboardingTracker(configService); await tracker.onApplicationBootstrap(); - const state = tracker.getState(); - expect(state.completed).toBe(true); - expect(state.completedAtVersion).toBe('7.1.0'); + await expect(tracker.getStateResult()).resolves.toEqual({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.1.0', + }, + }); }); it('marks onboarding as completed with current version', async () => { @@ -272,9 +280,13 @@ describe('OnboardingTracker', () => { const tracker = new OnboardingTrackerService(configService, overrides); await tracker.onApplicationBootstrap(); - const state = tracker.getState(); - expect(state.completed).toBe(true); - expect(state.completedAtVersion).toBe('6.12.0'); + await expect(tracker.getStateResult()).resolves.toEqual({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '6.12.0', + }, + }); }); }); diff --git a/api/src/unraid-api/config/onboarding-tracker.service.spec.ts b/api/src/unraid-api/config/onboarding-tracker.service.spec.ts index 1ec6c538aa..1b65e1886a 100644 --- a/api/src/unraid-api/config/onboarding-tracker.service.spec.ts +++ b/api/src/unraid-api/config/onboarding-tracker.service.spec.ts @@ -167,4 +167,38 @@ describe('OnboardingTrackerService tracker state availability', () => { }, }); }); + + it('propagates tracker read failures through isCompleted', async () => { + const config = createConfigService(); + const overrides = new OnboardingOverrideService(); + + mockReadFile.mockImplementation(async (filePath) => { + if (String(filePath).includes('unraid-version')) { + return 'version="7.2.0"\n'; + } + throw new Error('permission denied'); + }); + + const tracker = new OnboardingTrackerService(config, overrides); + await tracker.onApplicationBootstrap(); + + await expect(tracker.isCompleted()).rejects.toThrow('permission denied'); + }); + + it('propagates tracker read failures through getCompletedAtVersion', async () => { + const config = createConfigService(); + const overrides = new OnboardingOverrideService(); + + mockReadFile.mockImplementation(async (filePath) => { + if (String(filePath).includes('unraid-version')) { + return 'version="7.2.0"\n'; + } + throw new Error('permission denied'); + }); + + const tracker = new OnboardingTrackerService(config, overrides); + await tracker.onApplicationBootstrap(); + + await expect(tracker.getCompletedAtVersion()).rejects.toThrow('permission denied'); + }); }); diff --git a/api/src/unraid-api/config/onboarding-tracker.service.ts b/api/src/unraid-api/config/onboarding-tracker.service.ts index 02d25d1c55..44979383dc 100644 --- a/api/src/unraid-api/config/onboarding-tracker.service.ts +++ b/api/src/unraid-api/config/onboarding-tracker.service.ts @@ -51,10 +51,7 @@ export class OnboardingTrackerService implements OnApplicationBootstrap { this.syncConfig(); } - /** - * Get the current onboarding state. - */ - getState(): PublicTrackerState { + private getCachedState(): PublicTrackerState { // Check for override first (for testing) const overrideState = this.onboardingOverrides.getState(); if (overrideState?.onboarding !== undefined) { @@ -73,15 +70,25 @@ export class OnboardingTrackerService implements OnApplicationBootstrap { /** * Check if onboarding is completed. */ - isCompleted(): boolean { - return this.getState().completed; + async isCompleted(): Promise { + const stateResult = await this.getStateResult(); + if (stateResult.kind === 'error') { + throw stateResult.error; + } + + return stateResult.state.completed; } /** * Get the version at which onboarding was completed. */ - getCompletedAtVersion(): string | undefined { - return this.getState().completedAtVersion; + async getCompletedAtVersion(): Promise { + const stateResult = await this.getStateResult(); + if (stateResult.kind === 'error') { + throw stateResult.error; + } + + return stateResult.state.completedAtVersion; } /** @@ -96,7 +103,7 @@ export class OnboardingTrackerService implements OnApplicationBootstrap { if (overrideState?.onboarding !== undefined) { return { kind: 'ok', - state: this.getState(), + state: this.getCachedState(), }; } @@ -125,7 +132,7 @@ export class OnboardingTrackerService implements OnApplicationBootstrap { }, }; this.onboardingOverrides.setState(updatedOverride); - return this.getState(); + return this.getCachedState(); } const updatedState: TrackerState = { @@ -136,7 +143,7 @@ export class OnboardingTrackerService implements OnApplicationBootstrap { await this.writeTrackerState(updatedState); this.syncConfig(); - return this.getState(); + return this.getCachedState(); } /** @@ -155,7 +162,7 @@ export class OnboardingTrackerService implements OnApplicationBootstrap { }, }; this.onboardingOverrides.setState(updatedOverride); - return this.getState(); + return this.getCachedState(); } const updatedState: TrackerState = { @@ -166,7 +173,7 @@ export class OnboardingTrackerService implements OnApplicationBootstrap { await this.writeTrackerState(updatedState); this.syncConfig(); - return this.getState(); + return this.getCachedState(); } private syncConfig() { diff --git a/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts b/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts index 2b168296f6..cad4b65030 100644 --- a/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts +++ b/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts @@ -53,9 +53,7 @@ describe('CustomizationResolver', () => { error: new Error('permission denied'), }); - await expect(resolver.resolveOnboarding()).rejects.toThrow( - 'Onboarding tracker state is unavailable.' - ); + await expect(resolver.resolveOnboarding()).rejects.toThrow(); }); it('returns INCOMPLETE status when not completed', async () => { diff --git a/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts b/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts index 5f798d715a..5b8d3b3e73 100644 --- a/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts +++ b/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts @@ -54,7 +54,7 @@ export class OnboardingService implements OnModuleInit { private async ensureFirstBootCompletion(): Promise { await fs.mkdir(this.activationDir, { recursive: true }); // Check if onboarding has already been completed - const alreadyCompleted = this.onboardingTracker.isCompleted(); + const alreadyCompleted = await this.onboardingTracker.isCompleted(); if (alreadyCompleted) { this.logger.log('Onboarding already completed, skipping first boot setup.'); return true; diff --git a/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts b/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts index 97db6ca47f..15019c0953 100644 --- a/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts +++ b/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts @@ -8,7 +8,7 @@ describe('OnboardingMutationsResolver', () => { const onboardingTracker = { markCompleted: vi.fn(), reset: vi.fn(), - getState: vi.fn(), + getStateResult: vi.fn(), getCurrentVersion: vi.fn(), }; @@ -31,9 +31,12 @@ describe('OnboardingMutationsResolver', () => { beforeEach(() => { vi.clearAllMocks(); - onboardingTracker.getState.mockReturnValue({ - completed: false, - completedAtVersion: undefined, + onboardingTracker.getStateResult.mockResolvedValue({ + kind: 'ok', + state: { + completed: false, + completedAtVersion: undefined, + }, }); onboardingTracker.getCurrentVersion.mockReturnValue('7.2.0'); onboardingService.getPublicPartnerInfo.mockResolvedValue(null); @@ -66,9 +69,12 @@ describe('OnboardingMutationsResolver', () => { completed: true, completedAtVersion: '7.2.0', }); - onboardingTracker.getState.mockReturnValue({ - completed: true, - completedAtVersion: '7.2.0', + onboardingTracker.getStateResult.mockResolvedValue({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.2.0', + }, }); onboardingTracker.getCurrentVersion.mockReturnValue('7.2.0'); onboardingService.getPublicPartnerInfo.mockResolvedValue(null); @@ -90,9 +96,12 @@ describe('OnboardingMutationsResolver', () => { it('returns incomplete status after resetOnboarding', async () => { onboardingTracker.reset.mockResolvedValue(undefined); - onboardingTracker.getState.mockReturnValue({ - completed: false, - completedAtVersion: undefined, + onboardingTracker.getStateResult.mockResolvedValue({ + kind: 'ok', + state: { + completed: false, + completedAtVersion: undefined, + }, }); const result = await resolver.resetOnboarding(); @@ -104,9 +113,12 @@ describe('OnboardingMutationsResolver', () => { it('returns completed status when completed version is on a prior patch of current minor', async () => { onboardingTracker.markCompleted.mockResolvedValue(undefined); - onboardingTracker.getState.mockReturnValue({ - completed: true, - completedAtVersion: '7.2.1', + onboardingTracker.getStateResult.mockResolvedValue({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.2.1', + }, }); onboardingTracker.getCurrentVersion.mockReturnValue('7.2.4'); @@ -117,9 +129,12 @@ describe('OnboardingMutationsResolver', () => { it('returns upgrade status when completed version is behind current', async () => { onboardingTracker.markCompleted.mockResolvedValue(undefined); - onboardingTracker.getState.mockReturnValue({ - completed: true, - completedAtVersion: '7.1.0', + onboardingTracker.getStateResult.mockResolvedValue({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.1.0', + }, }); onboardingTracker.getCurrentVersion.mockReturnValue('7.2.0'); @@ -130,9 +145,12 @@ describe('OnboardingMutationsResolver', () => { it('returns downgrade status when completed version is ahead of current', async () => { onboardingTracker.markCompleted.mockResolvedValue(undefined); - onboardingTracker.getState.mockReturnValue({ - completed: true, - completedAtVersion: '7.2.0', + onboardingTracker.getStateResult.mockResolvedValue({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.2.0', + }, }); onboardingTracker.getCurrentVersion.mockReturnValue('7.1.0'); @@ -142,9 +160,12 @@ describe('OnboardingMutationsResolver', () => { }); it('setOnboardingOverride stores override, clears cache, and returns onboarding state', async () => { - onboardingTracker.getState.mockReturnValue({ - completed: true, - completedAtVersion: '7.2.0', + onboardingTracker.getStateResult.mockResolvedValue({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.2.0', + }, }); onboardingTracker.getCurrentVersion.mockReturnValue('7.2.0'); onboardingService.getPublicPartnerInfo.mockResolvedValue({ @@ -174,9 +195,12 @@ describe('OnboardingMutationsResolver', () => { }); it('clearOnboardingOverride clears override and cache', async () => { - onboardingTracker.getState.mockReturnValue({ - completed: false, - completedAtVersion: undefined, + onboardingTracker.getStateResult.mockResolvedValue({ + kind: 'ok', + state: { + completed: false, + completedAtVersion: undefined, + }, }); const result = await resolver.clearOnboardingOverride(); @@ -208,4 +232,14 @@ describe('OnboardingMutationsResolver', () => { output: 'done', }); }); + + it('propagates tracker read failures while building the completion response', async () => { + onboardingTracker.markCompleted.mockResolvedValue(undefined); + onboardingTracker.getStateResult.mockResolvedValue({ + kind: 'error', + error: new Error('tracker-read-failed'), + }); + + await expect(resolver.completeOnboarding()).rejects.toThrow('tracker-read-failed'); + }); }); diff --git a/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts b/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts index 358347fdcf..5ea9b84074 100644 --- a/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts +++ b/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts @@ -33,7 +33,12 @@ export class OnboardingMutationsResolver { * Build a full Onboarding response with computed status */ private async buildOnboardingResponse(): Promise { - const state = this.onboardingTracker.getState(); + const stateResult = await this.onboardingTracker.getStateResult(); + if (stateResult.kind === 'error') { + throw stateResult.error; + } + + const state = stateResult.state; const currentVersion = this.onboardingTracker.getCurrentVersion() ?? 'unknown'; const partnerInfo = await this.onboardingService.getPublicPartnerInfo(); const onboardingState = await this.onboardingService.getOnboardingState(); diff --git a/plugin/source/dynamix.unraid.net/install/doinst.sh b/plugin/source/dynamix.unraid.net/install/doinst.sh index 49d650686e..e18f5f64eb 100644 --- a/plugin/source/dynamix.unraid.net/install/doinst.sh +++ b/plugin/source/dynamix.unraid.net/install/doinst.sh @@ -31,15 +31,3 @@ cp usr/local/unraid-api/.env.production usr/local/unraid-api/.env ( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npm-cli.js npm ) ( cd usr/local/bin ; rm -rf npx ) ( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx ) -( cd usr/local/bin ; rm -rf corepack ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/corepack/dist/corepack.js corepack ) -( cd usr/local/bin ; rm -rf npm ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npm-cli.js npm ) -( cd usr/local/bin ; rm -rf npx ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx ) -( cd usr/local/bin ; rm -rf corepack ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/corepack/dist/corepack.js corepack ) -( cd usr/local/bin ; rm -rf npm ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npm-cli.js npm ) -( cd usr/local/bin ; rm -rf npx ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx ) diff --git a/web/__test__/store/onboardingStatus.test.ts b/web/__test__/store/onboardingStatus.test.ts index f4f2f761ee..76cbe3b4a8 100644 --- a/web/__test__/store/onboardingStatus.test.ts +++ b/web/__test__/store/onboardingStatus.test.ts @@ -71,7 +71,7 @@ describe('onboardingStatus store', () => { } as unknown as ReturnType); }); - it('blocks onboarding modal while the onboarding query is still loading', () => { + it('blocks auto-show while the onboarding query is still loading', () => { state.onboardingResult.value = null; state.onboardingLoading.value = true; @@ -139,6 +139,16 @@ describe('onboardingStatus store', () => { expect(store.shouldShowOnboarding).toBe(true); }); + it('keeps onboarding modal display enabled when onboarding data exists alongside an Apollo error', () => { + state.onboardingError.value = new Error('Partial data error'); + + const store = useOnboardingStore(); + + expect(store.hasOnboardingQueryError).toBe(false); + expect(store.canDisplayOnboardingModal).toBe(true); + expect(store.shouldShowOnboarding).toBe(true); + }); + it('keeps onboarding modal display enabled during refetch when onboarding data already exists', () => { state.onboardingLoading.value = true; diff --git a/web/src/components/Onboarding/store/onboardingStatus.ts b/web/src/components/Onboarding/store/onboardingStatus.ts index 7292d68b4d..7dc60cf422 100644 --- a/web/src/components/Onboarding/store/onboardingStatus.ts +++ b/web/src/components/Onboarding/store/onboardingStatus.ts @@ -87,7 +87,9 @@ export const useOnboardingStore = defineStore('onboarding', () => { } }; - const hasOnboardingQueryError = computed(() => Boolean(onboardingError.value)); + const hasOnboardingQueryError = computed( + () => Boolean(onboardingError.value) && !onboardingData.value + ); const canDisplayOnboardingModal = computed( () => isVersionSupported.value && (hasResumableDraft.value || !hasOnboardingQueryError.value) ); From aea36b87d252fc5b055013651046e0708044457c Mon Sep 17 00:00:00 2001 From: Ajit Mehrotra Date: Mon, 16 Mar 2026 11:20:28 -0400 Subject: [PATCH 05/16] refactor(onboarding): centralize onboarding response building - Purpose: address the PR feedback about duplicated onboarding response assembly in the mutation resolver. - Before: both the customization resolver and onboarding mutation resolver rebuilt the same onboarding payload, status calculation, and tracker error handling separately. - Problem: the duplicated logic was easy to let drift, harder to test cleanly, and made the mutation path smellier than it needed to be. - Change: move onboarding response construction into OnboardingService and make both resolvers delegate to that shared implementation. - How: add a shared service method for building onboarding responses, keep tracker/error handling in one place, and reshape the resolver specs to validate delegation while the service spec covers the response-building logic. --- .../customization.resolver.spec.ts | 139 ++++++----- .../customization/customization.resolver.ts | 41 +--- .../customization/onboarding.service.spec.ts | 109 ++++++++- .../customization/onboarding.service.ts | 49 ++++ .../onboarding/onboarding.mutation.spec.ts | 228 +++++------------- .../onboarding/onboarding.mutation.ts | 56 +---- 6 files changed, 308 insertions(+), 314 deletions(-) diff --git a/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts b/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts index cad4b65030..fdf679630f 100644 --- a/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts +++ b/api/src/unraid-api/graph/resolvers/customization/customization.resolver.spec.ts @@ -1,7 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { OnboardingTrackerStateResult } from '@app/unraid-api/config/onboarding-tracker.service.js'; -import { OnboardingTrackerService } from '@app/unraid-api/config/onboarding-tracker.module.js'; import { OnboardingStatus } from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js'; import { CustomizationResolver } from '@app/unraid-api/graph/resolvers/customization/customization.resolver.js'; import { OnboardingService } from '@app/unraid-api/graph/resolvers/customization/onboarding.service.js'; @@ -15,27 +13,16 @@ describe('CustomizationResolver', () => { getTheme: vi.fn(), isFreshInstall: vi.fn(), getOnboardingState: vi.fn(), + getOnboardingResponse: vi.fn(), } as unknown as OnboardingService; - const onboardingTracker = { - getStateResult: vi.fn(), - getCurrentVersion: vi.fn(), - } as unknown as OnboardingTrackerService; const displayService = { getAvailableLanguages: vi.fn(), } as unknown as DisplayService; - const resolver = new CustomizationResolver(onboardingService, onboardingTracker, displayService); + const resolver = new CustomizationResolver(onboardingService, displayService); beforeEach(() => { vi.clearAllMocks(); - vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ - kind: 'ok', - state: { - completed: false, - completedAtVersion: undefined, - }, - } satisfies OnboardingTrackerStateResult); - vi.mocked(onboardingTracker.getCurrentVersion).mockReturnValue('7.2.0'); vi.mocked(onboardingService.getPublicPartnerInfo).mockResolvedValue(null); vi.mocked(onboardingService.getActivationDataForPublic).mockResolvedValue(null); vi.mocked(onboardingService.getOnboardingState).mockResolvedValue({ @@ -45,23 +32,41 @@ describe('CustomizationResolver', () => { hasActivationCode: false, activationRequired: false, }); + vi.mocked(onboardingService.getOnboardingResponse).mockResolvedValue({ + status: OnboardingStatus.INCOMPLETE, + isPartnerBuild: false, + completed: false, + completedAtVersion: undefined, + onboardingState: { + registrationState: undefined, + isRegistered: false, + isFreshInstall: false, + hasActivationCode: false, + activationRequired: false, + }, + }); }); it('throws when tracker state could not be read', async () => { - vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ - kind: 'error', - error: new Error('permission denied'), - }); + vi.mocked(onboardingService.getOnboardingResponse).mockRejectedValue( + new Error('permission denied') + ); await expect(resolver.resolveOnboarding()).rejects.toThrow(); }); it('returns INCOMPLETE status when not completed', async () => { - vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ - kind: 'ok', - state: { - completed: false, - completedAtVersion: undefined, + vi.mocked(onboardingService.getOnboardingResponse).mockResolvedValue({ + status: OnboardingStatus.INCOMPLETE, + isPartnerBuild: false, + completed: false, + completedAtVersion: undefined, + onboardingState: { + registrationState: undefined, + isRegistered: false, + isFreshInstall: false, + hasActivationCode: false, + activationRequired: false, }, }); @@ -83,11 +88,17 @@ describe('CustomizationResolver', () => { }); it('returns COMPLETED status when completed on current version', async () => { - vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ - kind: 'ok', - state: { - completed: true, - completedAtVersion: '7.2.0', + vi.mocked(onboardingService.getOnboardingResponse).mockResolvedValue({ + status: OnboardingStatus.COMPLETED, + isPartnerBuild: false, + completed: true, + completedAtVersion: '7.2.0', + onboardingState: { + registrationState: undefined, + isRegistered: false, + isFreshInstall: false, + hasActivationCode: false, + activationRequired: false, }, }); @@ -109,14 +120,19 @@ describe('CustomizationResolver', () => { }); it('returns COMPLETED status when completed on a prior patch of current minor', async () => { - vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ - kind: 'ok', - state: { - completed: true, - completedAtVersion: '7.2.1', + vi.mocked(onboardingService.getOnboardingResponse).mockResolvedValue({ + status: OnboardingStatus.COMPLETED, + isPartnerBuild: false, + completed: true, + completedAtVersion: '7.2.1', + onboardingState: { + registrationState: undefined, + isRegistered: false, + isFreshInstall: false, + hasActivationCode: false, + activationRequired: false, }, }); - vi.mocked(onboardingTracker.getCurrentVersion).mockReturnValue('7.2.3'); const result = await resolver.resolveOnboarding(); @@ -136,11 +152,17 @@ describe('CustomizationResolver', () => { }); it('returns UPGRADE status when completed on older version', async () => { - vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ - kind: 'ok', - state: { - completed: true, - completedAtVersion: '7.1.0', + vi.mocked(onboardingService.getOnboardingResponse).mockResolvedValue({ + status: OnboardingStatus.UPGRADE, + isPartnerBuild: false, + completed: true, + completedAtVersion: '7.1.0', + onboardingState: { + registrationState: undefined, + isRegistered: false, + isFreshInstall: false, + hasActivationCode: false, + activationRequired: false, }, }); @@ -162,11 +184,17 @@ describe('CustomizationResolver', () => { }); it('returns DOWNGRADE status when completed on newer version', async () => { - vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ - kind: 'ok', - state: { - completed: true, - completedAtVersion: '7.3.0', + vi.mocked(onboardingService.getOnboardingResponse).mockResolvedValue({ + status: OnboardingStatus.DOWNGRADE, + isPartnerBuild: false, + completed: true, + completedAtVersion: '7.3.0', + onboardingState: { + registrationState: undefined, + isRegistered: false, + isFreshInstall: false, + hasActivationCode: false, + activationRequired: false, }, }); @@ -188,16 +216,17 @@ describe('CustomizationResolver', () => { }); it('returns isPartnerBuild true when partner info exists', async () => { - vi.mocked(onboardingTracker.getStateResult).mockResolvedValue({ - kind: 'ok', - state: { - completed: false, - completedAtVersion: undefined, - }, - }); - vi.mocked(onboardingService.getPublicPartnerInfo).mockResolvedValue({ - partner: { - name: 'Test Partner', + vi.mocked(onboardingService.getOnboardingResponse).mockResolvedValue({ + status: OnboardingStatus.INCOMPLETE, + isPartnerBuild: true, + completed: false, + completedAtVersion: undefined, + onboardingState: { + registrationState: undefined, + isRegistered: false, + isFreshInstall: false, + hasActivationCode: false, + activationRequired: false, }, }); diff --git a/api/src/unraid-api/graph/resolvers/customization/customization.resolver.ts b/api/src/unraid-api/graph/resolvers/customization/customization.resolver.ts index 94a95f7bb3..9c6966e37c 100644 --- a/api/src/unraid-api/graph/resolvers/customization/customization.resolver.ts +++ b/api/src/unraid-api/graph/resolvers/customization/customization.resolver.ts @@ -2,27 +2,22 @@ import { Query, ResolveField, Resolver } from '@nestjs/graphql'; import { AuthAction, Resource } from '@unraid/shared/graphql.model.js'; import { UsePermissions } from '@unraid/shared/use-permissions.directive.js'; -import { GraphQLError } from 'graphql'; import { Public } from '@app/unraid-api/auth/public.decorator.js'; -import { OnboardingTrackerService } from '@app/unraid-api/config/onboarding-tracker.module.js'; import { ActivationCode, Customization, Onboarding, - OnboardingStatus, } from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js'; import { OnboardingService } from '@app/unraid-api/graph/resolvers/customization/onboarding.service.js'; import { Theme } from '@app/unraid-api/graph/resolvers/customization/theme.model.js'; import { Language } from '@app/unraid-api/graph/resolvers/info/display/display.model.js'; import { DisplayService } from '@app/unraid-api/graph/resolvers/info/display/display.service.js'; -import { getOnboardingVersionDirection } from '@app/unraid-api/graph/resolvers/onboarding/onboarding-status.util.js'; @Resolver(() => Customization) export class CustomizationResolver { constructor( private readonly onboardingService: OnboardingService, - private readonly onboardingTracker: OnboardingTrackerService, private readonly displayService: DisplayService ) {} @@ -60,41 +55,7 @@ export class CustomizationResolver { resource: Resource.CUSTOMIZATIONS, }) async resolveOnboarding(): Promise { - const trackerStateResult = await this.onboardingTracker.getStateResult(); - if (trackerStateResult.kind === 'error') { - throw new GraphQLError('Onboarding tracker state is unavailable.'); - } - - const state = trackerStateResult.state; - const currentVersion = this.onboardingTracker.getCurrentVersion() ?? 'unknown'; - const partnerInfo = await this.onboardingService.getPublicPartnerInfo(); - const activationData = await this.onboardingService.getActivationData(); - const onboardingState = await this.onboardingService.getOnboardingState(); - const versionDirection = getOnboardingVersionDirection(state.completedAtVersion, currentVersion); - - // Compute the status based on completion state and version - let status: OnboardingStatus; - if (!state.completed) { - status = OnboardingStatus.INCOMPLETE; - } else if (versionDirection === 'DOWNGRADE') { - status = OnboardingStatus.DOWNGRADE; - } else if (versionDirection === 'UPGRADE') { - status = OnboardingStatus.UPGRADE; - } else { - status = OnboardingStatus.COMPLETED; - } - - // Get the activation code string if present and non-empty - const activationCode = activationData?.code?.trim() || undefined; - - return { - status, - isPartnerBuild: partnerInfo !== null, - completed: state.completed, - completedAtVersion: state.completedAtVersion, - activationCode, - onboardingState, - }; + return this.onboardingService.getOnboardingResponse({ includeActivationCode: true }); } @ResolveField(() => [Language], { nullable: true, name: 'availableLanguages' }) diff --git a/api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts b/api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts index 79ad738de2..10f4652c45 100644 --- a/api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts +++ b/api/src/unraid-api/graph/resolvers/customization/onboarding.service.spec.ts @@ -14,7 +14,10 @@ import { getters } from '@app/store/index.js'; import { OnboardingOverrideService } from '@app/unraid-api/config/onboarding-override.service.js'; import { OnboardingStateService } from '@app/unraid-api/config/onboarding-state.service.js'; import { OnboardingTrackerService } from '@app/unraid-api/config/onboarding-tracker.module.js'; -import { ActivationCode } from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js'; +import { + ActivationCode, + OnboardingStatus, +} from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js'; import { OnboardingService } from '@app/unraid-api/graph/resolvers/customization/onboarding.service.js'; vi.mock('@app/core/utils/files/file-exists.js'); @@ -115,9 +118,11 @@ vi.mock('@app/core/utils/misc/sleep.js', async () => { }); const onboardingTrackerMock = { - isCompleted: vi.fn<() => boolean>(), - getState: vi.fn<() => { completed: boolean; completedAtVersion?: string }>(), + isCompleted: vi.fn<() => Promise>(), + getStateResult: vi.fn(), + getCurrentVersion: vi.fn(), markCompleted: vi.fn<() => Promise<{ completed: boolean; completedAtVersion?: string }>>(), + reset: vi.fn<() => Promise<{ completed: boolean; completedAtVersion?: string }>>(), }; const onboardingOverridesMock = { getState: vi.fn(), @@ -182,7 +187,27 @@ describe('OnboardingService', () => { loggerWarnSpy = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => {}); loggerErrorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => {}); onboardingTrackerMock.isCompleted.mockReset(); - onboardingTrackerMock.isCompleted.mockReturnValue(false); + onboardingTrackerMock.isCompleted.mockResolvedValue(false); + onboardingTrackerMock.getStateResult.mockReset(); + onboardingTrackerMock.getStateResult.mockResolvedValue({ + kind: 'ok', + state: { + completed: false, + completedAtVersion: undefined, + }, + }); + onboardingTrackerMock.getCurrentVersion.mockReset(); + onboardingTrackerMock.getCurrentVersion.mockReturnValue('7.2.0'); + onboardingTrackerMock.markCompleted.mockReset(); + onboardingTrackerMock.markCompleted.mockResolvedValue({ + completed: true, + completedAtVersion: '7.2.0', + }); + onboardingTrackerMock.reset.mockReset(); + onboardingTrackerMock.reset.mockResolvedValue({ + completed: false, + completedAtVersion: undefined, + }); onboardingOverridesMock.getState.mockReset(); onboardingOverridesMock.getState.mockReturnValue(null); onboardingOverridesMock.setState.mockReset(); @@ -244,6 +269,72 @@ describe('OnboardingService', () => { expect(service).toBeDefined(); }); + describe('getOnboardingResponse', () => { + it('builds an onboarding response with activation code when requested', async () => { + onboardingTrackerMock.getStateResult.mockResolvedValue({ + kind: 'ok', + state: { + completed: true, + completedAtVersion: '7.2.0', + }, + }); + onboardingTrackerMock.getCurrentVersion.mockReturnValue('7.2.0'); + onboardingStateMock.getRegistrationState.mockReturnValue('PRO'); + onboardingStateMock.hasActivationCode.mockResolvedValue(true); + onboardingStateMock.isFreshInstall.mockReturnValue(false); + onboardingStateMock.isRegistered.mockReturnValue(true); + onboardingStateMock.requiresActivationStep.mockReturnValue(false); + + vi.spyOn(service, 'getPublicPartnerInfo').mockResolvedValue({ + partner: { name: 'Partner' }, + branding: null, + }); + vi.spyOn(service, 'getActivationData').mockResolvedValue({ + code: ' ABC123 ', + } as ActivationCode); + + await expect( + service.getOnboardingResponse({ includeActivationCode: true }) + ).resolves.toEqual({ + status: OnboardingStatus.COMPLETED, + isPartnerBuild: true, + completed: true, + completedAtVersion: '7.2.0', + activationCode: 'ABC123', + onboardingState: { + registrationState: 'PRO', + isRegistered: true, + isFreshInstall: false, + hasActivationCode: true, + activationRequired: false, + }, + }); + }); + + it('omits activation code when it is not requested', async () => { + vi.spyOn(service, 'getPublicPartnerInfo').mockResolvedValue(null); + vi.spyOn(service, 'getActivationData'); + + await expect(service.getOnboardingResponse()).resolves.toMatchObject({ + status: OnboardingStatus.INCOMPLETE, + completed: false, + completedAtVersion: undefined, + }); + expect(service.getActivationData).not.toHaveBeenCalled(); + }); + + it('throws when tracker state is unavailable', async () => { + onboardingTrackerMock.getStateResult.mockResolvedValue({ + kind: 'error', + error: new Error('permission denied'), + }); + + await expect(service.getOnboardingResponse()).rejects.toThrow( + 'Onboarding tracker state is unavailable.' + ); + }); + }); + describe('onModuleInit', () => { it('should log error if dynamix user config path is missing', async () => { const originalDynamixConfig = [...mockPaths['dynamix-config']]; @@ -289,7 +380,7 @@ describe('OnboardingService', () => { }); it('should skip customizations when first boot already completed', async () => { - onboardingTrackerMock.isCompleted.mockReturnValueOnce(true); + onboardingTrackerMock.isCompleted.mockResolvedValueOnce(true); await service.onModuleInit(); @@ -305,8 +396,8 @@ describe('OnboardingService', () => { it('should be idempotent across init calls when tracker marks setup complete', async () => { onboardingTrackerMock.isCompleted - .mockReturnValueOnce(false) // first init applies customizations - .mockReturnValueOnce(true); // second init should skip + .mockResolvedValueOnce(false) // first init applies customizations + .mockResolvedValueOnce(true); // second init should skip vi.mocked(fs.access).mockResolvedValue(undefined); vi.mocked(fs.readdir).mockResolvedValue([activationJsonFile as any]); vi.mocked(fs.readFile).mockImplementation(async (p) => { @@ -1363,7 +1454,7 @@ describe('applyActivationCustomizations specific tests', () => { loggerWarnSpy = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => {}); loggerErrorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => {}); onboardingTrackerMock.isCompleted.mockReset(); - onboardingTrackerMock.isCompleted.mockReturnValue(false); + onboardingTrackerMock.isCompleted.mockResolvedValue(false); onboardingOverridesMock.getState.mockReset(); onboardingOverridesMock.getState.mockReturnValue(null); onboardingOverridesMock.setState.mockReset(); @@ -1569,7 +1660,7 @@ describe('OnboardingService - updateCfgFile', () => { loggerLogSpy = vi.spyOn(Logger.prototype, 'log').mockImplementation(() => {}); loggerErrorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => {}); onboardingTrackerMock.isCompleted.mockReset(); - onboardingTrackerMock.isCompleted.mockReturnValue(false); + onboardingTrackerMock.isCompleted.mockResolvedValue(false); onboardingOverridesMock.getState.mockReset(); onboardingOverridesMock.getState.mockReturnValue(null); onboardingOverridesMock.setState.mockReset(); diff --git a/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts b/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts index 5b8d3b3e73..6f4c8b5e6e 100644 --- a/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts +++ b/api/src/unraid-api/graph/resolvers/customization/onboarding.service.ts @@ -19,7 +19,9 @@ import { OnboardingTrackerService } from '@app/unraid-api/config/onboarding-trac import { ActivationCode, BrandingConfig, + Onboarding, OnboardingState, + OnboardingStatus, PublicPartnerInfo, } from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js'; import { @@ -27,6 +29,7 @@ import { getActivationDirCandidates, } from '@app/unraid-api/graph/resolvers/customization/activation-steps.util.js'; import { Theme, ThemeName } from '@app/unraid-api/graph/resolvers/customization/theme.model.js'; +import { getOnboardingVersionDirection } from '@app/unraid-api/graph/resolvers/onboarding/onboarding-status.util.js'; @Injectable() export class OnboardingService implements OnModuleInit { @@ -340,6 +343,52 @@ export class OnboardingService implements OnModuleInit { }; } + public async getOnboardingResponse(options?: { + includeActivationCode?: boolean; + }): Promise { + const trackerStateResult = await this.onboardingTracker.getStateResult(); + if (trackerStateResult.kind === 'error') { + throw new GraphQLError('Onboarding tracker state is unavailable.'); + } + + const state = trackerStateResult.state; + const currentVersion = this.onboardingTracker.getCurrentVersion() ?? 'unknown'; + const partnerInfo = await this.getPublicPartnerInfo(); + const onboardingState = await this.getOnboardingState(); + const versionDirection = getOnboardingVersionDirection(state.completedAtVersion, currentVersion); + + let status: OnboardingStatus; + if (!state.completed) { + status = OnboardingStatus.INCOMPLETE; + } else if (versionDirection === 'DOWNGRADE') { + status = OnboardingStatus.DOWNGRADE; + } else if (versionDirection === 'UPGRADE') { + status = OnboardingStatus.UPGRADE; + } else { + status = OnboardingStatus.COMPLETED; + } + + const activationData = options?.includeActivationCode ? await this.getActivationData() : null; + const activationCode = activationData?.code?.trim() || undefined; + + return { + status, + isPartnerBuild: partnerInfo !== null, + completed: state.completed, + completedAtVersion: state.completedAtVersion, + activationCode, + onboardingState, + }; + } + + public async markOnboardingCompleted(): Promise { + await this.onboardingTracker.markCompleted(); + } + + public async resetOnboarding(): Promise { + await this.onboardingTracker.reset(); + } + public isFreshInstall(): boolean { return this.onboardingState.isFreshInstall(); } diff --git a/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts b/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts index 15019c0953..200291c1e8 100644 --- a/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts +++ b/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts @@ -5,21 +5,15 @@ import { CreateInternalBootPoolInput } from '@app/unraid-api/graph/resolvers/onb import { OnboardingMutationsResolver } from '@app/unraid-api/graph/resolvers/onboarding/onboarding.mutation.js'; describe('OnboardingMutationsResolver', () => { - const onboardingTracker = { - markCompleted: vi.fn(), - reset: vi.fn(), - getStateResult: vi.fn(), - getCurrentVersion: vi.fn(), - }; - const onboardingOverrides = { setState: vi.fn(), clearState: vi.fn(), }; const onboardingService = { - getPublicPartnerInfo: vi.fn(), - getOnboardingState: vi.fn(), + markOnboardingCompleted: vi.fn(), + resetOnboarding: vi.fn(), + getOnboardingResponse: vi.fn(), clearActivationDataCache: vi.fn(), }; @@ -27,151 +21,79 @@ describe('OnboardingMutationsResolver', () => { createInternalBootPool: vi.fn(), }; - let resolver: OnboardingMutationsResolver; - - beforeEach(() => { - vi.clearAllMocks(); - onboardingTracker.getStateResult.mockResolvedValue({ - kind: 'ok', - state: { - completed: false, - completedAtVersion: undefined, - }, - }); - onboardingTracker.getCurrentVersion.mockReturnValue('7.2.0'); - onboardingService.getPublicPartnerInfo.mockResolvedValue(null); - onboardingService.getOnboardingState.mockResolvedValue({ + const defaultOnboardingResponse = { + status: OnboardingStatus.INCOMPLETE, + isPartnerBuild: false, + completed: false, + completedAtVersion: undefined, + onboardingState: { registrationState: null, isRegistered: false, isFreshInstall: false, hasActivationCode: false, activationRequired: false, - }); + }, + }; + + let resolver: OnboardingMutationsResolver; + + beforeEach(() => { + vi.clearAllMocks(); + onboardingService.markOnboardingCompleted.mockResolvedValue(undefined); + onboardingService.resetOnboarding.mockResolvedValue(undefined); + onboardingService.getOnboardingResponse.mockResolvedValue(defaultOnboardingResponse); resolver = new OnboardingMutationsResolver( - onboardingTracker as any, - onboardingOverrides as any, - onboardingService as any, - onboardingInternalBootService as any + onboardingOverrides as never, + onboardingService as never, + onboardingInternalBootService as never ); }); - it('propagates tracker failure from completeOnboarding', async () => { + it('propagates completion failures', async () => { const error = new Error('tracker-write-failed'); - onboardingTracker.markCompleted.mockRejectedValue(error); + onboardingService.markOnboardingCompleted.mockRejectedValue(error); await expect(resolver.completeOnboarding()).rejects.toThrow('tracker-write-failed'); - expect(onboardingService.getPublicPartnerInfo).not.toHaveBeenCalled(); + expect(onboardingService.getOnboardingResponse).not.toHaveBeenCalled(); }); - it('returns completed onboarding state when markCompleted succeeds', async () => { - onboardingTracker.markCompleted.mockResolvedValue({ + it('delegates completeOnboarding through the onboarding service', async () => { + const response = { + ...defaultOnboardingResponse, + status: OnboardingStatus.COMPLETED, completed: true, completedAtVersion: '7.2.0', - }); - onboardingTracker.getStateResult.mockResolvedValue({ - kind: 'ok', - state: { - completed: true, - completedAtVersion: '7.2.0', - }, - }); - onboardingTracker.getCurrentVersion.mockReturnValue('7.2.0'); - onboardingService.getPublicPartnerInfo.mockResolvedValue(null); - - const result = await resolver.completeOnboarding(); - - expect(result.completed).toBe(true); - expect(result.completedAtVersion).toBe('7.2.0'); - expect(result.status).toBe(OnboardingStatus.COMPLETED); - expect(result.isPartnerBuild).toBe(false); - expect(result.onboardingState).toEqual({ - registrationState: null, - isRegistered: false, - isFreshInstall: false, - hasActivationCode: false, - activationRequired: false, - }); - }); - - it('returns incomplete status after resetOnboarding', async () => { - onboardingTracker.reset.mockResolvedValue(undefined); - onboardingTracker.getStateResult.mockResolvedValue({ - kind: 'ok', - state: { - completed: false, - completedAtVersion: undefined, - }, - }); - - const result = await resolver.resetOnboarding(); - - expect(onboardingTracker.reset).toHaveBeenCalledTimes(1); - expect(result.status).toBe(OnboardingStatus.INCOMPLETE); - expect(result.completed).toBe(false); - }); - - it('returns completed status when completed version is on a prior patch of current minor', async () => { - onboardingTracker.markCompleted.mockResolvedValue(undefined); - onboardingTracker.getStateResult.mockResolvedValue({ - kind: 'ok', - state: { - completed: true, - completedAtVersion: '7.2.1', - }, - }); - onboardingTracker.getCurrentVersion.mockReturnValue('7.2.4'); - - const result = await resolver.completeOnboarding(); - - expect(result.status).toBe(OnboardingStatus.COMPLETED); - }); - - it('returns upgrade status when completed version is behind current', async () => { - onboardingTracker.markCompleted.mockResolvedValue(undefined); - onboardingTracker.getStateResult.mockResolvedValue({ - kind: 'ok', - state: { - completed: true, - completedAtVersion: '7.1.0', - }, - }); - onboardingTracker.getCurrentVersion.mockReturnValue('7.2.0'); - - const result = await resolver.completeOnboarding(); + }; + onboardingService.getOnboardingResponse.mockResolvedValue(response); - expect(result.status).toBe(OnboardingStatus.UPGRADE); + await expect(resolver.completeOnboarding()).resolves.toEqual(response); + expect(onboardingService.markOnboardingCompleted).toHaveBeenCalledTimes(1); + expect(onboardingService.getOnboardingResponse).toHaveBeenCalledWith(); }); - it('returns downgrade status when completed version is ahead of current', async () => { - onboardingTracker.markCompleted.mockResolvedValue(undefined); - onboardingTracker.getStateResult.mockResolvedValue({ - kind: 'ok', - state: { - completed: true, - completedAtVersion: '7.2.0', - }, - }); - onboardingTracker.getCurrentVersion.mockReturnValue('7.1.0'); - - const result = await resolver.completeOnboarding(); + it('delegates resetOnboarding through the onboarding service', async () => { + const response = { + ...defaultOnboardingResponse, + status: OnboardingStatus.INCOMPLETE, + completed: false, + }; + onboardingService.getOnboardingResponse.mockResolvedValue(response); - expect(result.status).toBe(OnboardingStatus.DOWNGRADE); + await expect(resolver.resetOnboarding()).resolves.toEqual(response); + expect(onboardingService.resetOnboarding).toHaveBeenCalledTimes(1); + expect(onboardingService.getOnboardingResponse).toHaveBeenCalledWith(); }); - it('setOnboardingOverride stores override, clears cache, and returns onboarding state', async () => { - onboardingTracker.getStateResult.mockResolvedValue({ - kind: 'ok', - state: { - completed: true, - completedAtVersion: '7.2.0', - }, - }); - onboardingTracker.getCurrentVersion.mockReturnValue('7.2.0'); - onboardingService.getPublicPartnerInfo.mockResolvedValue({ - partner: { name: 'Partner' }, - branding: {}, - } as any); + it('stores overrides, clears activation cache, and returns the shared onboarding response', async () => { + const response = { + ...defaultOnboardingResponse, + status: OnboardingStatus.COMPLETED, + isPartnerBuild: true, + completed: true, + completedAtVersion: '7.2.0', + }; + onboardingService.getOnboardingResponse.mockResolvedValue(response); const input = { onboarding: { @@ -179,10 +101,9 @@ describe('OnboardingMutationsResolver', () => { completedAtVersion: '7.2.0', }, registrationState: undefined, - } as any; - - const result = await resolver.setOnboardingOverride(input); + } as never; + await expect(resolver.setOnboardingOverride(input)).resolves.toEqual(response); expect(onboardingOverrides.setState).toHaveBeenCalledWith({ onboarding: input.onboarding, activationCode: undefined, @@ -190,24 +111,20 @@ describe('OnboardingMutationsResolver', () => { registrationState: undefined, }); expect(onboardingService.clearActivationDataCache).toHaveBeenCalledTimes(1); - expect(result.status).toBe(OnboardingStatus.COMPLETED); - expect(result.isPartnerBuild).toBe(true); + expect(onboardingService.getOnboardingResponse).toHaveBeenCalledWith(); }); - it('clearOnboardingOverride clears override and cache', async () => { - onboardingTracker.getStateResult.mockResolvedValue({ - kind: 'ok', - state: { - completed: false, - completedAtVersion: undefined, - }, - }); - - const result = await resolver.clearOnboardingOverride(); - + it('clears overrides and returns the shared onboarding response', async () => { + await expect(resolver.clearOnboardingOverride()).resolves.toEqual(defaultOnboardingResponse); expect(onboardingOverrides.clearState).toHaveBeenCalledTimes(1); expect(onboardingService.clearActivationDataCache).toHaveBeenCalledTimes(1); - expect(result.status).toBe(OnboardingStatus.INCOMPLETE); + expect(onboardingService.getOnboardingResponse).toHaveBeenCalledWith(); + }); + + it('propagates onboarding response failures after completion', async () => { + onboardingService.getOnboardingResponse.mockRejectedValue(new Error('tracker-read-failed')); + + await expect(resolver.completeOnboarding()).rejects.toThrow('tracker-read-failed'); }); it('delegates createInternalBootPool to onboarding internal boot service', async () => { @@ -223,23 +140,12 @@ describe('OnboardingMutationsResolver', () => { bootSizeMiB: 16384, updateBios: true, }; - const result = await resolver.createInternalBootPool(input); - expect(onboardingInternalBootService.createInternalBootPool).toHaveBeenCalledWith(input); - expect(result).toEqual({ + await expect(resolver.createInternalBootPool(input)).resolves.toEqual({ ok: true, code: 0, output: 'done', }); - }); - - it('propagates tracker read failures while building the completion response', async () => { - onboardingTracker.markCompleted.mockResolvedValue(undefined); - onboardingTracker.getStateResult.mockResolvedValue({ - kind: 'error', - error: new Error('tracker-read-failed'), - }); - - await expect(resolver.completeOnboarding()).rejects.toThrow('tracker-read-failed'); + expect(onboardingInternalBootService.createInternalBootPool).toHaveBeenCalledWith(input); }); }); diff --git a/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts b/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts index 5ea9b84074..1011f65b2e 100644 --- a/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts +++ b/api/src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.ts @@ -5,15 +5,10 @@ import { UsePermissions } from '@unraid/shared/use-permissions.directive.js'; import type { OnboardingOverrideState } from '@app/unraid-api/config/onboarding-override.model.js'; import { OnboardingOverrideService } from '@app/unraid-api/config/onboarding-override.service.js'; -import { OnboardingTrackerService } from '@app/unraid-api/config/onboarding-tracker.module.js'; -import { - Onboarding, - OnboardingStatus, -} from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js'; +import { Onboarding } from '@app/unraid-api/graph/resolvers/customization/activation-code.model.js'; import { OnboardingService } from '@app/unraid-api/graph/resolvers/customization/onboarding.service.js'; import { OnboardingMutations } from '@app/unraid-api/graph/resolvers/mutation/mutation.model.js'; import { OnboardingInternalBootService } from '@app/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.js'; -import { getOnboardingVersionDirection } from '@app/unraid-api/graph/resolvers/onboarding/onboarding-status.util.js'; import { CreateInternalBootPoolInput, OnboardingInternalBootResult, @@ -23,48 +18,11 @@ import { @Resolver(() => OnboardingMutations) export class OnboardingMutationsResolver { constructor( - private readonly onboardingTracker: OnboardingTrackerService, private readonly onboardingOverrides: OnboardingOverrideService, private readonly onboardingService: OnboardingService, private readonly onboardingInternalBootService: OnboardingInternalBootService ) {} - /** - * Build a full Onboarding response with computed status - */ - private async buildOnboardingResponse(): Promise { - const stateResult = await this.onboardingTracker.getStateResult(); - if (stateResult.kind === 'error') { - throw stateResult.error; - } - - const state = stateResult.state; - const currentVersion = this.onboardingTracker.getCurrentVersion() ?? 'unknown'; - const partnerInfo = await this.onboardingService.getPublicPartnerInfo(); - const onboardingState = await this.onboardingService.getOnboardingState(); - const versionDirection = getOnboardingVersionDirection(state.completedAtVersion, currentVersion); - - // Compute the status based on completion state and version - let status: OnboardingStatus; - if (!state.completed) { - status = OnboardingStatus.INCOMPLETE; - } else if (versionDirection === 'DOWNGRADE') { - status = OnboardingStatus.DOWNGRADE; - } else if (versionDirection === 'UPGRADE') { - status = OnboardingStatus.UPGRADE; - } else { - status = OnboardingStatus.COMPLETED; - } - - return { - status, - isPartnerBuild: partnerInfo !== null, - completed: state.completed, - completedAtVersion: state.completedAtVersion, - onboardingState, - }; - } - @ResolveField(() => Onboarding, { description: 'Marks the onboarding flow as completed', }) @@ -73,8 +31,8 @@ export class OnboardingMutationsResolver { resource: Resource.WELCOME, }) async completeOnboarding(): Promise { - await this.onboardingTracker.markCompleted(); - return this.buildOnboardingResponse(); + await this.onboardingService.markOnboardingCompleted(); + return this.onboardingService.getOnboardingResponse(); } @ResolveField(() => Onboarding, { @@ -85,8 +43,8 @@ export class OnboardingMutationsResolver { resource: Resource.WELCOME, }) async resetOnboarding(): Promise { - await this.onboardingTracker.reset(); - return this.buildOnboardingResponse(); + await this.onboardingService.resetOnboarding(); + return this.onboardingService.getOnboardingResponse(); } @ResolveField(() => Onboarding, { @@ -105,7 +63,7 @@ export class OnboardingMutationsResolver { }; this.onboardingOverrides.setState(override); this.onboardingService.clearActivationDataCache(); - return this.buildOnboardingResponse(); + return this.onboardingService.getOnboardingResponse(); } @ResolveField(() => Onboarding, { @@ -118,7 +76,7 @@ export class OnboardingMutationsResolver { async clearOnboardingOverride(): Promise { this.onboardingOverrides.clearState(); this.onboardingService.clearActivationDataCache(); - return this.buildOnboardingResponse(); + return this.onboardingService.getOnboardingResponse(); } @ResolveField(() => OnboardingInternalBootResult, { From 6586b4958a3aa515260ab23907947d6759c5845f Mon Sep 17 00:00:00 2001 From: Ajit Mehrotra Date: Mon, 16 Mar 2026 11:52:53 -0400 Subject: [PATCH 06/16] fix(onboarding): persist stable resume step identity - Purpose: keep resumed onboarding drafts anchored to the intended step even when the visible step list changes while boot visibility loads. - Before: onboarding only persisted currentStepIndex, so a network-only internal boot visibility refetch could temporarily remove CONFIGURE_BOOT and reinterpret saved drafts against a shorter list. - Problem: resumed internal boot drafts could jump to the plugins step, and later-step resumes could briefly render a blank modal until the query returned. - Change: persist currentStepId in the draft store, resolve the active modal step by id first, and keep a resumed CONFIGURE_BOOT step available while the visibility query is still loading. - How: add normalized step-id persistence, sync modal navigation through draftStore.setCurrentStep, and cover the loading-window resume regression in OnboardingModal tests. --- .../Onboarding/OnboardingModal.test.ts | 33 +++++++- .../components/Onboarding/OnboardingModal.vue | 80 +++++++++++++++---- .../Onboarding/store/onboardingDraft.ts | 23 ++++++ 3 files changed, 120 insertions(+), 16 deletions(-) diff --git a/web/__test__/components/Onboarding/OnboardingModal.test.ts b/web/__test__/components/Onboarding/OnboardingModal.test.ts index 8d7ec6a29d..4c54f2e3bb 100644 --- a/web/__test__/components/Onboarding/OnboardingModal.test.ts +++ b/web/__test__/components/Onboarding/OnboardingModal.test.ts @@ -17,6 +17,7 @@ type InternalBootVisibilityResult = { const { mutateMock, internalBootVisibilityResult, + internalBootVisibilityLoading, onboardingModalStoreState, activationCodeDataStore, onboardingStatusStore, @@ -35,6 +36,7 @@ const { }, }, } as InternalBootVisibilityResult, + internalBootVisibilityLoading: { value: false }, onboardingModalStoreState: { isAutoVisible: { value: true }, isForceOpened: { value: false }, @@ -64,7 +66,12 @@ const { }, onboardingDraftStore: { currentStepIndex: { value: 0 }, + currentStepId: { value: null }, internalBootApplySucceeded: { value: false }, + setCurrentStep: vi.fn((stepId: string, stepIndex: number) => { + onboardingDraftStore.currentStepId.value = stepId; + onboardingDraftStore.currentStepIndex.value = stepIndex; + }), }, purchaseStore: { generateUrl: vi.fn(() => 'https://example.com/activate'), @@ -104,7 +111,7 @@ vi.mock('@heroicons/vue/24/solid', () => ({ vi.mock('@vue/apollo-composable', () => ({ useQuery: () => ({ result: internalBootVisibilityResult, - loading: { value: false }, + loading: internalBootVisibilityLoading, error: { value: null }, }), useMutation: () => ({ @@ -187,7 +194,9 @@ describe('OnboardingModal.vue', () => { onboardingStatusStore.canDisplayOnboardingModal.value = true; onboardingStatusStore.isPartnerBuild.value = false; onboardingDraftStore.currentStepIndex.value = 0; + onboardingDraftStore.currentStepId.value = null; onboardingDraftStore.internalBootApplySucceeded.value = false; + internalBootVisibilityLoading.value = false; internalBootVisibilityResult.value = { vars: { bootedFromFlashWithInternalBootSetup: false, @@ -326,6 +335,7 @@ describe('OnboardingModal.vue', () => { it('shows internal boot step for regular builds', () => { onboardingDraftStore.currentStepIndex.value = 2; + onboardingDraftStore.currentStepId.value = 'CONFIGURE_BOOT'; const wrapper = mountComponent(); @@ -340,6 +350,7 @@ describe('OnboardingModal.vue', () => { }, }; onboardingDraftStore.currentStepIndex.value = 2; + onboardingDraftStore.currentStepId.value = 'CONFIGURE_BOOT'; const wrapper = mountComponent(); @@ -349,6 +360,7 @@ describe('OnboardingModal.vue', () => { it('shows internal boot step for partner builds when boot transfer is available', () => { onboardingStatusStore.isPartnerBuild.value = true; onboardingDraftStore.currentStepIndex.value = 2; + onboardingDraftStore.currentStepId.value = 'CONFIGURE_BOOT'; const wrapper = mountComponent(); @@ -363,6 +375,7 @@ describe('OnboardingModal.vue', () => { }, }; onboardingDraftStore.currentStepIndex.value = 2; + onboardingDraftStore.currentStepId.value = 'CONFIGURE_BOOT'; const wrapper = mountComponent(); @@ -378,6 +391,7 @@ describe('OnboardingModal.vue', () => { }, }; onboardingDraftStore.currentStepIndex.value = 2; + onboardingDraftStore.currentStepId.value = 'CONFIGURE_BOOT'; const wrapper = mountComponent(); @@ -385,6 +399,23 @@ describe('OnboardingModal.vue', () => { expect(wrapper.find('[data-testid="plugins-step"]').exists()).toBe(true); }); + it('keeps the resumed internal boot step visible while boot visibility is still loading', () => { + internalBootVisibilityLoading.value = true; + internalBootVisibilityResult.value = { + vars: { + bootedFromFlashWithInternalBootSetup: null, + enableBootTransfer: null, + }, + }; + onboardingDraftStore.currentStepIndex.value = 2; + onboardingDraftStore.currentStepId.value = 'CONFIGURE_BOOT'; + + const wrapper = mountComponent(); + + expect(wrapper.find('[data-testid="internal-boot-step"]').exists()).toBe(true); + expect(wrapper.find('[data-testid="plugins-step"]').exists()).toBe(false); + }); + it('opens exit confirmation when close button is clicked', async () => { const wrapper = mountComponent(); diff --git a/web/src/components/Onboarding/OnboardingModal.vue b/web/src/components/Onboarding/OnboardingModal.vue index 839a15d449..e7a5043830 100644 --- a/web/src/components/Onboarding/OnboardingModal.vue +++ b/web/src/components/Onboarding/OnboardingModal.vue @@ -1,5 +1,5 @@