Refactor: Deprecate getPersonalPolicy (part 2)#78783
Conversation
|
@ahmedGaber93 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] |
|
@ahmedGaber93 Sorry, but @ikevin127 will review the PR here |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-01-02.at.17.01.58.mov |
|
@tgolen ✅ PR Reviewer Checklist completed. I cannot approve since I was removed from C+ team temporarely, but this is my 🟢 Approved signal - consider this PR ready to merge from my side 👍 |
| function addPersonalBankAccount(account: PlaidBankAccount, policyID?: string, source?: string, lastPaymentMethod?: LastPaymentMethodType | string | undefined) { | ||
| function addPersonalBankAccount( | ||
| account: PlaidBankAccount, | ||
| personalPolicyID: string | undefined, |
There was a problem hiding this comment.
Please add a unit test for this new parameter.
There was a problem hiding this comment.
@tgolen I don't know how to write a test case for the BA feature, as there are no tests present to mock PlaidBankAccount. Can you help me here?
There was a problem hiding this comment.
At a very minimum, I would add a test that does this:
- Mock all the parameters to
addPersonalBankAccount()with test data - Mock the network layer so that
API.write()doesn't do anything - Verify that the Onyx updates have the correct personal policy ID
I know that's very far from 100% coverage, but at least it's more than 0%.
There was a problem hiding this comment.
@tgolen I have talked about tests here https://expensify.slack.com/archives/C08CZDJFJ77/p1768229592680389 and it seems like someone will write the tests in another PR. So, we could merge the PR without any issues.
|
PR doesn’t need product input as a refactor PR. Unassigning us. |
|
✋ 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/tgolen in version: 9.3.0-0 🚀
|
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.3.0-8 🚀
|
Explanation of Change
Fixed Issues
$ #66397
Tests
Offline tests
Same as Tests
QA Steps
Same as Tests
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))npm run compress-svg)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.Screenshots/Videos
Screen.Recording.2026-01-02.at.6.19.42.PM.mov
Screen.Recording.2026-01-02.at.6.20.18.PM.mov