-
Notifications
You must be signed in to change notification settings - Fork 4k
Copy codes (2fa) page - use navigation based verify account modal #69345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
56f5343
616c5a9
8652005
ba7dcce
b856c42
1d04264
10623ec
fc41dfa
c062db1
b1c5e74
6b40c3f
d6c3a99
c88ae40
00bbda4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,4 +70,27 @@ function getSearchParamFromUrl(currentUrl: string, param: string) { | |
| return currentUrl ? new URL(currentUrl).searchParams.get(param) : null; | ||
| } | ||
|
|
||
| export {getSearchParamFromUrl, hasSameExpensifyOrigin, getPathFromURL, appendParam, hasURL, addLeadingForwardSlash, extractUrlDomain}; | ||
| type UrlWithParams<TBase extends string> = `${TBase}${'' | `?${string}` | `&${string}`}`; | ||
| type UrlParams = {backTo?: string; forwardTo?: string} & Record<string, string | number | undefined>; | ||
| /** | ||
| * Generate a URL with properly encoded query parameters. | ||
| * | ||
| * @param baseUrl - The base URL. | ||
| * @param params - Object containing key-value pairs for query parameters. | ||
| * @returns A URL string with encoded query parameters. | ||
| */ | ||
| function getUrlWithParams<TBase extends string, TParams extends UrlParams>(baseUrl: TBase, params: TParams): UrlWithParams<TBase> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this method needed now? Feels like something we should already have a solution for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't say it's needed, but nice to have. Assumed that if we want to get rid of Currently there's plenty of manually stitched urls in IMO would be cleaner if we can just pass an object to util.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok I see and it does look cleaner, but in general, lets please try to do these changes that are not necessary for the core purpose of the PR in separate PRs to keep scopes of changes as small as possible |
||
| const [path, existingQuery] = baseUrl.split('?', 2); | ||
| const searchParams = new URLSearchParams(existingQuery || ''); | ||
|
|
||
| for (const [key, value] of Object.entries(params)) { | ||
| if (value) { | ||
| searchParams.set(key, String(value)); | ||
| } | ||
| } | ||
|
|
||
| const queryString = searchParams.toString(); | ||
| return (queryString ? `${path}?${queryString}` : path) as UrlWithParams<TBase>; | ||
| } | ||
|
|
||
| export {getSearchParamFromUrl, hasSameExpensifyOrigin, getPathFromURL, appendParam, hasURL, addLeadingForwardSlash, extractUrlDomain, getUrlWithParams}; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,34 @@ | ||
| import React from 'react'; | ||
| import React, {useCallback} from 'react'; | ||
| import {View} from 'react-native'; | ||
| import type {OnyxEntry} from 'react-native-onyx'; | ||
| import Button from '@components/Button'; | ||
| import Icon from '@components/Icon'; | ||
| import {Encryption} from '@components/Icon/Illustrations'; | ||
| import ScreenWrapper from '@components/ScreenWrapper'; | ||
| import Text from '@components/Text'; | ||
| import useLocalize from '@hooks/useLocalize'; | ||
| import useOnyx from '@hooks/useOnyx'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import Navigation from '@libs/Navigation/Navigation'; | ||
| import variables from '@styles/variables'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import ROUTES from '@src/ROUTES'; | ||
| import type Account from '@src/types/onyx/Account'; | ||
|
|
||
| const accountValidatedSelector = (account: OnyxEntry<Account>) => account?.validated; | ||
|
|
||
| function RequireTwoFactorAuthenticationPage() { | ||
| const styles = useThemeStyles(); | ||
| const {translate} = useLocalize(); | ||
| const [isUserValidated = false] = useOnyx(ONYXKEYS.ACCOUNT, {selector: accountValidatedSelector, canBeMissing: true}); | ||
|
|
||
| const handleOnPress = useCallback(() => { | ||
| if (isUserValidated) { | ||
| Navigation.navigate(ROUTES.SETTINGS_2FA_ROOT.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH)); | ||
| return; | ||
| } | ||
| Navigation.navigate(ROUTES.SETTINGS_2FA_VERIFY_ACCOUNT.getRoute({backTo: ROUTES.REQUIRE_TWO_FACTOR_AUTH, forwardTo: ROUTES.SETTINGS_2FA_ROOT.getRoute()})); | ||
| }, [isUserValidated]); | ||
|
Comment on lines
+25
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The const handleOnPress = useCallback(() => {
if (isUserValidated) {
Navigation.navigate(settingsTwoFARoute);
return;
}
Navigation.navigate(verifyAccountRoute);
}, [isUserValidated, settingsTwoFARoute, verifyAccountRoute]);Where the route variables are defined and memoized outside the callback.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO this is memoization taken one step too far...
Comment on lines
+25
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The const settingsTwoFARoute = useMemo(() => ROUTES.SETTINGS_2FA_ROOT.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), []);
const verifyAccountRoute = useMemo(() => ROUTES.SETTINGS_2FA_VERIFY_ACCOUNT.getRoute({backTo: ROUTES.REQUIRE_TWO_FACTOR_AUTH, forwardTo: ROUTES.SETTINGS_2FA_ROOT.getRoute()}), []);
const handleOnPress = useCallback(() => {
if (isUserValidated) {
Navigation.navigate(settingsTwoFARoute);
return;
}
Navigation.navigate(verifyAccountRoute);
}, [isUserValidated, settingsTwoFARoute, verifyAccountRoute]); |
||
|
|
||
| return ( | ||
| <ScreenWrapper testID={RequireTwoFactorAuthenticationPage.displayName}> | ||
|
|
@@ -34,7 +49,7 @@ function RequireTwoFactorAuthenticationPage() { | |
| large | ||
| success | ||
| pressOnEnter | ||
| onPress={() => Navigation.navigate(ROUTES.SETTINGS_2FA_ROOT.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH))} | ||
| onPress={handleOnPress} | ||
| text={translate('twoFactorAuth.enableTwoFactorAuth')} | ||
| /> | ||
| </View> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import React, {useEffect, useMemo, useState} from 'react'; | ||
| import React, {useEffect, useState} from 'react'; | ||
| import {ActivityIndicator, View} from 'react-native'; | ||
| import Button from '@components/Button'; | ||
| import FixedFooter from '@components/FixedFooter'; | ||
|
|
@@ -9,51 +9,42 @@ import PressableWithDelayToggle from '@components/Pressable/PressableWithDelayTo | |
| import ScrollView from '@components/ScrollView'; | ||
| import Section from '@components/Section'; | ||
| import Text from '@components/Text'; | ||
| import ValidateCodeActionModal from '@components/ValidateCodeActionModal'; | ||
| import useBeforeRemove from '@hooks/useBeforeRemove'; | ||
| import useLocalize from '@hooks/useLocalize'; | ||
| import useOnyx from '@hooks/useOnyx'; | ||
| import useResponsiveLayout from '@hooks/useResponsiveLayout'; | ||
| import useTheme from '@hooks/useTheme'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import {READ_COMMANDS} from '@libs/API/types'; | ||
| import Clipboard from '@libs/Clipboard'; | ||
| import {getEarliestErrorField, getLatestErrorField} from '@libs/ErrorUtils'; | ||
| import localFileDownload from '@libs/localFileDownload'; | ||
| import Navigation from '@libs/Navigation/Navigation'; | ||
| import {toggleTwoFactorAuth} from '@userActions/Session'; | ||
| import {quitAndNavigateBack, setCodesAreCopied} from '@userActions/TwoFactorAuthActions'; | ||
| import {clearContactMethodErrors, requestValidateCodeAction, validateSecondaryLogin} from '@userActions/User'; | ||
| import CONST from '@src/CONST'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import ROUTES from '@src/ROUTES'; | ||
| import {isEmptyObject} from '@src/types/utils/EmptyObject'; | ||
| import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue'; | ||
| import type {TwoFactorAuthPageProps} from './TwoFactorAuthPage'; | ||
| import TwoFactorAuthWrapper from './TwoFactorAuthWrapper'; | ||
|
|
||
| function CopyCodesPage({route}: TwoFactorAuthPageProps) { | ||
| const theme = useTheme(); | ||
| const styles = useThemeStyles(); | ||
| const {translate, formatPhoneNumber} = useLocalize(); | ||
| const {translate} = useLocalize(); | ||
| // We need to use isSmallScreenWidth instead of shouldUseNarrowLayout to use correct style | ||
| // eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth | ||
| const {isExtraSmallScreenWidth, isSmallScreenWidth} = useResponsiveLayout(); | ||
| const [error, setError] = useState(''); | ||
|
|
||
| const [account, accountMetadata] = useOnyx(ONYXKEYS.ACCOUNT, {canBeMissing: true}); | ||
| const [loginList] = useOnyx(ONYXKEYS.LOGIN_LIST, {canBeMissing: true}); | ||
|
|
||
| const isUserValidated = account?.validated ?? false; | ||
| const contactMethod = account?.primaryLogin ?? ''; | ||
|
|
||
| const loginData = useMemo(() => loginList?.[contactMethod], [loginList, contactMethod]); | ||
| const validateLoginError = getEarliestErrorField(loginData, 'validateLogin'); | ||
|
|
||
| const [isValidateModalVisible, setIsValidateModalVisible] = useState(!isUserValidated); | ||
|
|
||
| useEffect(() => { | ||
| setIsValidateModalVisible(!isUserValidated); | ||
| if (!isUserValidated) { | ||
| Navigation.navigate(ROUTES.SETTINGS_2FA_VERIFY_ACCOUNT.getRoute()); | ||
| return; | ||
| } | ||
| // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||
| if (isLoadingOnyxValue(accountMetadata) || account?.requiresTwoFactorAuth || account?.recoveryCodes || !isUserValidated) { | ||
| return; | ||
|
Comment on lines
49
to
50
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a redundant condition check that could cause issues. The code first checks useEffect(() => {
if (!isUserValidated) {
Navigation.navigate(ROUTES.SETTINGS_2FA_VERIFY_ACCOUNT.getRoute());
return;
}
if (isLoadingOnyxValue(accountMetadata) || account?.requiresTwoFactorAuth || account?.recoveryCodes) {
return;
}
toggleTwoFactorAuth(true, READ_COMMANDS.GET_RECOVERY_CODES);
}, [isUserValidated, accountMetadata.status]); |
||
|
|
@@ -62,8 +53,6 @@ function CopyCodesPage({route}: TwoFactorAuthPageProps) { | |
| // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps -- We want to run this when component mounts | ||
| }, [isUserValidated, accountMetadata.status]); | ||
|
|
||
| useBeforeRemove(() => setIsValidateModalVisible(false)); | ||
|
|
||
| return ( | ||
| <TwoFactorAuthWrapper | ||
| title={translate('twoFactorAuth.headerTitle')} | ||
|
|
@@ -167,22 +156,6 @@ function CopyCodesPage({route}: TwoFactorAuthPageProps) { | |
| /> | ||
| </FixedFooter> | ||
| </ScrollView> | ||
| <ValidateCodeActionModal | ||
| title={translate('contacts.validateAccount')} | ||
| descriptionPrimary={translate('contacts.featureRequiresValidate')} | ||
| descriptionSecondary={translate('contacts.enterMagicCode', {contactMethod})} | ||
| isVisible={isValidateModalVisible} | ||
| validateCodeActionErrorField="validateLogin" | ||
| validatePendingAction={loginData?.pendingFields?.validateCodeSent} | ||
| sendValidateCode={() => requestValidateCodeAction()} | ||
| handleSubmitForm={(validateCode) => validateSecondaryLogin(loginList, contactMethod, validateCode, formatPhoneNumber, true)} | ||
| validateError={!isEmptyObject(validateLoginError) ? validateLoginError : getLatestErrorField(loginData, 'validateCodeSent')} | ||
| clearError={() => clearContactMethodErrors(contactMethod, !isEmptyObject(validateLoginError) ? 'validateLogin' : 'validateCodeSent')} | ||
| onClose={() => { | ||
| setIsValidateModalVisible(false); | ||
| quitAndNavigateBack(); | ||
| }} | ||
| /> | ||
| </TwoFactorAuthWrapper> | ||
| ); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import React from 'react'; | ||
| import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; | ||
| import type {TwoFactorAuthNavigatorParamList} from '@libs/Navigation/types'; | ||
| import VerifyAccountPageBase from '@pages/settings/VerifyAccountPageBase'; | ||
| import ROUTES from '@src/ROUTES'; | ||
| import type SCREENS from '@src/SCREENS'; | ||
|
|
||
| type VerifyAccountPageProps = PlatformStackScreenProps<TwoFactorAuthNavigatorParamList, typeof SCREENS.TWO_FACTOR_AUTH.VERIFY_ACCOUNT>; | ||
|
|
||
| function VerifyAccountPage({route}: VerifyAccountPageProps) { | ||
| return ( | ||
| <VerifyAccountPageBase | ||
| navigateBackTo={route?.params?.backTo ?? ROUTES.SETTINGS_SECURITY} | ||
| navigateForwardTo={route?.params?.forwardTo ?? ROUTES.SETTINGS_2FA_ROOT.getRoute()} | ||
| /> | ||
| ); | ||
|
mountiny marked this conversation as resolved.
|
||
| } | ||
|
jmusial marked this conversation as resolved.
|
||
| VerifyAccountPage.displayName = 'VerifyAccountPage'; | ||
| export default VerifyAccountPage; | ||
Uh oh!
There was an error while loading. Please reload this page.