From 3eb951b03744dd0b75d8bd3d2f9a5a8dd51a1f98 Mon Sep 17 00:00:00 2001 From: Wildan Muhlis Date: Fri, 17 Jul 2026 09:07:53 +0700 Subject: [PATCH 1/2] fix: stop the Account/Wallet RBR for personal cards past the 90-day grace period The 90-day grace correctly removed the home task, but the Account indicator and the Wallet row red dot stayed lit for personal cards. Both are driven by the card's own error, which the issue requires us to keep so the card stays fixable: - getShouldShowRBR short-circuits on hasFeedErrors before the grace check, so a past-grace card's error still forced the RBR. - hasPaymentMethodError counts any personal card with errors, and it feeds both the Account indicator (useAccountIndicatorChecks) and the Wallet row (InitialSettingsPage). Gate both on isBrokenConnectionPastDismissThreshold, leaving card.errors untouched so the Wallet-page card row keeps its error and Fix card action. Adds regression tests for a past-grace card that still carries the error (the case the original fixtures missed), plus guards that within-grace and unrelated errors still show. --- .../OnyxDerived/configs/cardFeedErrors.ts | 12 +++-- src/libs/actions/PaymentMethods.ts | 8 ++- tests/unit/CardFeedErrorsDerivedValueTest.ts | 52 +++++++++++++++++++ 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/libs/actions/OnyxDerived/configs/cardFeedErrors.ts b/src/libs/actions/OnyxDerived/configs/cardFeedErrors.ts index dcc020035351..0861b6a21e3c 100644 --- a/src/libs/actions/OnyxDerived/configs/cardFeedErrors.ts +++ b/src/libs/actions/OnyxDerived/configs/cardFeedErrors.ts @@ -53,7 +53,13 @@ export default createOnyxDerivedValueConfig({ const personalCardsWithBrokenConnection: Record = {}; function addErrorsForPersonalCard(card: Card) { - const hasCardErrors = !isEmptyObject(card.errors) || !isEmptyObject(card.errorFields); + // Stop surfacing the broken connection (task + RBR) once it has been unresolved past the + // grace period; the underlying error on the card is kept so the user can still fix it. + const isPastDismissThreshold = isBrokenConnectionPastDismissThreshold(card); + // The broken connection is reported as an error on the card itself, so past the grace period it must not + // count towards the RBR either — otherwise keeping the error (so the card stays fixable) would keep the + // Account/Wallet red dot lit forever. + const hasCardErrors = (!isEmptyObject(card.errors) || !isEmptyObject(card.errorFields)) && !isPastDismissThreshold; const cardErrors = { ...(hasCardErrors ? { @@ -66,9 +72,7 @@ export default createOnyxDerivedValueConfig({ : {}), } as Record; - // Stop surfacing the broken connection (task + RBR) once it has been unresolved past the - // grace period; the underlying error on the card is kept so the user can still fix it. - const isFeedConnectionBroken = isCardConnectionBroken(card) && !isBrokenConnectionPastDismissThreshold(card); + const isFeedConnectionBroken = isCardConnectionBroken(card) && !isPastDismissThreshold; // Track personal cards with broken feed connection if (isFeedConnectionBroken) { personalCardsWithBrokenConnection[card.cardID] = card; diff --git a/src/libs/actions/PaymentMethods.ts b/src/libs/actions/PaymentMethods.ts index 4703b305dfd9..c4792ce4475f 100644 --- a/src/libs/actions/PaymentMethods.ts +++ b/src/libs/actions/PaymentMethods.ts @@ -420,7 +420,8 @@ function dismissSuccessfulTransferBalancePage() { /** * Looks through each payment method to see if there is an existing error. * When session and policies are provided, card list errors are only counted if the current user - * is a member of that card's workspace (for company cards) or always counted for personal cards. + * is a member of that card's workspace (for company cards), or counted for personal cards unless the + * card's broken connection has been unresolved past the dismiss grace period. * Only cards with non-empty errors are considered (error cards). */ function hasPaymentMethodError( @@ -437,7 +438,10 @@ function hasPaymentMethodError( const policyList = Object.values(policies ?? {}).filter(Boolean); const hasRelevantCardError = cardsWithErrors.some((card) => { if (CardUtils.isPersonalCard(card)) { - return true; + // Once a personal card's broken connection has been unresolved past the grace period we stop leading the user + // to it, so it must not light the Account/Wallet RBR. The error itself is kept on the card so it can still be + // fixed from the Wallet page. + return !CardUtils.isBrokenConnectionPastDismissThreshold(card); } const workspaceAccountID = Number(card?.fundID); const policy = policyList.find((p) => p?.policyAccountID === workspaceAccountID); diff --git a/tests/unit/CardFeedErrorsDerivedValueTest.ts b/tests/unit/CardFeedErrorsDerivedValueTest.ts index 322209ae7f60..42afe62dbedd 100644 --- a/tests/unit/CardFeedErrorsDerivedValueTest.ts +++ b/tests/unit/CardFeedErrorsDerivedValueTest.ts @@ -211,6 +211,58 @@ describe('CardFeedErrors Derived Value', () => { expect(result.personalCard.isFeedConnectionBroken).toBe(false); expect(result.personalCardsWithBrokenConnection).toEqual({}); }); + + // A real broken personal card also carries the error on the card itself, which we intentionally keep so the + // card stays fixable. That error used to short-circuit getShouldShowRBR, leaving the Account/Wallet red dot + // lit past the grace period even though the home task was correctly removed. + it('should NOT show the RBR for a personal card past the grace period that still carries the broken-connection error', () => { + const card = createCard({ + cardID: CARD_IDS.card1, + lastScrapeResult: 403, // Broken connection + lastScrape: '2020-01-01 00:00:00', // Last successful scrape is well beyond the grace period + errors: {connectionError: 'Your card connection is broken.'}, // Kept so the card can still be fixed + }); + + const globalCardList: CardList = {card1: card}; + + const result = cardFeedErrorsConfig.compute([globalCardList, {}, {}, undefined], DERIVED_VALUE_CONTEXT); + + expect(result.personalCard.shouldShowRBR).toBe(false); + expect(result.personalCard.isFeedConnectionBroken).toBe(false); + }); + + it('should still show the RBR for a broken personal card within the grace period that carries the error', () => { + // Broken only since yesterday, so it is still well inside the grace period and must keep prompting. + const recentScrape = new Date(Date.now() - 24 * 60 * 60 * 1000).toISOString().slice(0, 19).replace('T', ' '); + const card = createCard({ + cardID: CARD_IDS.card1, + lastScrapeResult: 403, + lastScrape: recentScrape, + errors: {connectionError: 'Your card connection is broken.'}, + }); + + const globalCardList: CardList = {card1: card}; + + const result = cardFeedErrorsConfig.compute([globalCardList, {}, {}, undefined], DERIVED_VALUE_CONTEXT); + + expect(result.personalCard.shouldShowRBR).toBe(true); + expect(result.personalCard.isFeedConnectionBroken).toBe(true); + }); + + it('should still show the RBR for a personal card with an unrelated error and a healthy connection', () => { + const card = createCard({ + cardID: CARD_IDS.card1, + lastScrapeResult: 200, // Not a broken connection, so the grace period does not apply + lastScrape: '2020-01-01 00:00:00', + errors: {unrelatedError: 'Something else went wrong.'}, + }); + + const globalCardList: CardList = {card1: card}; + + const result = cardFeedErrorsConfig.compute([globalCardList, {}, {}, undefined], DERIVED_VALUE_CONTEXT); + + expect(result.personalCard.shouldShowRBR).toBe(true); + }); }); describe('processing cards from workspace cards collection', () => { From 4c2ad732aa38f06ef3861e6bcc4b96940fe642df Mon Sep 17 00:00:00 2001 From: Wildan Muhlis Date: Sat, 18 Jul 2026 14:40:07 +0700 Subject: [PATCH 2/2] fix: suppress only the broken-connection error past the grace period MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Codex review. The previous change dropped ALL of a stale-broken personal card's errors/errorFields, so an unrelated actionable error also lost its RBR — a failed reimbursable/start-date update (another errorFields entry) or a failed remove (card.errors). Now only errorFields.lastScrape (the broken-connection error the card detail page reads) is excluded from the RBR once past the grace period; every other error is kept. Reverts the over-broad hasPaymentMethodError gate, since card.errors on a personal card is an actionable error, not the connection error. Adds regression tests for a past-grace card that also carries an unrelated field error and one that carries a card-level error. --- .../OnyxDerived/configs/cardFeedErrors.ts | 14 ++--- src/libs/actions/PaymentMethods.ts | 8 +-- tests/unit/CardFeedErrorsDerivedValueTest.ts | 51 ++++++++++++++++--- 3 files changed, 54 insertions(+), 19 deletions(-) diff --git a/src/libs/actions/OnyxDerived/configs/cardFeedErrors.ts b/src/libs/actions/OnyxDerived/configs/cardFeedErrors.ts index 0861b6a21e3c..dbef7cf37bd3 100644 --- a/src/libs/actions/OnyxDerived/configs/cardFeedErrors.ts +++ b/src/libs/actions/OnyxDerived/configs/cardFeedErrors.ts @@ -53,13 +53,15 @@ export default createOnyxDerivedValueConfig({ const personalCardsWithBrokenConnection: Record = {}; function addErrorsForPersonalCard(card: Card) { - // Stop surfacing the broken connection (task + RBR) once it has been unresolved past the - // grace period; the underlying error on the card is kept so the user can still fix it. + // Once the broken connection is unresolved past the grace period we stop leading the user to it: the + // time-sensitive task and the RBR are removed. The broken-connection error surfaces via + // `errorFields.lastScrape`, so drop only that field when deciding the RBR — any other actionable error on + // the card (a failed reimbursable/start-date update in another `errorFields` entry, a failed remove in + // `card.errors`, etc.) is kept so it still surfaces. The error itself stays on the card so it's fixable. const isPastDismissThreshold = isBrokenConnectionPastDismissThreshold(card); - // The broken connection is reported as an error on the card itself, so past the grace period it must not - // count towards the RBR either — otherwise keeping the error (so the card stays fixable) would keep the - // Account/Wallet red dot lit forever. - const hasCardErrors = (!isEmptyObject(card.errors) || !isEmptyObject(card.errorFields)) && !isPastDismissThreshold; + const errorFieldsForRBR = + isPastDismissThreshold && card.errorFields ? Object.fromEntries(Object.entries(card.errorFields).filter(([field]) => field !== 'lastScrape')) : card.errorFields; + const hasCardErrors = !isEmptyObject(card.errors) || !isEmptyObject(errorFieldsForRBR); const cardErrors = { ...(hasCardErrors ? { diff --git a/src/libs/actions/PaymentMethods.ts b/src/libs/actions/PaymentMethods.ts index c4792ce4475f..4703b305dfd9 100644 --- a/src/libs/actions/PaymentMethods.ts +++ b/src/libs/actions/PaymentMethods.ts @@ -420,8 +420,7 @@ function dismissSuccessfulTransferBalancePage() { /** * Looks through each payment method to see if there is an existing error. * When session and policies are provided, card list errors are only counted if the current user - * is a member of that card's workspace (for company cards), or counted for personal cards unless the - * card's broken connection has been unresolved past the dismiss grace period. + * is a member of that card's workspace (for company cards) or always counted for personal cards. * Only cards with non-empty errors are considered (error cards). */ function hasPaymentMethodError( @@ -438,10 +437,7 @@ function hasPaymentMethodError( const policyList = Object.values(policies ?? {}).filter(Boolean); const hasRelevantCardError = cardsWithErrors.some((card) => { if (CardUtils.isPersonalCard(card)) { - // Once a personal card's broken connection has been unresolved past the grace period we stop leading the user - // to it, so it must not light the Account/Wallet RBR. The error itself is kept on the card so it can still be - // fixed from the Wallet page. - return !CardUtils.isBrokenConnectionPastDismissThreshold(card); + return true; } const workspaceAccountID = Number(card?.fundID); const policy = policyList.find((p) => p?.policyAccountID === workspaceAccountID); diff --git a/tests/unit/CardFeedErrorsDerivedValueTest.ts b/tests/unit/CardFeedErrorsDerivedValueTest.ts index 42afe62dbedd..6dd517591a4a 100644 --- a/tests/unit/CardFeedErrorsDerivedValueTest.ts +++ b/tests/unit/CardFeedErrorsDerivedValueTest.ts @@ -212,15 +212,15 @@ describe('CardFeedErrors Derived Value', () => { expect(result.personalCardsWithBrokenConnection).toEqual({}); }); - // A real broken personal card also carries the error on the card itself, which we intentionally keep so the - // card stays fixable. That error used to short-circuit getShouldShowRBR, leaving the Account/Wallet red dot - // lit past the grace period even though the home task was correctly removed. - it('should NOT show the RBR for a personal card past the grace period that still carries the broken-connection error', () => { + // A broken personal card surfaces its connection error via errorFields.lastScrape (this is what the card + // detail page reads). Past the grace period we stop leading the user to it, so that specific error must not + // light the Account/Wallet RBR — while the error itself is kept so the card can still be fixed. + it('should NOT show the RBR for a personal card past the grace period whose only error is the broken connection', () => { const card = createCard({ cardID: CARD_IDS.card1, lastScrapeResult: 403, // Broken connection lastScrape: '2020-01-01 00:00:00', // Last successful scrape is well beyond the grace period - errors: {connectionError: 'Your card connection is broken.'}, // Kept so the card can still be fixed + errorFields: {lastScrape: {error: 'Your card connection is broken.'}}, // Kept so the card can still be fixed }); const globalCardList: CardList = {card1: card}; @@ -231,14 +231,14 @@ describe('CardFeedErrors Derived Value', () => { expect(result.personalCard.isFeedConnectionBroken).toBe(false); }); - it('should still show the RBR for a broken personal card within the grace period that carries the error', () => { + it('should still show the RBR for a broken personal card within the grace period', () => { // Broken only since yesterday, so it is still well inside the grace period and must keep prompting. const recentScrape = new Date(Date.now() - 24 * 60 * 60 * 1000).toISOString().slice(0, 19).replace('T', ' '); const card = createCard({ cardID: CARD_IDS.card1, lastScrapeResult: 403, lastScrape: recentScrape, - errors: {connectionError: 'Your card connection is broken.'}, + errorFields: {lastScrape: {error: 'Your card connection is broken.'}}, }); const globalCardList: CardList = {card1: card}; @@ -249,6 +249,43 @@ describe('CardFeedErrors Derived Value', () => { expect(result.personalCard.isFeedConnectionBroken).toBe(true); }); + // Past the grace period we suppress ONLY the broken-connection error, not other actionable errors on the same + // card — e.g. a failed reimbursable/start-date update, which lands in a different errorFields entry. + it('should still show the RBR for a past-grace broken card that also has an unrelated field error', () => { + const card = createCard({ + cardID: CARD_IDS.card1, + lastScrapeResult: 403, + lastScrape: '2020-01-01 00:00:00', + errorFields: { + lastScrape: {error: 'Your card connection is broken.'}, + reimbursable: {error: 'Failed to update the reimbursable setting.'}, + }, + }); + + const globalCardList: CardList = {card1: card}; + + const result = cardFeedErrorsConfig.compute([globalCardList, {}, {}, undefined], DERIVED_VALUE_CONTEXT); + + expect(result.personalCard.shouldShowRBR).toBe(true); + }); + + // ...nor an error written to card.errors, e.g. a failed remove of the card. + it('should still show the RBR for a past-grace broken card that also has a card-level error', () => { + const card = createCard({ + cardID: CARD_IDS.card1, + lastScrapeResult: 403, + lastScrape: '2020-01-01 00:00:00', + errorFields: {lastScrape: {error: 'Your card connection is broken.'}}, + errors: {removeFailed: 'Failed to remove the card.'}, + }); + + const globalCardList: CardList = {card1: card}; + + const result = cardFeedErrorsConfig.compute([globalCardList, {}, {}, undefined], DERIVED_VALUE_CONTEXT); + + expect(result.personalCard.shouldShowRBR).toBe(true); + }); + it('should still show the RBR for a personal card with an unrelated error and a healthy connection', () => { const card = createCard({ cardID: CARD_IDS.card1,