[Paid] [HOLD for payment 2023-10-31] [$500] [HIGH] [P2P ACTIVATION] GBR is not shown when a pending payment is waiting on the user to enable their wallet

[kevinksullivan]

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

---

Coming from : https://expensify.slack.com/archives/C05RECHFBEW/p1697739357210919

And #29430 is the relevant PR that will likely need to be revisited.

Action Performed:

1. Send an IOU from user without enabled wallet to another account with an enabled wallet
2. Pay the IOU from the other account

Expected Result:

There should be a system message prompting the user to enable their wallet, and there should be a GBR in the chat for that user, since they have an outstanding action to collect the funds.

Actual Result:

There is no GBR

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d5e47fb869ecebeb
• Upwork Job ID: 1715313432707346432
• Last Price Increase: 2023-10-20
• Automatic offers:
• situchan | Contributor | 27292792
• b4s36t4 | Contributor | 27294338

[melvin-bot]

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

[trjExpensify]

I'll take over the BZ on this one. CC: @samh-nl @akinwale as we're discussing in this thread.

[samh-nl]

Proposal

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

Add a green brick road indicator

What is the root cause of that problem?

Missing check

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

In OptionRowLHN we should add a condition here:

App/src/components/LHNOptionsList/OptionRowLHN.js

Line 119 in 69cf29e

const shouldShowGreenDotIndicator =

What alternative solutions did you explore? (Optional)

[techievivek]

Let's also fix the double message coming from here: https://expensify.slack.com/archives/C01GTK53T8Q/p1697796531033319?thread_ts=1697788964.150409&cid=C01GTK53T8Q

[b4s36t4]

Proposal

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

GBR is not shown when a pending payment is waiting on the user to enable their wallet

What is the root cause of that problem?

We have written a condition to show the GBR if there is any action needed from the user here in this line

App/src/libs/ReportUtils.js

Line 1287 in ca51f6f

if (report.hasOutstandingIOU && report.ownerAccountID === currentUserAccountID && report.isWaitingOnBankAccount) {

But if see the condition it's going to be false in case isWaitingOnBankAccount this being true. As the other user paid the amount we mark the property hasOutStandIOU as false which makes the conditional doesn't work.

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

We need to update the condition here

App/src/libs/ReportUtils.js

Line 1287 in ca51f6f

if (report.hasOutstandingIOU && report.ownerAccountID === currentUserAccountID && report.isWaitingOnBankAccount) {

as followed.

if ((report.hasOutstandingIOU || report.isWaitingOnBankAccount) && report.ownerAccountID === currentUserAccountID) {
        return true;
    }

In this way if the user doesn't had setup a wallet we show GBR.

What alternative solutions did you explore? (Optional)

NA

For Double System Message

#30033 (comment) Starting from the RCA from this comment.

As Vivek suggested we need to stop rendering the reportAction.

const shouldHideAction = originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.PAY && props.action.actionName === CONST.REPORT.ACTIONS.TYPE.IOU;

The above lines does check for the reportAction which does have the action type as iou and the iou type is pay using this we can stop rendering the reportaction fully.

{!shouldHideAction && renderReportActionItem(hovered || isReportActionLinked, isWhisper, hasErrors)}

Kapture.2023-10-20.at.18.01.30.mp4

[techievivek]

RCA for double system message when a payer pays the money request via wallet flow:

We have two IOU actions when a user pays a money request via wallet(one for create and one for pay).

Create IOU action gets used for rendering the MoneyRequestAction from here:

App/src/pages/home/report/ReportActionItem.js

Line 319 in 69cf29e

<MoneyRequestAction

The reimbursememntQueued action gets used to show the enable wallet button and the system message informing we are waiting for the payee to enable their wallet/bank account.

The remaining IOU action(type:Pay) falls under else block here:

App/src/pages/home/report/ReportActionItem.js

Line 414 in 69cf29e

} else {

and we show a different system message saying the payee needs to enable their bank account(because isWaitingBankAccount is set on the report.(Checkout the getReportPreview method).

Solution:
We want to skip rendering anything if the request is not sendingMoney and IOU action is of type pay.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[situchan]

@b4s36t4 the root cause is correct but the solution is wrong.
isWaitingOnBankAccount is true on both accounts. We should not show GBR on payer side

[situchan]

@techievivek should we fix double message issue as well here and ask contributors update proposal?

[techievivek]

Yes, please.

@b4s36t4 the root cause is correct but the solution is wrong.
isWaitingOnBankAccount is true on both accounts. We should not show GBR on payer side

Good one. 👌

[b4s36t4]

@situchan @techievivek Will update my proposal soon.

[b4s36t4]

@situchan updated proposal for the first issue (GBR). Working on the double system message fix. Will update soon.

[b4s36t4]

Updated the proposal to include the double system message as well.

[trjExpensify]

👋 checklist time! Bump, @situchan.

[techievivek]

Not overdue, gentle bump @situchan ^

[melvin-bot]

@trjExpensify, @techievivek, @b4s36t4, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[situchan]

No regression. This is new feature.

I am not sure regression test is needed as it requires gold wallet which QA team doesn't have.

[trjExpensify]

Interesting. @kevinksullivan @joekaufmanexpensify are their no wallet regression tests in the app?

[trjExpensify]

Confirming payments as follows:

@situchan $500 for C+ review
@b4s36t4 $500 for the bug fix.

[trjExpensify]

Settled up with you both! Keeping open for a sec while we discuss await this.

[b4s36t4]

@trjExpensify isn't this also eligible for urgency bonus? We have a single pr for the both issues can you check please?

[trjExpensify]

I don't think we double up the urgency bonus for every issue a PR is linked to. It's a bonus for moving forward the PR with urgency, so I applied it on the payment for the other issue. 👍

[joekaufmanexpensify]

are their no wallet regression tests in the app?

I see a few in testrail. I think the original P2P implementation was before we had standardized on the current regression testing system in Testrail. I bet the few that have been added were part of the #gold-wallets room/project.

[trjExpensify]

Erm, I might be blind. Mind linking me to them?

[melvin-bot]

@trjExpensify, @techievivek, @b4s36t4, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[joekaufmanexpensify]

Erm, I might be blind. Mind linking me to them?

I see some in the Edit IOUs and other tests section: https://expensify.testrail.io/index.php?/suites/view/7722&group_by=cases:section_id&group_order=asc&display_deleted_cases=0&group_id=296079

[trjExpensify]

Hm, weird place for them. Also, it's creating a Wallet via OldDot.

Log in with a new applause.expensifail account
Navigate to OldDot > Account > Payments and add a Personal Bank account using the credentials shared below
Verify you see a payment type for WALLET with a deposit status of "Silver"

[joekaufmanexpensify]

Ah, interesting. Then, maybe there aren't really any for the NewDot P2P yet

[trjExpensify]

Asked here in the #gold-wallets channel.

[melvin-bot]

@trjExpensify, @techievivek, @b4s36t4, @situchan Huh... This is 4 days overdue. Who can take care of this?

[trjExpensify]

Okay, put that on the radar of the gold wallets team. Going to close this out now.