[WAITING ON MIROSLAV] [HOLD for payment 2023-10-02] 🟢 [$500] Update 'Add Stop' to directly navigate to the waypoint editor

[hayata-suenaga]

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

---

Action Performed:

I recommend following the following reproduction steps in a fresh account

1. Open NewDot on Chrome
2. Go to a workspace chat
3. Click on the "+" icon next to the text composer
4. Select "Request money" and then choose "Distance"
5. Add addresses for "Start" and "Finish"
6. Click "Add stop" several times to create empty "Stop"s
7. Click "Next"
8. Open the developer console of Chrome and go to the Application tab > Storage > IndexedDB -> OnyxDB -> keyvaluepairs
9. Find the draft transaction being created. You can type "transaction" on the search field.
   This is tricky if you have already several transactions. That's the reason I recommends using a new account
   

Expected Result:

Waypoints should only contains valid waypoints, and none of them should be unfilled.

Actual Result:

Invalid / empty waypoints are in transaction.

Workaround:

This doesn't affect the user directly, but the Onyx data should be cleaned to reflect the state of the database.

Platforms:

Which of our officially supported platforms is this issue occurring on?
You can inspect the issue only on Chrome

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number:
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): N/A
Logs: N/A
Notes/Photos/Videos:
Expensify/Expensify Issue URL: N/A
Issue reported by: Internal engineer
Slack conversation: https://expensify.slack.com/archives/C05DWUDHVK7/p1693959000958689?thread_ts=1693949572.430099&cid=C05DWUDHVK7

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01be476129f2f20c23
• Upwork Job ID: 1701078084614434816
• Last Price Increase: 2023-09-11
• Automatic offers:
• 0xmiroslav | Reviewer | 26728805
• DylanDylann | Contributor | 26728807

[neil-marcellini]

Following. I would like to review the PR.

[Nikhil-Vats]

Proposal

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

Delete unfilled waypoints from Onyx data before moving to the confirmation screen

What is the root cause of that problem?

Not a bug, this is an improvement.

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

We need to make a new function in Transaction.js -

function removeEmptyWaypoints(transactionID) {
    const transaction = lodashGet(allTransactions, transactionID, {});
    const existingWaypoints = lodashGet(transaction, 'comment.waypoints', {});
   

    const reIndexedWaypoints = TransactionUtils.getValidWaypoints(existingWaypoints, true);
   

    // Onyx.merge won't remove the null nested object values, this is a workaround
    // to remove nested keys while also preserving other object keys
    // Doing a deep clone of the transaction to avoid mutating the original object and running into a cache issue when using Onyx.set
    const newTransaction = {
        ...transaction,
        comment: {
            ...transaction.comment,
            waypoints: reIndexedWaypoints,
        },
        // Clear the existing route so that we don't show an old route
        routes: {
            route0: {
                geometry: {
                    coordinates: null,
                },
            },
        },
    };
    Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, newTransaction);
}

Then we can call this method on next button press like -

onPress={() => {
    Transaction.removeEmptyWaypoints(iou.transactionID);
    IOU.navigateToNextPage(iou, iouType, reportID, report)
}}

What alternative solutions did you explore? (Optional)

NA

Result -

onyx_waypoint.mp4

[Nikhil-Vats]

Hey @hayata-suenaga, I have added a proposal here because I have previously worked on the remove waypoint feature so I knew this functionality in depth. I have also added the video of how it works - when we click next button all the empty waypoints are removed from onyx so if we click back we won't see the empty waypoints. Let me know what you think, I can have a PR ready asap.

[fedirjh]

I propose an alternative approach: enhancing the UX by directing them to the waypoint editor instead of inserting empty stops. The user can then add a new waypoint only when they choose a new destination.

CleanShot.2023-09-06.at.20.01.11.mp4

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @jliexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[hayata-suenaga]

I don't have the bandwidth to open a PR right now, so I'm making this issue external

[DylanDylann]

@0xmiroslav Thanks so much for your reminder. Noted in the future

[melvin-bot]

🎯 ⚡️ Woah @0xmiroslav / @DylanDylann, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @DylanDylann got assigned: 2023-09-18 09:29:43 Z
• when the PR got merged: 2023-09-21 06:34:55 UTC

On to the next one 🚀

[jliexpensify]

Not overdue

[jliexpensify]

Payment Summary:

• Contributor: @DylanDylann $500 + $250 (speed bonus)
• C+: @0xmiroslav $500 + $250 (speed bonus)

Upwork job

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.73-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix/26888: New design when adding stop waypoint #27657

If no regressions arise, payment will be issued on 2023-10-02. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @0xmiroslav requires payment offer (Reviewer)
• @DylanDylann requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[jliexpensify]

Updated payment date, looks like it was just deployed to prod 3 days ago? Correct me if I am wrong here!

[0xmiros]

Deploy date was Sep 25 - #27657 (comment)
2nd post on Sep 27 was from wrong deploy process.

[jliexpensify]

Cheers, paying now.

@0xmiroslav any need for a checklist? Also are you ready tor receive payments yet?

[jliexpensify]

Paid out @DylanDylann

[jliexpensify]

Bumpo @0xmiroslav to complete the checklist. Also, do you want me to close this out?

[0xmiros]

This is a new feature.

Regression Test Proposal

1. Go to FAB > Request money
2. Select Distance tab
3. Fill Start point and Finish point
4. Click Add stop
5. Verify that app redirects to the waypoint editor page
6. Select address and click Save
7. Verify that Stop point is added between of Start point and Finish point

[0xmiros]

and yes, close this out