Refactor `putOnhold` to use the `useAncestors()` hook. by Tony-MK · Pull Request #74628 · Expensify/App

Tony-MK · 2025-11-08T01:19:20Z

Explanation of Change

Fixed Issues

$ #73566
PROPOSAL:

Tests

Create an expense.
Navigate to the expense thread.
Enable Force Offline.
Hold the expense.
Navigate back to the report preview.
Verify the reply count is one.
Disable Force Offline.
Verify the reply count doesn't change.

Verify that no errors appear in the JS console

Offline tests

QA Steps

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

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android.-.Native.webm

Android: mWeb Chrome

Android.-.mWeb.webm

iOS: Native

iOS.-.mWeb.mp4

iOS: mWeb Safari

iOS.-.Native.mp4

MacOS: Chrome / Safari

macOS.-.Chrome.mov

MacOS: Desktop

macOS.-.Desktop.mov

- Introduced `getAncestors` function to traverse report hierarchy and fetch ancestor reports and actions. - Updated `putOnHold` and `putTransactionsOnHold` functions to accept ancestor data. - Integrated `useTransactionsAncestors` hook in `SearchHoldReasonPage` to retrieve ancestors for selected transactions.

…mport in IOU actions

codecov · 2025-11-08T01:30:49Z

Codecov Report

❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.

Files with missing lines	Coverage Δ
src/pages/iou/HoldReasonPage.tsx	`0.00% <ø> (ø)`
src/libs/actions/IOU.ts	`64.64% <50.00%> (-0.04%)`	⬇️
src/pages/Search/SearchHoldReasonPage.tsx	`0.00% <0.00%> (ø)`
... and 3 files with indirect coverage changes

…into putOnHold

…nction to provide default value for ancestors

Tony-MK · 2025-11-10T21:47:42Z

Hey @luacmartins and @FitseTLT, if a report has multiple transactions. Since the report will still have no new hold comments, expect the transaction thread. We are supposed to update the report's parent action. E.g.: childCommentCount, etc...

luacmartins · 2025-11-10T22:32:27Z

@Tony-MK sorry, I'm not sure that I understood your comment. Could you please list the steps to reproduce the issue you're referring to?

Tony-MK · 2025-11-10T22:44:11Z

This is after holding and unholding a singular expense, then adding another expense to the report.

Mutiple.Transactions.mov

Should the reply count change like holding a single expense? I believe it should not update the REPORT_PREVIEW action's childCommentCount value. It should be zero since there are no visible comments.

luacmartins · 2025-11-10T22:47:41Z

@Tony-MK isn't the visible comment in the transaction thread?

Tony-MK · 2025-11-11T01:15:59Z

Apologies, this is a bug that has been happening since staging because putTransactionsOnHold doesn't update using the current new ancestors optimistic data; instead, it uses the old ancestors data value. In short, it only increases the childCommentCount by 1, regardless of the number of transaction threads that created a hold comment.

…handling

…eLogList

… and update dev server host

…r support and optimizing report name parsing

…emove unused parameters

…empty string

Tony-MK · 2025-11-19T19:46:29Z

Hey @luacmartins, could we create an issue for #74628 (comment)? I already started a draft PR. Thanks

situchan · 2025-11-19T19:48:17Z

we are checking if the ancestors report action is updated correctly.

ok, makes sense

Tony-MK · 2025-11-19T19:49:53Z

Updated the test case.

situchan · 2025-11-19T19:51:05Z

Please merge main

situchan · 2025-11-19T20:15:56Z

Expense thread after creating new expense seems broken on main

Screen.Recording.2025-11-20.at.2.14.14.AM.mov

Had to test with old expenses

Tony-MK · 2025-11-19T20:19:12Z

Let me look into it.

situchan · 2025-11-19T20:30:25Z

Pretty bad regression. I see it's reported as deploy blocker - #75565

situchan · 2025-11-20T09:01:33Z

Another bug happening on main:

duplicated/unnecessary report preview shows in transaction thread when offline. disappears after online

Screen.Recording.2025-11-20.at.2.55.46.PM.mov

situchan

Above bugs are not caused by this PR

OSBotify · 2025-11-20T17:43:26Z

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

OSBotify · 2025-11-21T12:26:33Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.2.62-0 🚀

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

deetergp · 2025-11-21T23:59:35Z

I'm pretty sure this PR caused this blocker. After confirming the behavior in my local Android simulator, I reverted this PR and I was able to hold the expense from the Reports screen drop down.

This reverts commit 5c64aa8, reversing changes made to 94d3272.

situchan · 2025-11-22T03:03:41Z

@Tony-MK please check ^

[CP Staging] Revert "Merge pull request #74628 from Tony-MK/putOnHold"

Tony-MK · 2025-11-24T01:41:33Z

Hey @situchan, created a PR for it.

situchan · 2025-11-24T02:52:02Z

src/pages/Search/SearchHoldReasonPage.tsx

                context.clearSelectedTransactions(true);
            } else {
-                holdMoneyRequestOnSearch(context.currentSearchHash, Object.keys(context.selectedTransactions), comment, allTransactions, allReportActions);
+                holdMoneyRequestOnSearch(context.currentSearchHash, context.selectedTransactionIDs, comment, allTransactions, allReportActions);


Looks like you reverted this change in new PR.
What was this change for and why did it cause the issue?

The issue was that the context was not available because of https://github.com/Expensify/App/pull/75833/files#r2554436669.

ok you answered to why did it cause the issue?

Now what was this change for?

@Tony-MK anything that I can help with to move this forward?

[CP Staging] Revert "Merge pull request #74628 from Tony-MK/putOnHold" (cherry picked from commit a60213e) (cherry-picked to staging by mountiny)

OSBotify · 2025-11-24T20:01:35Z

🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.62-5 🚀

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

OSBotify · 2025-11-24T20:23:15Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.2.63-0 🚀

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

OSBotify · 2025-11-26T01:21:41Z

🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.63-8 🚀

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

Tony-MK added 3 commits November 7, 2025 23:24

refactor: useTransactionsAncestors parameter type and remove unused i…

0e931fe

…mport in IOU actions

Merge branch 'Expensify:main' into putOnHold

50ce0a4

melvin-bot bot assigned Tony-MK Nov 8, 2025

Tony-MK and others added 8 commits November 8, 2025 04:57

Merge branch 'putOnHold' of https://github.com/Tony-MK/Expensify-App …

7d144aa

…into putOnHold

refactor:useAncestors imports for consistency and update putOnHold fu…

dd9b64b

…nction to provide default value for ancestors

refactor

ff783be

lint

5d48c91

Merge branch 'Expensify:main' into putOnHold

e210e29

Merge branch 'main' of https://github.com/Expensify/App into putOnHold

0154e03

chore: update IOU.ts with necessary changes

e177051

fix: streamline transaction selection in SearchHoldReasonPage

7848efb

Merge branch 'main' into putOnHold

2fd0f16

This comment was marked as resolved.

Sign in to view

Tony-MK added 8 commits November 11, 2025 01:48

refactor: update putTransactionsOnHold function to simplify ancestor …

1dbb724

…handling

refactor: remove unused imports and simplify attachment logic in Shar…

45e2a0f

…eLogList

refactor: remove unused ancestor handling from task-related functions…

7554239

… and update dev server host

Merge branch 'main' of https://github.com/Expensify/App into putOnHold

bd60f06

refactor: enhance task creation and report handling by adding ancesto…

0f6a89b

…r support and optimizing report name parsing

refactor: simplify ancestor fetching logic in useAncestors hook and r…

2f44302

…emove unused parameters

fix: ensure previousMessage is defined by providing a fallback to an …

79fc04a

…empty string

Merge branch 'main' into putOnHold

530617d

Tony-MK marked this pull request as ready for review November 11, 2025 20:10

Tony-MK requested a review from a team as a code owner November 11, 2025 20:10

Merge branch 'Expensify:main' into putOnHold

a901038

situchan approved these changes Nov 20, 2025

View reviewed changes

melvin-bot bot requested a review from luacmartins November 20, 2025 09:02

luacmartins approved these changes Nov 20, 2025

View reviewed changes

luacmartins merged commit 5c64aa8 into Expensify:main Nov 20, 2025
27 checks passed

OSBotify mentioned this pull request Nov 21, 2025

Deploy Checklist: New Expensify 2025-11-21 #75728

Closed

deetergp added a commit that referenced this pull request Nov 22, 2025

Revert "Merge pull request #74628 from Tony-MK/putOnHold"

002fa09

This reverts commit 5c64aa8, reversing changes made to 94d3272.

deetergp mentioned this pull request Nov 22, 2025

[CP Staging] Revert "Merge pull request #74628 from Tony-MK/putOnHold" #75803

Merged

54 tasks

situchan mentioned this pull request Nov 22, 2025

Removing violations when policy category is created #75485

Closed

54 tasks

mountiny added a commit that referenced this pull request Nov 23, 2025

Merge pull request #75803 from Expensify/scott-revertPutOnHoldRefactor

a60213e

[CP Staging] Revert "Merge pull request #74628 from Tony-MK/putOnHold"

situchan reviewed Nov 24, 2025

View reviewed changes

situchan mentioned this pull request Nov 24, 2025

Fix - Unable to hold expenses from dropdown menu #75833

Closed

54 tasks

melvin-bot bot mentioned this pull request Nov 26, 2025

[Due for payment 2026-01-21] [$125] Refactor putOnHold to use useAncestorReportsAndReportActions hook #73566

Closed

Conversation

Tony-MK commented Nov 8, 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!

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

Uh oh!

Codecov Report

Uh oh!

Tony-MK commented Nov 10, 2025

Uh oh!

luacmartins commented Nov 10, 2025

Uh oh!

Tony-MK commented Nov 10, 2025

Uh oh!

luacmartins commented Nov 10, 2025

Uh oh!

This comment was marked as resolved.

Tony-MK commented Nov 11, 2025

Uh oh!

Tony-MK commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

situchan commented Nov 19, 2025

Uh oh!

Tony-MK commented Nov 19, 2025

Uh oh!

situchan commented Nov 19, 2025

Uh oh!

situchan commented Nov 19, 2025

Uh oh!

Tony-MK commented Nov 19, 2025

Uh oh!

situchan commented Nov 19, 2025

Uh oh!

situchan commented Nov 20, 2025

Uh oh!

situchan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

OSBotify commented Nov 20, 2025

Uh oh!

OSBotify commented Nov 21, 2025

Uh oh!

deetergp commented Nov 21, 2025

Uh oh!

situchan commented Nov 22, 2025

Uh oh!

Tony-MK commented Nov 24, 2025

Uh oh!

situchan Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Tony-MK Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

situchan Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

luacmartins Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

OSBotify commented Nov 24, 2025

Uh oh!

OSBotify commented Nov 24, 2025

Uh oh!

OSBotify commented Nov 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Tony-MK commented Nov 8, 2025 •

edited

Loading

codecov bot commented Nov 8, 2025 •

edited by github-actions bot

Loading

Tony-MK commented Nov 19, 2025 •

edited

Loading