Repository navigation
Conversation
Co-authored-by: Nyoman Jyotisa <[email protected]>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
|
The failing Analysis: The failing test is Evidence:
Next step: Re-run the failed job. If it keeps failing, the flaky test needs a separate fix on |
|
🤖 Android test steps verification results: ❌ fail I couldn't test the Address Line 2 fix on Android. Tapping "Add bank account" opens a "Validate your account" magic-code screen, the same blocker as on Web. The code goes to the test account's email, which I can't read. The PR's Tests section is still empty, so I used the steps from the linked issue.
These failures mean the steps were blocked, not that the fix is broken. To test the fix, use an account that is already validated. |
|
@MelvinBot please replace PR description updateTests
Offline tests
QA StepsSame as Tests and Offline tests.
PR Author Checklist
Screenshots/VideosAndroid: NativePR-104823-android-native-online.mp4Android: mWeb ChromePR-104823-android-mweb-chrome-online-final.mp4iOS: NativePR-104823-ios-native-online-final.mp4iOS: mWeb SafariPR-104823-ios-mweb-safari-online-final.mp4MacOS: Chrome / SafariPR-104823-desktop-web-chrome-online.mp4 |
|
I replaced everything from |
|
@MelvinBot please mark this PR ready for review. |
|
I marked this PR as ready for review. |
|
@nyomanjyotisa 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] |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppPR-104823-android-native-online.mp4Android: mWeb ChromePR-104823-android-mweb-chrome-online-final.mp4iOS: HybridAppPR-104823-ios-native-online-final.mp4iOS: mWeb SafariPR-104823-ios-mweb-safari-online-final.mp4MacOS: Chrome / SafariPR-104823-desktop-web-chrome-online.mp4 |
|
@puneetlath While reviewing this PR, I discovered the international-bank terms checkbox has no accessible name. Confirmed on staging and main; changing |
|
@nyomanjyotisa 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] |
|
The failing What failed: What I did: I pushed merge commit 35052fa. After the merge, the diff against Details
|
|
The test checks now pass on the latest commit (35052fa), including The only failing check is |
|
@puneetlath I also found this while reviewing this PR: Wallet → Add personal card → Next without a country → select United States. The country error remains, although Next advances. The country selector should clear Chrome-F26-before.mp4 |


Explanation of Change
When you add a US personal bank account with Address Line 2,
addPersonalBankAccountjoined both lines with\nintoaddressStreet. The API rejects newlines in that field, so the whole address was lost and the Wallet row showed Review.This PR sends the two lines as separate params, the same way
updatePersonalBankAccountInfoalready does:addressStreet: Line 1addressStreet2: Line 2, or''when emptyThe backend now accepts
addressStreet2onAddPersonalBankAccount(already on staging, per the issue). It stores the address asline1\nline2, which the App already splits for display, so no read-side change is needed.Changes:
src/libs/actions/BankAccounts.ts: sendaddressStreetandaddressStreet2separately inaddPersonalBankAccount.src/libs/API/parameters/AddPersonalBankAccountParams.ts: addaddressStreet2.tests/unit/BankAccountsTest.ts: add tests for both params, with and without Line 2.AI tests run locally:
BankAccountsTestandAddPersonalBankAccountPageTestpass, plus typecheck, lint-changed, oxfmt, and spell-changed. An automated web UI check could not run because the test browser session rendered a blank page, so the end-to-end flow still needs manual verification on staging.Fixed Issues
$ #104165
PROPOSAL: #104165 (comment)
Tests
123 Main St/Apt 4Band the required US address fields.undefinedor extra address line after reload.Offline tests
QA Steps
Same as Tests and Offline tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
PR-104823-android-native-online.mp4
Android: mWeb Chrome
PR-104823-android-mweb-chrome-online-final.mp4
iOS: Native
PR-104823-ios-native-online-final.mp4
iOS: mWeb Safari
PR-104823-ios-mweb-safari-online-final.mp4
MacOS: Chrome / Safari
PR-104823-desktop-web-chrome-online.mp4