[Company Card] Update RBR logic of admin/member by hungvu193 · Pull Request #83159 · Expensify/App

hungvu193 · 2026-02-22T15:09:40Z

Correct the logic of RBR on the Wallet page. Per #78305 (comment):

The RBR should direct workspace admins with a broken connection to the Company cards page, as that's where they fix it. Workspace admins with an assigned card should also see the error on that card on Wallet when there's a broken connection, but no RBR leading there. Non-admin employees with an assigned card should see an RBR leading them to the Wallet error, which tells them to contact an admin to fix the broken connection.

Explanation of Change

Fixed Issues

$ #81724
PROPOSAL: N/A

Tests

Same as QA Tests.

Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

Login as User A > Create a workspace with at least one company card.
Invite User B to the workspace, change User B role to admin and assign them a card.
Break the card connection. (Check this slack thread for more ℹ️ ).
Login with User B > go to workspace that User A has just invited > Company Card > Verify there is RBR on company card page.
Go to Account > Wallet > Verify that there is no RBR display on Wallet page.
Verify that there is an error under the broken company card.
Login with User A again > Change the role of User B to Member.
Login with User B > Go to Account > Wallet.
Verify that there's RBR display on the Wallet page and Account tab.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

MacOS: Chrome / Safari

Screen.Recording.2026-02-23.at.13.53.00.mov

codecov · 2026-02-22T15:15:31Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
src/components/Navigation/DebugTabView.tsx	`35.22% <100.00%> (ø)`
src/hooks/useNavigationTabBarIndicatorChecks.ts	`100.00% <100.00%> (ø)`
src/pages/settings/InitialSettingsPage.tsx	`0.00% <0.00%> (ø)`
src/libs/actions/PaymentMethods.ts	`26.95% <58.33%> (+1.48%)`	⬆️
... and 11 files with indirect coverage changes

hungvu193 · 2026-02-22T16:20:36Z

@MelvinBot please review it in the meantime

MelvinBot · 2026-02-22T16:25:05Z

Code Review

Overall the approach is sound — filtering company card errors by workspace membership in hasPaymentMethodError so admins only see wallet RBR for cards on workspaces they're a member of (not admin of), and routing admin-level feed errors through the policy checks path instead. All CI checks pass.

A few observations:

1. Redundant null check (minor)

In src/libs/actions/PaymentMethods.ts:442-444, there's a redundant guard:

if (!currentUserLogin || !policies) {
    return true;
}

This code is inside the .some() callback, but currentUserLogin and policies are already guaranteed to be truthy by the early return at lines 431-433. This is dead code — it will never be reached. Consider removing it to keep the logic clear.

2. Fallback behavior when session/policies not loaded

When session or policies are not yet available (lines 431-433), the function returns hasBankOrFundError only — card errors are completely ignored. The old behavior counted all card errors unconditionally. This means during initial app load (before session/policies are hydrated from Onyx), card errors won't show in the wallet RBR temporarily.

This is probably fine in practice since session loads early, but worth being aware of. If you want to preserve the old behavior as a fallback, you could include all card errors when session/policies aren't available:

if (!currentUserLogin || !policies) {
    const hasAnyCardError = Object.values(cardList ?? {}).some(
        (card) => Object.keys(card?.errors ?? {}).length > 0
    );
    return hasBankOrFundError || hasAnyCardError;
}

Up to you whether this edge case matters.

3. `Number.isNaN(workspaceAccountID)` fallback returns `true`

At line 446-448, if card.fundID converts to NaN, the card error is counted. This is a safe conservative default (show the error rather than hide it), but it might be worth adding a brief comment explaining why this defaults to true — i.e., "if we can't determine the workspace, show the error to be safe."

4. All checks pass ✓

ESLint, TypeScript, Prettier, Jest, Perf tests, CLA, Storybook — all green.

Summary

The fix correctly addresses the issue: admins no longer see competing RBR paths for broken company card feeds. The fundID → workspaceAccountID mapping is consistent with established patterns in the codebase (TimeSensitiveSection, OnyxDerived/configs/cardFeedErrors.ts, CardFeedUtils). The isPersonalCard check and isPolicyMember usage are both appropriate.

The main actionable item is removing the redundant null check at lines 442-444.

hungvu193 · 2026-02-23T06:21:58Z

Note:
There are 2 lines of errors, it's on main too.

hungvu193 · 2026-02-23T07:16:17Z

Member:

Display RBR under Wallet page, Account tab indicator.
Error under the broken connection card.

Admin:

No RBR under Wallet page, Account tab indicator.
Only display error under the broken connection card.

Member	Admin

melvin-bot · 2026-02-23T07:17:56Z

@ikevin127 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]

github-actions · 2026-02-23T07:24:33Z

Code Review - PR #83159

Issue 1: `fundID` may not be the correct field for workspace matching (PaymentMethods.ts:440)

card.fundID is typed as a string and represents a payment funding source identifier. The new code does Number(card?.fundID) and matches it against policy.workspaceAccountID (a number). Are these actually the same value for company cards? If not, this lookup would never match, and company card errors would always be suppressed for non-personal cards.

Please verify this mapping is correct or use the appropriate card field that maps to the workspace.

Issue 2: `isPolicyUser` only matches the "user" role (PaymentMethods.ts:442)

isPolicyUser() returns true only when getPolicyRole() === CONST.POLICY.ROLE.USER. This excludes auditors and any other non-admin roles. The PR description says "Non-admin employees with an assigned card should see an RBR" — if auditors should also see RBR, this check is too narrow. Consider using a negated admin check instead (e.g., \!isPolicyAdmin(policy, currentUserLogin)) to capture all non-admin members.

Issue 3: `allCards` differs between callers (InitialSettingsPage.tsx:163)

In InitialSettingsPage.tsx, allCards comes from useNonPersonalCardList() which filters out personal cards. But inside hasPaymentMethodError, there is a CardUtils.isPersonalCard(card) check — that branch will never execute when called from InitialSettingsPage because personal cards are already filtered out.

In contrast, useNavigationTabBarIndicatorChecks.ts passes the full allCards (including personal cards) so the check works there. This means the RBR behavior for personal card errors will differ between the tab bar indicator and the wallet menu item.

Issue 4: Fallback silently drops card errors (PaymentMethods.ts:427)

When \!currentUserLogin || \!policies is true, the function returns only hasBankOrFundError, completely ignoring card errors. The old behavior checked all cards unconditionally. During app startup or if session/policies have not loaded yet, card errors will be silently suppressed. Consider falling back to the old behavior (checking all cards) rather than dropping them.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: acc3a7ec1c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-02-23T07:26:22Z

src/libs/actions/PaymentMethods.ts

+    if (!currentUserLogin || !policies) {
+        return hasBankOrFundError;


Preserve card errors when policy data is missing

Returning early with only hasBankOrFundError when policies is missing drops all card-list errors. useNavigationTabBarIndicatorChecks reads policies with canBeMissing: true, so users can temporarily or permanently have policies === undefined (for example accounts without workspace policies), and in that state personal/company card errors no longer trigger HAS_PAYMENT_METHOD_ERROR. Before this change, card errors were always included, so this regresses account-tab error visibility.

Useful? React with 👍 / 👎.

src/libs/actions/PaymentMethods.ts

hungvu193 · 2026-02-23T07:49:56Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 624cece03a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

src/pages/settings/InitialSettingsPage.tsx

github-actions · 2026-02-23T18:07:26Z

🚧 @joekaufmanexpensify has triggered a test Expensify/App build. You can view the workflow run here.

joekaufmanexpensify · 2026-02-24T18:31:13Z

BUG: errant RBRs on wallet page and workspace page

joekaufmanexpensify · 2026-02-24T18:43:04Z

Also seeing the errant RBR thing for the non-admin employee view

ShridharGoel · 2026-02-24T21:32:16Z

Taking this for review based on Slack

ShridharGoel · 2026-03-11T20:40:50Z

@kavimuru https://expensify.slack.com/archives/C01GTK53T8Q/p1771625126451409?thread_ts=1771625115.808499&cid=C01GTK53T8Q

m-natarajan · 2026-03-12T03:04:08Z

@ShridharGoel Break connection option not displayed

kavimuru · 2026-03-12T12:55:52Z

@ShridharGoel @hungvu193 can you please help us?

hungvu193 · 2026-03-12T13:21:00Z

Do you toggle "Use Staging Server" option?

hungvu193 · 2026-03-12T16:54:38Z

@kavimuru I've just checked the code and the condition to show the Break connection (Testing) button is that your card should be mock card and the environment is not production. So can you please add the Mock card and turn off then turn on the Use Staging Server option? If it still doesn't work, please comment here

hungvu193 · 2026-03-12T17:04:53Z

Oh I think I found the issue. This check is always false on Staging:

App/src/pages/workspace/companyCards/WorkspaceCompanyCardDetailsPage.tsx

Line 114 in 58253ec

    
           const isNonProductionBackend = CONFIG.EXPENSIFY.EXPENSIFY_URL.includes('.dev') || CONFIG.EXPENSIFY.EXPENSIFY_URL.includes('staging') || !!shouldUseStagingServer;

CONFIG.EXPENSIFY.EXPENSIFY_URL value is always https://www.expensify.com/ and I couldn't find shouldUseStagingServer inside Onyx DB, we should use !isProduction here instead.

@amyevans Do we allow Break connection (testing) on Staging?

amyevans · 2026-03-12T17:37:00Z

Toggle User Staging Server off and then on again and it will work:

It's a small bug - we need to update the logic to match the implementation here so that the toggling is not required:

App/src/pages/workspace/companyCards/addNew/SelectBankStep.tsx

Line 39 in 6e53bf9

const [shouldUseStagingServer = isUsingStagingApi()] = useOnyx(ONYXKEYS.SHOULD_USE_STAGING_SERVER);
App/src/pages/workspace/companyCards/addNew/SelectBankStep.tsx

Line 73 in 6e53bf9

return CONFIG.ENVIRONMENT !== CONST.ENVIRONMENT.PRODUCTION || shouldUseStagingServer;

OSBotify · 2026-03-12T22:48:56Z

🚀 Deployed to production by https://github.com/luacmartins in version: 9.3.36-10 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

joekaufmanexpensify · 2026-03-13T13:19:06Z

I just retested this as part of something else, and it's already not working again/. Do we think this is related to another PR @carlosmiceli @hungvu193 @ShridharGoel

2026-03-13_09-18-13.mp4

hungvu193 · 2026-03-13T13:26:58Z

Weird. I think there is another PR removed the RBR logic. I'll take a look later today

joekaufmanexpensify · 2026-03-13T19:49:24Z

TY!

hungvu193 · 2026-03-15T09:55:23Z

@MelvinBot Find the PR that removed the RBR from WorkspaceInitialPage when there's company card with broken connection.

MelvinBot · 2026-03-15T10:09:36Z

The RBR on WorkspaceInitialPage for company cards with broken connections was restructured (not explicitly removed) by PR #78305 (fix: Handle company Cards errors and initial loading states), merged on 2026-01-26.

That PR replaced the old useHasWorkspaceCompanyCardErrors hook with the new useCardFeedErrors derived value system on WorkspaceInitialPage. The Company Cards menu item still uses shouldShowRbrForWorkspaceAccountID[workspaceAccountID] from the derived value, and the underlying logic in cardFeedErrors.ts still includes isFeedConnectionBroken as a trigger for shouldShowRBR — so the RBR should still appear for broken connections.

If the RBR is currently not appearing as expected, the more likely culprit is PR #84803 (Add 434 to broken connection ignored statuses for card feeds), merged on 2026-03-11. That PR added 434 to BROKEN_CONNECTION_IGNORED_STATUSES in src/CONST/index.ts, which causes isCardConnectionBroken() in CardUtils.ts to return false for cards with lastScrapeResult = 434. If the test card's broken connection has that scrape status, the entire RBR chain would break — no RBR on WorkspaceInitialPage.

TL;DR:

PR fix: Handle company Cards errors and initial loading states #78305 — restructured the RBR computation for company cards on WorkspaceInitialPage (old hook → derived value)
PR Add 434 to broken connection ignored statuses for card feeds #84803 — could suppress the RBR if the card's lastScrapeResult is 434 (newly ignored status)

hungvu193 · 2026-03-15T10:29:44Z

I think #84803 is the offending.

@carlosmiceli Can you confirm?

situchan · 2026-03-15T10:31:27Z

It's expected as per #84803 (comment)

hungvu193 · 2026-03-15T13:35:11Z

@joekaufmanexpensify #84803 added 434 status (broken connection card status) to the ignore list so we don't see the RBR anymore.

joekaufmanexpensify · 2026-03-16T13:22:00Z

I see. I think this might just be an issue with the Mock bank then. I tested my real Amex card and the broken connection doesn't scrape 434, so the flow worked correctly. We might need to update the error code we throw when breaking a connection for the mock bank cc @amyevans

joekaufmanexpensify · 2026-03-16T13:23:07Z

It's expected as per #84803 (comment)

If we aren't supposed to show the error for this case, why does the error momentarily appear below the card @situchan ? See here.

joekaufmanexpensify · 2026-03-16T13:24:29Z

I'm also seeing a double error with my Amex card on the company cards page. Something definitely is not working 100% correctly here...

hungvu193 · 2026-03-17T10:07:16Z

I'm also seeing a double error with my Amex card on the company cards page. Something definitely is not working 100% correctly here...

The RBR under the card looks redundant. How do you reproduce it btw?

I think when the feed connection is broken (RBR in Company Cards and Workspace), we shouldn't show the RBR under the card.

joekaufmanexpensify · 2026-03-17T12:53:06Z

The RBR under the card looks redundant. How do you reproduce it btw?
I think when the feed connection is broken (RBR in Company Cards and Workspace), we shouldn't show the RBR under the card.

Agreed. We shouldn't show this second error on the company cards page ever really IMO. We didn't when this PR originally went out. I'm wondering if that other PR caused this? For reproducing, I just broke the feed connection to see it.

hungvu193 · 2026-03-18T02:57:36Z

@MelvinBot Can you check the PR that added the RBR under the card inside WorkspaceCompanyCardPage?

MelvinBot · 2026-03-18T03:08:45Z

Here's the history of PRs that added RBR under cards on the WorkspaceCompanyCards page:

PR	Merged	What it did
#48176	2024-09-03	First created `WorkspaceCompanyCardsList.tsx` with `OfflineWithFeedback` wrapping each card row, showing `errors={item.errors}` — the original RBR under cards
#78069	2025-12-20	Replaced the old list with the new `WorkspaceCompanyCardsTableItem.tsx` (Table component). Added failed card assignment errors + `OfflineWithFeedback` with errors/pendingAction. Included sub-PRs: #77788 (table item), #78062 (failed assignment error states), #78178 (added `useCardFeedErrors` hook and `brickRoadIndicator` on feed selector page)
#78305	2026-01-26	Centralized all card error checking into `cardFeedErrors.ts` Onyx derived value. Added `isFeedConnectionBroken`, `shouldShowRbrForWorkspaceAccountID`, and `shouldShowRbrForFeedNameWithDomainID`. Updated `WorkspaceCompanyCardsTableItem` to use `assignedCard?.errors` directly

If you're specifically asking about the broken connection RBR under cards, PR #78305 is the one that wired up isFeedConnectionBroken into the centralized error system that feeds into the card-level and feed-level RBR indicators.

hungvu193 · 2026-03-18T03:18:13Z

For reproducing, I just broke the feed connection to see it.

After broke card connection, I only see the RBR under the card 🤔

Screen.Recording.2026-03-18.at.10.17.53.mov

joekaufmanexpensify · 2026-03-18T12:36:54Z

I think that's because of this and you using the mock bank. The mock bank uses 434 to show the broken feed error, but we don't show it for that anymore. I think it's incorrect that you're seeing that error at all.

joekaufmanexpensify · 2026-03-18T12:37:44Z

IMO, we need to:

Investigate why we're showing this error below individual cards on Company cards. We should show the feed-level error only.
Change the mock bank to use a different error code for broken connection, so it can still be used to test this feature.

hungvu193 · 2026-03-18T12:39:08Z

For the 2, I think @amyevans had a draft PR for it.

Ideally, we can wait for that PR to be merged so I can reproduce and find the RCA

joekaufmanexpensify · 2026-03-18T12:42:45Z

Works for me!

hungvu193 · 2026-03-20T09:35:52Z

#85591 is merged. I'll try to reproduce the bug and find the offending/solution.

update RBR of admin logic

c088d0e

melvin-bot bot assigned hungvu193 Feb 22, 2026

hungvu193 mentioned this pull request Feb 22, 2026

[Due for payment 2026-03-19] Admin seeing competing RBR paths with broken company card connection #81724

Closed

7 tasks

hungvu193 added 3 commits February 23, 2026 13:26

cleanup

92668bc

update hasPaymentMethodError logic

aa3f336

correct RBR logic

acc3a7e

hungvu193 marked this pull request as ready for review February 23, 2026 07:17

hungvu193 requested review from a team as code owners February 23, 2026 07:17

melvin-bot bot requested review from ikevin127 and joekaufmanexpensify and removed request for a team February 23, 2026 07:17

hungvu193 changed the title ~~[Company Card] Update RBR of admin/member logic~~ [Company Card] Update RBR logic of admin/member Feb 23, 2026

chatgpt-codex-connector bot reviewed Feb 23, 2026

View reviewed changes

update early return

624cece

chatgpt-codex-connector bot reviewed Feb 23, 2026

View reviewed changes

src/pages/settings/InitialSettingsPage.tsx Show resolved Hide resolved

This comment has been minimized.

Sign in to view

amyevans mentioned this pull request Mar 17, 2026

Improve Mock Bank testing utility #85591

Merged

52 tasks

		if (!currentUserLogin \|\| !policies) {
		return hasBankOrFundError;

Conversation

hungvu193 commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

codecov bot commented Feb 22, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hungvu193 commented Feb 22, 2026

Uh oh!

MelvinBot commented Feb 22, 2026

Code Review

1. Redundant null check (minor)

2. Fallback behavior when session/policies not loaded

3. Number.isNaN(workspaceAccountID) fallback returns true

4. All checks pass ✓

Summary

Uh oh!

hungvu193 commented Feb 23, 2026

Uh oh!

hungvu193 commented Feb 23, 2026

Member:

Admin:

Uh oh!

melvin-bot bot commented Feb 23, 2026

Uh oh!

github-actions bot commented Feb 23, 2026

Code Review - PR #83159

Issue 1: fundID may not be the correct field for workspace matching (PaymentMethods.ts:440)

Issue 2: isPolicyUser only matches the "user" role (PaymentMethods.ts:442)

Issue 3: allCards differs between callers (InitialSettingsPage.tsx:163)

Issue 4: Fallback silently drops card errors (PaymentMethods.ts:427)

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hungvu193 commented Feb 23, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

github-actions bot commented Feb 23, 2026

Uh oh!

This comment has been minimized.

joekaufmanexpensify commented Feb 24, 2026

BUG: errant RBRs on wallet page and workspace page

Uh oh!

joekaufmanexpensify commented Feb 24, 2026

Uh oh!

ShridharGoel commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShridharGoel commented Mar 11, 2026

Uh oh!

m-natarajan commented Mar 12, 2026

Uh oh!

kavimuru commented Mar 12, 2026

Uh oh!

hungvu193 commented Mar 12, 2026

Uh oh!

hungvu193 commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hungvu193 commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hungvu193 commented Feb 22, 2026 •

edited

Loading

codecov bot commented Feb 22, 2026 •

edited by github-actions bot

Loading

3. `Number.isNaN(workspaceAccountID)` fallback returns `true`

Issue 1: `fundID` may not be the correct field for workspace matching (PaymentMethods.ts:440)

Issue 2: `isPolicyUser` only matches the "user" role (PaymentMethods.ts:442)

Issue 3: `allCards` differs between callers (InitialSettingsPage.tsx:163)

ShridharGoel commented Feb 24, 2026 •

edited

Loading

hungvu193 commented Mar 12, 2026 •

edited

Loading

hungvu193 commented Mar 12, 2026 •

edited

Loading

hungvu193 commented Mar 18, 2026 •

edited

Loading