Skip to content

Commit ed69907

Browse files
elirangoshenclaude
andcommitted
Address review feedback on PR #88431
- Skip auto-sync effect in useFixPersonalCardConnection when isPlaid is true. The Plaid flow drives its own SyncCard from onSuccess; the hook was firing a duplicate call because the explicit update optimistically flipped isCardBroken before the screen unmounted. - Add unit tests covering the approved/settled guards added to isMarkAsCashAction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5668c35 commit ed69907

2 files changed

Lines changed: 80 additions & 2 deletions

File tree

src/pages/settings/Wallet/PersonalCards/FixPersonalCardConnectionPage/useFixPersonalCardConnection.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ function useFixPersonalCardConnection(cardID: string) {
2626
if (isLoadingOnyxValue(cardListMetadata)) {
2727
return;
2828
}
29-
if (!card || isCardBroken) {
29+
// The Plaid flow drives its own SyncCard from onSuccess, so skip the auto-sync here
30+
// to avoid a duplicate call when our optimistic update flips isCardBroken before unmount.
31+
if (!card || isCardBroken || isPlaid) {
3032
return;
3133
}
3234
updatePersonalCardConnection(card.cardID.toString(), card.lastScrapeResult);
3335
Navigation.goBack(ROUTES.SETTINGS_WALLET_PERSONAL_CARD_DETAILS.getRoute(cardID));
34-
}, [isCardBroken, card, cardID, cardListMetadata]);
36+
}, [isCardBroken, card, cardID, cardListMetadata, isPlaid]);
3537

3638
return {card, bankDisplayName, url, isCardBroken, isOffline, isPlaid, country};
3739
}

tests/unit/ReportPrimaryActionUtilsTest.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -958,6 +958,82 @@ describe('getPrimaryAction', () => {
958958
).toBe(CONST.REPORT.PRIMARY_ACTIONS.MARK_AS_CASH);
959959
});
960960

961+
it('should not return MARK AS CASH for broken connection on approved report', async () => {
962+
const report = {
963+
reportID: REPORT_ID,
964+
ownerAccountID: CURRENT_USER_ACCOUNT_ID,
965+
stateNum: CONST.REPORT.STATE_NUM.APPROVED,
966+
statusNum: CONST.REPORT.STATUS_NUM.APPROVED,
967+
type: CONST.REPORT.TYPE.EXPENSE,
968+
} as unknown as Report;
969+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
970+
const policy = {};
971+
const TRANSACTION_ID = 'TRANSACTION_ID';
972+
973+
const transaction = {
974+
transactionID: TRANSACTION_ID,
975+
} as unknown as Transaction;
976+
977+
const violation = {
978+
name: CONST.VIOLATIONS.RTER,
979+
data: {
980+
rterType: CONST.RTER_VIOLATION_TYPES.BROKEN_CARD_CONNECTION,
981+
},
982+
} as unknown as TransactionViolation;
983+
984+
expect(
985+
getReportPrimaryAction({
986+
currentUserLogin: CURRENT_USER_EMAIL,
987+
currentUserAccountID: CURRENT_USER_ACCOUNT_ID,
988+
report,
989+
chatReport,
990+
reportTransactions: [transaction],
991+
violations: {[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`]: [violation]},
992+
bankAccountList: {},
993+
policy: policy as Policy,
994+
isChatReportArchived: false,
995+
}),
996+
).not.toBe(CONST.REPORT.PRIMARY_ACTIONS.MARK_AS_CASH);
997+
});
998+
999+
it('should not return MARK AS CASH for broken connection on settled (reimbursed) report', async () => {
1000+
const report = {
1001+
reportID: REPORT_ID,
1002+
ownerAccountID: CURRENT_USER_ACCOUNT_ID,
1003+
stateNum: CONST.REPORT.STATE_NUM.APPROVED,
1004+
statusNum: CONST.REPORT.STATUS_NUM.REIMBURSED,
1005+
type: CONST.REPORT.TYPE.EXPENSE,
1006+
} as unknown as Report;
1007+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
1008+
const policy = {};
1009+
const TRANSACTION_ID = 'TRANSACTION_ID';
1010+
1011+
const transaction = {
1012+
transactionID: TRANSACTION_ID,
1013+
} as unknown as Transaction;
1014+
1015+
const violation = {
1016+
name: CONST.VIOLATIONS.RTER,
1017+
data: {
1018+
rterType: CONST.RTER_VIOLATION_TYPES.BROKEN_CARD_CONNECTION,
1019+
},
1020+
} as unknown as TransactionViolation;
1021+
1022+
expect(
1023+
getReportPrimaryAction({
1024+
currentUserLogin: CURRENT_USER_EMAIL,
1025+
currentUserAccountID: CURRENT_USER_ACCOUNT_ID,
1026+
report,
1027+
chatReport,
1028+
reportTransactions: [transaction],
1029+
violations: {[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`]: [violation]},
1030+
bankAccountList: {},
1031+
policy: policy as Policy,
1032+
isChatReportArchived: false,
1033+
}),
1034+
).not.toBe(CONST.REPORT.PRIMARY_ACTIONS.MARK_AS_CASH);
1035+
});
1036+
9611037
it('should not return SUBMIT for expense report with smartscan failed violation', async () => {
9621038
const report = {
9631039
reportID: REPORT_ID,

0 commit comments

Comments
 (0)