Fix duplicate card entries and missing assigned cards for commercial feeds#82788
Fix duplicate card entries and missing assigned cards for commercial feeds#82788carlosmiceli merged 15 commits intoExpensify:mainfrom
Conversation
|
@linhvovan29546 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] |
|
@MelvinBot Please review this PR. |
Code ReviewOverall, the approach is sound — the cascading lookup in A few observations: 1. Potential issue:
|
…n of card names and encrypted numbers
…me and encrypted number
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
|
@fedirjh Could you please add the screenshot to the PR description? |
I think the test may not be accurate; the backend should return the same suffix for all cards. So other cards should be displayed with the last 5 digits as well.
This is expected as well, we have seen same behaviour in https://github.com/Expensify/Expensify/issues/600254#issuecomment-3917763404. |
|
I'm blocked by this bug. I'll test the PR once it's resolved. |
|
Bug: App crashes when opening Company Card (crash after commit 5f39330) Precondition: My workspace contains one company card. |
Fixed with 32c668a |
Reviewer Checklist
Screenshots/Videos |
|
🎯 @linhvovan29546, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #83135. |
|
🚧 @carlosmiceli 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/carlosmiceli in version: 9.3.25-0 🚀
|
|
@fedirjh @carlosmiceli @joekaufmanexpensify Can this one please be tested internally, We don't have access to CDF commercial feeds |
|
I can QA it with the affected customer's account 👍 |
|
Not working as expected. Discussing. |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.25-13 🚀
|
|
@m-natarajan mmm, why was this deployed to production without QA being complete? |
|
@m-natarajan any updates regarding @carlosmiceli's latest comment? @carlosmiceli confirming that the issue is being fixed here now? |
|
@brianlee-expensify yes, just waiting for the PR to be reviewed, just bumped the C+. |
|
@carlosmiceli @brianlee-expensify Not sure, was waiting for answer #82788 (comment) |
|
I checked this off the checklist since Joe had QA'd it and opened a discussion for what wasn't working. Do you think I should have done differently? |










Explanation of Change
Duplicate entries for CDF commercial feeds — Commercial feeds (CDF) can have outdated card names (e.g.,
XXXXXXXXXXX1234) whilecardListhas the up-to-date name (e.g.,111222XXXX31234). Since neither the name nor theencryptedCardNumbermatched, the same card appeared twice.How it's fixed:
Added
resolveCardListEntrywith a cascading lookup to link assigned cards to theircardListcounterpart:Fixed Issues
$ https://github.com/Expensify/Expensify/issues/600254
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Before the fix
After the fix
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari