Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@jayeshmangwani 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] |
melvin-bot
bot
requested review from
joekaufmanexpensify
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
src/pages/ReimbursementAccount/NonUSD/utils/getSignerDetailsAndSignerFilesForSignerInfo.ts
Show resolved
Hide resolved
src/pages/ReimbursementAccount/NonUSD/utils/getSignerDetailsAndSignerFilesForSignerInfo.ts
Show resolved
Hide resolved
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6131c9f7fc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/pages/ReimbursementAccount/NonUSD/utils/getSignerDetailsAndSignerFilesForSignerInfo.ts
Show resolved
Hide resolved
|
No product review needed |
|
@jayeshmangwani are you able to review please 🙇 |
|
Yes, I’m testing the PR now. |
|
@NikkiWines , when we send Note: There is also an existing bug in prod. When the Date of Birth date picker is opened and the user clicks outside the modal without selecting a date, a default date appears to be selected initially. However, after completing all the steps and returning to the final Signer Info review page, no date is selected. Additionally, no validation error is shown on the FE side. I’ve attached a video below showing the steps to reproduce the issue. date-selection-bug.mov |
|
@NikkiWines , whenever you get a chance, please see the comment above #81602 (comment) and confirm whether we need to make any BE changes so it sends the specific error message for the missing value. |
|
Sorry for the delay @jayeshmangwani, I'm unable to reproduce the bug you identified in dev (even on main), there is no default DOB selected (as least for UK bank accounts). I'm also not able to reproduce the bug with these steps:
Screen.Recording.2026-02-18.at.13.36.36.movI'll try now with a CAD bank account to see Edit: Same behavior as the GBP |
|
@jayeshmangwani responded to your review above, can you take a look please 🙇 |
|
Sorry, I missed the above comments. @NikkiWines , I’m still seeing the issue. Here are the detailed steps to reproduce it:
bug-repro.movI’m adding the cURL and response below. If you need the specific account I’m testing with, please let me know and I’ll DM you. cURLcurl 'https://dev.new.expensify.com:8082/api/SaveCorpayOnboardingDirectorInformation?' \
-H 'Accept: */*' \
-H 'Accept-Language: en-US,en;q=0.9' \
-H 'Connection: keep-alive' \
-H 'Content-Type: multipart/form-data; boundary=----WebKitFormBoundaryh0Ple4nZgjU4xqsI' \
-H 'Origin: https://dev.new.expensify.com:8082' \
-H 'Referer: https://dev.new.expensify.com:8082/bank-account/validate?backTo=workspaces%2F4EDECBD7692AD744%2Fworkflows&policyID=4EDECBD7692AD744' \
-H 'Sec-Fetch-Dest: empty' \
-H 'Sec-Fetch-Mode: cors' \
-H 'Sec-Fetch-Site: same-origin' \
-H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/145.0.0.0 Safari/537.36' \
-H 'sec-ch-ua: "Not:A-Brand";v="99", "Google Chrome";v="145", "Chromium";v="145"' \
-H 'sec-ch-ua-mobile: ?0' \
-H 'sec-ch-ua-platform: "macOS"' \
--data-raw $'------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="inputs"\r\n\r\n{"signerFullName":"testuing","signerEmail":"jm9828951745@gmail.com","signerJobTitle":"testing","signerDateOfBirth":"","signerCompleteResidentialAddress":"Janes Lane, Almont, CO, 81210"}\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="proofOfDirectors"; filename="1.png"\r\nContent-Type: image/png\r\n\r\n\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="bankAccountID"\r\n\r\n8950554\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="apiRequestType"\r\n\r\nwrite\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="pusherSocketID"\r\n\r\n885823.8345608\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="shouldRetry"\r\n\r\ntrue\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="canCancel"\r\n\r\ntrue\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="authToken"\r\n\r\nBFFD4538086541DB8E92F980E4E0963E1739271CC10A86D819C9C5AFA898A224F60B635887508DD3F850E66AB3B3222E1585B27F7B140651B0AA4C6F7C067CAB04BC0469F9675DAA6D735DC8C1C01D635CECCDA23A6E8EE833BA2FE806EBEC45E7B54E0F28E0453074118D73F75F5D0FEDDCB42976E06F019303D0B83586367E7681C2B7EFDD2CBF406AF9438CF03160ADC871FBB287EECEE40C3052165B36FB42866E3582B98DE0075818578643DCB9B74F159A3C482ADA83B7D0D06455DC4B6D2713A877D22670344A9F943273F1FBFA2942ADFDC246B0094E4C5FFEC6FE3F964389471078264861631E2923B2D03038AF4777D877736550F1E36C4491A6E48385880CA4439545C4C5F2E5422658B7971E57C82704930F7C65BF1E0668258E15BFECA2F111A6E1428A2459ED6935652FD5A44B267D2244FA6D098765E91EB8901CEFD4251B605CCC7BFDFE70EFB8225D4F38DBBB4EC91F3A15F9785FEC0809\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="referer"\r\n\r\necash\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="platform"\r\n\r\nweb\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="api_setCookie"\r\n\r\nfalse\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="email"\r\n\r\njm98289517+45@gmail.com\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="isFromDevEnv"\r\n\r\ntrue\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="appversion"\r\n\r\n9.3.25-4\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI\r\nContent-Disposition: form-data; name="clientUpdateID"\r\n\r\n11413631321\r\n------WebKitFormBoundaryh0Ple4nZgjU4xqsI--\r\n'
Response:{
"code": 666,
"jsonCode": 666,
"type": "Expensify\\Libs\\Error\\ExpError",
"UUID": "1301298F-9273-44A4-AB99-289B9FCF5DA2",
"message": "Auth UpdateCorpayOnboardingAdditionalData returned an error",
"title": "",
"data": {
"onyxData": [
{
"onyxMethod": "merge",
"key": "reimbursementAccount",
"value": {
"errors": {
"1771929450323043": "Auth UpdateCorpayOnboardingAdditionalData returned an error"
}
}
}
]
},
"htmlMessage": "",
"onyxData": [
{
"onyxMethod": "merge",
"key": "reimbursementAccount",
"value": {
"errors": {
"1771929450323043": "Auth UpdateCorpayOnboardingAdditionalData returned an error"
}
}
}
],
"requestID": "9d2e53f05f839094-SJC"
}
If this BE error is unrelated, the PR looks good otherwise, and I’ll proceed with the checklist. |
|
Thanks for the added details @jayeshmangwani. I still haven't been able to reproduce, but I will see if our internalQA team is able to. Since this bug also exists on prod I think we can handle it separately from this issue, especially since the signer fields are now no longer silently failing |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
Sounds good to me. I’ll try to add reliable repro steps and will post in the Slack #expensify-bugs channel to investigate that issue separately. We’re good to go with this PR. |
|
@NikkiWines we need to merge main to fix the failing Prettier check. |
|
🎯 @jayeshmangwani, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #83655. |
|
🚧 @Gonals has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ 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/Gonals in version: 9.3.28-0 🚀
|
|
🚀 Deployed to staging by https://github.com/Gonals in version: 9.3.30-0 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.3.30-3 🚀
|
5 participants
Explanation of Change
This PR fixes an issue where required signer fields (
signerFullName,signerDateOfBirth,signerJobTitle) were being silently omitted from the API request payload when not present in the form draft. This caused the Auth backend to receive incomplete data and fail with cryptic validation errors like "Invalid inputs" instead of properly identifying which required fields were missing.Fix:
requiredSignerFieldsconstant defining which fields must always be sent to the backendSafeString()to ensure consistent string handlingFixed Issues
$ https://github.com/Expensify/Expensify/issues/593959
PROPOSAL: N/A
Tests
signerDateOfBirth,signerFullName, andsignerJobTitleare present in the payloadOffline tests
N/A - This change affects form submission, which requires network connectivity
QA Steps
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
Android: Native
N/A - Backend/utility change only, no UI changes
Android: mWeb Chrome
N/A - Backend/utility change only, no UI changes
iOS: Native
N/A - Backend/utility change only, no UI changes
iOS: mWeb Safari
N/A - Backend/utility change only, no UI changes
MacOS: Chrome / Safari
N/A - Backend/utility change only, no UI changes