Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.89k forks source link

[LOW] [Splits] [$500] IOU - When paying a deleted IOU disappears and RBR does not appear when going online #38338

Closed kbecciv closed 1 month ago

kbecciv commented 8 months ago

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.52-0 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4424536 Issue reported by: Applause - Internal Team

Action Performed:

  1. Open NewDot app or https://staging.new.expensify.com/
  2. Log in to account A and B in incognito mode
  3. [User B] Request money from User A
  4. [User A] Go offline
  5. [User A] Open the IOU report and tap Pay elsewhere
  6. [User B] Delete the IOU
  7. [User A] Go online

Expected Result:

The deleted IOU should not disappear when you pay and should appear RBR when you go online

Actual Result:

When paying a deleted IOU disappears and RBR does not appear when going online

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/93399543/7933a1bd-6eb5-48ef-9ff0-25f3404b2ac3

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01606c57f19fab1b0c
  • Upwork Job ID: 1768749861959938048
  • Last Price Increase: 2024-08-08
mallenexpensify commented 5 months ago

@abdulrahuman5196 , ๐Ÿ‘€ on @ijmalik 's proposal above plz

abdulrahuman5196 commented 5 months ago

On @ijmalik 's proposal here - #38338 (comment). I don't agree on the solution suggested to make changes in the central SaveResponseInOnyx.ts component to fix this particular case. I think we don't have any proposals to approve yet.

I am still on this. I don't think its a good solution to update the central middlewares and API infra to reflect this exception for a specific case. @ijmalik Is there a different way where we can set optimistic failure report action when the API fails?

Currently we don't have any proposals to be approved yet.

ijmalik commented 5 months ago

I am still on this. I don't think its a good solution to update the central middlewares and API infra to reflect this exception for a specific case. @ijmalik Is there a different way where we can set optimistic failure report action when the API fails?

Currently we don't have any proposals to be approved yet.

@mallenexpensify

@abdulrahuman5196 Thank you for your feedback on proposal in #38338 (comment) . I understand your concerns regarding the suggested changes to the central SaveResponseInOnyx.ts component.

I agree that updating central middlewares and API infrastructure for this specific case may not be the best approach. Instead, could we explore alternative solutions?

The root cause of our issue is not related to an API failure. If it were, setting optimistic data would indeed be the correct approach. However, this case is different and warrants a different strategy.

Up to this point, everything is working as expected.

The issue arises when the API ReconnectApp response is received. The response includes onyxData related to this specific IOU and other activities associated with User-A while offline. The IOU deletion data updates Onyx directly, bypassing most of our app's code except for the middleware.

Therefore, we need to identify IOU deletions in the response and skip these updates to Onyx.

abdulrahuman5196 commented 5 months ago

@mallenexpensify Can we involve an internal engineer here to provide information from backend as well? Or some other direction as well.

Basically, the reportAction(IOU) is deleted from the ReconnectApp API call response. So we don't have a report action where we can show an error.

melvin-bot[bot] commented 5 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

mallenexpensify commented 5 months ago

@abdulrahuman5196 before assigning an engineer, can you post in #expensify-open-source to see if you can get a reply from someone from the team? Thx

sonialiap commented 5 months ago

@mallenexpensify @abdulrahuman5196 I came across a similar sounding issue, do you think they're connected https://github.com/Expensify/App/issues/40705

melvin-bot[bot] commented 5 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

mallenexpensify commented 4 months ago

Good ๐Ÿ‘€ @sonialiap , the steps are near identical. @abdulrahuman5196 can you review that issue and comment here or there with any thoughts? Normally we'd keep the older issue open but it looks like both these have quite a few comments (and no suggested proposals).

abdulrahuman5196 commented 4 months ago

@mallenexpensify Both are almost similar issues. And I can see comments that other issue is not reproducible. So we can focus on this issue.

But anyways I agree on this https://github.com/Expensify/App/issues/40705#issuecomment-2176587187, we need internal guidance on if we should fix this, and if so in what direction.

melvin-bot[bot] commented 4 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

melvin-bot[bot] commented 4 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

melvin-bot[bot] commented 4 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

mallenexpensify commented 4 months ago

Checking in #vip-split to see what folks think https://expensify.slack.com/archives/C05RECHFBEW/p1720481477873549

melvin-bot[bot] commented 4 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

melvin-bot[bot] commented 3 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

mallenexpensify commented 3 months ago

From chat a couple weeks ago

I think we should fix this bug but not now. You can make it a monthly and add it under the paused items under this proje

I removed Help Wanted

mallenexpensify commented 3 months ago

Gonna be a min til we get to this

melvin-bot[bot] commented 3 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

mallenexpensify commented 2 months ago

IOUs are a priority right now

mallenexpensify commented 1 month ago

Closing as we're closing #vip-split