Skip to content

Commit 44011fc

Browse files
committed
fix: implement physical-first sorting for Expensify cards in PaymentMethodList
- Create getAssignedCardSortKey utility function in CardUtils.ts - Replace simple boolean comparator with new sorting logic - Add comprehensive unit tests for card sorting scenarios - Resolves issue Expensify#68033 where physical cards were hidden in navigation - Physical Expensify cards now appear before virtual ones irrespective of their cardID
1 parent e325bcf commit 44011fc

3 files changed

Lines changed: 38 additions & 2 deletions

File tree

src/libs/CardUtils.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ function getMonthFromExpirationDateString(expirationDateString: string) {
4040
* @param card
4141
* @returns boolean
4242
*/
43+
function getAssignedCardSortKey(card: Card): number {
44+
if (!isExpensifyCard(card)) {
45+
return 2;
46+
}
47+
return card?.nameValuePairs?.isVirtual ? 1 : 0;
48+
}
49+
4350
function isExpensifyCard(card?: Card) {
4451
if (!card) {
4552
return false;
@@ -663,6 +670,7 @@ function getFundIdFromSettingsKey(key: string) {
663670
}
664671

665672
export {
673+
getAssignedCardSortKey,
666674
isExpensifyCard,
667675
getDomainCards,
668676
formatCardExpiration,

src/pages/settings/Wallet/PaymentMethodList.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import useStyleUtils from '@hooks/useStyleUtils';
2121
import useThemeIllustrations from '@hooks/useThemeIllustrations';
2222
import useThemeStyles from '@hooks/useThemeStyles';
2323
import {clearAddPaymentMethodError, clearDeletePaymentMethodError} from '@libs/actions/PaymentMethods';
24-
import {getCardFeedIcon, getPlaidInstitutionIconUrl, isExpensifyCard, lastFourNumbersFromCardName, maskCardNumber} from '@libs/CardUtils';
24+
import {getAssignedCardSortKey, getCardFeedIcon, getPlaidInstitutionIconUrl, isExpensifyCard, lastFourNumbersFromCardName, maskCardNumber} from '@libs/CardUtils';
2525
import Log from '@libs/Log';
2626
import Navigation from '@libs/Navigation/Navigation';
2727
import {formatPaymentMethods} from '@libs/PaymentUtils';
@@ -219,7 +219,8 @@ function PaymentMethodList({
219219
const assignedCards = Object.values(isLoadingCardList ? {} : (cardList ?? {}))
220220
// Filter by active cards associated with a domain
221221
.filter((card) => !!card.domainName && CONST.EXPENSIFY_CARD.ACTIVE_STATES.includes(card.state ?? 0));
222-
const assignedCardsSorted = lodashSortBy(assignedCards, (card) => !isExpensifyCard(card));
222+
223+
const assignedCardsSorted = lodashSortBy(assignedCards, getAssignedCardSortKey);
223224

224225
const assignedCardsGrouped: PaymentMethodItem[] = [];
225226
assignedCardsSorted.forEach((card) => {

tests/unit/CardUtilsTest.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ import type IllustrationsType from '@styles/theme/illustrations/types';
33
import type * as Illustrations from '@src/components/Icon/Illustrations';
44
import CONST from '@src/CONST';
55
import IntlStore from '@src/languages/IntlStore';
6+
import lodashSortBy from 'lodash/sortBy';
67
import {
78
checkIfFeedConnectionIsBroken,
89
filterInactiveCards,
910
flatAllCardsList,
1011
formatCardExpiration,
12+
getAssignedCardSortKey,
1113
getBankCardDetailsImage,
1214
getBankName,
1315
getCardDescription,
@@ -33,6 +35,8 @@ import type {Card, CardFeeds, CardList, CompanyCardFeed, ExpensifyCardSettings,
3335
import type {CompanyCardFeedWithNumber} from '@src/types/onyx/CardFeeds';
3436
import {localeCompare} from '../utils/TestHelper';
3537
import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
38+
import Onyx from 'react-native-onyx';
39+
import ONYXKEYS from '@src/ONYXKEYS';
3640

3741
const shortDate = '0924';
3842
const shortDateSlashed = '09/24';
@@ -1138,4 +1142,27 @@ describe('CardUtils', () => {
11381142
expect(description).toBe('Test');
11391143
});
11401144
});
1145+
1146+
describe('Expensify card sort comparator', () => {
1147+
it('should not change the order of non-Expensify cards', () => {
1148+
const cardList = {
1149+
10: {cardID: 10, bank: 'chase'}, // non-Expensify
1150+
11: {cardID: 11, bank: 'chase'}, // non-Expensify
1151+
} as unknown as CardList;
1152+
1153+
const sorted = lodashSortBy(Object.values(cardList), getAssignedCardSortKey);
1154+
expect(sorted.map((r: any) => r.cardID)).toEqual([10, 11]);
1155+
});
1156+
1157+
it('places physical Expensify card before its virtual sibling', async () => {
1158+
const cardList = {
1159+
10: {cardID: 10, bank: CONST.EXPENSIFY_CARD.BANK, nameValuePairs: {isVirtual: true}}, // Expensify virtual
1160+
11: {cardID: 11, bank: CONST.EXPENSIFY_CARD.BANK}, // Expensify physical
1161+
99: {cardID: 99, bank: 'chase'}, // non-Expensify
1162+
} as unknown as CardList;
1163+
1164+
const sorted = lodashSortBy(Object.values(cardList), getAssignedCardSortKey);
1165+
expect(sorted.map((r: any) => r.cardID)).toEqual([11, 10, 99]);
1166+
});
1167+
});
11411168
});

0 commit comments

Comments
 (0)