fix: hide setup checklist sidebar for existing users (#4584)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson
GitHub
parent
5142a24536
commit
1ebbab575c
|
|
@ -25,7 +25,7 @@ import type {
|
|||
} from '../shared/types'
|
||||
import { isTerminalLeafId, makePaneKey } from '../shared/stable-pane-id'
|
||||
import { MAX_BROWSER_HISTORY_ENTRIES } from '../shared/workspace-session-browser-history'
|
||||
import { ONBOARDING_FLOW_VERSION } from '../shared/constants'
|
||||
import { ONBOARDING_FINAL_STEP, ONBOARDING_FLOW_VERSION } from '../shared/constants'
|
||||
|
||||
// Shared mutable state so the electron mock can reference a per-test directory
|
||||
const testState = { dir: '' }
|
||||
|
|
@ -286,6 +286,173 @@ describe('Store', () => {
|
|||
expect(ui.lastActiveRepoId).toBeNull()
|
||||
expect(ui.dismissedUpdateVersion).toBeNull()
|
||||
expect(ui.lastUpdateCheckAt).toBeNull()
|
||||
expect(ui.setupGuideSidebarDismissed).toBe(false)
|
||||
})
|
||||
|
||||
it('hides the setup guide sidebar entry for existing users backfilled as completed', async () => {
|
||||
writeDataFile({
|
||||
schemaVersion: 1,
|
||||
ui: {}
|
||||
})
|
||||
|
||||
const store = await createStore()
|
||||
const onboarding = store.getOnboarding()
|
||||
|
||||
expect(onboarding.closedAt).not.toBeNull()
|
||||
expect(onboarding.outcome).toBe('completed')
|
||||
expect(onboarding.lastCompletedStep).toBe(ONBOARDING_FINAL_STEP)
|
||||
expect(store.getUI().setupGuideSidebarDismissed).toBe(true)
|
||||
})
|
||||
|
||||
it('persists the existing-user onboarding backfill back to disk', async () => {
|
||||
// Why: the upgrade-cohort backfill is derived at load; this asserts the
|
||||
// backfilled onboarding+gate state round-trips through a write intact (the
|
||||
// load-time scheduleSave that triggers it without a manual flush is wired
|
||||
// via loadNeedsSave at the no-onboarding-block branch).
|
||||
writeDataFile({
|
||||
schemaVersion: 1,
|
||||
ui: {}
|
||||
})
|
||||
|
||||
const store = await createStore()
|
||||
store.flush()
|
||||
const persisted = readDataFile() as {
|
||||
onboarding?: { closedAt: number | null; outcome: string | null; lastCompletedStep: number }
|
||||
ui?: { setupGuideSidebarDismissed?: boolean }
|
||||
}
|
||||
|
||||
expect(persisted.onboarding?.closedAt).not.toBeNull()
|
||||
expect(persisted.onboarding?.outcome).toBe('completed')
|
||||
expect(persisted.onboarding?.lastCompletedStep).toBe(ONBOARDING_FINAL_STEP)
|
||||
expect(persisted.ui?.setupGuideSidebarDismissed).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps the setup guide sidebar entry available while onboarding is open', async () => {
|
||||
writeDataFile({
|
||||
onboarding: {
|
||||
flowVersion: ONBOARDING_FLOW_VERSION,
|
||||
closedAt: null,
|
||||
outcome: null,
|
||||
lastCompletedStep: -1,
|
||||
checklist: {}
|
||||
},
|
||||
ui: {}
|
||||
})
|
||||
|
||||
const store = await createStore()
|
||||
|
||||
expect(store.getOnboarding().closedAt).toBeNull()
|
||||
expect(store.getUI().setupGuideSidebarDismissed).toBe(false)
|
||||
})
|
||||
|
||||
it('treats persisted false setup guide sidebar dismissal as stale once onboarding is closed', async () => {
|
||||
writeDataFile({
|
||||
onboarding: {
|
||||
flowVersion: ONBOARDING_FLOW_VERSION,
|
||||
closedAt: 123,
|
||||
outcome: 'dismissed',
|
||||
lastCompletedStep: 2,
|
||||
checklist: {}
|
||||
},
|
||||
ui: {
|
||||
setupGuideSidebarDismissed: false
|
||||
}
|
||||
})
|
||||
|
||||
const store = await createStore()
|
||||
|
||||
expect(store.getUI().setupGuideSidebarDismissed).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps malformed completed onboarding closed for the setup guide sidebar gate', async () => {
|
||||
writeDataFile({
|
||||
onboarding: {
|
||||
flowVersion: ONBOARDING_FLOW_VERSION,
|
||||
closedAt: 'yesterday',
|
||||
outcome: 'completed',
|
||||
lastCompletedStep: ONBOARDING_FINAL_STEP,
|
||||
checklist: {}
|
||||
},
|
||||
ui: {
|
||||
setupGuideSidebarDismissed: false
|
||||
}
|
||||
})
|
||||
|
||||
const store = await createStore()
|
||||
const onboarding = store.getOnboarding()
|
||||
|
||||
expect(onboarding.closedAt).not.toBeNull()
|
||||
expect(onboarding.outcome).toBe('completed')
|
||||
expect(onboarding.lastCompletedStep).toBe(ONBOARDING_FINAL_STEP)
|
||||
expect(store.getUI().setupGuideSidebarDismissed).toBe(true)
|
||||
})
|
||||
|
||||
it('does not reopen the setup guide sidebar when closed onboarding has a null timestamp', async () => {
|
||||
writeDataFile({
|
||||
onboarding: {
|
||||
flowVersion: ONBOARDING_FLOW_VERSION,
|
||||
closedAt: null,
|
||||
outcome: 'dismissed',
|
||||
lastCompletedStep: 1,
|
||||
checklist: {}
|
||||
},
|
||||
ui: {}
|
||||
})
|
||||
|
||||
const store = await createStore()
|
||||
|
||||
expect(store.getOnboarding().closedAt).not.toBeNull()
|
||||
expect(store.getUI().setupGuideSidebarDismissed).toBe(true)
|
||||
})
|
||||
|
||||
it('recovers a close timestamp when closed onboarding omits the closedAt key', async () => {
|
||||
// Why: a persisted block missing `closedAt` entirely (vs an explicit null)
|
||||
// must still stay closed via outcome recovery, guarding the
|
||||
// `'closedAt' in raw` sanitizer branch separately from the null case.
|
||||
writeDataFile({
|
||||
onboarding: {
|
||||
flowVersion: ONBOARDING_FLOW_VERSION,
|
||||
outcome: 'completed',
|
||||
lastCompletedStep: ONBOARDING_FINAL_STEP,
|
||||
checklist: {}
|
||||
},
|
||||
ui: {}
|
||||
})
|
||||
|
||||
const store = await createStore()
|
||||
|
||||
expect(store.getOnboarding().closedAt).not.toBeNull()
|
||||
expect(store.getUI().setupGuideSidebarDismissed).toBe(true)
|
||||
})
|
||||
|
||||
it('does not mutate gate fields for a consistent closed-onboarding existing user', async () => {
|
||||
// Why: the gate must be idempotent. A user already persisted as
|
||||
// closed+completed must round-trip unchanged — the backfill path must not
|
||||
// fire and stomp the real closedAt with a fresh Date.now() each launch.
|
||||
const consistent = {
|
||||
onboarding: {
|
||||
flowVersion: ONBOARDING_FLOW_VERSION,
|
||||
closedAt: 123,
|
||||
outcome: 'completed',
|
||||
lastCompletedStep: ONBOARDING_FINAL_STEP,
|
||||
checklist: {}
|
||||
},
|
||||
ui: {
|
||||
setupGuideSidebarDismissed: true
|
||||
}
|
||||
}
|
||||
writeDataFile(consistent)
|
||||
|
||||
const store = await createStore()
|
||||
expect(store.getUI().setupGuideSidebarDismissed).toBe(true)
|
||||
|
||||
store.flush()
|
||||
const persisted = readDataFile() as typeof consistent
|
||||
|
||||
// Flushing the loaded state preserves the persisted gate fields verbatim.
|
||||
expect(persisted.onboarding.closedAt).toBe(123)
|
||||
expect(persisted.onboarding.outcome).toBe('completed')
|
||||
expect(persisted.ui.setupGuideSidebarDismissed).toBe(true)
|
||||
})
|
||||
|
||||
it.each([
|
||||
|
|
|
|||
|
|
@ -583,6 +583,60 @@ export function sanitizeOnboardingUpdate(
|
|||
return out
|
||||
}
|
||||
|
||||
function normalizeLoadedOnboardingState(
|
||||
input: unknown,
|
||||
defaults: OnboardingState
|
||||
): OnboardingState {
|
||||
// Why: if we successfully parsed an existing orca-data.json that lacks an
|
||||
// onboarding block, this is an upgrade-cohort user — backfill as completed
|
||||
// (not dismissed) so they don't get dropped into the wizard regardless of
|
||||
// whether they currently have repos, SSH targets, or just non-default
|
||||
// settings. Analytics still distinguish this from users who explicitly
|
||||
// bailed mid-funnel.
|
||||
if (!input) {
|
||||
return {
|
||||
...defaults,
|
||||
closedAt: Date.now(),
|
||||
outcome: 'completed',
|
||||
lastCompletedStep: ONBOARDING_FINAL_STEP
|
||||
}
|
||||
}
|
||||
// Why: validate every persisted onboarding key explicitly via the shared
|
||||
// sanitizer instead of spreading raw values. A type-flipped field on disk
|
||||
// (string where number expected, unknown checklist key) is dropped or
|
||||
// coerced to the default rather than poisoning in-memory state.
|
||||
const sanitized = sanitizeOnboardingUpdate(input, {
|
||||
migrateLegacyProgress: true
|
||||
})
|
||||
// Why: a persisted completed/dismissed outcome means the user left
|
||||
// onboarding. Recover from a bad/missing/null closedAt instead of reopening
|
||||
// the new-user sidebar checklist.
|
||||
const recoveredClosedAt =
|
||||
typeof sanitized.closedAt === 'number'
|
||||
? sanitized.closedAt
|
||||
: sanitized.outcome !== null && sanitized.outcome !== undefined
|
||||
? Date.now()
|
||||
: sanitized.closedAt
|
||||
return {
|
||||
...defaults,
|
||||
...sanitized,
|
||||
closedAt: recoveredClosedAt ?? defaults.closedAt,
|
||||
checklist: {
|
||||
...defaults.checklist,
|
||||
...sanitized.checklist
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function resolveSetupGuideSidebarDismissedOnLoad(
|
||||
persistedDismissed: unknown,
|
||||
onboarding: OnboardingState
|
||||
): boolean {
|
||||
// Why: the sidebar checklist is a new-user prompt. Once onboarding is
|
||||
// closed, persisted false is just the old default value, not a user opt-in.
|
||||
return onboarding.closedAt !== null || persistedDismissed === true
|
||||
}
|
||||
|
||||
// Why: read a settings field that was removed from the GlobalSettings type
|
||||
// but still round-trips on disk via the ...parsed.settings spread. One-shot
|
||||
// use only — for the inline-agents default-on migration's Case B discriminator.
|
||||
|
|
@ -1755,6 +1809,13 @@ export class Store {
|
|||
if (!visibleTaskProvidersDefaultedForJira) {
|
||||
this.loadNeedsSave = true
|
||||
}
|
||||
const normalizedOnboarding = normalizeLoadedOnboardingState(
|
||||
parsed.onboarding,
|
||||
defaults.onboarding
|
||||
)
|
||||
if (!parsed.onboarding) {
|
||||
this.loadNeedsSave = true
|
||||
}
|
||||
result = {
|
||||
...defaults,
|
||||
...parsed,
|
||||
|
|
@ -1935,6 +1996,16 @@ export class Store {
|
|||
) {
|
||||
this.loadNeedsSave = true
|
||||
}
|
||||
const setupGuideSidebarDismissed = resolveSetupGuideSidebarDismissedOnLoad(
|
||||
parsed.ui?.setupGuideSidebarDismissed,
|
||||
normalizedOnboarding
|
||||
)
|
||||
if (
|
||||
parsed.ui?.setupGuideSidebarDismissed !== setupGuideSidebarDismissed &&
|
||||
(setupGuideSidebarDismissed || parsed.ui?.setupGuideSidebarDismissed !== undefined)
|
||||
) {
|
||||
this.loadNeedsSave = true
|
||||
}
|
||||
return {
|
||||
...defaults.ui,
|
||||
...parsed.ui,
|
||||
|
|
@ -1942,6 +2013,7 @@ export class Store {
|
|||
// when no explicit persisted chrome preference exists yet.
|
||||
rightSidebarOpen,
|
||||
rightSidebarTab: normalizeRightSidebarTab(parsed.ui?.rightSidebarTab),
|
||||
setupGuideSidebarDismissed,
|
||||
sortBy: migrate ? ('smart' as const) : sort,
|
||||
showDotfilesByWorktree: normalizeShowDotfilesByWorktree(
|
||||
parsed.ui?.showDotfilesByWorktree
|
||||
|
|
@ -1995,38 +2067,7 @@ export class Store {
|
|||
),
|
||||
automations: Array.isArray(parsed.automations) ? parsed.automations : [],
|
||||
automationRuns: Array.isArray(parsed.automationRuns) ? parsed.automationRuns : [],
|
||||
onboarding: (() => {
|
||||
// Why: if we successfully parsed an existing orca-data.json that
|
||||
// lacks an onboarding block, this is an upgrade-cohort user —
|
||||
// backfill as completed (not dismissed) so they don't get dropped
|
||||
// into the wizard regardless of whether they currently have repos,
|
||||
// SSH targets, or just non-default settings. Analytics still
|
||||
// distinguish this from users who explicitly bailed mid-funnel.
|
||||
if (!parsed.onboarding) {
|
||||
return {
|
||||
...defaults.onboarding,
|
||||
closedAt: Date.now(),
|
||||
outcome: 'completed' as const,
|
||||
lastCompletedStep: ONBOARDING_FINAL_STEP
|
||||
}
|
||||
}
|
||||
// Why: validate every persisted onboarding key explicitly via the
|
||||
// shared sanitizer instead of spreading raw values. A type-flipped
|
||||
// field on disk (string where number expected, unknown checklist
|
||||
// key) is dropped or coerced to the default rather than poisoning
|
||||
// in-memory state.
|
||||
const sanitized = sanitizeOnboardingUpdate(parsed.onboarding, {
|
||||
migrateLegacyProgress: true
|
||||
})
|
||||
return {
|
||||
...defaults.onboarding,
|
||||
...sanitized,
|
||||
checklist: {
|
||||
...defaults.onboarding.checklist,
|
||||
...sanitized.checklist
|
||||
}
|
||||
}
|
||||
})()
|
||||
onboarding: normalizedOnboarding
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue