From d5d471a9d024593e77963dd6a261461bef837334 Mon Sep 17 00:00:00 2001 From: Kathy Luo Date: Thu, 28 Nov 2024 14:52:30 +0100 Subject: [PATCH 1/3] fix: phone verification ends at CYA screen always --- src/navigator/types.tsx | 1 + .../VerificationCodeInputScreen.test.tsx | 1 + src/verify/VerificationCodeInputScreen.tsx | 5 +- src/verify/VerificationStartScreen.test.tsx | 74 ++++++++++--------- src/verify/VerificationStartScreen.tsx | 6 ++ 5 files changed, 49 insertions(+), 38 deletions(-) diff --git a/src/navigator/types.tsx b/src/navigator/types.tsx index 54632963234..e49625b51bb 100644 --- a/src/navigator/types.tsx +++ b/src/navigator/types.tsx @@ -308,6 +308,7 @@ export type StackParamList = { registrationStep?: { step: number; totalSteps: number } e164Number: string countryCallingCode: string + verificationCompletionScreen: keyof StackParamList } [Screens.OnboardingSuccessScreen]: undefined [Screens.WalletConnectRequest]: diff --git a/src/verify/VerificationCodeInputScreen.test.tsx b/src/verify/VerificationCodeInputScreen.test.tsx index 9a3eefbd1d2..03c5554bb05 100644 --- a/src/verify/VerificationCodeInputScreen.test.tsx +++ b/src/verify/VerificationCodeInputScreen.test.tsx @@ -45,6 +45,7 @@ const renderComponent = () => params={{ countryCode: '+31', e164Number, + verificationCompletionScreen: Screens.OnboardingSuccessScreen, }} /> diff --git a/src/verify/VerificationCodeInputScreen.tsx b/src/verify/VerificationCodeInputScreen.tsx index 6b6db5b93d9..0d3c561c749 100644 --- a/src/verify/VerificationCodeInputScreen.tsx +++ b/src/verify/VerificationCodeInputScreen.tsx @@ -4,8 +4,8 @@ import React, { useLayoutEffect, useState } from 'react' import { useTranslation } from 'react-i18next' import { StyleSheet } from 'react-native' import { SafeAreaView } from 'react-native-safe-area-context' -import { PhoneVerificationEvents } from 'src/analytics/Events' import AppAnalytics from 'src/analytics/AppAnalytics' +import { PhoneVerificationEvents } from 'src/analytics/Events' import BackButton from 'src/components/BackButton' import InfoBottomSheet from 'src/components/InfoBottomSheet' import { HeaderTitleWithSubtitle } from 'src/navigator/Headers' @@ -21,6 +21,7 @@ function VerificationCodeInputScreen({ route, navigation, }: NativeStackScreenProps) { + const nextScreen = route.params.verificationCompletionScreen const [showHelpDialog, setShowHelpDialog] = useState(false) const { t } = useTranslation() @@ -82,7 +83,7 @@ function VerificationCodeInputScreen({ setSmsCode={setSmsCode} onResendSms={onResendSms} onSuccess={() => { - navigate(Screens.OnboardingSuccessScreen) + navigate(nextScreen) }} containerStyle={{ marginTop: headerHeight }} /> diff --git a/src/verify/VerificationStartScreen.test.tsx b/src/verify/VerificationStartScreen.test.tsx index a038056586b..21b32d512b3 100644 --- a/src/verify/VerificationStartScreen.test.tsx +++ b/src/verify/VerificationStartScreen.test.tsx @@ -1,4 +1,4 @@ -import { act, fireEvent, render, waitFor, within } from '@testing-library/react-native' +import { fireEvent, render, waitFor, within } from '@testing-library/react-native' import React from 'react' import * as Keychain from 'react-native-keychain' import { Provider } from 'react-redux' @@ -60,10 +60,6 @@ describe('VerificationStartScreen', () => { }) const { getByText, getByTestId, queryByTestId, queryByText } = renderComponent() - await act(() => { - jest.advanceTimersByTime(5000) - }) - await waitFor(() => expect(getByText('phoneVerificationScreen.startButtonLabel')).toBeTruthy()) expect(getByText('phoneVerificationScreen.title')).toBeTruthy() expect(getByText('phoneVerificationScreen.description')).toBeTruthy() @@ -83,10 +79,6 @@ describe('VerificationStartScreen', () => { }) const { getByText, getByTestId } = renderComponent({ hasOnboarded: false }) - await act(() => { - jest.advanceTimersByTime(5000) - }) - await waitFor(() => expect(getByText('phoneVerificationScreen.startButtonLabel')).toBeTruthy()) expect(getByText('phoneVerificationScreen.title')).toBeTruthy() expect(getByText('phoneVerificationScreen.description')).toBeTruthy() @@ -108,10 +100,6 @@ describe('VerificationStartScreen', () => { }) const { getByText, getByTestId, queryByTestId } = renderComponent({ hasOnboarded: false }, true) - await act(() => { - jest.advanceTimersByTime(5000) - }) - await waitFor(() => expect(getByText('phoneVerificationScreen.startButtonLabel')).toBeTruthy()) expect(getByText('phoneVerificationScreen.title')).toBeTruthy() expect(getByText('phoneVerificationScreen.description')).toBeTruthy() @@ -126,13 +114,9 @@ describe('VerificationStartScreen', () => { mockedKeychain.getGenericPassword.mockResolvedValue(false) const { queryByText, getByTestId } = renderComponent() - await act(() => { - jest.advanceTimersByTime(5000) - // enter a valid phone number - fireEvent.changeText(getByTestId('PhoneNumberField'), '619123456') - }) + fireEvent.changeText(getByTestId('PhoneNumberField'), '619123456') - expect(getByTestId('PhoneVerificationContinue')).toBeDisabled() + await waitFor(() => expect(getByTestId('PhoneVerificationContinue')).toBeDisabled()) expect(getByTestId('Button/Loading')).toBeTruthy() expect(queryByText('phoneVerificationScreen.startButtonLabel')).toBeFalsy() }) @@ -140,9 +124,8 @@ describe('VerificationStartScreen', () => { it('shows the learn more dialog', async () => { const { getByTestId, getByText } = renderComponent() - await act(() => { - fireEvent.press(getByText('phoneVerificationScreen.learnMore.buttonLabel')) - }) + fireEvent.press(getByText('phoneVerificationScreen.learnMore.buttonLabel')) + await waitFor(() => expect(getByTestId('PhoneVerificationLearnMoreDialog')).toBeTruthy()) const LearnMoreDialog = getByTestId('PhoneVerificationLearnMoreDialog') expect( @@ -154,14 +137,14 @@ describe('VerificationStartScreen', () => { it('skip button works', async () => { const { getByText } = renderComponent({ hasOnboarded: false }) - await act(() => { - fireEvent.press(getByText('skip')) - }) + fireEvent.press(getByText('skip')) - expect(goToNextOnboardingScreen).toHaveBeenCalledWith({ - firstScreenInCurrentStep: Screens.VerificationStartScreen, - onboardingProps: mockOnboardingProps, - }) + await waitFor(() => + expect(goToNextOnboardingScreen).toHaveBeenCalledWith({ + firstScreenInCurrentStep: Screens.VerificationStartScreen, + onboardingProps: mockOnboardingProps, + }) + ) }) it('proceeds to the next verification step', async () => { @@ -173,10 +156,7 @@ describe('VerificationStartScreen', () => { }) const { getByText, getByTestId } = renderComponent({ hasOnboarded: false }) - await act(() => { - jest.advanceTimersByTime(5000) - fireEvent.changeText(getByTestId('PhoneNumberField'), '619123456') - }) + fireEvent.changeText(getByTestId('PhoneNumberField'), '619123456') await waitFor(() => expect(getByText('phoneVerificationScreen.startButtonLabel')).toBeTruthy()) fireEvent.press(getByText('phoneVerificationScreen.startButtonLabel')) @@ -186,6 +166,7 @@ describe('VerificationStartScreen', () => { countryCallingCode: '+31', e164Number: '+31619123456', registrationStep: { step: 3, totalSteps: 3 }, + verificationCompletionScreen: 'OnboardingSuccessScreen', }) }) @@ -198,10 +179,30 @@ describe('VerificationStartScreen', () => { }) const { getByText, getByTestId } = renderComponent({ hasOnboarded: false }, true) - await act(() => { - jest.advanceTimersByTime(5000) - fireEvent.changeText(getByTestId('PhoneNumberField'), '619123456') + fireEvent.changeText(getByTestId('PhoneNumberField'), '619123456') + + await waitFor(() => expect(getByText('phoneVerificationScreen.startButtonLabel')).toBeTruthy()) + fireEvent.press(getByText('phoneVerificationScreen.startButtonLabel')) + + expect(navigate).toHaveBeenCalledTimes(1) + expect(navigate).toHaveBeenCalledWith(Screens.VerificationCodeInputScreen, { + countryCallingCode: '+31', + e164Number: '+31619123456', + registrationStep: undefined, + verificationCompletionScreen: 'OnboardingSuccessScreen', }) + }) + + it('proceeds to the next verification step with the correct navigation props after onboarding', async () => { + mockedKeychain.getGenericPassword.mockResolvedValue({ + password: 'some signed message', + username: 'username', + service: 'service', + storage: 'storage', + }) + const { getByText, getByTestId } = renderComponent({ hasOnboarded: true }, false) + + fireEvent.changeText(getByTestId('PhoneNumberField'), '619123456') await waitFor(() => expect(getByText('phoneVerificationScreen.startButtonLabel')).toBeTruthy()) fireEvent.press(getByText('phoneVerificationScreen.startButtonLabel')) @@ -211,6 +212,7 @@ describe('VerificationStartScreen', () => { countryCallingCode: '+31', e164Number: '+31619123456', registrationStep: undefined, + verificationCompletionScreen: 'TabHome', }) }) }) diff --git a/src/verify/VerificationStartScreen.tsx b/src/verify/VerificationStartScreen.tsx index 7bbd1884ea9..33c3ee0d8c6 100644 --- a/src/verify/VerificationStartScreen.tsx +++ b/src/verify/VerificationStartScreen.tsx @@ -82,10 +82,16 @@ function VerificationStartScreen({ countryCallingCode: country?.countryCallingCode || '', }) + const routes = navigation.getState().routes + const prevRoute = routes[routes.length - 2] // -2 because -1 is the current route + navigate(Screens.VerificationCodeInputScreen, { registrationStep: showSteps ? { step, totalSteps } : undefined, e164Number: phoneNumberInfo.e164Number, countryCallingCode: country?.countryCallingCode || '', + verificationCompletionScreen: !route.params?.hasOnboarded + ? Screens.OnboardingSuccessScreen + : (prevRoute?.name ?? Screens.TabHome), }) } From 2e1bc5e9e44339ba32809592b266fe384bb45f91 Mon Sep 17 00:00:00 2001 From: Kathy Luo Date: Mon, 2 Dec 2024 10:52:14 +0100 Subject: [PATCH 2/3] chore: add comment --- src/verify/VerificationStartScreen.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/verify/VerificationStartScreen.tsx b/src/verify/VerificationStartScreen.tsx index 33c3ee0d8c6..3f494fb80e4 100644 --- a/src/verify/VerificationStartScreen.tsx +++ b/src/verify/VerificationStartScreen.tsx @@ -84,14 +84,18 @@ function VerificationStartScreen({ const routes = navigation.getState().routes const prevRoute = routes[routes.length - 2] // -2 because -1 is the current route + // Usually it makes sense to navigate the user back to where they launched + // the verification flow after they complete it, but during onboarding we + // want to navigate to the next step. + const verificationCompletionScreen = !route.params?.hasOnboarded + ? Screens.OnboardingSuccessScreen + : (prevRoute?.name ?? Screens.TabHome) navigate(Screens.VerificationCodeInputScreen, { registrationStep: showSteps ? { step, totalSteps } : undefined, e164Number: phoneNumberInfo.e164Number, countryCallingCode: country?.countryCallingCode || '', - verificationCompletionScreen: !route.params?.hasOnboarded - ? Screens.OnboardingSuccessScreen - : (prevRoute?.name ?? Screens.TabHome), + verificationCompletionScreen, }) } From e27539689ee5de43cdc40135ca93f0ac51b20669 Mon Sep 17 00:00:00 2001 From: Kathy Luo Date: Mon, 2 Dec 2024 10:53:42 +0100 Subject: [PATCH 3/3] fix: test title --- src/verify/VerificationStartScreen.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/verify/VerificationStartScreen.test.tsx b/src/verify/VerificationStartScreen.test.tsx index 21b32d512b3..ccbd30713de 100644 --- a/src/verify/VerificationStartScreen.test.tsx +++ b/src/verify/VerificationStartScreen.test.tsx @@ -147,7 +147,7 @@ describe('VerificationStartScreen', () => { ) }) - it('proceeds to the next verification step', async () => { + it('proceeds to the next verification step during onboarding', async () => { mockedKeychain.getGenericPassword.mockResolvedValue({ password: 'some signed message', username: 'username',