Move selected transactions by jnowakow · Pull Request #61839 · Expensify/App

jnowakow · 2025-05-12T08:32:58Z

Explanation of Change

Fixed Issues

$ #61770
PROPOSAL: N/A

Tests

Create at least two reports.
Add some expenses to one of them.
Select at least one expense.
Click "X Selected" -> "Move transactions"
Select second report

Verify that no errors appear in the JS console

Offline tests

Create at least two reports.
Add some expenses to one of them.
Select at least one expense.
Click "X Selected" -> "Move transactions"
Select second report

QA Steps

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

Create at least two reports.
Add some expenses to one of them.
Select at least one expense.
Click "X Selected" -> "Move transactions"
Select second report

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

android-web.mov

iOS: Native

iOS: mWeb Safari

ios.mov

MacOS: Chrome / Safari

web.mov

MacOS: Desktop

melvin-bot · 2025-05-12T08:33:27Z

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

jnowakow · 2025-05-12T15:19:13Z

@waterim I used your implementation here and move UI to separate component. I think you'll work on follow up so please don't be surprised 😅

@sumo-slonik I slightly modified logic here so please take a look 😇

cc @luacmartins

jnowakow · 2025-05-12T15:36:41Z

@waterim I made a mistake and wrote that I'll work on follow up 😅 I think you're the one who's doing it so I don't want to interfere 😅 but of course I can help with it :D

jnowakow · 2025-05-12T15:47:18Z

src/libs/Navigation/Navigation.ts

-    const policyID = policyIDToCheck ?? getPolicyIDFromState(navigationRef.getRootState() as State<RootNavigatorParamList>);
-    const policyMemberAccountIDs = getPolicyEmployeeAccountIDs(policyID);
-    const shouldOpenAllWorkspace = isEmptyObject(targetReport) ? true : !doesReportBelongToWorkspace(targetReport, policyMemberAccountIDs, policyID);
+    isNavigationReady().then(() => {


This change was approved by @WojtekBoman himself 🙇

the man, the legend

luacmartins

Looking good. Left a few minor comments

luacmartins · 2025-05-12T16:14:52Z

assets/images/document-merge.svg

This asset doesn't seem to follow the same pattern as others. @jnowakow did you just extract it directly from figma?

Yes and then I removed attributes that were not present in assets/images/document-plus.svg

In App it looks good

I think we have a different process to export these assets. @Expensify/design could you please assist?

Yeah, who exported this? Did it come from the design team or not?

Try this one: document-move.svg.zip

Yeah, who exported this? Did it come from the design team or not?

I don't think so, I think @jnowakow just got it from figma

Is this addressed @jnowakow?

src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx

src/hooks/useSelectedTransactionsActions.ts

luacmartins · 2025-05-12T16:26:41Z

src/hooks/useSelectedTransactionsActions.ts

+        options.push({
+            text: 'Move Expenses',
+            icon: Expensicons.DocumentMerge,
+            value: 'MOVE',


Let's use a const for this, probably declared in CONST.REPORT.SECONDARY_ACTIONS.MOVE_TRANSACTIONS

I'll declare it as const on top of the file because it's ignored anyway

luacmartins · 2025-05-12T16:27:27Z

src/libs/Navigation/Navigation.ts

-    const policyID = policyIDToCheck ?? getPolicyIDFromState(navigationRef.getRootState() as State<RootNavigatorParamList>);
-    const policyMemberAccountIDs = getPolicyEmployeeAccountIDs(policyID);
-    const shouldOpenAllWorkspace = isEmptyObject(targetReport) ? true : !doesReportBelongToWorkspace(targetReport, policyMemberAccountIDs, policyID);
+    isNavigationReady().then(() => {


the man, the legend

luacmartins · 2025-05-12T16:32:03Z

src/pages/iou/request/step/IOURequestEditReportCommon.tsx

+
+function IOURequestEditReportCommon({backTo, transactionReport, selectReport}: Props) {
+    const {translate} = useLocalize();
+    const [allReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {selector: (c) => mapOnyxCollectionItems(c, reportSelector), canBeMissing: true});


I know this is not your code, but what is c? Can we use a better variable name in this case?

shawnborton · 2025-05-12T18:42:57Z

Where do we have the most up-to-date videos for review? Otherwise we can make a test build when this is ready too. Just let us know!

github-actions · 2025-05-12T20:25:05Z

🚧 @luacmartins has triggered a test app build. You can view the workflow run here.

shawnborton · 2025-05-12T21:18:39Z

Hmm at this specific breakpoint, should we be showing checkboxes? I would think not, right?

I would think at that point we just mirror mobile styles:

shawnborton · 2025-05-12T21:19:24Z

I can see the option to move an expense on a report that is already approved/paid, but when I try to move it, I see "not found" in the RHP:

allgandalf · 2025-05-14T04:27:03Z

bug: submitted expense has move expense option, nothing happens when we click on it:

Screen.Recording.2025-05-14.at.9.56.24.AM.mov

…ve-transactions

jnowakow · 2025-05-14T07:36:09Z

Bug: There is a move expense option even when the policy chat has only one report:

It was done this way before so I didn't alter this behaviour

jnowakow · 2025-05-14T07:42:40Z

bug: submitted expense has move expense option, nothing happens when we click on it:

I can't reproduce this one

allgandalf

I also cannot reproduce the second bug with new report! thanks

jnowakow · 2025-05-14T08:53:22Z

@luacmartins @trjExpensify should it be possible to move all expenses from submitted report? It leads to situation where there's a report without expenses and there's no option to add expense to it

Screen.Recording.2025-05-14.at.10.50.16.mov

jnowakow · 2025-05-14T08:58:54Z

Ahh okey after #61465 is merged it will be possible to bring expenses back but this submitted $0.00 looks strange and there's no button above 🤔

shawnborton · 2025-05-14T09:24:27Z

Hmm interesting, I would think that in that case above, we should probably just delete the report since it no longer has expenses on it? Curious what Tom thinks though.

dannymcclain · 2025-05-14T13:00:20Z

Hmm interesting, I would think that in that case above, we should probably just delete the report since it no longer has expenses on it? Curious what Tom thinks though.

Yeah, it seems pretty unlikely that someone is going to move all expenses off a report and then add new ones to that old report? Curious what Tom thinks though for sure.

luacmartins · 2025-05-14T15:38:37Z

@luacmartins @trjExpensify should it be possible to move all expenses from submitted report? It leads to situation where there's a report without expenses and there's no option to add expense to it

Yea, this is possible.

I would think that in that case above, we should probably just delete the report since it no longer has expenses on it?

We don't do this in OldDot, so I don't think we should start it now. Let's just keep the empty report.

MarioExpensify

LGTM!

luacmartins

LGTM

luacmartins · 2025-05-14T18:26:49Z

src/libs/ReportSecondaryActionUtils.ts

+    const canMoveExpense = canEditFieldOfMoneyRequest(iouReportAction, CONST.EDIT_REQUEST_FIELD.REPORT);
+
+    return canMoveExpense;


NAB

Suggested change

const canMoveExpense = canEditFieldOfMoneyRequest(iouReportAction, CONST.EDIT_REQUEST_FIELD.REPORT);

return canMoveExpense;

return canEditFieldOfMoneyRequest(iouReportAction, CONST.EDIT_REQUEST_FIELD.REPORT);

luacmartins · 2025-05-14T18:29:50Z

@jnowakow we have conflicts

jnowakow · 2025-05-15T09:17:37Z

@luacmartins conflicts solved

luacmartins

LGTM

OSBotify · 2025-05-15T13:56:02Z

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

github-actions · 2025-05-20T10:00:14Z

🚀 Deployed to production by https://github.com/mountiny in version: 9.1.46-12 🚀

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

rojiphil · 2025-06-07T01:15:17Z

src/pages/iou/request/step/IOURequestEditReportCommon.tsx

+    const currentUserPersonalDetails = useCurrentUserPersonalDetails();
+    const [searchValue, debouncedSearchValue, setSearchValue] = useDebouncedState('');
+
+    const expenseReports = getOutstandingReportsForUser(transactionReport?.policyID, transactionReport?.ownerAccountID ?? currentUserPersonalDetails.accountID, allReports ?? {});


We need to exclude reports from deleted workspaces when moving transactions. We fixed this in #62552

Add screen - not working

12a5296

jnowakow requested a review from a team as a code owner May 12, 2025 08:32

jnowakow marked this pull request as draft May 12, 2025 08:33

melvin-bot bot requested review from MarioExpensify and removed request for a team May 12, 2025 08:33

Move selected transactions

4c06432

jnowakow changed the title ~~Add screen - not working~~ Move selected transactions May 12, 2025

Fix navigation on goBack

4ea1d2a

jnowakow added 2 commits May 12, 2025 17:21

fix linter

bfb15db

Add missing return

319ffd2

jnowakow marked this pull request as ready for review May 12, 2025 15:36

jnowakow requested a review from a team as a code owner May 12, 2025 15:36

jnowakow commented May 12, 2025

View reviewed changes

jnowakow mentioned this pull request May 12, 2025

[Due for payment 2025-05-30] [Due for payment 2025-05-27] [Report Creation UI] Add Move expense to the bulk action menu #61770

Closed

2 tasks

luacmartins requested review from DylanDylann, allgandalf and luacmartins and removed request for allgandalf May 12, 2025 16:10

luacmartins assigned jnowakow May 12, 2025

luacmartins requested changes May 12, 2025

View reviewed changes

Apply suggestions after review

eae4929

jnowakow requested a review from luacmartins May 12, 2025 17:35

This comment has been minimized.

Sign in to view

Merge branch 'main' of github.com:Expensify/App into jnowakow/bulk-mo…

b3e80b3

…ve-transactions

Apply review suggestions

8b9cc72

allgandalf approved these changes May 14, 2025

View reviewed changes

MarioExpensify previously approved these changes May 14, 2025

View reviewed changes

luacmartins previously approved these changes May 14, 2025

View reviewed changes

Merge branch 'main' into jnowakow/bulk-move-transactions

2183b33

jnowakow dismissed stale reviews from luacmartins and MarioExpensify via 2183b33 May 15, 2025 09:17

refactor getting transactions

de2f138

luacmartins approved these changes May 15, 2025

View reviewed changes

luacmartins merged commit 5aa3ba0 into Expensify:main May 15, 2025
16 checks passed

francoisl mentioned this pull request May 16, 2025

Deploy Checklist: New Expensify 2025-05-15 #62161

Closed

88 tasks

nkdengineer mentioned this pull request May 16, 2025

[Due for payment 2025-05-28] [Due for payment 2025-05-27] Report field opens Not here page when sharing expense to accountant on confirmation page #62156

Closed

8 tasks

robertjchen mentioned this pull request May 16, 2025

[HOLD] Expense - Unable to move expense to the report of a track expense. #62154

Open

8 tasks

nkdengineer mentioned this pull request May 20, 2025

[Due for payment 2025-06-05] [$250] Expense-App opens expense report instead of returning to confirm page after selecting a report #62343

Closed

8 tasks

This was referenced May 21, 2025

[$250] Web-RHN-After creating a room, pressing back button navigates to room creation #60609

Closed

[Due for payment 2025-06-17] [$250] Android - search-Search results are opened with delay #62026

Closed

rojiphil reviewed Jun 7, 2025

View reviewed changes

		const canMoveExpense = canEditFieldOfMoneyRequest(iouReportAction, CONST.EDIT_REQUEST_FIELD.REPORT);

		return canMoveExpense;

Conversation

jnowakow commented May 12, 2025 • 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!

melvin-bot bot commented May 12, 2025

Uh oh!

jnowakow commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnowakow commented May 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luacmartins left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnowakow May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnowakow May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shawnborton commented May 12, 2025

Uh oh!

github-actions bot commented May 12, 2025

Uh oh!

This comment has been minimized.

shawnborton commented May 12, 2025

Uh oh!

shawnborton commented May 12, 2025

Uh oh!

allgandalf commented May 14, 2025

Uh oh!

jnowakow commented May 14, 2025

Uh oh!

jnowakow commented May 14, 2025

Uh oh!

allgandalf left a comment

Choose a reason for hiding this comment

Uh oh!

jnowakow commented May 14, 2025

Uh oh!

jnowakow commented May 14, 2025

Uh oh!

shawnborton commented May 14, 2025

Uh oh!

dannymcclain commented May 14, 2025

jnowakow commented May 12, 2025 •

edited

Loading

jnowakow commented May 12, 2025 •

edited

Loading

jnowakow May 12, 2025 •

edited

Loading

jnowakow May 12, 2025 •

edited

Loading