[HOLD for payment 2024-11-21] [$250] After hold and unhold expense page is not scrolling down to comments section

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.41-6
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dukenv0307
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1727724158339229

Action Performed:

1. Go to a workspace
2. Submit an expense
3. Go to the expense detail page and click on the header
4. Hold this expense
5. Unhold this expense

Expected Result:

After hold and unhold expense, it is scroll to bottom

Actual Result:

After hold and unhold expense, it isn't scrolling to the comment section

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

bug.1.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021841168390578719829
• Upwork Job ID: 1841168390578719829
• Last Price Increase: 2024-10-29
• Automatic offers:
• dukenv0307 | Reviewer | 104747445
• nkdengineer | Contributor | 104747448

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @sakluger

[melvin-bot]

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[nkdengineer]

Edited by proposal-police: This proposal was edited at 2024-10-11 04:44:02 UTC.

Proposal

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

After hold and unhold expense is not scrolling down

What is the root cause of that problem?

After hold and unhold expense, we don't have any logic to scroll

App/src/libs/actions/IOU.ts

Line 7924 in 96d8973

'HoldRequest',

App/src/libs/actions/IOU.ts

Line 8025 in 96d8973

'UnHoldRequest',

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

We should use Report.notifyNewAction in 2 function unholdRequest and putOnHold to scroll to the bottom of this report like we did in other places

    API.write(
        'HoldRequest',
    ...
    );
    const report = ReportUtils.getReport(reportID);
    const parentReportAction = ReportActionsUtils.getReportAction(report?.parentReportID ?? '-1', report?.parentReportActionID ?? '-1');
    const currentReportID = ReportUtils.isOneTransactionThread(report?.reportID ?? '', report?.parentReportID ?? '', parentReportAction) ? report?.parentReportID : reportID ;
    
    Report.notifyNewAction(currentReportID, userAccountID);

    API.write(
        'UnHoldRequest',
    ...
    );
    const report = ReportUtils.getReport(reportID);
    const parentReportAction = ReportActionsUtils.getReportAction(report?.parentReportID ?? '-1', report?.parentReportActionID ?? '-1');
    const currentReportID = ReportUtils.isOneTransactionThread(report?.reportID ?? '', report?.parentReportID ?? '', parentReportAction) ? report?.parentReportID : reportID ;

    Report.notifyNewAction(currentReportID, userAccountID);

What alternative solutions did you explore? (Optional)

[dukenv0307]

@sakluger I report this bug so I can help take it as C+

[huult]

Proposal

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

After hold and unhold expense page is not scrolling down to comments section

What is the root cause of that problem?

We haven't handled scrolling to the bottom when HOLD or UNHOLD changes yet

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

We should check if sortedVisibleReportActions has changed to include HOLD or UNHOLD in order to determine if we need to scroll to the bottom. Something like this:

//.src/pages/home/report/ReportActionsList.tsx#L203
+const previousReport = useRef<OnyxTypes.ReportAction[]>([]);
+ useEffect(() => {
+        if (!previousReport.current && Array.isArray(sortedVisibleReportActions) && sortedVisibleReportActions.length) {
+            previousReport.current = sortedVisibleReportActions;
+            return;
+        }

+        if (Array.isArray(sortedVisibleReportActions) && sortedVisibleReportActions.length) {
+            const previousActions = previousReport.current || [];

+            const newActions = sortedVisibleReportActions.filter((newAction) => !previousActions.some((prevAction) => +prevAction.reportActionID === newAction.reportActionID));
+            const containsHoldOrUnhold = newActions.some((action) => action?.actionName === 'HOLD' || action?.actionName + === 'UNHOLD');

+            if (containsHoldOrUnhold) {
+                InteractionManager.runAfterInteractions(() => {
+                    requestAnimationFrame(() => {
+                        reportScrollManager.scrollToBottom();
+                    });
+                });
+            }

+            previousReport.current = sortedVisibleReportActions;
+        }
+    }, [reportScrollManager, sortedVisibleReportActions]);

POC

Screen.Recording.2024-10-01.at.11.59.24.mov

[melvin-bot]

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

[melvin-bot]

Current assignee @dukenv0307 is eligible for the External assigner, not assigning anyone new.

[sakluger]

Thanks @dukenv0307!

[raza-ak]

Proposal

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

After hold and unhold expense page is not scrolling down to comments section

What is the root cause of that problem?

The "ReportActionsList.tsx" component does not currently handle auto-scrolling when new messages or comments are added. As a result, the page does not scroll to the latest comment when an expense is held or unheld.

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

We will update the "ReportActionsList.tsx" component to detect when a new message arrives and implement automatic scrolling to the latest comment whenever an expense is held or unheld. This will ensure that the page always scrolls down to display the newest message.

Video:

372422564-47842cb7-2be7-4562-bd9d-3b2cac706e1d.mp4

[dukenv0307]

I'm reviewing...

[sakluger]

@dukenv0307 any updates?

[dukenv0307]

@sakluger I'll update in a few hours. The first proposal looks promising, but sometime it doesn't work

[dukenv0307]

@nkdengineer I tested your proposal, but sometime it doesn't work, especially when there's only one money request. Can you check again?

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[nkdengineer]

@dukenv0307 i'll update today

[huult]

@dukenv0307 Could you please review my proposal and POC? It is working as expected.

[dukenv0307]

@huult

We should check if sortedVisibleReportActions has changed to include HOLD or UNHOLD in order to determine if we need to scroll to the bottom. Something like this:

I don't think it's a good idea, when users add a new comment, the list automatically scrolls to the bottom without checking sortedVisibleReportActions has new action. We should do the same as Add comment

[raza-ak]

@dukenv0307 Did you check my proposal?

[dukenv0307]

@raza-ak We don't need to do that for every new message. For example the whisper message, new message from others, ...

[nkdengineer]

@dukenv0307 i looks like with one transaction thread, the reportID in this function is the reportID of the transaction

App/src/libs/actions/IOU.ts

Line 7906 in 4c90d62

function putOnHold(transactionID: string, comment: string, reportID: string, searchHash?: number) {

and the reportID in here is the reportID of transaction thread

App/src/pages/home/report/ReportActionsList.tsx

Line 371 in 4c90d62

const unsubscribe = Report.subscribeToNewActionEvent(report.reportID, scrollToBottomForCurrentUserAction);

so when we call notifyNewAction it can't run callback function scrollToBottomForCurrentUserAction

updated proposal

[sakluger]

We're still reviewing proposals.

[melvin-bot]

Triggered auto assignment to @mjasikowski, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[mjasikowski]

let's go with @nkdengineer's proposal!

[melvin-bot]

📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.61-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: hold and unhold expense page is not scrolling down #52089

If no regressions arise, payment will be issued on 2024-11-21. 🎊

For reference, here are some details about the assignees on this issue:

• @dukenv0307 requires payment automatic offer (Reviewer)
• @nkdengineer requires payment automatic offer (Contributor)
• @dukenv0307 requires payment automatic offer (Reviewer)

[melvin-bot]

@dukenv0307 @sakluger @dukenv0307 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[sakluger]

Payment date is tomorrow. @dukenv0307 - please complete the BZ checklist by tomorrow, thanks.

[dukenv0307]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: https://github.com/Expensify/App/pull/33897/files#r1851278165

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again. Yes

Regression Test Proposal Template

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Test:

1. Go to a workspace
2. Submit an expense
3. Go to the expense detail page and click on the header
4. Hold this expense
5. Unhold this expense
6. Verify that: After hold and unhold expense, it is scroll to bottom

Do we agree 👍 or 👎

[mjasikowski]

@dukenv0307 looks good, I'd suggest to make point 6 a bit more precise:

Verify that the expense view automatically scrolls to bottom of the page

[sakluger]

Summarizing payment on this issue:

Contributor: @nkdengineer $250, paid via Upwork
Contributor+: @dukenv0307 $250, paid via Upwork

FYI, I received errors when completing both contracts in Upwork. I double-checked that they were paid, but please raise in Slack and tag me if there were any issues with payment.