Web - Workspace - IOU moves in the LHN every time the merchant is changed #38425

kbecciv · 2024-03-15T20:28:40Z

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: 1.4.53.1
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4428435&group_by=cases:section_id&group_id=294998&group_order=asc
Issue reported by: Applause - Internal Team

Action Performed:

Admin: Log in with a new Gmail account
Admin: Create a workspace
Admin: Invite a member to the workspace
Member: Create a scan IOU in the workspace chat
Member: Open the IOU and add the "Amount" and "Merchant" fields manually
Admin: Open the IOU and edit the "Merchant" field

Expected Result:

It shouldn't move. If an order change is needed due to the unread status, it should only happen once.

Actual Result:

IOU moves in the LHN every time the merchant is changed.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6415247_1710525981908.bandicam_2024-03-15_18-47-10-675.mp4

View all open jobs on GitHub

github-actions · 2024-03-15T20:28:53Z

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

melvin-bot · 2024-03-15T20:29:05Z

Triggered auto assignment to @hayata-suenaga (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

kbecciv · 2024-03-15T20:29:38Z

We think that this bug might be related to #wave-collect - Release 1

kbecciv · 2024-03-15T20:30:36Z

Issue is not reproduce in Prod

bandicam.2024-03-15.22-26-02-304.mp4

hayata-suenaga · 2024-03-15T21:01:06Z

taking a look at this issue

hayata-suenaga · 2024-03-15T21:11:29Z

This is reproducible
https://github.com/Expensify/App/assets/98560306/6c2e7584-ce99-44be-ae27-cf17595cbdc2

hayata-suenaga · 2024-03-15T21:27:58Z

asking for help in Slack

paultsimura · 2024-03-15T22:19:10Z

This might be considered a regression from #38027 – now, every time we update the MoneyRequest field, we call OpenReport because of this hook:

App/src/pages/home/ReportScreen.tsx

Lines 365 to 374 in d78c0f1

    
           // If a user has chosen to leave a thread, and then returns to it (e.g. with the back button), we need to call `openReport` again in order to allow the user to rejoin and to receive real-time updates 
        
           useEffect(() => { 
        
               if (!isFocused || prevIsFocused || !ReportUtils.isChatThread(report) || report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN) { 
        
                   return; 
        
               } 
        
               Report.openReport(report.reportID); 
        
               // We don't want to run this useEffect every time `report` is changed 
        
               // eslint-disable-next-line react-hooks/exhaustive-deps 
        
           }, [prevIsFocused, report.notificationPreference, isFocused]);

The reason why the issue happens is that on updating the money request, we set the hasOutstandingChildRequest: false for the chat report in the optimistic data – that's why the chat report moves below the money request in LHN – it has to RBR for a moment. But after we call OpenReport, the chat report gets updated with hasOutstandingChildRequest: true, and the RBR appears again, moving the chat report above the open Money Request.

However, I would blame this PR: #34866 – it changed the way we calculate the optimistic hasOutstandingChildRequest, so now we are setting it to false, when we shouldn't.

paultsimura · 2024-03-15T22:36:35Z

Proposal

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

The chat report's GBR disappears for a moment when modifying the money request data.

Note: this shouldn't really be a blocker, because the version on Staging only allowed this bug to be more visible – it exists in Prod as well.

What is the root cause of that problem?

The reason why the issue happens is that on updating the money request, we set the hasOutstandingChildRequest: false for the chat report in the optimistic data – that's why the chat report moves below the money request in LHN – it has no GBR for a moment.

In this PR, we've added the logic for calculating the optimistic hasOutstandingChildRequest:

App/src/libs/actions/IOU.ts

Lines 1261 to 1262 in 2979e3c

    
           hasOutstandingChildRequest: 
        
               iouReport && needsToBeManuallySubmitted(iouReport) && updatedMoneyRequestReport.managerID === userAccountID && updatedMoneyRequestReport.total !== 0,

However, this code doesn't work as expected. e.g. it strongly checks needsToBeManuallySubmitted(iouReport) for every iouReport, but I should have the GBR if I'm the admin of the workspace even if the request doesn't need to be manually submitted. Also, the mentioned PR broke the existing logic: #33030 (the first test).

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

First, we should modify the getOutstandingChildRequest to correctly calculate the GBR for different scenarios (it may require some further alterations, need to test different cases deeper):

function getOutstandingChildRequest(policy: OnyxEntry<Policy> | EmptyObject, iouReport: OnyxEntry<Report> | EmptyObject): OutstandingChildRequest {
    if (!iouReport || isEmptyObject(iouReport)) {
        return {};
    }

    if (!isPolicyExpenseChat(iouReport)) {
        return {
            hasOutstandingChildRequest: iouReport.managerID === currentUserAccountID && iouReport.total !== 0,
        };
    }

    if (!needsToBeManuallySubmitted(iouReport)) {
        return {
            hasOutstandingChildRequest: false,
        };
    }

    if (PolicyUtils.isPolicyAdmin(policy)) {
        return {
            hasOutstandingChildRequest: true,
        };
    }

    // We don't need to update hasOutstandingChildRequest in this case
    return {};
}

App/src/libs/actions/IOU.ts

Lines 419 to 438 in 5770e23

    
           /** 
        
            * Return the object to update hasOutstandingChildRequest 
        
            */ 
        
           function getOutstandingChildRequest(policy: OnyxEntry<OnyxTypes.Policy> | EmptyObject, iouReport: OnyxTypes.Report): OutstandingChildRequest { 
        
               if (!needsToBeManuallySubmitted(iouReport)) { 
        
                   return { 
        
                       hasOutstandingChildRequest: false, 
        
                   }; 
        
               } 
        
               if (PolicyUtils.isPolicyAdmin(policy)) { 
        
                   return { 
        
                       hasOutstandingChildRequest: true, 
        
                   }; 
        
               } 
        
               return { 
        
                   hasOutstandingChildRequest: iouReport.managerID === userAccountID && iouReport.total !== 0, 
        
               }; 
        
           }

Second, use this function instead of the incorrect custom checks:

        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport?.parentReportID}`,
            value: ReportUtils.getOutstandingChildRequest(policy, updatedMoneyRequestReport),
        },

App/src/libs/actions/IOU.ts

Lines 1257 to 1264 in 5770e23

    
           { 
        
               onyxMethod: Onyx.METHOD.MERGE, 
        
               key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport?.parentReportID}`, 
        
               value: { 
        
                   hasOutstandingChildRequest: 
        
                       iouReport && needsToBeManuallySubmitted(iouReport) && updatedMoneyRequestReport.managerID === userAccountID && updatedMoneyRequestReport.total !== 0, 
        
               }, 
        
           },

App/src/libs/actions/IOU.ts

Lines 3029 to 3035 in 5770e23

    
           { 
        
               onyxMethod: Onyx.METHOD.MERGE, 
        
               key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport?.reportID}`, 
        
               value: { 
        
                   hasOutstandingChildRequest: iouReport && needsToBeManuallySubmitted(iouReport) && updatedIOUReport?.managerID === userAccountID && updatedIOUReport.total !== 0, 
        
               }, 
        
           },

What alternative solutions did you explore? (Optional)

hayata-suenaga · 2024-03-15T23:23:58Z

@paultsimura, I opened a PR to implement the fix you suggested. As you're also a C+, I assigned in the PR as a reviewer.

paultsimura · 2024-03-18T20:53:18Z

Updated proposal: #38425 (comment)
Added more details after deeper testing of the initial PR.

hayata-suenaga · 2024-03-19T17:07:55Z

@paultsimura let's go with your updated proposal. Please open a PR when you have time 🙇

situchan · 2024-03-19T17:09:38Z

Looks like I was recommended as C+ 😄
Happy to help

hayata-suenaga · 2024-04-03T15:29:46Z

I'm sorry @situchan, but a C+ seems to be automatically assigned to the PR and they already reviewed it 🙇

paultsimura · 2024-04-03T16:53:40Z

I'm sorry @situchan, but a C+ seems to be automatically assigned to the PR and they already reviewed it 🙇

It's OK, Situ went OOO and asked to re-assign this issue 👍

melvin-bot · 2024-04-09T12:36:10Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

paultsimura · 2024-04-11T07:05:03Z

This was deployed to production yesterday, the automation didn't work: #38675 (comment)

paultsimura · 2024-04-16T08:05:46Z

This is due payment tomorrow.
@hayata-suenaga could you please add a corresponding label to assign a BZ member?

hayata-suenaga · 2024-04-16T16:01:09Z

sorry about that. apparently, i added and then removed the label 😆

paultsimura · 2024-04-16T16:19:06Z

Also @akinwale bump for a checklist, please 🙏

paultsimura · 2024-04-17T06:38:21Z

@hayata-suenaga I'm not sure that was the correct label though because it didn't assign a BZ member. I think you should have added & removed the External & Help Wanted labels first 🤔

akinwale · 2024-04-17T06:48:05Z

@hayata-suenaga @paultsimura I believe adding the Bug label should assign a BZ member.

akinwale · 2024-04-17T07:14:34Z

[@akinwale] The PR that introduced the bug has been identified. Link to the PR:

[@akinwale] 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:

[@akinwale] A discussion in #expensify-bugs 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:

Likely regression from #34866, where the way the value for hasOutstandingChildRequest is determined was changed.

[@akinwale] Determine if we should create a regression test for this bug.

[@akinwale] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Steps

Test 1

Launch Expensify.
Request money from a workspace that you own.
Open the report for the IOU request.
Set the device offline.
Modify the amount on the request.
Verify that the GBR is displayed for the workspace chat in the LHN.
Go back online.
Verify that the GBR is still visible for the workspace chat in the LHN.

Test 2 (Scheduled Submit disabled)

Prerequisites

Log in as an employee in a Collect workspace with the Scheduled Submit option disabled
The employee has no pending money requests in the workspace

Go offline on the device.
Request money from the workspace.
Verify that the GBR is displayed for the workspace chat in the LHN.
Go back online.
Verify that the GBR is still visible for the workspace chat in the LHN.

Test 3 (Scheduled Submit enabled)

Prerequisites

Log in as an employee in a Collect workspace with the Scheduled Submit option enabled
The employee has no pending money requests in the workspace

Go offline on the device.
Request money from the workspace.
Verify that the GBR is not displayed for the workspace chat in the LHN.
Go back online.
Verify that the GBR is still not visible for the workspace chat in the LHN.

Test 4

Prerequisite: The money request currency should match the workspace currency

Log in to Expensify as user A and user B on different devices
Request money from each user as the other as follows:
- User A to User B: Request $20
- User B to User A: Request $10

As User A

Verify that the GBR is not displayed on the chat report with B in the LHN.
Go offline on the device.
Edit the amount on the request to $5
Verify that the GBR is displayed on the chat report with user B as A now owes money to B.
Go back online.
Verify that the GBR is still displayed.
Go offline again.
Create a new request to user B, with the total amount being larger than what B requested from A ($10). For example, send a request for $10, so that the total amount becomes $15 ($10 + $5).
Verify that the GBR is not displayed on the chat report with user B as A does not owe money to B.
Go back online.
Verify that the GBR is still not displayed.
Go offline again.
Delete the request created in step 8 ($10).
Verify that the GBR is displayed on the chat report with user B as A now owes money to B.
Go back online.
Verify that the GBR is still displayed.

Do we agree 👍 or 👎?

melvin-bot · 2024-04-17T16:15:12Z

Triggered auto assignment to @laurenreidexpensify (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.

laurenreidexpensify · 2024-04-18T10:12:28Z

@paultsimura @akinwale offers sent in Upwork 👍

akinwale · 2024-04-18T10:39:45Z

@laurenreidexpensify Accepted. Thanks!

paultsimura · 2024-04-18T11:46:07Z

@laurenreidexpensify accepted, thanks

laurenreidexpensify · 2024-04-18T12:39:13Z

Payment Summary:

C+ @akinwale $500 paid in upwork
Contributor @paultsimura $500 paid in upwork

kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Mar 15, 2024

github-actions bot added Engineering Hourly KSv2 labels Mar 15, 2024

melvin-bot bot assigned hayata-suenaga Mar 15, 2024

github-actions bot mentioned this issue Mar 15, 2024

Deploy Checklist: New Expensify 2024-03-15 #38382

Closed

35 tasks

melvin-bot bot mentioned this issue Mar 15, 2024

[HOLD for payment 2024-03-25] [$500] Chat - Unable to re-join thread after leaving thread and returning with back button #37612

Closed

6 tasks

melvin-bot bot mentioned this issue Mar 15, 2024

[HOLD for payment 2024-03-07] [LOW] [P2P] [$500] IOU - Delay in updating green dot and total amount when the net amounts cancel each other out for 2 users #33774

Closed

6 tasks

hayata-suenaga removed the DeployBlockerCash This issue or pull request should block deployment label Mar 15, 2024

hayata-suenaga mentioned this issue Mar 15, 2024

Fix conditional statement for hasOutstandingChildRequest when calculating the optimistic data for IOU requests #38431

Closed

48 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Mar 15, 2024

hayata-suenaga assigned paultsimura Mar 19, 2024

hayata-suenaga assigned situchan Mar 19, 2024

paultsimura mentioned this issue Mar 20, 2024

Fix the optimistic GBR for chats with money requests #38675

Merged

48 tasks

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Mar 20, 2024

hayata-suenaga assigned akinwale and unassigned situchan Apr 3, 2024

hayata-suenaga added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review Awaiting Payment Auto-added when associated PR is deployed to production labels Apr 11, 2024

hayata-suenaga added the Awaiting Payment Auto-added when associated PR is deployed to production label Apr 16, 2024

akinwale mentioned this issue Apr 17, 2024

Fix: Delay in updating green dot and total amount #34866

Merged

56 tasks

hayata-suenaga added the Bug Something is broken. Auto assigns a BugZero manager. label Apr 17, 2024

melvin-bot bot assigned laurenreidexpensify Apr 17, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 17, 2024

laurenreidexpensify mentioned this issue Apr 18, 2024

[$500] Payment for 38425 #40415

Closed

laurenreidexpensify closed this as completed Apr 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Web - Workspace - IOU moves in the LHN every time the merchant is changed #38425

Web - Workspace - IOU moves in the LHN every time the merchant is changed #38425

kbecciv commented Mar 15, 2024 •

edited

Loading

github-actions bot commented Mar 15, 2024

melvin-bot bot commented Mar 15, 2024

kbecciv commented Mar 15, 2024

kbecciv commented Mar 15, 2024

hayata-suenaga commented Mar 15, 2024

hayata-suenaga commented Mar 15, 2024

hayata-suenaga commented Mar 15, 2024

paultsimura commented Mar 15, 2024 •

edited

Loading

paultsimura commented Mar 15, 2024 •

edited

Loading

hayata-suenaga commented Mar 15, 2024

paultsimura commented Mar 18, 2024

hayata-suenaga commented Mar 19, 2024

situchan commented Mar 19, 2024

hayata-suenaga commented Apr 3, 2024

paultsimura commented Apr 3, 2024

melvin-bot bot commented Apr 9, 2024

paultsimura commented Apr 11, 2024

paultsimura commented Apr 16, 2024

hayata-suenaga commented Apr 16, 2024

paultsimura commented Apr 16, 2024

paultsimura commented Apr 17, 2024

akinwale commented Apr 17, 2024 •

edited

Loading

akinwale commented Apr 17, 2024 •

edited

Loading

melvin-bot bot commented Apr 17, 2024

laurenreidexpensify commented Apr 18, 2024

akinwale commented Apr 18, 2024

paultsimura commented Apr 18, 2024

laurenreidexpensify commented Apr 18, 2024

Web - Workspace - IOU moves in the LHN every time the merchant is changed #38425

Web - Workspace - IOU moves in the LHN every time the merchant is changed #38425

Comments

kbecciv commented Mar 15, 2024 • edited Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

github-actions bot commented Mar 15, 2024

melvin-bot bot commented Mar 15, 2024

kbecciv commented Mar 15, 2024

kbecciv commented Mar 15, 2024

hayata-suenaga commented Mar 15, 2024

hayata-suenaga commented Mar 15, 2024

hayata-suenaga commented Mar 15, 2024

paultsimura commented Mar 15, 2024 • edited Loading

paultsimura commented Mar 15, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

hayata-suenaga commented Mar 15, 2024

paultsimura commented Mar 18, 2024

hayata-suenaga commented Mar 19, 2024

situchan commented Mar 19, 2024

hayata-suenaga commented Apr 3, 2024

paultsimura commented Apr 3, 2024

melvin-bot bot commented Apr 9, 2024

paultsimura commented Apr 11, 2024

paultsimura commented Apr 16, 2024

hayata-suenaga commented Apr 16, 2024

paultsimura commented Apr 16, 2024

paultsimura commented Apr 17, 2024

akinwale commented Apr 17, 2024 • edited Loading

akinwale commented Apr 17, 2024 • edited Loading

Regression Test Steps

melvin-bot bot commented Apr 17, 2024

laurenreidexpensify commented Apr 18, 2024

akinwale commented Apr 18, 2024

paultsimura commented Apr 18, 2024

laurenreidexpensify commented Apr 18, 2024

kbecciv commented Mar 15, 2024 •

edited

Loading

paultsimura commented Mar 15, 2024 •

edited

Loading

paultsimura commented Mar 15, 2024 •

edited

Loading

akinwale commented Apr 17, 2024 •

edited

Loading

akinwale commented Apr 17, 2024 •

edited

Loading