[Better Expense Reports] Add MoneyRequestReportView #58360

Kicu · 2025-03-13T10:40:24Z

This PR adds new view called MoneyRequestReportView which will display a grouped list of report transactions at top and chat items below.

It implements parts of: https://docs.google.com/document/d/12RD9MO-CXFwcehkqLdWvx0__mK8Ou_0cMBYG5BVNULc/edit?tab=t.0#bookmark=id.vbs1mmt4xhw4

Explanation of Change

Introduces cleaned up MoneyRequestReportView, which displays list of transactions and chat items
- transaction list consists of all transactions filtered by reportID
- the chat list uses the same rendering logic as normal ReportScreen but we filter out CREATED and IOU actions because CREATED is not relevant to this view and IOU is the transaction list ☝️
reuses a lot of logic from: ReportScreen, ReportActionsView and ReportActionsList
currently new view is only displayed in Search context (Search LHN)
uses tableReportView beta to redirect Search report items to the new view - without this beta the user cannot access new screen
abstracts away SearchTableHeader into component called SortableTableHeader to use the same header both in Search table and MoneyRequestReport table

NOTE: this PR is already quite big so I wanted to divide it into smaller parts to allow for reasonable review.

These don't yet work and will be done in separated PR

rest of report actions list functionalities like divider line etc.
sorting transaction list (at the top of screen)
chat message count on transaction row

Fixed Issues

$ #57508
PROPOSAL:

Tests

Important: in order to test change function canUseTableReportView to return true, or have BETA tableReportView set

open Search, go to Expense Reports
click on any report
verify you see the new view, and not report in RHP
verify that transactions are visible, note in this view the user always starts at the top of chat messages
verify that composer works
verify that report actions are correctly displayed
Please note that there are parts of this not done yet, described above in the description

Offline tests

QA Steps

Same as Tests

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

rec-report-ios.mp4

iOS: mWeb Safari

rec-report-ios-mweb.mp4

MacOS: Chrome / Safari

rec-report-web.mp4

MacOS: Desktop

…st and chat

…list

Kicu · 2025-03-17T14:14:59Z

src/pages/home/report/ReportActionsView.tsx

    const {isOffline} = useNetwork();

    const {shouldUseNarrowLayout} = useResponsiveLayout();
-    const contentListHeight = useRef(0);


Please note this is a cleanup.
I have noticed that this ref was not used anywhere. It was updated in onContentSizeChange and never used after that nor passed down.

Kinda feel like ideally we would do that in a separate PR in case there is any regression with the hook

src/pages/home/report/ReportActionsView.tsx

Kicu · 2025-03-17T15:26:55Z

I have some problems building android hybrid app. Need to do a clean install, so I will upload android videos tomorrow.

melvin-bot · 2025-03-17T15:31:51Z

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

Kicu · 2025-03-17T15:33:25Z

CC @mountiny @luacmartins ready to review, please take a look at the description.

shawnborton · 2025-03-17T17:13:07Z

Do you want the @Expensify/design to do a thorough review of this or not yet?

luacmartins · 2025-03-17T18:51:14Z

@DylanDylann let's prioritize reviewing this PR when you're online

luacmartins · 2025-03-17T18:52:55Z

Do you want the @Expensify/design to do a thorough review of this or not yet?

@shawnborton I think we can save design review for later when we have more of the functionality in place.

luacmartins

Left a few comments. Additionally, found some minor issues:

Table header is not aligned with table data
Date from previous year is cut off
Incorrect cursor shown on hover

Screen.Recording.2025-03-17.at.1.26.44.PM.mov

Screen.Recording.2025-03-17.at.1.27.08.PM.mov

Violations don't have padding on small screens
Items without violations have bigger height than needed

src/components/SelectionList/SearchTableHeader.tsx

luacmartins · 2025-03-17T19:10:36Z

src/components/SelectionList/SearchTableHeader.tsx

-    shouldShow: (data: OnyxTypes.SearchResults['data'], metadata: OnyxTypes.SearchResults['search']) => boolean;
+};
+
+const shouldShowColumnConfig: Record<SearchColumnType, ShouldShowSearchColumnFn> = {


Why did we move this out of SearchColumnConfig?

Same question?

Short answer: code decoupling + TS types

In search table some of shouldShow functions needed extra search data to make the decision whether to display a column. These function had signatures like this:

(data: OnyxTypes.SearchResults['data'], metadata: OnyxTypes.SearchResults['search']) => boolean

I needed to create a clean reusable SortableTableHeader component. Inside this component I need a shouldShow function, but I don't want to make its arguments depend on search data/metadata. It would be hard to use it for ReportsView then.
Because of that, inside SearchTableHeader I had to split types that are connected to search data (shouldShow) from columnsConfig. That means columnsConfig is now only names of columns + translation keys - generic types.
I'm quite happy with this solution.
(If you remember @luacmartins we had similar problems with chat name generation, where it was tightly coupled to reports data in Onyx and hard to reuse in Search - I want to avoid that here)

src/hooks/useReportActionsLoading.ts

src/libs/ReportActionsUtils.ts

luacmartins · 2025-03-17T19:19:41Z

src/libs/ReportActionsUtils.ts

+    const currentAction = reportActions.at(actionIndex);
+    const nextAction = findNextAction(reportActions, actionIndex);
+
+    // Todo first should have avatar - verify that this works with long chats


Same here, are we doing this in a follow up?

Suggested change

// Todo first should have avatar - verify that this works with long chats

// Todo first should have avatar - verify that this works with long chats

+1 lets link issues in which we would handle this in each TODO

let's combine these two, because both touch the same part of code - displaying avatars on grouped comments.
Since the existing solution is working specifically only on InvertedList I had to re-implement it for non-inverted lists.

Both TODO comments are leftovers after my change so they should be fixed together.

issue created: #58625

Kicu · 2025-03-19T09:44:00Z

Yes correct 👍 the reportActions are reversed but the List is not.
In this view you "start at the top", so if there are just 1-2 comments then they should be aligned to the top of the whole view, not to the bottom as currently when looking at chats/expenses.

mountiny

Thanks! going to merge this so @JakubKorytko and other PRs can base their changes from this more easily today but @luacmartins please feel free to review this PR still

trjExpensify · 2025-03-19T10:56:44Z

The LHN Expenses is still highlighted when in an individual report view. Is removing that on the follow-up list? (I seem to recall it mentioned somewhere else already).

Similarly, is the table footer with the Comments header and report Total amount slated for a follow-up? I see those missing as well. Ref:

mountiny · 2025-03-19T11:02:53Z

@JakubKorytko Could work those notes into his PR

JakubKorytko · 2025-03-19T11:08:53Z

Noted, will take a look later, focusing on sorting functionality rn.

OSBotify · 2025-03-19T11:10:58Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

Kicu · 2025-03-19T12:00:40Z

I will also work on those. I'm adding comments to the original issue so that we don't forget about anything 👍

github-actions · 2025-03-20T15:37:03Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.16-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	success ✅
🍎🔄 iOS HybridApp 🍎🔄	success ✅

deetergp · 2025-03-20T20:18:24Z

I believe that this PR is the cause of this deploy blocker. I have reverted this in main locally and confirmed that the bug goes away.

trjExpensify · 2025-03-20T21:37:17Z

It's behind a beta 👍

deetergp · 2025-03-20T22:17:01Z

Noted. We demoted it from being a blocker.

github-actions · 2025-03-21T22:21:05Z

🚀 Deployed to production by https://github.com/luacmartins in version: 9.1.16-4 🚀

platform	result
🤖 android 🤖	true ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	failure ❌
🍎🔄 iOS HybridApp 🍎🔄	failure ❌

s77rt · 2025-05-05T10:58:35Z

src/libs/ReportActionsUtils.ts

+function findNextAction(reportActions: ReportAction[], actionIndex: number): OnyxEntry<ReportAction> {
+    for (let i = actionIndex - 1; i > 0; i--) {


This loop stops at 1 (i > 0) and does not check the first report action. The condition should have been i >= 0.

Coming from #59948

ikevin127 · 2025-05-08T04:14:27Z

src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx

+                    shouldUseThreadDividerLine={shouldUseThreadDividerLine}
+                    shouldDisplayReplyDivider={visibleReportActions.length > 1}
+                    isFirstVisibleReportAction={firstVisibleReportActionID === reportAction.reportActionID}
+                />


Missed passing linkedReportActionID here for message highlighting when clicking header to go back to a linkedReportAction, leading to this issue:

[Due for payment 2025-05-14] [$250] Expense - Message is not highlighted after returning from thread via header subtitle link #60768

fedirjh · 2025-05-13T17:09:00Z

src/components/MoneyRequestReportView/MoneyRequestReportView.tsx

+                    lastReportAction={lastReportAction}
+                />
+            ) : null}
+        </View>


Coming from #60747, missing <PortalHost name="suggestions" /> resulted in a broken suggestion list.

Kicu force-pushed the kicu/57508-add-report-view branch from 6506e89 to 8f630ba Compare March 13, 2025 13:26

luacmartins self-requested a review March 13, 2025 15:23

mountiny self-requested a review March 13, 2025 15:28

mountiny assigned Kicu Mar 13, 2025

Kicu force-pushed the kicu/57508-add-report-view branch 2 times, most recently from ceca442 to 5ed0a75 Compare March 14, 2025 13:35

Kicu and others added 3 commits March 17, 2025 15:04

Add basic version of MoneyRequestReportView with basic transaction li…

0040f0d

…st and chat

Add TransactionItemRowRBR (#211)

b544883

Fix avatars grouping working for the reverse direction of ReportView …

b387580

…list

Kicu force-pushed the kicu/57508-add-report-view branch 2 times, most recently from 33c9d0e to 35d7b65 Compare March 17, 2025 14:10

Kicu commented Mar 17, 2025

View reviewed changes

src/pages/home/report/ReportActionsView.tsx Show resolved Hide resolved

Refactor code for displaying MoneyRequestReportView

dd279c5

Kicu force-pushed the kicu/57508-add-report-view branch from 35d7b65 to dd279c5 Compare March 17, 2025 14:21

Add fetching report

9803b3f

Kicu changed the title ~~[WIP] Add MoneyRequestReportView~~ Add MoneyRequestReportView Mar 17, 2025

Kicu marked this pull request as ready for review March 17, 2025 15:31

Kicu requested review from a team as code owners March 17, 2025 15:31

melvin-bot bot requested review from DylanDylann and removed request for a team March 17, 2025 15:31

mountiny changed the title ~~Add MoneyRequestReportView~~ [Better Expense Reports] Add MoneyRequestReportView Mar 17, 2025

luacmartins requested changes Mar 17, 2025

View reviewed changes

Kicu dismissed mountiny’s stale review via dd8660a March 19, 2025 09:42

Kicu mentioned this pull request Mar 19, 2025

[Due for payment 2025-04-16] [Due for payment 2025-04-10] [Better Expense Report View] Implement updated Expense Report View #57508

Closed

DylanDylann approved these changes Mar 19, 2025

View reviewed changes

melvin-bot bot requested a review from mountiny March 19, 2025 10:26

mountiny approved these changes Mar 19, 2025

View reviewed changes

mountiny merged commit e292fa1 into Expensify:main Mar 19, 2025
20 checks passed

JakubKorytko mentioned this pull request Mar 19, 2025

[Better Expense Report View] Add sorting functionality to MoneyRequestReportView #58713

Merged

48 tasks

github-actions bot mentioned this pull request Mar 20, 2025

Deploy Checklist: New Expensify 2025-03-20 #58801

Closed

72 tasks

carlosmiceli mentioned this pull request Mar 20, 2025

[$250] Expense Reports - Expense category field is not positioned under Category column #58822

Closed

8 tasks

jponikarchuk mentioned this pull request Mar 20, 2025

[Due for payment 2025-04-10] [Better Expense Report View] Expense Reports - Submit and approve system message appear in report, but not pay message #58851

Closed

8 tasks

deetergp mentioned this pull request Mar 20, 2025

[Better Expense Report View] Reports - LHN tab highlights Reports when expense report is opened #58810

Closed

8 tasks

Kicu mentioned this pull request Mar 25, 2025

Improve MoneyRequestReportView #58903

Merged

48 tasks

s77rt reviewed May 5, 2025

View reviewed changes

s77rt mentioned this pull request May 5, 2025

[Due for payment 2025-05-05] [$250] Expense Reports - Invite user whisper is missing Concierge user name and avatar #59948

Closed

8 tasks

ikevin127 reviewed May 8, 2025

View reviewed changes

fedirjh reviewed May 13, 2025

View reviewed changes

fedirjh mentioned this pull request May 13, 2025

[Due for payment 2025-05-13] [$250] iOS - Expense Reports - No suggestion list when typing emoji or user mention in expense report #60747

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Better Expense Reports] Add MoneyRequestReportView #58360

[Better Expense Reports] Add MoneyRequestReportView #58360

Kicu commented Mar 13, 2025 •

edited

Loading

Kicu Mar 17, 2025 •

edited

Loading

mountiny Mar 17, 2025

Kicu commented Mar 17, 2025

melvin-bot bot commented Mar 17, 2025

Kicu commented Mar 17, 2025

shawnborton commented Mar 17, 2025

luacmartins commented Mar 17, 2025

luacmartins commented Mar 17, 2025

luacmartins left a comment

luacmartins Mar 17, 2025

DylanDylann Mar 18, 2025

Kicu Mar 18, 2025

luacmartins Mar 17, 2025

mountiny Mar 17, 2025

Kicu Mar 18, 2025 •

edited

Loading

Kicu Mar 18, 2025

Kicu commented Mar 19, 2025

mountiny left a comment

trjExpensify commented Mar 19, 2025

mountiny commented Mar 19, 2025

JakubKorytko commented Mar 19, 2025

OSBotify commented Mar 19, 2025

Kicu commented Mar 19, 2025

github-actions bot commented Mar 20, 2025

deetergp commented Mar 20, 2025

trjExpensify commented Mar 20, 2025

deetergp commented Mar 20, 2025

github-actions bot commented Mar 21, 2025

s77rt May 5, 2025

ikevin127 May 8, 2025

fedirjh May 13, 2025

	// Todo first should have avatar - verify that this works with long chats
	// Todo first should have avatar - verify that this works with long chats

		function findNextAction(reportActions: ReportAction[], actionIndex: number): OnyxEntry<ReportAction> {
		for (let i = actionIndex - 1; i > 0; i--) {

[Better Expense Reports] Add MoneyRequestReportView #58360

[Better Expense Reports] Add MoneyRequestReportView #58360

Conversation

Kicu commented Mar 13, 2025 • edited Loading

Explanation of Change

These don't yet work and will be done in separated PR

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Kicu Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kicu commented Mar 17, 2025

melvin-bot bot commented Mar 17, 2025

Kicu commented Mar 17, 2025

shawnborton commented Mar 17, 2025

luacmartins commented Mar 17, 2025

luacmartins commented Mar 17, 2025

luacmartins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kicu Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kicu commented Mar 19, 2025

mountiny left a comment

Choose a reason for hiding this comment

trjExpensify commented Mar 19, 2025

mountiny commented Mar 19, 2025

JakubKorytko commented Mar 19, 2025

OSBotify commented Mar 19, 2025

Kicu commented Mar 19, 2025

github-actions bot commented Mar 20, 2025

deetergp commented Mar 20, 2025

trjExpensify commented Mar 20, 2025

deetergp commented Mar 20, 2025

github-actions bot commented Mar 21, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kicu commented Mar 13, 2025 •

edited

Loading

Kicu Mar 17, 2025 •

edited

Loading

Kicu Mar 18, 2025 •

edited

Loading