Skip to content

Refactor Terms step of Wallet_Activate into AcceptWalletTerms #10443

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

Merged
merged 12 commits into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from 10 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
3 changes: 2 additions & 1 deletion src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,8 @@ export default {
},
activateStep: {
headerTitle: 'Enable payments',
activated: 'Your Expensify Wallet is ready to use.',
activatedTitle: 'Wallet activated!',
activatedMessage: 'Congrats, your wallet is set up and ready to make payments.',
checkBackLater: 'We\'re still reviewing your information. Please check back later.',
},
companyStep: {
Expand Down
3 changes: 2 additions & 1 deletion src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,8 @@ export default {
},
activateStep: {
headerTitle: 'Habilitar pagos',
activated: 'Su billetera Expensify está lista para usar.',
activatedTitle: '¡Billetera activada!',
activatedMessage: 'Felicidades, tu Billetera está configurada y lista para hacer pagos.',
checkBackLater: 'Todavía estamos revisando tu información. Por favor, vuelva más tarde.',
},
companyStep: {
Expand Down
1 change: 1 addition & 0 deletions src/libs/actions/BankAccounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export {
activateWallet,
fetchUserWallet,
verifyIdentity,
acceptWalletTerms,
} from './Wallet';

function clearPersonalBankAccount() {
Expand Down
42 changes: 42 additions & 0 deletions src/libs/actions/Wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,47 @@ function verifyIdentity(parameters) {
});
}

/**
* Complete the "Accept Terms" step of the wallet activation flow.
*
* @param {Object} parameters
* @param {Boolean} parameters.hasAcceptedTerms
*/
function acceptWalletTerms(parameters) {
const optimisticData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.USER_WALLET,
value: {
shouldShowWalletActivationSuccess: true,
},
},
];

const successData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.WALLET_TERMS,
value: {
errors: null,
},
},
];

const failureData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.USER_WALLET,
value: {
shouldShowWalletActivationSuccess: null,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this to null instead of false so it gets removed, I figured it's no longer needed at that point.

shouldShowFailedKYC: true,
},
},
];

API.write('AcceptWalletTerms', {hasAcceptedTerms: parameters.hasAcceptedTerms}, {optimisticData, successData, failureData});
}

/**
* Fetches information about a user's Expensify Wallet
*
Expand Down Expand Up @@ -481,4 +522,5 @@ export {
updateCurrentStep,
updatePersonalDetails,
verifyIdentity,
acceptWalletTerms,
};
82 changes: 64 additions & 18 deletions src/pages/EnablePayments/ActivateStep.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import styles from '../../styles/styles';
import userWalletPropTypes from './userWalletPropTypes';
import CONST from '../../CONST';
import Text from '../../components/Text';
import Icon from '../../components/Icon';
import * as Illustrations from '../../components/Icon/Illustrations';
import defaultTheme from '../../styles/themes/default';
import FixedFooter from '../../components/FixedFooter';
import Button from '../../components/Button';
import * as PaymentMethods from '../../libs/actions/PaymentMethods';

const propTypes = {
...withLocalizePropTypes,
Expand All @@ -21,24 +27,64 @@ const defaultProps = {
userWallet: {},
};

const ActivateStep = props => (
<ScreenWrapper>
<HeaderWithCloseButton
title={props.translate('activateStep.headerTitle')}
onCloseButtonPress={() => Navigation.dismissModal()}
shouldShowBackButton
onBackButtonPress={() => Navigation.goBack()}
/>
<View style={[styles.mh5, styles.flex1]}>
{props.userWallet.tierName === CONST.WALLET.TIER_NAME.GOLD && (
<Text>{props.translate('activateStep.activated')}</Text>
)}
{props.userWallet.tierName === CONST.WALLET.TIER_NAME.SILVER && (
<Text>{props.translate('activateStep.checkBackLater')}</Text>
)}
</View>
</ScreenWrapper>
);
class ActivateStep extends React.Component {
constructor(props) {
super(props);

this.renderGoldWalletActivationStep = this.renderGoldWalletActivationStep.bind(this);
}

renderGoldWalletActivationStep() {
return (
<>
<View style={[styles.pageWrapper, styles.flex1, styles.flexColumn, styles.alignItemsCenter, styles.justifyContentCenter]}>
<Icon
src={Illustrations.TadaBlue}
height={100}
width={100}
fill={defaultTheme.iconSuccessFill}
/>
<View style={[styles.ph5]}>
<Text style={[styles.mt5, styles.h1, styles.textAlignCenter]}>
{this.props.translate('activateStep.activatedTitle')}
</Text>
<Text style={[styles.mt3, styles.textAlignCenter]}>
{this.props.translate('activateStep.activatedMessage')}
</Text>
</View>
</View>
<FixedFooter>
<Button
text={this.props.translate('common.continue')}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal but we can probably adjust this now or in a follow up PR.

The green success button will either state “Continue to payment” or “Continue to transfer” based on the action that we will be returning the user to.

https://docs.google.com/document/d/1PYqKcBpjPxewL5OneCmqbMJNH8G3CNkqbJWqzHSaXhU/edit#bookmark=id.8y4wjrbvjh9z

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah at the moment I don't see how we would do that so I don't mind if we do this in a follow-up PR.

onPress={PaymentMethods.continueSetup}
Copy link
Contributor

Choose a reason for hiding this comment

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

Haven't tested but just wanted to confirm if clicking this will lead the user back to the action they were previously taking i.e. paying via the wallet or transferring their wallet balance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a huge pain to test on dev for some reason, but yes. It calls this function, which at the bottom ends up calling the onSuccessfulKYC() prop method.

Screen.Recording.2022-08-22.at.3.38.56.PM.mov

style={[styles.mt4]}
iconStyles={[styles.mr5]}
success
/>
</FixedFooter>
</>
);
}

render() {
return (
<ScreenWrapper>
<HeaderWithCloseButton
title={this.props.translate('activateStep.headerTitle')}
onCloseButtonPress={() => Navigation.dismissModal()}
shouldShowBackButton
onBackButtonPress={() => Navigation.goBack()}
/>
<View style={[styles.mh5, styles.flex1]}>
{this.props.userWallet.tierName === CONST.WALLET.TIER_NAME.GOLD && this.renderGoldWalletActivationStep()}
{this.props.userWallet.tierName === CONST.WALLET.TIER_NAME.SILVER && (
<Text>{this.props.translate('activateStep.checkBackLater')}</Text>
)}
Comment on lines +80 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

Created an issue for a mockup for the Check back later screen here: https://github.com/Expensify/Expensify/issues/223443

Doesn't have to be part of this PR, can be a follow up improvement.

</View>
</ScreenWrapper>
);
}
}

ActivateStep.propTypes = propTypes;
ActivateStep.defaultProps = defaultProps;
Expand Down
5 changes: 5 additions & 0 deletions src/pages/EnablePayments/EnablePaymentsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ class EnablePaymentsPage extends React.Component {
</ScreenWrapper>
);
}
if (this.props.userWallet.shouldShowWalletActivationSuccess) {
return (
<ActivateStep userWallet={this.props.userWallet} />
);
}

const currentStep = this.props.userWallet.currentStep || CONST.WALLET.STEP.ADDITIONAL_DETAILS;

Expand Down
19 changes: 12 additions & 7 deletions src/pages/EnablePayments/TermsStep.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import React from 'react';
import {ScrollView} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
import lodashGet from 'lodash/get';
import _ from 'underscore';
import HeaderWithCloseButton from '../../components/HeaderWithCloseButton';
import Navigation from '../../libs/Navigation/Navigation';
import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize';
import styles from '../../styles/styles';
import Button from '../../components/Button';
import * as BankAccounts from '../../libs/actions/BankAccounts';
import CONST from '../../CONST';
import TextLink from '../../components/TextLink';
import compose from '../../libs/compose';
import ONYXKEYS from '../../ONYXKEYS';
Expand All @@ -20,15 +21,15 @@ import LongTermsForm from './TermsPage/LongTermsForm';
const propTypes = {
/** Comes from Onyx. Information about the terms for the wallet */
walletTerms: PropTypes.shape({
/** Whether or not the information is currently loading */
loading: PropTypes.bool,
/** Any additional error message to show */
errors: PropTypes.objectOf(PropTypes.string),
}),
...withLocalizePropTypes,
};

const defaultProps = {
walletTerms: {
loading: false,
errors: null,
},
};

Expand Down Expand Up @@ -64,6 +65,7 @@ class TermsStep extends React.Component {
}

render() {
const errors = lodashGet(this.props, 'walletTerms.errors', {});
return (
<>
<HeaderWithCloseButton
Expand Down Expand Up @@ -112,11 +114,15 @@ class TermsStep extends React.Component {
{this.props.translate('common.error.acceptedTerms')}
</Text>
)}
{!_.isEmpty(errors) && (
<Text style={[styles.formError, styles.mb2]}>
{_.last(_.values(errors))}
</Text>
)}
<Button
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's replace the Button component with the FormAlertWithSubmitButton. It's more consistent with all other forms and we can get rid of these lines

const bool isButtonDisabled = !this.state.hasAcceptedDisclosure || !this.state.hasAcceptedPrivacyPolicyAndWalletAgreement;
...
<FormAlertWithSubmitButton
    buttonText={this.props.translate('termsStep.enablePayments')}
    onSubmit={() => {
        if (!this.state.hasAcceptedDisclosure
            || !this.state.hasAcceptedPrivacyPolicyAndWalletAgreement) {
            this.setState({error: true});
            return;
        }

        this.setState({error: false});
        BankAccounts.acceptWalletTerms({
            hasAcceptedTerms: this.state.hasAcceptedDisclosure
                && this.state.hasAcceptedPrivacyPolicyAndWalletAgreement,
        });
    }}
    isDisabled={isButtonDisabled || this.props.network.isOffline}
     message={this.state.error ? this.props.translate('common.error.acceptedTerms') : !_.isEmpty(errors) ? _.last(_.values(errors)) : ''}
    isAlertVisible={this.state.error || !_.isEmpty(errors)}
/>

Screen Shot 2022-08-22 at 1 46 47 PM

success
style={[styles.mv4]}
text={this.props.translate('termsStep.enablePayments')}
isLoading={this.props.walletTerms.loading}
onPress={() => {
if (!this.state.hasAcceptedDisclosure
|| !this.state.hasAcceptedPrivacyPolicyAndWalletAgreement) {
Expand All @@ -125,7 +131,7 @@ class TermsStep extends React.Component {
}

this.setState({error: false});
BankAccounts.activateWallet(CONST.WALLET.STEP.TERMS, {
BankAccounts.acceptWalletTerms({
hasAcceptedTerms: this.state.hasAcceptedDisclosure
&& this.state.hasAcceptedPrivacyPolicyAndWalletAgreement,
});
Expand All @@ -144,7 +150,6 @@ export default compose(
withOnyx({
walletTerms: {
key: ONYXKEYS.WALLET_TERMS,
initWithStoredValues: false,
},
}),
)(TermsStep);
3 changes: 3 additions & 0 deletions src/pages/EnablePayments/userWalletPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,7 @@ export default PropTypes.shape({

/** Status of wallet - e.g. SILVER or GOLD */
tierName: PropTypes.string,

/** Whether we should show the ActivateStep view success view after the user finished the KYC flow */
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/** Whether we should show the ActivateStep view success view after the user finished the KYC flow */
/** Whether we should show the ActivateStep success view after the user finished the KYC flow */

shouldShowWalletActivationSuccess: PropTypes.bool,
});