Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion plans/store-split.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Recommended implementation order:
- [x] `settings-email`
- [x] `settings-phone`
- [x] `people`
- [ ] `recover-password`
- [x] `recover-password`
- [ ] `settings-password`
- [ ] `tracker`
- [ ] `team-building`
Expand Down Expand Up @@ -103,6 +103,9 @@ Planned change:
- submit paper key
- submit password
- submit reset password
- Use scoped `flow-handles` registrations that return disposers and keep those disposers local to the flow owner.
- Clear those named handlers when each step or the listener finishes; do not rely only on an inactive guard, and use generation-safe cleanup so an older flow cannot wipe handlers from a newer restart.
- Dispose keyed handlers too if the owning route or listener exits before the token is consumed.
- Move `resetEmailSent` into route params or local screen state.
- Move orchestration into a recover-password feature module, not a store.
- Update screens to read explicit params/handlers rather than selecting `dispatch.dynamic.*`.
Expand Down
19 changes: 8 additions & 11 deletions shared/login/recover-password/device-selector.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,23 @@
import SelectOtherDevice from '@/provision/select-other-device'
import type {Device} from '@/stores/provision'
import {useState as useRecoverState} from '@/stores/recover-password'
import {
cancelRecoverPassword,
submitRecoverPasswordDeviceSelect,
submitRecoverPasswordNoDevice,
} from './flow'

type Props = {route: {params: {devices: ReadonlyArray<Device>}}}

const RecoverPasswordDeviceSelector = ({route}: Props) => {
const {devices} = route.params
const submitDeviceSelect = useRecoverState(s => s.dispatch.dynamic.submitDeviceSelect)
const submitNoDevice = useRecoverState(s => s.dispatch.dynamic.submitNoDevice)
const cancel = useRecoverState(s => s.dispatch.dynamic.cancel)
const onBack = () => {
cancel?.()
cancelRecoverPassword()
}
const onResetAccount = () => {
submitNoDevice?.()
submitRecoverPasswordNoDevice()
}
const onSelect = (name: string) => {
if (submitDeviceSelect) {
submitDeviceSelect(devices.find(device => device.name === name)?.id)
} else {
console.log('Missing device select?')
}
submitRecoverPasswordDeviceSelect(devices.find(device => device.name === name)?.id)
}
const props = {
devices,
Expand Down
3 changes: 1 addition & 2 deletions shared/login/recover-password/explain-device.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@ import * as Kb from '@/common-adapters'
import * as T from '@/constants/types'
import type {ButtonType} from '@/common-adapters/button'
import {SignupScreen} from '@/signup/common'
import {useState as useRecoverState} from '@/stores/recover-password'
import {startRecoverPassword} from './flow'

type Props = {route: {params: {deviceName: string; deviceType: T.RPCGen.DeviceType; username: string}}}

const ConnectedExplainDevice = ({route}: Props) => {
const {deviceName, deviceType, username} = route.params
const startRecoverPassword = useRecoverState(s => s.dispatch.startRecoverPassword)
const onBack = () => {
startRecoverPassword({
replaceRoute: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,31 @@ jest.mock('@/constants/router', () => {
const actual = jest.requireActual('@/constants/router')
return {
...actual,
clearModals: jest.fn(),
navigateAppend: jest.fn(),
navigateUp: jest.fn(),
}
})

import {useState as useRecoverPasswordState} from '../recover-password'

const {navigateAppend: mockNavigateAppend, navigateUp: mockNavigateUp} = require('@/constants/router') as {
import {
startRecoverPassword,
submitRecoverPasswordDeviceSelect,
submitRecoverPasswordReset,
} from './flow'

const {
clearModals: mockClearModals,
navigateAppend: mockNavigateAppend,
navigateUp: mockNavigateUp,
} = require('@/constants/router') as {
clearModals: jest.Mock
navigateAppend: jest.Mock
navigateUp: jest.Mock
}

afterEach(() => {
jest.restoreAllMocks()
mockClearModals.mockReset()
mockNavigateAppend.mockReset()
mockNavigateUp.mockReset()
resetAllStores()
Expand Down Expand Up @@ -56,12 +67,9 @@ test('startRecoverPassword exposes device selection handlers', async () => {
})

try {
useRecoverPasswordState.getState().dispatch.startRecoverPassword({username: 'alice'})
startRecoverPassword({username: 'alice'})
await flush()

const state = useRecoverPasswordState.getState()
expect(state.dispatch.dynamic.submitDeviceSelect).toBeDefined()
expect(state.dispatch.dynamic.cancel).toBeDefined()
expect(mockNavigateAppend).toHaveBeenCalledWith(
{
name: 'recoverPasswordDeviceSelector',
Expand All @@ -78,28 +86,30 @@ test('startRecoverPassword exposes device selection handlers', async () => {
false
)

state.dispatch.dynamic.submitDeviceSelect?.(T.Devices.stringToDeviceID('device-1'))
submitRecoverPasswordDeviceSelect(T.Devices.stringToDeviceID('device-1'))
submitRecoverPasswordDeviceSelect(T.Devices.stringToDeviceID('device-1'))

expect(chooserResponse?.result).toHaveBeenCalledTimes(1)
expect(chooserResponse?.result).toHaveBeenCalledWith(T.Devices.stringToDeviceID('device-1'))
expect(useRecoverPasswordState.getState().dispatch.dynamic.submitDeviceSelect).toBeUndefined()
expect(useRecoverPasswordState.getState().dispatch.dynamic.cancel).toBeUndefined()
} finally {
finishListener()
await flush()
}
})

test('resetState clears recover-password state after it has been populated', async () => {
test('resetAllStores clears pending recover-password handlers', async () => {
let chooserResponse: {error: jest.Mock; result: jest.Mock} | undefined
let finishListener = () => {}

jest.spyOn(T.RPCGen, 'loginRecoverPassphraseRpcListener').mockImplementation(async listener => {
chooserResponse = {error: jest.fn(), result: jest.fn()}
const chooseDevice = listener.customResponseIncomingCallMap?.['keybase.1.loginUi.chooseDeviceToRecoverWith']
if (!chooseDevice) {
throw new Error('chooseDeviceToRecoverWith handler missing')
}
chooseDevice(
{devices: [makeRpcDevice('tablet', 'device-2', 'desktop')]} as any,
{error: jest.fn(), result: jest.fn()} as any
chooserResponse as any
)
await new Promise<void>(resolve => {
finishListener = resolve
Expand All @@ -108,14 +118,56 @@ test('resetState clears recover-password state after it has been populated', asy
})

try {
useRecoverPasswordState.getState().dispatch.startRecoverPassword({username: 'alice'})
startRecoverPassword({username: 'alice'})
await flush()

resetAllStores()
submitRecoverPasswordDeviceSelect(T.Devices.stringToDeviceID('device-2'))

expect(chooserResponse?.result).not.toHaveBeenCalled()
} finally {
finishListener()
await flush()
expect(useRecoverPasswordState.getState().dispatch.dynamic.submitDeviceSelect).toBeDefined()
}
})

test('reset-password prompt resolves callback and local banner handler', async () => {
let promptResponse: {result: jest.Mock} | undefined
let finishListener = () => {}
const onResetEmailSent = jest.fn()

jest.spyOn(T.RPCGen, 'loginRecoverPassphraseRpcListener').mockImplementation(async listener => {
promptResponse = {result: jest.fn()}
const promptReset = listener.customResponseIncomingCallMap?.['keybase.1.loginUi.promptResetAccount']
if (!promptReset) {
throw new Error('promptResetAccount handler missing')
}
promptReset(
{prompt: {t: T.RPCGen.ResetPromptType.enterResetPw}} as any,
promptResponse as any
)
await new Promise<void>(resolve => {
finishListener = resolve
})
return undefined as any
})

try {
startRecoverPassword({onResetEmailSent, username: 'alice'})
await flush()

expect(mockNavigateAppend).toHaveBeenCalledWith({
name: 'recoverPasswordPromptResetPassword',
params: {username: 'alice'},
})

useRecoverPasswordState.getState().dispatch.resetState()
submitRecoverPasswordReset(T.RPCGen.ResetPromptResponse.confirmReset)
submitRecoverPasswordReset(T.RPCGen.ResetPromptResponse.confirmReset)

expect(useRecoverPasswordState.getState().resetEmailSent).toBe(false)
expect(useRecoverPasswordState.getState().dispatch.dynamic.submitDeviceSelect).toBeUndefined()
expect(promptResponse?.result).toHaveBeenCalledTimes(1)
expect(promptResponse?.result).toHaveBeenCalledWith(T.RPCGen.ResetPromptResponse.confirmReset)
expect(onResetEmailSent).toHaveBeenCalledTimes(1)
expect(mockNavigateUp).toHaveBeenCalledTimes(1)
} finally {
finishListener()
await flush()
Expand Down
Loading