-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Conversation
4aba17e
to
3cc5916
Compare
Tested the Idology Questions flow. You need to have your IP whitelisted by Idology. In src/setup/index.js, hardcode Access http://localhost:8080/settings/payments/enable-payments Run in the console: Enter the info as in the screenshot (double check the address as the google autocomplete tends to mess it up) then Save & Continue After 5 questions badly answered, you're redirected to the KYC failed page: |
bfa36b6
to
c6adaa3
Compare
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? |
c6adaa3
to
fb8574e
Compare
9c18087
to
9cdf993
Compare
370137c
to
f9350d3
Compare
f9350d3
to
1ed21d8
Compare
Looks good. Going to test first thing tomorrow |
995ec14
to
5457046
Compare
5457046
to
27b5777
Compare
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! |
There was a problem hiding this 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.
src/libs/actions/Wallet.js
Outdated
CONST.WALLET.ERROR.UNEXPECTED, | ||
CONST.WALLET.ERROR.UNABLE_TO_VERIFY, | ||
]; | ||
let qualifiers = _.get(response, ['data', 'requestorIdentityID', 'apiResult', 'qualifiers', 'qualifier'], []); |
There was a problem hiding this comment.
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.
src/libs/actions/Wallet.js
Outdated
} | ||
} | ||
|
||
if (_.get(response, ['data', 'requestorIdentityID', 'apiResult', 'results', 'key']) === 'result.no.match' |
There was a problem hiding this comment.
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) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const possibleAnwers = _.filter(_.map(question.answer, (answer) => { | |
const possibleAnswers = _.filter(_.map(question.answer, (answer) => { |
Updated |
f38fb95
to
0b9f587
Compare
0b9f587
to
42459e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great thanks!
✋ 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 @marcaaron in version: 1.1.43-0 🚀
|
🚀 Deployed to production by @roryabraham in version: 1.1.43-2 🚀
|
Details
Add more address fields
Show Idology Questions when needed
Handle Idology errors and copies
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