-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Update Onfido SDK to fix package dependencies #34509
Conversation
@cubuspl42 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@cubuspl42 We need to check that I didn't break the Onfido flow for adding a bank account after bumping the version. |
I don't think I ever managed to complete the real Onfido flow. I'll try my best. It's my end of the day, though, so I can pick this up tomorrow. |
@cubuspl42 yeah, no rush, I also didn't have time to test it today, that's why I added the HOLD, I will test it tomorrow. |
I don't know how to pass the "Additional details" step (which is before the Onfido flow). I tried fake SSN generators, but nothing seemed to work. When I hack it around so the flow starts with Onfido step, it also gives me an error. I'm not sure how to properly test this as non-American. |
@cubuspl42 I think you can just add a video up to that step, the fact that it's showing is a good sign that I didn't break it completely 😄 |
I wasn't able to get to what you showed on your screenshots without errors. Did you use your real US data in steps 1 and 2? |
@cubuspl42 I used my real first/last name and a US address. For the last 4 SSN I used |
@cubuspl42 Which flow are you using? I am going to the workspace settings > Reimbursements > Connect Bank Account. The screen I see it's different: |
I went through the Wallet path! I'll try your way. |
Reviewer Checklist
Screenshots/VideosAndroid: Nativeonfido-sdk-upgrade-android-compressed.mp4Android: mWeb Chromeonfido-sdk-upgrade-android-web-compressed.mp4iOS: Nativeonfido-sdk-upgrade-ios-compressed.mp4iOS: mWeb Safarionfido-sdk-upgrade-ios-web-compressed.mp4MacOS: Chrome / Safarionfido-sdk-upgrade-web-converted.mp4MacOS: Desktoponfido-sdk-upgrade-desktop-converted.mp4 |
@MonilBhavsar Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
🎯 @cubuspl42, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #34737. |
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.
Thanks for fixing it!
Could you please fill the author checklist, tests and QA steps
I have ni clue why Snyk is failing, I will try to figure it out on Monday |
Nvm, I did the same as https://expensify.slack.com/archives/C07J32337/p1705544162525359?thread_ts=1705543165.610339&cid=C07J32337. @MonilBhavsar Feel free to merge! |
Github actions test is failing
|
What on earth? I have no clue why this would be failing, I am going to check in Slack |
I don't understand this either... Doesn't |
I tried to run |
Diff stats are +244 −41,893. Very red. Interesting, not necessarily a problem. |
It seems to be working, it just removed a lot in those compiled files. @MonilBhavsar All yours! |
Thanks for looking! Makes sense and works fine |
✋ 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 https://github.com/MonilBhavsar in version: 1.4.30-0 🚀
|
🚀 Deployed to production by https://github.com/thienlnam in version: 1.4.30-1 🚀
|
1 similar comment
🚀 Deployed to production by https://github.com/thienlnam in version: 1.4.30-1 🚀
|
There are some vulnerabilities in the dependencies for Onfido, so this should fix it. I didn't upgrade to the latest one because there are no changelogs and I wasn't sure of something big could have changed.
Details
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/286999
PROPOSAL:
Tests
Same as QA
Offline tests
N/A
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop