Skip to content
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

Jump to bottom

RequestorStep.js: class to functional component refactor #27455

Merged
merged 22 commits into from
Sep 27, 2023
Merged

RequestorStep.js: class to functional component refactor #27455

Changes from 4 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f21cc76
initial rewrite of RequestorStep
kacper-mikolajczak Sep 14, 2023
062cfac
add forwardRef compatibility
kacper-mikolajczak Sep 14, 2023
f4e9c7d
extraction of constants and memoization
kacper-mikolajczak Sep 14, 2023
c2c3f35
extract styles
kacper-mikolajczak Sep 14, 2023
6dfd524
use INPUT_KEYS for form default values object
kacper-mikolajczak Sep 15, 2023
8e1d212
use lodashGet defaultValue arg
kacper-mikolajczak Sep 15, 2023
16169e8
remove redundant template literals
kacper-mikolajczak Sep 15, 2023
0110dfe
add displayName
kacper-mikolajczak Sep 15, 2023
a20cb71
swap withLocalize for useLocalize
kacper-mikolajczak Sep 15, 2023
43e915e
add braces to early return
kacper-mikolajczak Sep 15, 2023
7f5655f
Revert "extract styles"
kacper-mikolajczak Sep 15, 2023
22ae3d3
rename LabelComponent to renderLabelComponent
kacper-mikolajczak Sep 15, 2023
e32147b
propTypes omit improvement
kacper-mikolajczak Sep 15, 2023
0e0c07c
extract urls
kacper-mikolajczak Sep 15, 2023
c75ad40
remove renderLabelComponent memoization
kacper-mikolajczak Sep 15, 2023
4ede3a6
Merge branch 'main'
kacper-mikolajczak Sep 19, 2023
6e5fa8a
fix lodash issues
kacper-mikolajczak Sep 19, 2023
375c85f
remove unused ref
kacper-mikolajczak Sep 19, 2023
9de749d
fix PropTypes
kacper-mikolajczak Sep 22, 2023
67116fc
Merge branch 'main' into refactor/16247/requestor-step-class-to-function
kacper-mikolajczak Sep 22, 2023
f127451
add PropTypes to inner forward ref component
kacper-mikolajczak Sep 22, 2023
08ba6ac
fix displayName issues and remove inner component
kacper-mikolajczak Sep 22, 2023
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
315 changes: 165 additions & 150 deletions src/pages/ReimbursementAccount/RequestorStep.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, {useCallback, useMemo} from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import lodashGet from 'lodash/get';
Expand All @@ -25,170 +25,185 @@ const propTypes = {
shouldShowOnfido: PropTypes.bool.isRequired,
};

class RequestorStep extends React.Component {
constructor(props) {
super(props);

this.validate = this.validate.bind(this);
this.submit = this.submit.bind(this);
}

/**
* @param {Object} values
* @returns {Object}
*/
validate(values) {
const requiredFields = ['firstName', 'lastName', 'dob', 'ssnLast4', 'requestorAddressStreet', 'requestorAddressCity', 'requestorAddressState', 'requestorAddressZipCode'];
const errors = ValidationUtils.getFieldRequiredErrors(values, requiredFields);

if (values.dob) {
if (!ValidationUtils.isValidPastDate(values.dob) || !ValidationUtils.meetsMaximumAgeRequirement(values.dob)) {
errors.dob = 'bankAccount.error.dob';
} else if (!ValidationUtils.meetsMinimumAgeRequirement(values.dob)) {
errors.dob = 'bankAccount.error.age';
}
}

if (values.ssnLast4 && !ValidationUtils.isValidSSNLastFour(values.ssnLast4)) {
errors.ssnLast4 = 'bankAccount.error.ssnLast4';
}
const REQUIRED_FIELDS = ['firstName', 'lastName', 'dob', 'ssnLast4', 'requestorAddressStreet', 'requestorAddressCity', 'requestorAddressState', 'requestorAddressZipCode'];
const INPUT_KEYS = {
firstName: 'firstName',
lastName: 'lastName',
dob: 'dob',
ssnLast4: 'ssnLast4',
street: 'requestorAddressStreet',
city: 'requestorAddressCity',
state: 'requestorAddressState',
zipCode: 'requestorAddressZipCode',
};
const STEP_COUNTER = {step: 3, total: 5};

const STYLES = {
Label: [styles.flex1, styles.pr1],
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
Form: [styles.mh5, styles.flexGrow1],
LearnMoreContainer: [styles.mb5, styles.mt1, styles.dFlex, styles.flexRow],
LearnMoreLink: [styles.textMicro],
LearnMoreSeparator: [styles.textMicroSupporting],
DataSafeLink: [styles.textMicro, styles.textLink],
ControllingOfficerCheckbox: [styles.mt4],
ConditionsContainer: [styles.mt3, styles.textMicroSupporting],
ConditionsLink: [styles.textMicro],
};

if (values.requestorAddressStreet && !ValidationUtils.isValidAddress(values.requestorAddressStreet)) {
errors.requestorAddressStreet = 'bankAccount.error.addressStreet';
}
const validate = (values) => {
const errors = ValidationUtils.getFieldRequiredErrors(values, REQUIRED_FIELDS);

if (values.requestorAddressZipCode && !ValidationUtils.isValidZipCode(values.requestorAddressZipCode)) {
errors.requestorAddressZipCode = 'bankAccount.error.zipCode';
if (values.dob) {
if (!ValidationUtils.isValidPastDate(values.dob) || !ValidationUtils.meetsMaximumAgeRequirement(values.dob)) {
errors.dob = 'bankAccount.error.dob';
} else if (!ValidationUtils.meetsMinimumAgeRequirement(values.dob)) {
errors.dob = 'bankAccount.error.age';
}
}

if (!ValidationUtils.isRequiredFulfilled(values.isControllingOfficer)) {
errors.isControllingOfficer = 'requestorStep.isControllingOfficerError';
}
if (values.ssnLast4 && !ValidationUtils.isValidSSNLastFour(values.ssnLast4)) {
errors.ssnLast4 = 'bankAccount.error.ssnLast4';
}

return errors;
if (values.requestorAddressStreet && !ValidationUtils.isValidAddress(values.requestorAddressStreet)) {
errors.requestorAddressStreet = 'bankAccount.error.addressStreet';
}

submit(values) {
const payload = {
bankAccountID: lodashGet(this.props.reimbursementAccount, 'achData.bankAccountID') || 0,
...values,
};
if (values.requestorAddressZipCode && !ValidationUtils.isValidZipCode(values.requestorAddressZipCode)) {
errors.requestorAddressZipCode = 'bankAccount.error.zipCode';
}

BankAccounts.updatePersonalInformationForBankAccount(payload);
if (!ValidationUtils.isRequiredFulfilled(values.isControllingOfficer)) {
errors.isControllingOfficer = 'requestorStep.isControllingOfficerError';
}

render() {
if (this.props.shouldShowOnfido) {
return (
<RequestorOnfidoStep
reimbursementAccount={this.props.reimbursementAccount}
reimbursementAccountDraft={this.props.reimbursementAccountDraft}
onBackButtonPress={this.props.onBackButtonPress}
getDefaultStateForField={this.props.getDefaultStateForField}
/>
);
}
return errors;
};

function InnerRequestorStep({reimbursementAccount, shouldShowOnfido, reimbursementAccountDraft, onBackButtonPress, getDefaultStateForField, translate}, ref) {
const defaultValues = useMemo(
() => ({
firstName: getDefaultStateForField('firstName'),
lastName: getDefaultStateForField('lastName'),
street: getDefaultStateForField('requestorAddressStreet'),
city: getDefaultStateForField('requestorAddressCity'),
state: getDefaultStateForField('requestorAddressState'),
zipCode: getDefaultStateForField('requestorAddressZipCode'),
dob: getDefaultStateForField('dob'),
ssnLast4: getDefaultStateForField('ssnLast4'),
}),
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
[getDefaultStateForField],
);

const submit = useCallback(
(values) => {
const payload = {
bankAccountID: lodashGet(reimbursementAccount, 'achData.bankAccountID') || 0,
...values,
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
};

BankAccounts.updatePersonalInformationForBankAccount(payload);
},
[reimbursementAccount],
);

const LabelComponent = useCallback(
() => (
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
<View style={STYLES.Label}>
<Text>{translate('requestorStep.isControllingOfficer')}</Text>
</View>
),
[translate],
);

if (shouldShowOnfido)
return (
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
<ScreenWrapper includeSafeAreaPaddingBottom={false}>
<HeaderWithBackButton
title={this.props.translate('requestorStep.headerTitle')}
stepCounter={{step: 3, total: 5}}
shouldShowGetAssistanceButton
guidesCallTaskID={CONST.GUIDES_CALL_TASK_IDS.WORKSPACE_BANK_ACCOUNT}
onBackButtonPress={this.props.onBackButtonPress}
/>
<Form
formID={ONYXKEYS.REIMBURSEMENT_ACCOUNT}
submitButtonText={this.props.translate('common.saveAndContinue')}
validate={this.validate}
scrollContextEnabled
onSubmit={this.submit}
style={[styles.mh5, styles.flexGrow1]}
>
<Text>{this.props.translate('requestorStep.subtitle')}</Text>
<View style={[styles.mb5, styles.mt1, styles.dFlex, styles.flexRow]}>
<TextLink
style={[styles.textMicro]}
// eslint-disable-next-line max-len
href="https://community.expensify.com/discussion/6983/faq-why-do-i-need-to-provide-personal-documentation-when-setting-up-updating-my-bank-account"
>
{`${this.props.translate('requestorStep.learnMore')}`}
</TextLink>
<Text style={[styles.textMicroSupporting]}>{' | '}</Text>
<TextLink
style={[styles.textMicro, styles.textLink]}
// eslint-disable-next-line max-len
href="https://community.expensify.com/discussion/5677/deep-dive-security-how-expensify-protects-your-information"
>
{`${this.props.translate('requestorStep.isMyDataSafe')}`}
</TextLink>
</View>
<IdentityForm
translate={this.props.translate}
defaultValues={{
firstName: this.props.getDefaultStateForField('firstName'),
lastName: this.props.getDefaultStateForField('lastName'),
street: this.props.getDefaultStateForField('requestorAddressStreet'),
city: this.props.getDefaultStateForField('requestorAddressCity'),
state: this.props.getDefaultStateForField('requestorAddressState'),
zipCode: this.props.getDefaultStateForField('requestorAddressZipCode'),
dob: this.props.getDefaultStateForField('dob'),
ssnLast4: this.props.getDefaultStateForField('ssnLast4'),
}}
inputKeys={{
firstName: 'firstName',
lastName: 'lastName',
dob: 'dob',
ssnLast4: 'ssnLast4',
street: 'requestorAddressStreet',
city: 'requestorAddressCity',
state: 'requestorAddressState',
zipCode: 'requestorAddressZipCode',
}}
shouldSaveDraft
/>
<CheckboxWithLabel
accessibilityLabel={this.props.translate('requestorStep.isControllingOfficer')}
inputID="isControllingOfficer"
defaultValue={this.props.getDefaultStateForField('isControllingOfficer', false)}
LabelComponent={() => (
<View style={[styles.flex1, styles.pr1]}>
<Text>{this.props.translate('requestorStep.isControllingOfficer')}</Text>
</View>
)}
style={[styles.mt4]}
shouldSaveDraft
/>
<Text style={[styles.mt3, styles.textMicroSupporting]}>
{this.props.translate('requestorStep.onFidoConditions')}
<TextLink
href="https://onfido.com/facial-scan-policy-and-release/"
style={[styles.textMicro]}
>
{this.props.translate('onfidoStep.facialScan')}
</TextLink>
{', '}
<TextLink
href="https://onfido.com/privacy/"
style={[styles.textMicro]}
>
{this.props.translate('common.privacy')}
</TextLink>
{` ${this.props.translate('common.and')} `}
<TextLink
href="https://onfido.com/terms-of-service/"
style={[styles.textMicro]}
>
{this.props.translate('common.termsOfService')}
</TextLink>
</Text>
</Form>
</ScreenWrapper>
<RequestorOnfidoStep
reimbursementAccount={reimbursementAccount}
reimbursementAccountDraft={reimbursementAccountDraft}
onBackButtonPress={onBackButtonPress}
getDefaultStateForField={getDefaultStateForField}
/>
);
}

return (
<ScreenWrapper includeSafeAreaPaddingBottom={false}>
<HeaderWithBackButton
title={translate('requestorStep.headerTitle')}
stepCounter={STEP_COUNTER}
guidesCallTaskID={CONST.GUIDES_CALL_TASK_IDS.WORKSPACE_BANK_ACCOUNT}
onBackButtonPress={onBackButtonPress}
shouldShowGetAssistanceButton
/>
<Form
formID={ONYXKEYS.REIMBURSEMENT_ACCOUNT}
submitButtonText={translate('common.saveAndContinue')}
validate={validate}
onSubmit={submit}
style={STYLES.Form}
scrollContextEnabled
>
<Text>{translate('requestorStep.subtitle')}</Text>
<View style={STYLES.LearnMoreContainer}>
<TextLink
style={STYLES.LearnMoreLink}
// eslint-disable-next-line max-len
href="https://community.expensify.com/discussion/6983/faq-why-do-i-need-to-provide-personal-documentation-when-setting-up-updating-my-bank-account"
>
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
{`${translate('requestorStep.learnMore')}`}
</TextLink>
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
<Text style={STYLES.LearnMoreSeparator}>{' | '}</Text>
<TextLink
style={STYLES.DataSafeLink}
href="https://community.expensify.com/discussion/5677/deep-dive-security-how-expensify-protects-your-information"
>
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
{`${translate('requestorStep.isMyDataSafe')}`}
</TextLink>
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
</View>
<IdentityForm
translate={translate}
defaultValues={defaultValues}
inputKeys={INPUT_KEYS}
shouldSaveDraft
/>
<CheckboxWithLabel
accessibilityLabel={translate('requestorStep.isControllingOfficer')}
inputID="isControllingOfficer"
defaultValue={getDefaultStateForField('isControllingOfficer', false)}
LabelComponent={LabelComponent}
style={STYLES.ControllingOfficerCheckbox}
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
shouldSaveDraft
/>
<Text style={STYLES.ConditionsContainer}>
{translate('requestorStep.onFidoConditions')}
<TextLink
href="https://onfido.com/facial-scan-policy-and-release/"
style={STYLES.ConditionsLink}
>
{translate('onfidoStep.facialScan')}
</TextLink>
{', '}
<TextLink
href="https://onfido.com/privacy/"
style={STYLES.ConditionsLink}
>
{translate('common.privacy')}
</TextLink>
{` ${translate('common.and')} `}
<TextLink
href="https://onfido.com/terms-of-service/"
style={STYLES.ConditionsLink}
>
{translate('common.termsOfService')}
</TextLink>
</Text>
</Form>
</ScreenWrapper>
);
}

const RequestorStep = React.forwardRef(InnerRequestorStep);

kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@gedu gedu Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is spreading to other components @mountiny, what format should we stick for the forwardRef? In our codebase we use:

export default React.forwardRef((props, ref) => (
    <RequestorStep
        // eslint-disable-next-line react/jsx-props-no-spreading
        {...props}
        forwardedRef={ref}
    />
));

sometimes forwardedRef is innerRef
But I wanna bring into a discussion this format proposed by @kacper-mikolajczak

Copy link
Contributor

Choose a reason for hiding this comment

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

I think these are best in Slack where more people can weigh in

Copy link
Contributor

Choose a reason for hiding this comment

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

Thread created to discuss this

RequestorStep.propTypes = propTypes;
kacper-mikolajczak marked this conversation as resolved.
Show resolved Hide resolved

export default withLocalize(RequestorStep);
Loading