Skip to content

[Payment due][$500] Distance Expense - Impossible to remove a waypoint when editing a distance request offline #34686

Closed

Description

@isagoico

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: v1.4.26-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @paultsimura
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1705505264249909 & https://expensify.slack.com/archives/C01GTK53T8Q/p1705484273717249

Action Performed:

  1. Create a distance request with 3 waypoints
  2. Navigate to the distance request details
  3. Disable the internet connection
  4. Click on the distance field to edit
  5. Click on a waypoint
  6. Click on the 3 dot menu > Delete waypoint
  7. Hit Save
  8. Go back online

Expected Result:

The waypoint should be deleted.

Actual Result:

The waypoint is not deleted after hitting the option to delete. After going back online, the 3 waypoints are still displayed.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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

Screen.Recording.2024-01-17.at.10.28.52.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01979a5064cc9154e6
  • Upwork Job ID: 1747739146196148224
  • Last Price Increase: 2024-01-24
  • Automatic offers:
    • alitoshmatov | Reviewer | 28124284
    • paultsimura | Contributor | 28124285

Activity

  1. added
    ExternalAdded to denote the issue can be worked on by a contributor
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jan 17, 2024
  2. changed the title [-]Distance Expense - Impossible to remove a waypoint when editing a distance request offline[/-] [+][$500] Distance Expense - Impossible to remove a waypoint when editing a distance request offline[/+] on Jan 17, 2024
  3. melvin-bot commented on Jan 17, 2024

    @melvin-bot
  4. melvin-bot commented on Jan 17, 2024

    @melvin-bot

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

  5. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Jan 17, 2024
  6. melvin-bot commented on Jan 17, 2024

    @melvin-bot

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

  7. isagoico commented on Jan 17, 2024

    @isagoico
    Author

    Posting @paultsimura proposal here:

    Proposal

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

    When editing a Distance request offline, the waypoint cannot be removed

    What is the root cause of that problem?

    This is because we do not handle the Editing flow correctly when passing the isDraft parameter to Transaction.removeWaypoint: we pass true always:

    Transaction.removeWaypoint(transaction, pageIndex, true);

    Transaction.removeWaypoint(transaction, pageIndex, true);

    When we are creating a new request, we correctly remove the waypoint from a draft transaction. But when we edit an existing transaction, we should modify the real, not draft transaction.

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

    We should pass the isDraft parameter conditionally depending on the IOU action, similar to how we pass it to Transaction.saveWaypoint:
    action === CONST.IOU.ACTION.CREATE

    const saveWaypoint = (waypoint) => Transaction.saveWaypoint(transactionID, pageIndex, waypoint, action === CONST.IOU.ACTION.CREATE);

    What alternative solutions did you explore? (Optional)

  8. isagoico commented on Jan 17, 2024

    @isagoico
    Author
  9. Vlad-AIMaster commented on Jan 18, 2024

    @Vlad-AIMaster

    I will take a look at the issue and come up with a solution.

  10. melvin-bot commented on Jan 18, 2024

    @melvin-bot

    📣 @Vlad-AIMaster! 📣
    Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
    Please follow these steps:

    1. Make sure you've read and understood the contributing guidelines.
    2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
    3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
    4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
      Screen Shot 2022-11-16 at 4 42 54 PM
      Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
    
  11. DylanDylann commented on Jan 18, 2024

    @DylanDylann
    Contributor

    Proposal

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

    The waypoint is not deleted after hitting the option to delete. After going back online, the 3 waypoints are still displayed.

    What is the root cause of that problem?

    The RCA here

    Transaction.removeWaypoint(transaction, pageIndex, true);

    Transaction.removeWaypoint(transaction, pageIndex, true);

    We always pass isDraft: true to removeWaypoint function, so that new updates will be saved to the draft transaction

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

    After refactoring, we use action field from the route to indentify if users create or edit their request. So we also need to rely on action field from the route to determine whether we should save new data into a transaction or a draft transaction

    So, in these places

    Transaction.removeWaypoint(transaction, pageIndex, true);

    Transaction.removeWaypoint(transaction, pageIndex, true);

    We should update isDraft pram to action === CONST.IOU.ACTION.CREATE like we did here

    const saveWaypoint = (waypoint) => Transaction.saveWaypoint(transactionID, pageIndex, waypoint, action === CONST.IOU.ACTION.CREATE);

    One more thing that the above proposal is omitted, we also need to update isDraft pram to action === CONST.IOU.ACTION.CREATE for updateWaypoint function here

    Transaction.updateWaypoints(transactionID, newWaypoints, true).then(() => {

    The updateWaypoint function also be used here

    Transaction.updateWaypoints(transactionID, newWaypoints).then(() => {

    But this component will be removed here and we only use IOURequestStepDistance so that we don't need to fix in here

    What alternative solutions did you explore? (Optional)

    NA

  12. 102 remaining items

  13. paultsimura commented on Aug 9, 2024

    @paultsimura
    Contributor

    @lschurr this can be taken off hold – #37560 is in Production already.
    Also, it seems like the issue was fixed by 4641b08 while the PR was on hold for Onyx fix.

    Here's the timeline:

    1. The PR was created and ready for review
    2. I've noticed an Onyx bug that had to be fixed for this issue to be fully unblocked – [HOLD for payment 2024-08-07] [$500] Onyx updates are not being queued in the correct order #37560 – we've been holding for it the whole time.
    3. Fix/34610: Remove NewDistanceRequestPage and EditRequestDistancePage #35302 was merged, which solved this issue from the Client side.

    Should we be eligible for compensation here if the issue was resolved by another PR while this one was in progress?

  14. alitoshmatov commented on Aug 10, 2024

    @alitoshmatov
    Contributor

    Recently had a similar case and got this comment by @mallenexpensify . Not completely sure if it also applies here

    VictoriaExpensify both nyomanjyotisa and alitoshmatov are due compensation here. Per the internal SO

    If a contributor has been hired for a job and we decide to close the job before it is successfully completed, full payment is due for C+ and the contributor. One caveat here might be if the hired contributor wasn't working on their PR with urgency. If you're unsure, post in #bug-zero.

  15. lschurr commented on Aug 12, 2024

    @lschurr
    Contributor

    Thanks @paultsimura and @alitoshmatov - I checked with the team and we agreed that we should pay full comp for the contributor on the PR and half comp to the C+ since proposals were reviewed but the PR was not.

    Payment summary:

  16. changed the title [-][HOLD #37560][$500] Distance Expense - Impossible to remove a waypoint when editing a distance request offline[/-] [+][Payment due][$500] Distance Expense - Impossible to remove a waypoint when editing a distance request offline[/+] on Aug 12, 2024
  17. added
    Awaiting PaymentAuto-added when associated PR is deployed to production
    on Aug 12, 2024
  18. paultsimura commented on Aug 12, 2024

    @paultsimura
    Contributor

    Thanks, I've accepted the offer.

  19. lschurr commented on Aug 19, 2024

    @lschurr
    Contributor

    All set!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions