Skip to content

feat: migrate VerifyIdentity to ts #34338

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.

Sign up for GitHub

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

Merged
mountiny merged 2 commits into from
Jan 11, 2024
Merged

feat: migrate VerifyIdentity to ts #34338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
106a602
feat: migrate VerifyIdentity to ts
Swor71 Jan 11, 2024
fad1257
fix: removed reimbursementAccountDefaultProps
Swor71 Jan 11, 2024
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
3 changes: 2 additions & 1 deletion src/hooks/useSubStep/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type {ComponentType} from 'react';
import type {OnfidoData} from '@src/types/onyx/ReimbursementAccountDraft';

type SubStepProps = {
/** value indicating whether user is editing one of the sub steps */
Expand All @@ -22,7 +23,7 @@ type UseSubStep<T = void> = {
bodyContent: Array<ComponentType<SubStepProps & T>>;

/** called on last sub step */
onFinished: () => void;
onFinished: (data?: OnfidoData) => void;

/** index of initial sub step to display */
startFrom?: number;
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/ReimbursementAccount/navigation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import ROUTES from '@src/ROUTES';
* Navigate to a specific step in the VBA flow
*
* @param {String} stepID
* @param {Object} newAchData
* @param {Object} [newAchData]
*/
function goToWithdrawalAccountSetupStep(stepID, newAchData) {
Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {achData: {...newAchData, currentStep: stepID}});
Expand Down
40 changes: 20 additions & 20 deletions ...tAccount/VerifyIdentity/VerifyIdentity.js → ...Account/VerifyIdentity/VerifyIdentity.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,53 @@
import PropTypes from 'prop-types';
import React, {useCallback} from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import InteractiveStepSubHeader from '@components/InteractiveStepSubHeader';
import ScreenWrapper from '@components/ScreenWrapper';
import useLocalize from '@hooks/useLocalize';
import useSubStep from '@hooks/useSubStep';
import type {SubStepProps} from '@hooks/useSubStep/types';
import useThemeStyles from '@hooks/useThemeStyles';
import {reimbursementAccountPropTypes} from '@pages/ReimbursementAccount/reimbursementAccountPropTypes';
import * as ReimbursementAccountProps from '@pages/ReimbursementAccount/reimbursementAccountPropTypes';
import getDefaultValueForReimbursementAccountField from '@pages/ReimbursementAccount/utils/getDefaultValueForReimbursementAccountField';
import * as BankAccounts from '@userActions/BankAccounts';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {ReimbursementAccount} from '@src/types/onyx';
import type {OnfidoData} from '@src/types/onyx/ReimbursementAccountDraft';
import OnfidoInitialize from './substeps/OnfidoInitialize';

const propTypes = {
type VerifyIdentityOnyxProps = {
/** Reimbursement account from ONYX */
reimbursementAccount: reimbursementAccountPropTypes,
reimbursementAccount: OnyxEntry<ReimbursementAccount>;
};

type VerifyIdentityProps = VerifyIdentityOnyxProps & {
/** Goes to the previous step */
onBackButtonPress: PropTypes.func.isRequired,
onBackButtonPress: () => void;

/** Exits flow and goes back to the workspace initial page */
onCloseButtonPress: PropTypes.func.isRequired,
};

const defaultProps = {
reimbursementAccount: ReimbursementAccountProps.reimbursementAccountDefaultProps,
onCloseButtonPress: () => void;
};

const bodyContent = [OnfidoInitialize];
const bodyContent: Array<React.ComponentType<SubStepProps>> = [OnfidoInitialize];

function VerifyIdentity({reimbursementAccount, onBackButtonPress, onCloseButtonPress}) {
function VerifyIdentity({reimbursementAccount, onBackButtonPress, onCloseButtonPress}: VerifyIdentityProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();

const submit = useCallback(
(onfidoData) => {
BankAccounts.verifyIdentityForBankAccount(getDefaultValueForReimbursementAccountField(reimbursementAccount, 'bankAccountID', 0), onfidoData);
(onfidoData?: OnfidoData) => {
if (!onfidoData) {
return;
}
Comment on lines +40 to +42
Copy link
Copy Markdown
Contributor

@VickyStash VickyStash Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested before/after flow with this new condition? I just want to make sure it's okay that we skip 2 methods calls, even before it was probably called without onfido data.

Copy link
Copy Markdown
Contributor Author

@Swor71 Swor71 Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's going to be there, this is just to satisfy the TS change to onFinished, as this step is unlike the others within the flow and would require a refactor in my opinion (already discussed), however this is just the migration part. (Onfido itself needs to be migrated to provide proper typings)

Copy link
Copy Markdown
Contributor

@VickyStash VickyStash Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still looks strange. I've checked the code, and it looks like onFinished is never called with onfido data (or with any data). So your submit function will never execute methods below. Am I wrong?

Copy link
Copy Markdown
Contributor Author

@Swor71 Swor71 Jan 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I understand correctly, it will be called with arguments https://github.com/Expensify/App/blob/vit-tieredBankAccountFlow/src/components/Onfido/BaseOnfidoWeb.js#L82 but implicitly. onFinished is just an alias for submit, which in turn is the onSuccess there


BankAccounts.verifyIdentityForBankAccount(reimbursementAccount?.achData?.bankAccountID ?? 0, onfidoData);
BankAccounts.updateReimbursementAccountDraft({isOnfidoSetupComplete: true});
},
[reimbursementAccount],
);

const {componentToRender: SubStep, isEditing, nextScreen, moveTo} = useSubStep({bodyContent, startFrom: 0, onFinished: submit});
const {componentToRender: SubStep, isEditing, moveTo, nextScreen} = useSubStep({bodyContent, startFrom: 0, onFinished: submit});

return (
<ScreenWrapper testID={VerifyIdentity.displayName}>
Expand All @@ -71,11 +73,9 @@ function VerifyIdentity({reimbursementAccount, onBackButtonPress, onCloseButtonP
);
}

VerifyIdentity.propTypes = propTypes;
VerifyIdentity.defaultProps = defaultProps;
VerifyIdentity.displayName = 'VerifyIdentity';

export default withOnyx({
export default withOnyx<VerifyIdentityProps, VerifyIdentityOnyxProps>({
reimbursementAccount: {
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
},
Expand Down
21 changes: 8 additions & 13 deletions ...rifyIdentity/substeps/OnfidoInitialize.js → ...ifyIdentity/substeps/OnfidoInitialize.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,28 @@
import PropTypes from 'prop-types';
import React from 'react';
import {ScrollView} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import FullPageOfflineBlockingView from '@components/BlockingViews/FullPageOfflineBlockingView';
// @ts-expect-error TODO: Remove this once Onfido (https://github.com/Expensify/App/issues/25136) is migrated to TypeScript.
import Onfido from '@components/Onfido';
import useLocalize from '@hooks/useLocalize';
import type {SubStepProps} from '@hooks/useSubStep/types';
import useThemeStyles from '@hooks/useThemeStyles';
import Growl from '@libs/Growl';
import subStepPropTypes from '@pages/ReimbursementAccount/subStepPropTypes';
import * as BankAccounts from '@userActions/BankAccounts';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';

const propTypes = {
type OnfidoInitializeOnyxProps = {
/** The token required to initialize the Onfido SDK */
onfidoToken: PropTypes.string,

...subStepPropTypes,
onfidoToken: OnyxEntry<string>;
};

const defaultProps = {
onfidoToken: null,
};
type OnfidoInitializeProps = SubStepProps & OnfidoInitializeOnyxProps;

const ONFIDO_ERROR_DISPLAY_DURATION = 10000;

function OnfidoInitialize({onfidoToken, onNext}) {
function OnfidoInitialize({onfidoToken, onNext}: OnfidoInitializeProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();

Expand Down Expand Up @@ -55,11 +52,9 @@ function OnfidoInitialize({onfidoToken, onNext}) {
);
}

OnfidoInitialize.propTypes = propTypes;
OnfidoInitialize.defaultProps = defaultProps;
OnfidoInitialize.displayName = 'OnfidoInitialize';

export default withOnyx({
export default withOnyx<OnfidoInitializeProps, OnfidoInitializeOnyxProps>({
onfidoToken: {
key: ONYXKEYS.ONFIDO_TOKEN,
},
Expand Down