[$250] IOU - Green dot shows up in LHN briefly when requeting money from another user

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.59-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to 1:1 DM
3. Pin the chat
4. Create an IOU request
5. Go to IOU report
6. Create a second request

Expected Result:

Green dot will not show up in LHN when requeting money from another user

Actual Result:

Green dot shows up in LHN briefly when requeting money from another user

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

Bug6435614_1712065747860.20240402_214557.mp4

Bug6435614_1712065828320.20240402_214912.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017992a05a461e2089
• Upwork Job ID: 1775313856042840064
• Last Price Increase: 2024-04-03

[melvin-bot]

Triggered auto assignment to @roryabraham (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 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:

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

[lanitochka17]

@roryabraham FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

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

[roryabraham]

reproduced locally on main

[roryabraham]

reproduced locally on the production branch as well

[roryabraham]

reproduced in production, demoting

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 (External)

[melvin-bot]

Triggered auto assignment to @twisterdotcom (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[roryabraham]

pro-tip: go offline before step 6 in the reproduction steps and the green dot that shouldn't be there is persistent

[roryabraham]

related function is ReportUtils.requiresAttentionFromCurrentUser

[roryabraham]

the reason the green dot is there is because the report has hasOutstandingChildRequest: true

[roryabraham]

I suspect this is a problem introduced by https://github.com/Expensify/App/pull/34866/files, IOU.needsToBeManuallySubmitted will return true for any report that is not a policyExpenseChat. I think it's default should be false, since IOU reports by default do not need to be manually submitted

[roryabraham]

removing help wanted, will just open the PR myself

[roryabraham]

PR ready for review: #39470

[roryabraham]

this one was super simple, reducing to $250

[melvin-bot]

Upwork job price has been updated to $250

[nkdengineer]

@roryabraham @abdulrahuman5196 I don't think this is the right way to fix the issue, it will cause regression because for 1-1 IOU, this line will never be evaluated, and hasOutstandingChildRequest not set to true in case after requesting the money, the requester is the one owing money.

It's expected that the needsToBeManuallySubmitted will return true by default if it's not policy expense, it's so that we'll not early return here and will set hasOutstandingChildRequest based on this

I agree the name is a bit confusing, we might want to rename it to be clear, but we can't change the default return to false

Proposal

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

Green dot shows up in LHN briefly when requeting money from another user

What is the root cause of that problem?

In here, we're not checking if policy is not a personal policy before checking isPolicyAdmin. This leads to in 1-1 DM, the current user is always the admin of their own personal policy, thus it will early return here with hasOutstandingChildRequest as true.

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

In here, check that the policy is not a personal policy first.

if (policy?.type !== CONST.POLICY.TYPE.PERSONAL && PolicyUtils.isPolicyAdmin(policy)) {

Or can check iouReport is a policy expense chat, like here

What alternative solutions did you explore? (Optional)

We can avoid passing personal policy to getOutstandingChildRequest in the first place (or higher up in the buildOnyxDataForMoneyRequest)

[roryabraham]

@nkdengineer I have not been able to reproduce any regression with my solution. If there is indeed a regression, can you please provide clear reproduction steps?

[nkdengineer]

@roryabraham Please try these steps (after applying your solution):

1. Have 2 accounts, with no IOU with each other
2. From User A, send a money request for $500 to User B
3. From User B, wait for the money request from User A to show up
4. From User B, go offline
5. From User B, send a money request for $100 to User A

Now, the GBR in User B's LHN will disappear, which is not correct because User B still owes User A money after step 5. If user B is online, the GBR will disappear momentarily then will appear again after the back-end request is successful, the step of going offline is so that the issue is more visible.

Meanwhile, in staging and prod now, the GBR will stay visible in User B's LHN after step 5. So the above behavior is a regression.

The reason why this is happening is explained on top of my proposal

[roryabraham]

thanks for the reproduction steps @nkdengineer, I can reproduce the regression you're reporting. I'm still not sure about your RCA though - I'm quite certain that needsToBeManuallySubmitted should only be true for expense reports.

[roryabraham]

thanks again for pointing out that issue @nkdengineer - the reproduction steps were very helpful. I ended up fixing the problem in a different way.

[nkdengineer]

thanks again for pointing out that issue @nkdengineer - the reproduction steps were very helpful. I ended up fixing the problem in a different way.

@roryabraham There're other regressions with that approach too, please note needsToBeManuallySubmitted is also used here and here

I'm quite certain that needsToBeManuallySubmitted should only be true for expense reports.

@roryabraham I think the problem here is that the naming of needsToBeManuallySubmitted is confusing.

The use case for that method is:
If this report is an expense report, and it has automatic submit, then hasOutstandingChildRequest will always be false (it's the intention of this condition)

That's why needsToBeManuallySubmitted returns true for personal request, so that the hasOutstandingChildRequest will not be forced to be false here, but will depend on other conditions.

We should instead rename to hasAutomaticSubmission (which is an inverted version of needsToBeManuallySubmitted)

And we'll use hasAutomaticSubmission(iouReport) here

And use !hasAutomaticSubmission here and here

[nkdengineer]

I'm still not sure about your RCA though

@roryabraham Did you try keeping the some logic we had to make admins see expense reports that will be automatically submitted with a green dot (mentioned here)? You'll see the bug in the OP is not fixed right? That's because the RCA is there as I mentioned in my proposal

And the needsToBeManuallySubmitted fix doesn't fix the root cause but it's more like a code polish.

[roryabraham]

There're other regressions with that approach too

Can you please provide reproduction steps for these other regressions?

[melvin-bot]

This issue has not been updated in over 15 days. @twisterdotcom, @abdulrahuman5196, @roryabraham eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[twisterdotcom]

Are we still waiting on a response to @roryabraham from @nkdengineer here?

[abdulrahuman5196]

Are we still waiting on a response to @roryabraham from @nkdengineer here?

Yes.

[melvin-bot]

@twisterdotcom, @abdulrahuman5196, @roryabraham, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[mvtglobally]

Issue is not reproducible

Recording.2174.mp4

[roryabraham]

That adds up. When I went back to my draft PR, merging main it looked like the core changes had already been made somewhere else. Closing this out