Refactor Terms step of Wallet_Activate into AcceptWalletTerms
#10443
Conversation
francoisl
changed the title
AcceptWalletTermsAcceptWalletTerms
melvin-bot
bot
requested review from
Justicea83
and removed request for
a team
| <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.activated')} | ||
| </Text> | ||
| </View> | ||
| </View> | ||
| <FixedFooter> | ||
| <Button | ||
| text={this.props.translate('common.continue')} | ||
| onPress={PaymentMethods.continueSetup} | ||
| style={[styles.mt4]} | ||
| iconStyles={[styles.mr5]} | ||
| success | ||
| /> | ||
| </FixedFooter> |
There was a problem hiding this comment.
This doesn't quite match the mockup from the doc :)
| {this.props.userWallet.tierName === CONST.WALLET.TIER_NAME.SILVER && ( | ||
| <Text>{this.props.translate('activateStep.checkBackLater')}</Text> | ||
| )} |
There was a problem hiding this comment.
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.
| onyxMethod: CONST.ONYX.METHOD.MERGE, | ||
| key: ONYXKEYS.USER_WALLET, | ||
| value: { | ||
| shouldShowWalletActivationSuccess: null, |
There was a problem hiding this comment.
I changed this to null instead of false so it gets removed, I figured it's no longer needed at that point.
src/languages/es.js
Outdated
| activatedTitle: 'Billetera Expensify lista!', | ||
| activatedMessage: 'Enhorabuena, tu billetera Expensify está lista para usar.', |
There was a problem hiding this comment.
Getting confirmation for these here.
francoisl
changed the title
AcceptWalletTermsAcceptWalletTerms
|
Updated and removed the hold. One thing I'm not sure about is that I get an error in the JS console that seems to point to something unrelated, but it's also happening on it points to the |
francoisl
changed the title
AcceptWalletTermsAcceptWalletTerms
|
Fixed the conflicts with your other PR we just merged, and also I'm putting this back on hold until https://github.com/Expensify/Web-Expensify/pull/34588 is on staging. |
…rAcceptWalletTerms
| @@ -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 */ | |||
There was a problem hiding this comment.
| /** 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 */ |
| <Text style={[styles.formError, styles.mb2]}> | ||
| {_.last(_.values(errors))} | ||
| </Text> | ||
| )} | ||
| <Button |
There was a problem hiding this comment.
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)}
/>
| <FixedFooter> | ||
| <Button | ||
| text={this.props.translate('common.continue')} | ||
| onPress={PaymentMethods.continueSetup} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
| </View> | ||
| <FixedFooter> | ||
| <Button | ||
| text={this.props.translate('common.continue')} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Yeah, I see this is happening for the |
Co-authored-by: Maria D'Costa <maria@expensify.com>
|
Updated, thanks for the reviews! |
francoisl
changed the title
AcceptWalletTermsAcceptWalletTerms
|
Removed the Hold on this. I don't know if @Justicea83 you want to review? Otherwise Maria feel free to merge! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by @Justicea83 in version: 1.1.89-0 🚀
|
cc @MariaHCD
Hold until https://github.com/Expensify/Web-Expensify/pull/34588 is at least on staging.
Details
AcceptWalletTermswhen the user accepts the terms of the Expensify Wallet, instead of using the deprecatedWallet_ActivatecommandActivateStepcomponent to be prettier when the user finishes setting up a Gold wallet.This PR does not implement the Red Brick Road for KYC errors. This will come in follow-up PRs.
Fixed Issues
Part of https://github.com/Expensify/Expensify/issues/218502
Tests
Pre-requisites:
bankAccountIDof that account in auth, and then run (replace thebankAccountIDat the end):Tests
/enable-payments
3. Scroll down to the bottom, and make sure you can't click the "Enable Payments" button without checking both buttonsCheck both checkboxes, click the Enable Payments button
Since the wallet technically doesn't pass all identity checks at that point, it will be a Silver wallet, and you should see a "We're still reviewing your information. Please check back later." message
Run the flow again but for a Gold wallet. Start by resetting and configuring the wallet as follows:
validator:walletCLI Tool on the accountUPDATE bankAccounts SET additionalData = JSON_SET(COALESCE(NULLIF(additionalData, ''), '{}'), '$.currentStep', 'TermsStep') WHERE bankAccountID = <walletID>;Onyx.merge('userWallet', {shouldShowWalletActivationSuccess:null})Navigate to
/enable-paymentsagain. Check both boxes at the bottom, and click the buttonMake sure you see the following Tada page
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarhave been tested & I retested again)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)QA Steps
Screenshots
Web
Mobile Web
Desktop
iOS
Android