fix: company card list order changes everytime while assigning card#65233
fix: company card list order changes everytime while assigning card#65233nkdengineer wants to merge 5 commits intoExpensify:mainfrom
Conversation
|
@hoangzinh 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] |
|
@hoangzinh As far as I know, contributors currently can't add more than 1 company card, so I'm not able to take records across different platforms. Do you know any way I could still do it? |
|
Can you share a recording about it? Originally, I thought we can delete previous and re-create new on other platforms |
|
@hoangzinh To reproduce the bug in OP, we need to have at least 2 cards in the company card, but currently I can only create 1 card. |
|
@nkdengineer I see. Then, can you mock the API OpenPolicyCompanyCardsFeed? I mean mock successData of App/src/libs/actions/CompanyCards.ts Lines 753 to 761 in 5b2b9c6 |
|
@hoangzinh I tried mock data but still can't follow steps in OP, how about you? Should we ask the QA team to test here? |
|
I'm unable to reproduce neither. Do you know why it can't now? |
|
An interesting thing is when I unassigned a card, the order is changed Screen.Recording.2025-07-08.at.23.38.19.mov |
@hoangzinh is that after applying the solution in this PR? |
|
@hoangzinh Can you share with me an account with multiple company cards so I can investigate further? |
|
I'm afraid that I can't share @nkdengineer. Hmm, let me think what is our next move here. |
|
@nkdengineer I confirmed that the original issue is still reproducible in main branch. Btw, can you describe what is your issue with mock data #65233 (comment)? Then I will see how we go with it |
|
@hoangzinh i'll try again and give an update soon |
|
Thank you @nkdengineer. Keep me updated on how it is going. It's part of checklist, so we're hard to skip. |
|
@hoangzinh I'm still trying to get videos here, will give an update today |
|
@hoangzinh i added the records and some checks failing is not related to this PR |
|
Yep, the above issue is reported here https://expensify.slack.com/archives/C01GTK53T8Q/p1753070902108769 |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-07-21.at.18.08.06.android.movAndroid: mWeb ChromeScreen.Recording.2025-07-21.at.17.17.03.android.chrome.moviOS: HybridAppScreen.Recording.2025-07-21.at.17.22.34.ios.moviOS: mWeb SafariScreen.Recording.2025-07-21.at.18.01.51.ios.safari.movMacOS: Chrome / SafariScreen.Recording.2025-07-21.at.17.13.03.web.movMacOS: DesktopScreen.Recording.2025-07-21.at.17.14.09.desktop.mov |
|
The PR was merged. @nkdengineer can you try to merge the latest main? |
|
@hoangzinh i merged |
|
@nkdengineer Thank you. Can you try to solve another conflict? |
|
@hoangzinh looks like this PR is fixed this issue, should we close this PR? |
|
Agreed. I couldn't reproduce the original issue in latest main branch anymore. Can you post it in our GH issue? @nkdengineer |
Explanation of Change
Fixed Issues
$ #64254
PROPOSAL: #64254 (comment)
Tests
Offline tests
Same as tests
QA Steps
Same as tests
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))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
android.mov
Android: mWeb Chrome
android-mweb.mov
iOS: Native
ios.mov
iOS: mWeb Safari
ios-mweb.mov
MacOS: Chrome / Safari
wb.mov
MacOS: Desktop
desktop.mov