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

[$500] Distance - Additional edit system messages are shown when request is created offline with edits #33671

Closed
6 tasks done
kbecciv opened this issue Dec 27, 2023 · 32 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kbecciv
Copy link

kbecciv commented Dec 27, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: v1.4.18-2
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:

  1. Go offline.
  2. Create a distance request.
  3. Go to the request details page.
  4. Click description and enter one.
  5. Click description again and enter a new one.
  6. Go online.
  7. Go back to expense report (because of the grayed out expense view bug).
  8. Click on the distance expense.
    Note that it shows additional edit system message that user did not perform.

Expected Result:

Only the system message for description edit will be shown.

Actual Result:

The system message also shows for amount change and category removal when neither have been performed at all.

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

Bug6327019_1703708554183.20231228_011015.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ba249cef3e37408f
  • Upwork Job ID: 1740121158920376320
  • Last Price Increase: 2024-01-24
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 27, 2023
@melvin-bot melvin-bot bot changed the title Distance - Additional edit system messages are shown when request is created offline with edits [$500] Distance - Additional edit system messages are shown when request is created offline with edits Dec 27, 2023
Copy link

melvin-bot bot commented Dec 27, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 27, 2023
Copy link

melvin-bot bot commented Dec 27, 2023

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

Copy link

melvin-bot bot commented Dec 27, 2023

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

Copy link

melvin-bot bot commented Dec 27, 2023

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

@unidev727
Copy link
Contributor

unidev727 commented Dec 27, 2023

Proposal

from: @unicorndev-727

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

Additional edit system messages are shown when request is created offline with edits

What is the root cause of that problem?

The root cause is that we set all transaction data to be update here when we handle distance IOU edition here and iou distance request will be performed when network is online.

const updatedTransaction = {...transaction};

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

We need to update this line

const updatedTransaction = {...transaction};

to

const updatedTransaction = {} as Transaction

What alternative solutions did you explore? (Optional)

N/A

@Tony-MK
Copy link
Contributor

Tony-MK commented Dec 28, 2023

Proposal

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

Additional edit system messages are shown when request is created offline with edits

What is the root cause of that problem?

The backend is returning the amount and the category car in the first response. Before, the user goes online, any edited transactions will create a new request waiting to be sent. The problem is that these persisting requests have outdated amount and category data.

Therefore, once the user goes online, the response from the initial CreateDistanceRequest returns the category car and the amount. Later, both UpdateDistanceRequest send the value amount (0.00) and category data (''). Hence, the server makes the modifications with the previous data.

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

This issue needs both a backend and a frontend fix. From the front end, create an attribute, eg: isOptimistic, updateAttr, in the UpdateDistanceRequest data for the backend to update specific attributes from the payload. Hence, the server can ignore outdated attributes that are not supposed to be updated.

What alternative solutions did you explore? (Optional)

This is more of a theory than a solution. If the amount and category are not being edited, using we can modify the onyx data of the initial OnyxRequest from (CreateDistanceRequest) before sending it to the backend.

@melvin-bot melvin-bot bot added the Overdue label Jan 1, 2024
Copy link

melvin-bot bot commented Jan 2, 2024

@peterdbarkerUK, @0xmiroslav Huh... This is 4 days overdue. Who can take care of this?

@peterdbarkerUK
Copy link
Contributor

I'm not sure how to prioritise this, internal discussion

@melvin-bot melvin-bot bot removed the Overdue label Jan 2, 2024
Copy link

melvin-bot bot commented Jan 3, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Jan 5, 2024
Copy link

melvin-bot bot commented Jan 8, 2024

@peterdbarkerUK, @0xmiroslav Eep! 4 days overdue now. Issues have feelings too...

@kmbcook
Copy link
Contributor

kmbcook commented Jan 9, 2024

Proposal

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

Distance - Additional edit system messages are shown when request is created offline with edits.

What is the root cause of that problem?

New update distance request API is not yet finished. See #28358.

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

Use the the new update distance request API, when it is ready.

@0xmiros
Copy link
Contributor

0xmiros commented Jan 9, 2024

@peterdbarkerUK what is result after discussion?

@melvin-bot melvin-bot bot removed the Overdue label Jan 9, 2024
Copy link

melvin-bot bot commented Jan 10, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Jan 10, 2024

@peterdbarkerUK @0xmiroslav this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Jan 11, 2024
Copy link

melvin-bot bot commented Jan 15, 2024

@peterdbarkerUK, @0xmiroslav Huh... This is 4 days overdue. Who can take care of this?

@peterdbarkerUK
Copy link
Contributor

Apologies, this one slipped off my radar.

I've got a DM going, will update tomorrow

@melvin-bot melvin-bot bot removed the Overdue label Jan 16, 2024
Copy link

melvin-bot bot commented Jan 17, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Jan 17, 2024

@peterdbarkerUK @0xmiroslav 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!

@peterdbarkerUK
Copy link
Contributor

Dylan and I misunderstood something in our DM and he didn't get a chance to look at this. I've updated

@melvin-bot melvin-bot bot removed the Overdue label Jan 22, 2024
@dylanexpensify
Copy link
Contributor

@neil-marcellini to confirm, but this looks like a bug to me

@neil-marcellini
Copy link
Contributor

I agree it's a bug. @peterdbarkerUK would you please search for similar issues to make sure it's not a duplicate? I recall something similar recently. Also, please assign me when it's time for an internal engineer proposal review 🙂

@mallenexpensify mallenexpensify added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Jan 24, 2024
Copy link

melvin-bot bot commented Jan 24, 2024

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

@mallenexpensify
Copy link
Contributor

@situchan reassigning, please take over as C+. If you don't have bandwidth, unassign yourself. Thanks

Copy link

melvin-bot bot commented Jan 24, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@situchan
Copy link
Contributor

I got this error after making online, so description is not updated actually

Screenshot 2024-01-24 at 10 20 22 PM

@situchan
Copy link
Contributor

#33997 is supposed to fix this. It already reached production

@situchan
Copy link
Contributor

@kbecciv is this still reproducible on latest staging?

@kbecciv
Copy link
Author

kbecciv commented Jan 25, 2024

@situchan Not reproducible v1.4.32-2

20240126_043110.mp4

@situchan
Copy link
Contributor

Thanks. Let's close this

@melvin-bot melvin-bot bot added the Overdue label Jan 29, 2024
Copy link

melvin-bot bot commented Jan 29, 2024

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

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@peterdbarkerUK
Copy link
Contributor

Fixed and no longer reproducible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests