[HOLD for payment 2023-09-12] [$500] [Distance] - App crashes while using distance feature

[lanitochka17]

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

---

Issue found when executing PR #25980

Action Performed:

1. Launch app
2. Turn off mobile date
3. Tap fab
4. Tap Request money
5. Tap Distance
6. Enter Start address and save
7. Enter Finish address and save (make sure you hit save)
8. Turn on mobile data

Expected Result:

The app must generate map for address entered

Actual Result:

In online, the app crashes while using distance feature

Workaround:

Unknown

Platforms:

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

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

Version Number: 1.3.62-1

Reproducible in staging?: Yes

Reproducible in production?: No

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6186208_20230902_160813.mp4

test.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014a7eeceb0e8a9b8a
• Upwork Job ID: 1698183373609881600
• Last Price Increase: 2023-09-03
• Automatic offers:
• fedirjh | Reviewer | 26478908
• pradeepmdk | Contributor | 26478909

[OSBotify]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @hayata-suenaga (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014a7eeceb0e8a9b8a

[melvin-bot]

Current assignee @JmillsExpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[b4s36t4]

This issue only happens only when we click on Save button. Because we've wrapped the WaypointEditor component with a From when we click on save only string get's saved. No lat & lognitude is being saved.

If we're offline we save lat & lag as null which is causing the app to crash. We seems to handle the null values but not the app though.

So What I want to confirm is even if we go online because we've saved lat & lan as null or 0 route markers won't be pointed which results in no route paint on the map.

what shall we do here? when user become online fetch the lat & lng for the waypoints which have lat & lng null values??

if not doesn't that cause an issue itself??.

This save button also raises an issue flow follows this way

Open Requst Money
Select a waypoint from the autocomplete input
Select the same waypoint but now use the save button
Select finish address
Observe only one marker is showing in the app (Finish marker since we have lat & lng for the waypoint).

Whenever we click on Save there's no lat & lng.

Shall we just remove the save button?? That reduces the 90% of effort.

[b4s36t4]

https://developers.google.com/maps/documentation/places/web-service/search-text#:~:text=A%20Text%20Search%20returns%20information,bias%20that%20has%20been%20set. We can use this API if we're planning to refetch lat & lng once back online.

[shubham1206agra]

https://developers.google.com/maps/documentation/places/web-service/search-text#:~:text=A%20Text%20Search%20returns%20information,bias%20that%20has%20been%20set. We can use this API if we're planning to refetch lat & lng once back online.

This requires backend changes

[b4s36t4]

I don't think so. Backend handles places API using in a proxy way.

[hayata-suenaga]

okay I feel like the easy fix for now is to disable the save button when offline...

[b4s36t4]

Proposal

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

Distance - App crashes while using distance feature

What is the root cause of that problem?

lat & lng being null causing the MapBox component in mobile to crash because user clicks on Save button which only saves waypoint not lat & lng

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

App/src/pages/iou/WaypointEditor.js

Line 197 in 7217a18

validate={validate}

Setting false here will disable the save button in offline.

What alternative solutions did you explore? (Optional)

NA

[b4s36t4]

Warning

Whole save button is causing the issue. But with a user perspective when he's online and typing he usually select only
from auto complete. At this point solving online issue not so important I feel.

[shubham1206agra]

okay I feel like the easy fix for now is to disable the save button when offline...

I have another fix
Requires hiding the 2 text input and updating the value of lat lng when selecting
Not sure rn

[shubham1206agra]

But I think disable should work rn

[shubham1206agra]

But I have another case in mind
What about some custom addresses when offline and we just want to save in draft??

[b4s36t4]

@shubham1206agra we have to launch few things tmrw. Maybe you can come up with a plan which can by solved by EOD. please mind today is sunday for us, tmrw is sunday for them.

[shubham1206agra]

I am proposing removing these lines.

App/src/pages/iou/WaypointEditor.js

Lines 129 to 137 in 7217a18

	if (isOffline && waypointValue) {
	const waypoint = {
	lat: null,
	lng: null,
	address: waypointValue,
	};
	Transaction.saveWaypoint(transactionID, waypointIndex, waypoint);
	}

And add this logic in validate

if (isOffline && waypointValue !== '') {
            errors[`waypoint${waypointIndex}`] = 'bankAccount.error.addressWhenOffline';
        }

[shubham1206agra]

It will look something like this. Text will be decided by @hayata-suenaga

[shubham1206agra]

Do you like this method more?

[shubham1206agra]

I will also add logic to disable Next button

[pradeepmdk]

@mountiny i think we can go with normal process @hayata-suenaga also removed this deploy blocker due to this I'm removing the deploy blocker label per this instruction

[fedirjh]

I agree, let's proceed with the normal deploy process.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• 26558-App crashes while using distance feature  #26622

If no regressions arise, payment will be issued on 2023-09-12. 🎊

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:

• @pradeepmdk requires payment offer (Contributor)
• @fedirjh requires payment offer (Reviewer)

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

[JmillsExpensify]

I don't see the normal BZ checklist on this issue. @fedirjh Do you think we require any regression tests in light of the crash?

[JmillsExpensify]

Payment summary:

• Issue reporter: N/A
• Contributor: @pradeepmdk $750 (incl. urgency bonus)
• Contributor+: @fedirjh $750 (incl. urgency bonus)

Offers are out. Let me know when you've accepted and we'll get this kicked off.

[fedirjh]

Do you think we require any regression tests in light of the crash?

@JmillsExpensify I believe we should require a regression test for this issue #26589, as it addresses the offline flow and should cover this crash.

Let me know when you've accepted and we'll get this kicked off.

Accepted.

[melvin-bot]

@JmillsExpensify, @pradeepmdk, @fedirjh, @hayata-suenaga Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@JmillsExpensify, @pradeepmdk, @fedirjh, @hayata-suenaga 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[hayata-suenaga]

do we have a regression test?

[pradeepmdk]

@fedirjh here #26589 (comment) we are fixing here only the map is not loading when offline -> online on the web.

if we need a regression test here can we take care here itself?

[fedirjh]

Regression Test Proposal

1. Open app
2. Go offline
3. Tap fab -> Request money -> Distance
4. Enter Start address and save
5. Enter Finish address and save
6. Turn on mobile data
7. Verify the app doesn’t crash

• Do we agree 👍 or 👎

[hayata-suenaga]

The regression test proposal is above

[JmillsExpensify]

Great thanks. I'll issue payouts now and get a regression test created.

[JmillsExpensify]

All contributors paid out via Upwork. Regession test has also been created. Closing