[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

[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.19-1
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: Applause - Internal Team
Slack conversation:

Action Performed:

Precondition: User A has requested money from User B

1. Log in as User B
2. Go to 1:1 DM with User A > Click IOU preview
3. Request the same amount from User A to make the net owe become 0
4. Note the the green dot for the main chat in LHN
5. Return to main chat
6. Return to the IOU report
7. Delete the requested amount created in Step 3

Expected Result:

In Step 3, once the net owe becomes 0, the green dot for the main chat in LHN will disappear
In Step 7, the new amount should update instantly

Actual Result:

In Step 3, once the net owe becomes 0, the green dot for the main chat in LHN does not disappear, unless the main chat is visited
In Step 7, the total amount remains at 0, unless the report is revisited

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

Bug6328606_1703873451494.20231229_081427.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017d07091eedfc5a43
• Upwork Job ID: 1740800068445937664
• Last Price Increase: 2023-12-29
• Automatic offers:
• shubham1206agra | Reviewer | 28079575
• tienifr | Contributor | 28079576

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[abzokhattab]

Proposal

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

green dot should be removed when the net amount is 0 in iou

What is the root cause of that problem?

inside the requiresAttentionFromCurrentUser we define if the current report should show the green dot or not and currently we dont consider the case where the net is zero

App/src/libs/ReportUtils.ts

Line 1596 in 48953d0

function requiresAttentionFromCurrentUser(optionOrReport: OnyxEntry<Report> | OptionData, parentReportAction: EmptyObject | OnyxEntry<ReportAction> = {}) {

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

we need to add the following condition:

    if ( optionOrReport?.iouReportAmount === 0) {
        return false;
    }

also, we might add a condition that hasOutstandingChildRequest is true to make sure that it has child iou requests but I believe this is redundant

for the delay in the message in point 7, this is a backend pusher issue

[paultsimura]

Proposal

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

The green dot of the parent chat report doesn't update immediately.

What is the root cause of that problem?

This is happening because we do not update the hasOutstandingChildRequest of the parent 1:1 report when requesting the money.

As a result, the chat report's green dot is displayed because of this check:

App/src/libs/ReportUtils.ts

Lines 1614 to 1617 in a74cc7a

	// Has a child report that is awaiting action (e.g. approve, pay, add bank account) from current user
	if (optionOrReport.hasOutstandingChildRequest) {
	return true;
	}

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

In IOU.buildOnyxDataForMoneyRequest, we need to optimistically set the chat report's hasOutstandingChildRequest to false if the IOU Report's total will become 0 after the request is added:

hasOutstandingChildRequest: iouReport.total !== 0,

App/src/libs/actions/IOU.js

Lines 307 to 319 in 433bc47

	const optimisticData = [
	{
	// Use SET for new reports because it doesn't exist yet, is faster and we need the data to be available when we navigate to the chat page
	onyxMethod: isNewChatReport ? Onyx.METHOD.SET : Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`,
	value: {
	...chatReport,
	lastReadTime: DateUtils.getDBTime(),
	lastMessageTranslationKey: '',
	iouReportID: iouReport.reportID,
	...(isNewChatReport ? {pendingFields: {createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}} : {}),
	},
	},

We need to do the same on the money request removal:

        ...(shouldDeleteIOUReport
            ? [
                  {
                      onyxMethod: Onyx.METHOD.MERGE,
                      key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`,
                      value: {
                          hasOutstandingChildRequest: false,
                          iouReportID: null,
                          lastMessageText: ReportActionsUtils.getLastVisibleMessage(iouReport.chatReportID, {[reportPreviewAction.reportActionID]: null}).lastMessageText,
                          lastVisibleActionCreated: lodashGet(ReportActionsUtils.getLastVisibleAction(iouReport.chatReportID, {[reportPreviewAction.reportActionID]: null}), 'created'),
                      },
                  },
              ]
            : [
                {
                    onyxMethod: Onyx.METHOD.MERGE,
                    key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`,
                    value: {
                        hasOutstandingChildRequest: updatedIOUReport.total !== 0,
                    },
                },
            ]),

App/src/libs/actions/IOU.js

Lines 2422 to 2435 in 433bc47

	...(shouldDeleteIOUReport
	? [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`,
	value: {
	hasOutstandingChildRequest: false,
	iouReportID: null,
	lastMessageText: ReportActionsUtils.getLastVisibleMessage(iouReport.chatReportID, {[reportPreviewAction.reportActionID]: null}).lastMessageText,
	lastVisibleActionCreated: lodashGet(ReportActionsUtils.getLastVisibleAction(iouReport.chatReportID, {[reportPreviewAction.reportActionID]: null}), 'created'),
	},
	},
	]
	: []),

What alternative solutions did you explore? (Optional)

[tienifr]

Proposal

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

1. In Step 3, once the net owe becomes 0, the green dot for the main chat in LHN does not disappear, unless the main chat is visited
2. In Step 7, the total amount remains at 0, unless the report is revisited

What is the root cause of that problem?

1. We're not optimistically setting hasOutstandingChildRequest to the correct value after user B creates a money request that offsets the initial request from user A
2. We have a regression from this TS migration, as we can see, in here, we're checking if iouReportUpdate.total is truthy before making the calculation. This is wrong because the total could be 0, we should still calculate in that case. This is probably because when migrating to TS, we have to make sure the total is not undefined to avoid type issue, so if (iouReportUpdate.total) was added. This leads to the amount will not optimistically change when we delete the money request.

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

1. We should set hasOutstandingChildRequest in optimistic data of the chat report here

hasOutstandingChildRequest: iouReport.managerID === userAccountID && iouReport.total !== 0,

We have to compare managerID === userAccountID as well because hasOutstandingChildRequest happens only if the current user owes the money, if the user is owed money, it should not be true and GBR should not show, even though in this case total is still more than 0. Can see more explanation here

We should do the same when deleting money request as well here (checking both iouReport.managerID and iouReport.total)

2. Instead of checking if iouReportUpdate.total is truthy here, we should check that iouReportUpdate.total is a number.

if (typeof iouReportUpdate.total === 'number') {

(or we can check that iouReportUpdate.total is specifically not null/undefined)

What alternative solutions did you explore? (Optional)

NA

[melvin-bot]

@jliexpensify, @shubham1206agra Whoops! This issue is 2 days overdue. Let's get this updated quick!

[jliexpensify]

@lanitochka17 I don't think the title is very clear.

Am I correct in my understanding that the title should be something like: Delay in updating green dot and total amount when the net amounts cancel each other out for 2 users?

EDIT: Tagged Gabi in #vip-split here - https://expensify.slack.com/archives/C05RECHFBEW/p1704162130168679

[shubham1206agra]

@tienifr's proposal looks good to me.

🎀 👀 🎀 C+ Reviewed

[melvin-bot]

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

[melvin-bot]

📣 @shubham1206agra 🎉 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]

📣 @tienifr 🎉 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 📖

[tienifr]

@thienlnam @mobilify @shubham1206agra The recent change indicates that The policy expense chat should have the GBR only when its a paid policy that means the green dot won't be shown in DM chat -> the expectation above is incorrect?

[shubham1206agra]

@mountiny Can you clarify the above comment?

[mountiny]

Well regarding that line, thats about request money and setting the GBR optimistically which only happens in the workspace/ expense report case. You cannot request money from yourself in IOU case

[jliexpensify]

Aha, thank you for clarifying!

[melvin-bot]

@thienlnam @jliexpensify @mountiny @shubham1206agra @tienifr this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[jliexpensify]

Not overdue!

[jliexpensify]

@shubham1206agra Hi, looks like you need to review the PR?

[shubham1206agra]

I am OOO right now. Will review it on 30th

[shubham1206agra]

@mountiny @thienlnam The current solution does not work if the currency in which money is requested is different the default currency of the user. Should we skip these cases as it seems impossible from FE to fix these cases?

[mountiny]

If there is mix of currencies on the report or the report has different currency compared to the expense itself, then yes we cannot do much about it offline in App

[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 1.4.45-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

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

If no regressions arise, payment will be issued on 2024-03-07. 🎊

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

• @shubham1206agra requires payment automatic offer (Reviewer)
• @tienifr requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR:
• [@shubham1206agra] 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:
• [@shubham1206agra] 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:
• [@shubham1206agra] Determine if we should create a regression test for this bug.
• [@shubham1206agra] 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.
• [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jliexpensify]

Payment Summary

• Contributor: @tienifr $500
• C+: @shubham1206agra $500 - Please complete checklist!

NEW UPWORKS JOB

[shubham1206agra]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR: [No QA][TS migration] Migrate 'IOUUtils.js' lib to TypeScript #27332
• [@shubham1206agra] 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/27332/files#r1517175949
• [@shubham1206agra] 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: Not Required
• [@shubham1206agra] Determine if we should create a regression test for this bug. No
• [@shubham1206agra] 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.
• [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jliexpensify]

Paid and job closed

[melvin-bot]

⚠️ 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.