Prevent empty values for mandatory fields in CSV Card Import#95488
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@shubham1206agra 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] |
| return errors; | ||
| } | ||
|
|
||
| const columnWithEmptyValues = requiredColumns.find((requiredColumn) => { |
There was a problem hiding this comment.
@Gonals, I think we should throw an error only if all values in the column are empty. Otherwise, the check will become too rigid.
There was a problem hiding this comment.
I disagree. These fields are mandatory. No transactions should be missing them
There was a problem hiding this comment.
Since no one responded in Slack thread. Let's proceed with this as it is.
Reviewer Checklist
Screenshots/Videos
|
|
@youssef-lr 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] |
|
🚧 youssef-lr has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/youssef-lr in version: 9.4.32-0 🚀
|
|
🤖 I reviewed the changes in this PR against the help site articles under Yes, a help site update is warranted. This PR adds a new user-facing validation to the Import transactions from file flow: mapping a required field (Card Number, Date, Amount, or Merchant) to a spreadsheet column that contains empty values now shows an error ( The Import Company Card Transactions From a Spreadsheet article already documents the related "map the same column twice" error but didn't cover empty-value validation, so I added a short FAQ entry to match. Draft PR: #95688 (labeled What I checked
@Gonals, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.4.32-3 🚀
Bundle Size Analysis (Sentry): |

Explanation of Change
Fixed Issues
$ #93704
PROPOSAL:
Tests
Go to Workspace > Company Cards > Add cards > USA > Import from file and import a file with an empty column. You can use this one:
Expensify_usd.xlsx
Map the mandatory fields.
Confirm it doesn't let you map a column with empty fields:
Offline tests
None
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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.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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari