[$250] Broken card connection violation message not shown to workspace admins

[MelvinBot]

If you haven't already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Connection with other issues: Split from #72055

---

Action Performed:

1. Create a workspace with a company card feed connected
2. As a workspace admin (who is not the expense submitter), view an expense that has a brokenCardConnection violation (i.e., the bank connection for the company card is broken)
3. Observe the violation message displayed on the expense

Expected Result:

The workspace admin should see the message: "Receipt pending due to broken bank connection. Please resolve in Company cards" — with a link to the Company cards settings page.

This is the behavior implemented in BrokenConnectionDescription for the isPolicyAdmin && !isCurrentUserSubmitter(report) code path.

Actual Result:

The admin only sees a generic "Review" message. The brokenCardConnection-specific violation message does not appear.

Root Cause

There is a conflict between two layers of violation filtering logic:

1. shouldShowViolation gates all rter violations (including brokenCardConnection type) on (isSubmitter || isInstantSubmitEnabled(policy)). For a workspace admin who is not the expense submitter, on a policy without instant submit, the RTER violation is filtered out entirely.

2. BrokenConnectionDescription has a code path specifically for isPolicyAdmin && !isCurrentUserSubmitter(report) that renders the admin message — but this is dead code because the violation was already removed upstream by shouldShowViolation.

3. shouldShowBrokenConnectionViolationInternal also has logic to show the violation to admins for open reports, but its input is already empty because shouldShowViolation filtered the violations first.

The filtering chain is: useTransactionViolations → calls shouldShowViolation → removes the RTER violation → shouldShowBrokenConnectionViolation receives empty array → BrokenConnectionDescription never renders.

Workaround

None known.

Platform:

All platforms.

Version Number: Main (current)

Reproducible in staging?: Yes

Reproducible in production?: Yes

Logs: N/A

Expensify/Expensify Issue URL: N/A

Notes/Photos/Videos: N/A

---

Originated from: #72055 — MelvinBot's analysis comment

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @thelullabyy

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~022042689725596467142
• Upwork Job ID: 2042689725596467142
• Last Price Increase: 2026-04-10

[MelvinBot]

Proposal

What is the root cause of that problem?

shouldShowViolation at TransactionUtils/index.ts:1680-1681 gates all RTER violations on (isSubmitter || isInstantSubmitEnabled(policy)). Since brokenCardConnection is an RTER subtype, it gets filtered out for admin non-submitters before any downstream logic runs.

This makes two code paths dead code for that scenario:

• The admin message in BrokenConnectionDescription.tsx:42-43 (isPolicyAdmin && !isCurrentUserSubmitter)
• The admin logic in shouldShowBrokenConnectionViolationInternal (returns true for admins on open reports)

The filtering chain: useTransactionViolations → shouldShowViolation removes RTER → empty violations reach BrokenConnectionDescription → returns ''. Same thing happens in shouldShowBrokenConnectionViolationForMultipleTransactions at line 1629.

What changes do you think we should make in order to solve the problem?

Modify shouldShowViolation to accept the full TransactionViolation object (or add a violation parameter alongside the existing violationName). At the RTER check on line 1680, exempt brokenCardConnection violations from the submitter gate by returning true for them — letting the existing downstream logic in shouldShowBrokenConnectionViolationInternal (which already correctly handles admin visibility) control whether the violation is actually displayed.

This also fixes the shouldShowBrokenConnectionViolationForMultipleTransactions path at line 1629, since it calls shouldShowViolation too.

The helper isBrokenConnectionViolation at line 1570 already exists and can be reused for this check.

What alternative solutions did you explore?

Modifying useTransactionViolations to skip the shouldShowViolation filter for brokenCardConnection violations specifically. This was rejected because it would only fix one of the two filtering paths — the multi-transaction path at line 1629 would still be broken. Fixing shouldShowViolation itself addresses both paths with a single change.

---

Next Steps for Contributor+ team: Reply with @MelvinBot implement this to create a draft PR, @MelvinBot <your feedback> to refine this analysis, or explain why you are rejecting Melvin's proposal.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @thelullabyy (External)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~022042689725596467142

[wildan-m]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Workspace admins who are not the expense submitter cannot see the brokenCardConnection violation message on expenses. Instead of the expected "Receipt pending due to broken bank connection" admin message, they see only a generic "Review" message (or nothing at all).

What is the root cause of that problem?

There are two separate gates that filter out brokenCardConnection violations for admin non-submitters.

Gate 1 — shouldShowViolation gates all RTER violations (which includes brokenCardConnection as a subtype) on (isSubmitter || isInstantSubmitEnabled(policy)). For a workspace admin who is not the expense submitter, on a policy without instant submit enabled, this check returns false and the RTER violation is removed from the filtered violations list.

Gate 2 — shouldShowBrokenConnectionViolationInternal has a second independent check on its final line: return isProcessingReport(report) && isInstantSubmitEnabled(policy). Even after Gate 1 lets the violation through, this second gate still returns false for an admin non-submitter on a processing report unless the policy has instant submit enabled. This is the gate that matches the reviewer's reported failure.

Together, these two gates make the admin-specific code path in BrokenConnectionDescription (isPolicyAdmin && !isCurrentUserSubmitter) unreachable for the common case of a workspace that does not use instant submit.

The submitter gate is appropriate for regular RTER violations (pending receipt during real-time expense reporting), but brokenCardConnection is a different category — it represents a broken card integration that an admin can actually fix in the Company cards settings, so it should be visible to admins regardless of the submit frequency as long as the report is in an actionable state (open or processing).

What changes do you think we should make in order to solve the problem?

Gate 1 fix: Add an optional violation parameter to shouldShowViolation so the RTER branch can inspect the violation's rterType. When the violation is a brokenCardConnection subtype (detected via the existing isBrokenConnectionViolation helper), bypass only the submitter/instant-submit gate while preserving the settled-report filter (shouldShowRterForSettledReport || !isSettled). This ensures broken connection violations are visible in active reports but still correctly hidden in search results for settled reports.

Update all callers of shouldShowViolation (in useTransactionViolations, ViolationsUtils, shouldShowBrokenConnectionViolationForMultipleTransactions, allHavePendingRTERViolation, and the Search list item components) to pass the violation object through.

Gate 2 fix: In shouldShowBrokenConnectionViolationInternal, drop the isInstantSubmitEnabled(policy) requirement on the final line so it simply returns isProcessingReport(report). The early returns above already handle the other cases: non-admin users and submitters return true unconditionally, open reports return true, and settled/approved/reimbursed reports correctly fall through to false. The only state where the instant-submit requirement was acting as an extra gate was the admin non-submitter + processing report case — which is exactly the case the reviewer reported as broken.

Compare branch: https://github.com/Expensify/App/compare/main...wildan-m:App:wildan/87673-broken-connection-admin-visibility?expand=1

[JasimMalook]

🚫 Duplicated proposal withdrawn by 🤖 ProposalPolice.

[github-actions]

⚠️ @JasimMalook Your proposal is a duplicate of an already existing proposal and has been automatically withdrawn to prevent spam. Please review the existing proposals before submitting a new one.

[Nghondzweni]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Workspace admins who are not the expense submitter cannot see the brokenCardConnection violation message. Instead of the expected admin-specific message — "Receipt pending due to broken bank connection. Please resolve in Company cards" — they see only a generic "Review" message.

What is the root cause of that problem?

shouldShowViolation in TransactionUtils/index.ts gates all RTER violations (including brokenCardConnection, which is an RTER subtype) on the condition isSubmitter || isInstantSubmitEnabled(policy). For a workspace admin who is not the submitter on a policy without instant submit, this check returns false and the violation is stripped before any downstream logic runs.

This makes two downstream code paths dead code:

• The isPolicyAdmin && !isCurrentUserSubmitter(report) branch in BrokenConnectionDescription.tsx — the violation never reaches it
• shouldShowBrokenConnectionViolationInternal — which correctly handles admin visibility, but receives an empty violations array

The same problem exists in shouldShowBrokenConnectionViolationForMultipleTransactions (line ~1629), which also routes through shouldShowViolation.

What changes do you think we should make in order to solve the problem?

Rather than modifying shouldShowViolation's signature (which has many callers), I propose a targeted post-filter approach at the two consumption points:

1. In useTransactionViolations — after the existing filter pass, re-merge any brokenCardConnection violations for policy admins that were stripped by the submitter gate. The existing isBrokenConnectionViolation helper and isPolicyAdmin flag are already in scope:

const visibleViolations = violations.filter((v) => shouldShowViolation(...));

// Re-include broken connection violations for policy admins (submitter gate incorrectly removed them)
const adminBrokenConnectionViolations =
  isPolicyAdmin && !isSubmitter
    ? violations.filter(isBrokenConnectionViolation).filter((v) => !visibleViolations.includes(v))
    : [];

return [...visibleViolations, ...adminBrokenConnectionViolations];

2. In shouldShowBrokenConnectionViolationForMultipleTransactions — apply the same post-filter logic so the multi-transaction code path is also fixed.

This keeps shouldShowViolation entirely unchanged — no signature changes, no caller updates needed elsewhere. The existing admin-specific logic in BrokenConnectionDescription and shouldShowBrokenConnectionViolationInternal then runs correctly, since the violation is no longer stripped before reaching them.

What alternative solutions did you explore?

• Modifying shouldShowViolation to accept the full TransactionViolation object and special-case brokenCardConnection — valid, but requires updating all callers across the codebase, increasing the blast radius.
• Fixing only useTransactionViolations — misses the shouldShowBrokenConnectionViolationForMultipleTransactions path at line 1629. My approach fixes both call sites with a contained, additive change.