Skip to content

[$250] Expense carousel doesn't show RBR-actionable expenses first #85553

Closed

Description

@grgia

Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/603735
Slack conversation: https://expensify.slack.com/archives/C03U7DCU4/p1771944902661119?thread_ts=1771944902.661119&cid=C03U7DCU4


Action Performed

  1. Open a report that has multiple expenses, where at least one expense has an RBR indicator (violation, hold, field error, receipt error, etc.)
  2. Look at the expense carousel in the report preview

Expected Result

The expense that requires action (has RBR) should appear first in the carousel so the user can immediately identify and act on it.

Actual Result

Expenses appear in backend insertion order. RBR-flagged expenses can be buried at position 5+ in the carousel, making them easy to miss entirely — especially since only the first 11 are shown.

Root Cause

There is no RBR-aware sorting anywhere in the transaction display path:

  1. Carousel — MoneyRequestReportPreviewContent.tsx:~506 does transactions.slice(0, 11) with no sort beforehand.
  2. Report transaction list — MoneyRequestReportTransactionList.tsx:~325 sorts by date/column only; visualOrderTransactionIDs (which drives RHP carousel navigation) inherits this non-RBR-aware order.

The data flows from Onyx derived store → Object.values() (insertion order) → carousel/list — no step prioritizes actionable items.

Proposed Fix

Sort RBR-flagged transactions to the front in both locations:

  • Carousel: Sort transactions by RBR status before the .slice(0, 11) in MoneyRequestReportPreviewContent.
  • Transaction list: Use RBR status as a primary sort key in MoneyRequestReportTransactionList, with the existing date/column sort as secondary.

Existing infrastructure to reuse

  • shouldShowRBR from TransactionPreviewUtils.createTransactionPreviewConditionals()
  • filteredViolations already computed per-transaction in MoneyRequestReportTransactionList (~lines 216-237)
  • Report-level RBR prioritization pattern in SidebarUtils.ts:~533

Open Questions

  • Should RBR-first sorting override a user-selected column sort (date, merchant, etc.) in the report view, or only apply as the default?
  • Should sorting be centralized in useReportWithTransactionsAndViolations so all consumers benefit?

Platforms

  • Android: App
  • Android: mWeb Chrome
  • iOS: App
  • iOS: mWeb Safari
  • iOS: mWeb Chrome
  • Windows: Chrome
  • MacOS: Chrome / Safari
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~022033929183187241566
  • Upwork Job ID: 2033929183187241566
  • Last Price Increase: 2026-03-17

Activity

  1. self-assigned this
    on Mar 17, 2026
  2. added
    BugSomething is broken. Auto assigns a BugZero manager.
    ExternalAdded to denote the issue can be worked on by a contributor
    on Mar 17, 2026
  3. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Mar 17, 2026
  4. melvin-bot commented on Mar 17, 2026

    @melvin-bot

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

  5. changed the title [-]Expense carousel doesn't show RBR-actionable expenses first[/-] [+][$250] Expense carousel doesn't show RBR-actionable expenses first[/+] on Mar 17, 2026
  6. melvin-bot commented on Mar 17, 2026

    @melvin-bot
  7. MelvinBot commented on Mar 17, 2026

    @MelvinBot
    Contributor

    Proposal

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

    When a report has multiple expenses and some have RBR indicators (violations, holds, field errors, receipt errors, etc.), the expense carousel in the report preview doesn't prioritize those actionable expenses. They appear in backend insertion order, meaning RBR-flagged expenses can be buried at position 5+ and hidden behind the "+N more" button (only the first 10 are shown as previews).

    What is the root cause of that problem?

    There is no RBR-aware sorting anywhere in the transaction display path:

    1. Carousel path: Transactions are stored in Onyx as a Record<string, Transaction>. The derived config (reportTransactionsAndViolations.ts) stores them in a plain object. useReportWithTransactionsAndViolations retrieves them via Object.values() (insertion order), filters out deleted ones, and passes them as a prop. At MoneyRequestReportPreviewContent.tsx:506, transactions.slice(0, 11) takes the first 11 items (10 previews + 1 "show more" slot) with no .sort() call anywhere in the chain.

    2. Transaction list path: MoneyRequestReportTransactionList.tsx:269-271 sorts transactions by a single user-selected column (default: date ASC) using compareValues(). The 6 sortable columns are date, merchant, description, category, tag, and total_amount — none are violation/RBR-related. visualOrderTransactionIDs (line 325-330) inherits this non-RBR-aware order. While filteredViolations are computed per-item via useTransactionViolations, they are only used for display and never feed back into sort order.

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

    Carousel (MoneyRequestReportPreviewContent.tsx): Before the .slice(0, 11) at line 506, sort the transactions array so that RBR-flagged transactions come first. This can leverage shouldShowRBR from TransactionPreviewUtils.createTransactionPreviewConditionals() (which OR-combines violations, field errors/hold, report violations/action errors, receipt errors, and DEW submit failures). Among RBR and non-RBR groups, preserve the existing insertion order as a stable secondary sort.

    Transaction list (MoneyRequestReportTransactionList.tsx): Use RBR status as a default primary sort key, with the existing column sort as secondary. This follows the same pattern used in SidebarUtils.ts:586-613 where the LHN elevates error reports to a higher priority group (group 2) above drafts and regular reports.

    For both locations, the violation data is already available — the carousel's parent has access to violations via useReportWithTransactionsAndViolations, and the transaction list already computes filteredViolations per-item. The key question is whether RBR-first sorting should override a user-selected column sort in the transaction list or only apply as the default — this is flagged as an open question in the issue.

    What alternative solutions did you explore? (Optional)

    • Centralizing the sort in useReportWithTransactionsAndViolations: This would give all consumers RBR-aware ordering automatically, but it tightly couples the hook to violation data that not all consumers may need, and it removes the ability for individual consumers to opt out.
    • Adding a "Violations" sortable column to the transaction list: This would let users explicitly sort by violation status, but it doesn't solve the default ordering problem and adds UI complexity.

    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.

  8. abelhailefen commented on Mar 17, 2026

    @abelhailefen

    Proposal

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

    When an expense report contains multiple transactions, those requiring action (RBR indicators like violations, field errors, holds) can be buried past the 5th position in the report preview carousel and the transaction list. Because the preview only shows the first 11 items, users can completely miss actionable expenses.

    What is the root cause of that problem?

    There is no RBR-aware sorting in the presentation layer:

    • Carousel: In MoneyRequestReportPreviewContent.tsx, the component calls .slice(0, 11) strictly based on backend insertion order without any prior sorting.
    • Transaction List: In MoneyRequestReportTransactionList.tsx, transactions are sorted strictly by a standard column value (defaulting to date), completely ignoring RBR status. Since the Right-Hand Panel (RHP) carousel navigation (visualOrderTransactionIDs) is derived directly from this list, it inherits the same flawed order.

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

    • Create a lightweight RBR helper:
      Add a minimal transactionHasRBR utility in TransactionPreviewUtils.ts. This will evaluate transaction-level violations, holds, and receipt errors without needing heavy report/policy context.

    • Fix the Carousel (MoneyRequestReportPreviewContent):
      Before calling .slice(0, 11), sort the transactions array using the new transactionHasRBR helper and the already-available lastTransactionViolations prop. RBR items will sort to the front, preserving insertion order as a secondary tiebreaker.

    • Fix the Transaction List (MoneyRequestReportTransactionList):
      Modify the sortedTransactions useMemo to evaluate RBR status using useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS).

    • Crucial UX detail:
      Apply RBR-first sorting only when the list is set to the default sort (Date / ASC). If a user explicitly clicks a column header (e.g., Merchant or Amount), we respect their choice and bypass the RBR override.

    • Fix the RHP Navigation:
      By fixing the transaction list above, visualOrderTransactionIDs automatically inherits the correct RBR-first order, requiring zero extra changes.

    What alternative solutions did you explore? (Optional)

    • Always forcing RBR-first in the transaction list:
      I opted against this. Overriding an explicit user action (like sorting by Amount) with RBR priority leads to broken user expectations. Applying it only to the default sort guarantees visibility on first load while keeping the table interactive.

    • Centralizing the sort in useReportWithTransactionsAndViolations:
      While tempting, this tightly couples the hook to violation logic and forces all hook consumers to inherit an RBR-sorted array, which might cause regressions in non-UI data processors. Handling the sort at the presentation layer is safer and more isolated.

  9. mendarb commented on Mar 17, 2026

    @mendarb

    🚫 Duplicated proposal withdrawn by 🤖 ProposalPolice.

  10. melvin-bot commented on Mar 17, 2026

    @melvin-bot

    📣 @mendarb! 📣
    Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
    Please follow these steps:

    1. Make sure you've read and understood the contributing guidelines.
    2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
    3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
    4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
      Screen Shot 2022-11-16 at 4 42 54 PM
      Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
    
  11. github-actions commented on Mar 17, 2026

    @github-actions
    Contributor

    ⚠️ @mendarb 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.

  12. ikevin127 commented on Mar 18, 2026

    @ikevin127
    Contributor

    🎉 Thanks everybody for proposals!

    Detailed proposals review

    Here is my detailed review of the proposals based on Expensify's engineering guidelines and the current codebase logic.

    Decision

    Proposal 2 is the best fit for this issue and provides a much more mature and performant solution.

    Detailed Code Review & Justification

    1. React Code Performance & Pure Functions

    • Why Proposal 2 Wins: Proposal 1 suggests reusing shouldShowRBR from TransactionPreviewUtils.createTransactionPreviewConditionals(). This approach is an anti-pattern for array sorting. createTransactionPreviewConditionals is a massive function that requires 13 arguments (including transactionDetails, currentUserEmail, reportActions, etc.) and performs heavy string truncations and UI-layout specific mapping logic. Calling this inside a .sort() looping over potentially hundreds of transactions will severely bottleneck React render performance, causing dropped frames on mobile, especially older Android devices.
    • Proposal 2 correctly identifies the need for a lightweight, pure function (transactionHasRBR). This satisfies the requirement to separate computation logic from side-effects and prevents expensive layout logic from running during a simple sort operation.
    • Actionable Feedback for PR Author:
      When you write transactionHasRBR, ensure it remains a pure function with strict type definitions aligned with our repository standards. Example:
      // src/libs/TransactionPreviewUtils.ts
      export function transactionHasRBR(
          transaction: OnyxEntry<OnyxTypes.Transaction>, 
          transactionReport: OnyxEntry<OnyxTypes.Report>,
          violations: OnyxTypes.TransactionViolations, 
      ): boolean {
          if (!transaction) {
              return false;
          }
      
          const isTransactionOnHold = isOnHold(transaction);
          const hasErrors = hasMissingSmartscanFields(transaction, transactionReport) || hasReceiptError(transaction);
          const hasViolations = !!violations && violations.some((violation) => violation.type === CONST.VIOLATION_TYPES.VIOLATION);
      
          return isTransactionOnHold || hasErrors || hasViolations;
      }

    2. Responsiveness & User Experience

    • Why Proposal 2 Wins: The product decision to only apply the RBR-first sort when the default sort (Date / ASC) is active is outstanding. Overriding an explicit user action (e.g., a user clicking the 'Amount' column header to find a specific expensive item, but getting RBR items forced to the top instead) breaks standard responsive UI expectations. Proposal 2 respects the user's intent while still solving the default visibility issue.
    • Actionable Feedback for PR Author: In MoneyRequestReportTransactionList.tsx, ensure the sort conditionally checks the configuration:
      if (sortBy === CONST.SEARCH.TABLE_COLUMNS.DATE && sortOrder === CONST.SEARCH.SORT_ORDER.ASC) {
          // Execute lightweight RBR sort
      } else {
          // Respect user's chosen column sort
      }

    3. State Management with Onyx

    • Proposal 2 suggests adding useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS) to MoneyRequestReportTransactionList.tsx. This aligns perfectly with Expensify's Onyx best practices, as we do not use the deprecated Onyx.connect in components.
    • Actionable Feedback for PR Author: Because it's an Onyx collection, this hook triggers a re-render anytime any violation in the collection updates. To satisfy the React Code Performance guidelines, ensure that you use a selector to only subscribe to violations relevant to the transactions array passed to the component, preventing unnecessary re-renders on unrelated background validation updates.

    4. Edge Cases: Grouping Integration

    • Actionable Feedback for PR Author: In MoneyRequestReportTransactionList.tsx, transactions can be grouped (e.g., Category, Tag) via reportLayoutGroupBy. If grouping is active, groupedTransactions determines the visual order, not just sortedTransactions. You must ensure that your RBR sorting logic correctly bubbles up actionable items within their respective groups, or falls back gracefully so we don't accidentally scramble grouped layouts.

    5. Testing Requirements

    • To adhere to our Testing guidelines:
      1. Write Unit Tests for the pure transactionHasRBR utility to verify it catches Holds, field errors, and receipt violations correctly.
      2. The carousel (MoneyRequestReportPreviewContent) modification requires an integration test to guarantee that a mocked RBR transaction correctly renders at index 0 ahead of older insertion-order transactions.

    @abelhailefen's main proposal solution LGTM as it has a clearly superior architectural and product design, see Detailed proposals review above for details on why.

    Note for contributor: Please refer to the Detailed proposals review - Actionable Feedback for PR Author notes for guidelines during PR creation.

    If this is your first contribution:

    🎉 Congratulations on your first contribution assignment, and seeing that it's your first, here's what's gonna happen next:

    • an Expensify engineer will be assigned for final review & assignment
    • they will assing you to the issue as contributor
    • once assigned you can move forward with opening the PR, making sure to follow bot's directions for first time contribution
    • I'll review the PR and once we get it merged, it'll hit the Staging env first, then Production after which there will be a 7-day regression period
    • once the 7-day regression passed, being your first time contributing, you will manually be sent an Upwork offer and receive your payment / if there will be any regressions we'll have to fix them in a follow-up PR and we will both get a -50% penalty on the bounty for the first regression (= $125) and this will apply for each regression

    🎀👀🎀 C+ reviewed

  13. 91 remaining items

  14. melvin-bot commented on Jun 2, 2026

    @melvin-bot

    Current assignee @mallenexpensify is eligible for the Awaiting Payment assigner, not assigning anyone new.

  15. mallenexpensify commented on Jun 2, 2026

    @mallenexpensify
    Contributor

    Payment Summary

    Contributor: @abelhailefen paid $75 via Upwork
    Contributor+: @ikevin127 due $250 via NewDot

    @abelhailefen can you please apply to the job below? Please reply here and tag me once you have.
    https://www.upwork.com/jobs/~022061936512836955018

    @abelhailefen - possibly due $125 for their contributor / author work on the #86477 (which had 1 regression), later #90464 (comment) the regression follow-up PR

    $125 would have been price if they completed the follow-up PR. Since they didn't pay should be less so I made it $75.

  16. grgia commented on Jun 4, 2026

    @grgia
    ContributorAuthor

    Thank you!

  17. abelhailefen2 commented on Jun 4, 2026

    @abelhailefen2

    @mallenexpensify you told me to apply but my main account is banned?

  18. melvin-bot commented on Jun 4, 2026

    @melvin-bot

    📣 @abelhailefen2! 📣
    Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
    Please follow these steps:

    1. Make sure you've read and understood the contributing guidelines.
    2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
    3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
    4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
      Screen Shot 2022-11-16 at 4 42 54 PM
      Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
    
  19. abelhailefen2 commented on Jun 4, 2026

    @abelhailefen2

    @mallenexpensify i have already applied please let me know if i should withdraw my proposal in upwork

  20. mallenexpensify commented on Jun 5, 2026

    @mallenexpensify
    Contributor

    @abelhailefen can you please accept the job below? Please reply here and tag me once you have.
    If you're not sent an offer in Upwork when you're assigned to an issue, please tag me. If an Upwork job exists, please submit a proposal.
    https://www.upwork.com/jobs/~022061936512836955018

  21. abelhailefen2 commented on Jun 5, 2026

    @abelhailefen2

    @mallenexpensify i have accepted the offer on upwork

  22. mallenexpensify commented on Jun 6, 2026

    @mallenexpensify
    Contributor

    @abelhailefen paid, summary updated above. Thx.

  23. quinthar commented on Jun 7, 2026

    @quinthar
    Contributor

    Approved $250 to @ikevin127

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.ExternalAdded to denote the issue can be worked on by a contributorReviewingHas a PR in reviewWeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions