Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to request money after settling existing ious #18718

Closed
Julesssss opened this issue May 10, 2023 · 13 comments
Closed

Unable to request money after settling existing ious #18718

Julesssss opened this issue May 10, 2023 · 13 comments
Assignees
Labels
Engineering Hourly KSv2 Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff

Comments

@Julesssss
Copy link
Contributor

Julesssss commented May 10, 2023

cc @mountiny @luacmartins

Problem

Some IOURequests are failing. This is a new regression.

You need to settle a few requests before the bug occurs. It might also be related to workspaces. In my case it is happenening to the workspace admin.

  • As userA, Request money
  • As userB, pay it
  • As userB, Request money
  • As userA, pay it
  • As userA, request money
  • At some point we start. reusing the iouReportID which is a major regression. Theory is listed below

Screenshot 2023-05-10 at 16 53 50

This appears to be occurring because we no longer clear the chatReport.iouReportID property when an IOU is paid. Because of this, this line is reusing an iouReportID instead of generating a new one optimistically.

Screenshot 2023-05-10 at 16 51 38 (2)

Screenshot 2023-05-10 at 17 04 25

Solution

Figure out why we aren't clearing the iouReport.iouReportID, or update the mentioned logic so that we don't pass OLD iouReportIDs to the requestMoney API.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~013b510aa28fd9747e
  • Upwork Job ID: 1656328070402617344
  • Last Price Increase: 2023-05-10
@Julesssss Julesssss added Daily KSv2 Internal Requires API changes or must be handled by Expensify staff labels May 10, 2023
@Julesssss Julesssss self-assigned this May 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 10, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented May 10, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @0xmiroslav (Internal)

@Julesssss Julesssss added Hourly KSv2 Improvement Item broken or needs improvement. Engineering and removed Daily KSv2 labels May 10, 2023
@Julesssss
Copy link
Contributor Author

This might be related to missing optimistic data being persisted to Onyx.

@Julesssss Julesssss removed their assignment May 10, 2023
@trjExpensify
Copy link
Contributor

Asked @roryabraham in the slack thread if he can take a look at it, as he worked on this optimistic stuff recently.

@roryabraham roryabraham self-assigned this May 10, 2023
@roryabraham
Copy link
Contributor

Let me see what I can find out

@roryabraham
Copy link
Contributor

I'm so far unable to reproduce this while using the Settle up elsewhere payment method. Going to try provisioning bank accounts and see if it's related to the PayWithExpensify flow

@roryabraham
Copy link
Contributor

Ok, was able to reproduce using the PayWithExpensify flow:

image

Here's the failed IOU reportAction:

image

@roryabraham
Copy link
Contributor

roryabraham commented May 10, 2023

Maybe a red herring, but AFAICT this is the only IOU reportAction where originalMessage.IOUReportID is a string instead of a number. Also, I see that @Julesssss is correct that it's the only one that re-used an existing IOUReportID from an IOU report that's already settled.

@roryabraham
Copy link
Contributor

So I think it's probable that this is related to the PayMoneyRequest flows somewhere.

@roryabraham
Copy link
Contributor

roryabraham commented May 10, 2023

Documenting my testing/investigation:

  1. I have two accounts A and C, both with bank accounts added, and wallets upgraded to Gold. These accounts had never chatted before.

  2. A uses global create to request money from C. Looking at the IOUReportAction on their newly created chat report:

    image

    Then also at the IOU report:

    image
  3. Then, as C, I paid the IOU report via my Expensify wallet:

    There's a new IOU pay action:

    image

    As user A, I see the same pay action:

    image

    The IOU report is MANUALREIMBURSED:

    image

    User A sees the same thing:

    image

    I do see a stacktrace in server logs, not sure if these are a red herring:

    2023-05-10T22:14:37.363137+00:00 expensidev2004 php-fpm: Wtdoxt /api.php rory+c@gmail.com !ecash1.3.12-1! ?api? [info] [trace] StackTrace start
    2023-05-10T22:14:37.363141+00:00 expensidev2004 php-fpm: Wtdoxt /api.php rory+c@gmail.com !ecash1.3.12-1! ?api? [info] [trace] 0/4. file '/vagrant/Web-Expensify/lib/Push/API.php' function 'traceFunctionCall' line '253'
    2023-05-10T22:14:37.363145+00:00 expensidev2004 php-fpm: Wtdoxt /api.php rory+c@gmail.com !ecash1.3.12-1! ?api? [info] [trace] 1/4. file '/vagrant/Web-Expensify/lib/Push/API.php' function 'send' line '159'
    2023-05-10T22:14:37.363149+00:00 expensidev2004 php-fpm: Wtdoxt /api.php rory+c@gmail.com !ecash1.3.12-1! ?api? [info] [trace] 2/4. file '/vagrant/Web-Expensify/lib/ReportUtils.php' function 'sendToUserChannelByAccountID' line '1990'
    2023-05-10T22:14:37.363156+00:00 expensidev2004 php-fpm: Wtdoxt /api.php rory+c@gmail.com !ecash1.3.12-1! ?api? [info] [trace] 3/4. file '/vagrant/Web-Expensify/lib/ReportAPI.php' function 'notifyParticipants' line '4004'
    2023-05-10T22:14:37.363160+00:00 expensidev2004 php-fpm: Wtdoxt /api.php rory+c@gmail.com !ecash1.3.12-1! ?api? [info] [trace] 4/4. file '/vagrant/Web-Expensify/api.php' function 'payWithWallet' line '2866'
    2023-05-10T22:14:37.363164+00:00 expensidev2004 php-fpm: Wtdoxt /api.php rory+c@gmail.com !ecash1.3.12-1! ?api? [info] [trace] StackTrace end
    
  4. Now user C is going to request money from user A (offline first):

    And we see that the IOUReportID of the already-settled IOU is used 💥

    image

@roryabraham
Copy link
Contributor

Opened a PR to fix this issue: https://github.com/Expensify/Web-Expensify/pull/37386

@roryabraham
Copy link
Contributor

Thanks @Julesssss for helping take the PR over the finish line here. It was a back-end fix so no C+ payment here. Closing this out.

@trjExpensify
Copy link
Contributor

Nice one, guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Hourly KSv2 Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

4 participants