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.

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

P2P KYC - Handle Idology questions and errors #7813

Merged
merged 5 commits into from
Mar 14, 2022
Merged

Conversation

nkuoch
Copy link
Contributor

@nkuoch nkuoch commented Feb 18, 2022

Details

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/193505
$ https://github.com/Expensify/Expensify/issues/193504

Tests

See comments below

QA Steps

None, under beta and will be tested as a whole once everything is done

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

@nkuoch nkuoch self-assigned this Feb 18, 2022
@nkuoch nkuoch force-pushed the nat-p2pkycquestions branch 3 times, most recently from 4aba17e to 3cc5916 Compare February 20, 2022 13:20
@nkuoch
Copy link
Contributor Author

nkuoch commented Feb 20, 2022

Tested the Idology Questions flow.

You need to have your IP whitelisted by Idology.

In src/setup/index.js, hardcode window.Onyx = Onyx; so you can later access it from your console.

Access http://localhost:8080/settings/payments/enable-payments

Run in the console: Onyx.merge('userWallet', {currentStep: 'AdditionalDetailsStep'});

image

Enter the info as in the screenshot (double check the address as the google autocomplete tends to mess it up) then Save & Continue

image

After 5 questions badly answered, you're redirected to the KYC failed page:
image

@nkuoch nkuoch force-pushed the nat-p2pkycquestions branch 3 times, most recently from bfa36b6 to c6adaa3 Compare February 22, 2022 12:37
@botify
Copy link

botify commented Feb 22, 2022

Hey, I noticed you changed some webpack configuration files. This can break production builds. Did you remember to run a production build locally to verify they still work?

@nkuoch nkuoch mentioned this pull request Feb 22, 2022
7 tasks
@nkuoch nkuoch force-pushed the nat-p2pkycquestions branch 7 times, most recently from 9c18087 to 9cdf993 Compare February 28, 2022 12:40
@nkuoch nkuoch marked this pull request as ready for review February 28, 2022 12:41
@nkuoch nkuoch requested a review from a team as a code owner February 28, 2022 12:41
@nkuoch nkuoch removed the request for review from a team February 28, 2022 12:41
@nkuoch nkuoch removed the request for review from PauloGasparSv February 28, 2022 12:41
@nkuoch nkuoch force-pushed the nat-p2pkycquestions branch 5 times, most recently from 370137c to f9350d3 Compare March 1, 2022 16:11
@nkuoch
Copy link
Contributor Author

nkuoch commented Mar 2, 2022

Tested birthDate error:
image

Tested address error:
image

Tested ssn error:
image

Tested lastName error:
image

Tested combining 2 errors:
image

Tested combining 3 errors:
image

And in spanish:
image

@nkuoch nkuoch changed the title P2P KYC - Handle Idology questions and errors [HOLD] P2P KYC - Handle Idology questions and errors Mar 4, 2022
@ctkochan22
Copy link
Contributor

Looks good. Going to test first thing tomorrow

@nkuoch nkuoch changed the title [HOLD] P2P KYC - Handle Idology questions and errors P2P KYC - Handle Idology questions and errors Mar 8, 2022
@Expensify Expensify deleted a comment from marcaaron Mar 8, 2022
@Expensify Expensify deleted a comment from shawnborton Mar 8, 2022
@nkuoch
Copy link
Contributor Author

nkuoch commented Mar 8, 2022

Ok, I rebased main, squashed all my commits together so it's cleaner, forced push, and now that it's not holding on anything anymore, it's ready to be reviewed, tested and merged!

ctkochan22
ctkochan22 previously approved these changes Mar 10, 2022
Copy link
Contributor

@marcaaron marcaaron left a comment

Choose a reason for hiding this comment

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

Really great stuff, thanks for the changes. Just spotted a few minor things after doing a full review. Will give it a test first thing tomorrow.

CONST.WALLET.ERROR.UNEXPECTED,
CONST.WALLET.ERROR.UNABLE_TO_VERIFY,
];
let qualifiers = _.get(response, ['data', 'requestorIdentityID', 'apiResult', 'qualifiers', 'qualifier'], []);
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah small thing but we have standardized on using lodashGet(). A get() was added to underscore in a recent version, but doesn't have the path based syntax like (lodashGet(response, 'data. requestorIdentityID.apiResult.etc')) so we avoid using it to keep things consistent.

}
}

if (_.get(response, ['data', 'requestorIdentityID', 'apiResult', 'results', 'key']) === 'result.no.match'
Copy link
Contributor

Choose a reason for hiding this comment

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

lodashGet()

render() {
const questionIndex = this.state.questionNumber;
const question = this.props.questions[questionIndex] || {};
const possibleAnwers = _.filter(_.map(question.answer, (answer) => {
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
const possibleAnwers = _.filter(_.map(question.answer, (answer) => {
const possibleAnswers = _.filter(_.map(question.answer, (answer) => {

@nkuoch
Copy link
Contributor Author

nkuoch commented Mar 10, 2022

Updated

@marcaaron marcaaron self-requested a review March 14, 2022 18:57
Copy link
Contributor

@marcaaron marcaaron left a comment

Choose a reason for hiding this comment

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

Looks great thanks!

@marcaaron marcaaron merged commit c1bb574 into main Mar 14, 2022
@marcaaron marcaaron deleted the nat-p2pkycquestions branch March 14, 2022 21:00
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @marcaaron in version: 1.1.43-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.43-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants