[NO QA] Coerce instanceID to string in CSV company-card import to fix legacy-feed rejection#95695
Conversation
Co-authored-by: cretadn22 <cretadn22@users.noreply.github.com>
|
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. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
I analyzed the checks — there's no code-related failure to fix. The only failing check is
This is the human author checklist, not a CI/test failure. It's failing because the To clear it, Every real CI check has passed, including the ones relevant to this change:
(The So nothing to push here — the failing check just needs the author checklist filled in by a human. |
|
@MelvinBot Mark this PR as ready |
|
@cretadn22 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] |
|
🤖 Done — this PR is now marked Ready for review (no longer a draft). |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-07-09.at.20.19.43.movAndroid: mWeb ChromeScreen.Recording.2026-07-09.at.20.12.51.moviOS: HybridAppScreen.Recording.2026-07-09.at.20.20.05.moviOS: mWeb SafariScreen.Recording.2026-07-09.at.20.20.39.movMacOS: Chrome / SafariScreen.Recording.2026-07-09.at.20.10.01.mov |
|
@blimpich Could you mind completing the author checklist (because I can't edit)? Additionally, it might be worth generating an ad-hoc build so @VictoriaExpensify can test it on her account Testing Steps:
|
|
🚧 blimpich has triggered a test Expensify/App build. You can view the workflow run here. |
|
🤖 I analyzed the failing checks — there's no code fix to push here. The only failing check is the PR Author Checklist ( The checklist action reports: There are no unchecked boxes ( This is a human-owned checklist, not a code or test failure, so it isn't something I can "fix" by pushing a commit. To clear it, the human co-author needs to update the PR description using the most up-to-date template (
|
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@cretadn22 given this comment and how we'd need a "legacy" feed I think we just mark this as NO QA and ship it and then ask the customer if it's fixed for them. I don't think @VictoriaExpensify reproduced this issue personally |
|
🤖 Re-checked — the situation is unchanged. The only failing check is still the PR Author Checklist ( There's no code fix to push here. This isn't a bug in the change — the author checklist in the PR description is stale relative to the current This is a human-owned verification gate — several items (offline testing, high-traffic-account testing, etc.) attest that a person actually performed those checks, so I won't tick them on a human's behalf. To turn it green, the human co-author (
Once that's done the checklist check will pass on its next run. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🤖 I re-checked — there is still no code fix to push. The only failing check is the PR Author Checklist; every test, lint, typecheck, build, and perf check passes. The
I can't resolve this by pushing a commit, and I won't check these boxes for you — they are human attestations ("I tested this offline", "I tested with a High Traffic account", etc.) that only the human co-author can truthfully confirm. To clear the check, the human co-author needs to:
|
|
🚀 Deployed to staging by https://github.com/blimpich in version: 9.4.33-0 🚀
|
|
🤖 Help site review: no docs changes required. I reviewed this PR against the help articles under Why: This is a purely internal bug fix. It coerces The relevant article, Import Company Card Transactions From a Spreadsheet, already describes the import and "upload additional transactions to an existing card feed" steps accurately, and its error/troubleshooting FAQ is about file contents/formatting — not this backend rejection. Nothing in it becomes inaccurate as a result of this change, so there's no draft PR to open. @cretadn22, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
Explanation of Change
CSV company-card transaction imports fail with the generic error "Please ensure all fields are filled out correctly and try again…" when re-importing into a pre-existing (legacy) feed. The failure is unrelated to the CSV contents — the request is rejected before any row is parsed.
Root cause:
importCSVCompanyCardssendsinstanceIDin the requestsettingspayload.existingInstanceIDis typed as astring, but for legacy feeds the value stored in Onyx/NVP can actually be a number at runtime. Because the code usedexistingInstanceID ?? Date.now().toString(),??only substitutes onnull/undefined, so a numeric value passes straight through andJSON.stringifyemits it as a bare number ("instanceID":1745351414130instead of"instanceID":"1745351414130"). The backend readsinstanceIDwith a string-only accessor, treats the numeric value as empty, and rejects the request as missing a required field.This is why the bug only reproduces on legacy feeds with a stored numeric
instanceID: the Add New Card wizard has noexistingInstanceIDand falls back toDate.now().toString(), a genuine string the backend accepts — so clean test accounts never hit it.Fix: coerce
existingInstanceIDto a string before it is sent, so a numeric legacy value is serialized as a string. No data migration required.Fixed Issues
$ #94178
PROPOSAL: #94178 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review".
// Please describe what tests you performed that validate your change worked.
Offline tests
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas they need to test for regressions.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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