Remove Onyx.connect() for the key: ONYXKEYS.BANK_ACCOUNT_LIST in src/libs/actions/ReimbursementAccount/store.ts#67521
Conversation
|
@ikevin127 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] |
Reviewer Checklist
Screenshots/VideosScreen.Recording.2025-08-20.at.16.57.31.mov |
| }); | ||
|
|
||
| function hasCreditBankAccount(): boolean { | ||
| function hasCreditBankAccount(bankAccountList: OnyxEntry<OnyxTypes.BankAccountList>): boolean { |
There was a problem hiding this comment.
add a test to this function please
There was a problem hiding this comment.
I added the tests please review
| import type {SearchFilterKey} from '@components/Search/types'; | ||
| import type ResponsiveLayoutResult from '@hooks/useResponsiveLayout/types'; | ||
| import type {MileageRate} from '@libs/DistanceRequestUtils'; | ||
| import BankAccount from '@libs/models/BankAccount'; |
There was a problem hiding this comment.
A bit context here @ikevin127 @danieldoglas , There was a circular dependency here since we import const in libs/models/BankAccount, I observed this during writing unit tests which also made the tests to fail, so i defined the const here
|
|
|
Need to merge main on this one. |
|
@ikevin127 I think this is ready for you now, reassure might fail but it's broken on main too |
|
I'm not so sure we should merge without the reassure performance tests passing. The issue we saw before was fixed EOD yesterday |
|
@allgandalf Merging main should solve the reassure as I've noticed all workflows passing on PRs opened yesterday / today. |
@ikevin127 done! |
|
@danieldoglas All yours! |
|
✋ 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/danieldoglas in version: 9.2.3-0 🚀
|
|
@allgandalf @danieldoglas Could you help with last step please, how can we use |
Isn't it showing you the option to pay with expensify from User A account in the test steps? |
|
🚀 Deployed to staging by https://github.com/danieldoglas in version: 9.2.5-0 🚀
|
|
🚀 Deployed to staging by https://github.com/danieldoglas in version: 9.2.8-0 🚀
|
|
Hi @danieldoglas, I can see only bandicam.2025-09-10.10-52-10-938.mp4 |
|
@jponikarchuk I think the text got updated recently so it says only pay, if you are able to verify that we see the options as mentioned in the QA steps, i think that successfully passes this PR testing |
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.2.8-4 🚀
|
Explanation of Change
Fixed Issues
$ #67559
PROPOSAL:
Tests
Offline tests
QA Steps
Precondition: User A has not added any wallet or bank account
Verify that user A is shown the option to add personal bank account and business bank account
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screen.Recording.2025-08-20.at.5.02.03.PM.mov