[HOLD for payment 2024-06-11] [HOLD for payment 2024-06-06] [$250] Distance Expense - Map is not centered over San Francisco when location settings are turned off

[lanitochka17]

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.74-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4561935
Email or phone of affected tester (no customers): applausetester+azadmin@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Disable the location settings for New Expensify in Chrome
2. Navigate to Request Money > Distance expense

Expected Result:

The map is centered over San Francisco

Actual Result:

The map is shown over my current city

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

Bug6482032_1715806856342.map_San_Fransico.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015240613a85d23a6c
• Upwork Job ID: 1790944314356584448
• Last Price Increase: 2024-05-16
• Automatic offers:
• fedirjh | Reviewer | 0
• tienifr | Contributor | 0

Issue Owner

Current Issue Owner: @jliexpensify

[melvin-bot]

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

1. If you find which PR caused the issue/bug, you can reassign it to the person responsible for it.
   • If the author is OOO or won’t get online before the daily deploy is due, you are responsible for finding the best fix/path forward. Don’t hesitate to ask for help!
2. Try to reproduce the issue, if the bug is on production, remove the DeployBlocker label but stay assigned to fix it (or find out which PR broke it to get help from the author).
   • You can adjust the urgency of the issue to better represent the gravity of the bug.
   • If the issue is super low priority, feel free to un-assign yourself.
   • Be careful with PHP warnings, sometimes it is more complex than just adding a null coalescing operator as they might be uncovering some bigger bug.
   • If it was a one-off issue that requires no action (for example, Bedrock was down or it is a duplicated issue), you can close it.

Remember rule #2: Never un-assign yourself from a real DeployBlocker unless you are 100% sure someone else is assigned and will take care of it.

[melvin-bot]

Triggered auto assignment to @luacmartins (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 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.

[luacmartins]

This is pretty small and NAB.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015240613a85d23a6c

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @slafortune (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[tienifr]

Proposal

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

The map is shown over my current city

What is the root cause of that problem?

User location is cached in Onyx so when we revoked location permission, the previous location was still there.

When location request failed, we ran a callback to reset to initialLocation which is the default San Francisco but we ealry returned if cachedUserLocation existed:

App/src/components/MapView/MapView.website.tsx

Lines 67 to 69 in 0d45c55

	if (cachedUserLocation || !initialState) {
	return;
	}

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

We need to reset location if permission was not granted by removing the cachedUserLocation early return above.

We also need to clear the location stored in Onyx for privacy. We should only save user location as long as they permit it. Otherwise, we might get flickering between the cached location and the current or initial location.

if (!initialState) {
    return;
}
UserLocation.clear();
setCurrentPosition(initialState.location);

What alternative solutions did you explore? (Optional)

We can optionally check for PERMISSION_DENIED error code to only clear the cache for this specific case when user denied permission.

App/src/libs/getCurrentPosition/getCurrentPosition.types.ts

Lines 21 to 23 in c5f84b6

	PERMISSION_DENIED: typeof GeolocationErrorCode.PERMISSION_DENIED;
	POSITION_UNAVAILABLE: typeof GeolocationErrorCode.POSITION_UNAVAILABLE;
	TIMEOUT: typeof GeolocationErrorCode.TIMEOUT;

There's another issue that when there's cached location and the permission was reset, while the permission prompt was still open, the map would show the cached location instead of default location (San Francisco) because userLocation is initialized with cachedLocation while it should be the default location initialState.location:

App/src/components/MapView/MapView.website.tsx

Line 53 in 0d45c55

const [currentPosition, setCurrentPosition] = useState(cachedUserLocation);

[szhaqypbek]

Proposal

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

The map is shown over my current city when user geolocation settings are disabled

What is the root cause of that problem?

setCurrentPositionToInitialState callback returns when the user's location is already cached in Onyx.

App/src/components/MapView/MapView.website.tsx

Lines 67 to 69 in 0d45c55

	if (cachedUserLocation || !initialState) {
	return;
	}

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

We should check for an error code returned from the getCurrentPosition callback and set a default location when error.code equals GeolocationErrorCode.PERMISSION_DENIED

if (error.code !== GeolocationErrorCode.PERMISSION_DENIED && (cachedUserLocation || !initialState)) {
  return;
}

What alternative solutions did you explore? (Optional)

Screenshots/Videos

Screen.Recording.2024-05-16.at.09.mp4

[tienifr]

Proposal updated to add optional solution and detailed explaination.

[fedirjh]

Although both proposals are valid, I am inclined to favor the idea of completely removing the cached data when the location permission is revoked.

Let's move forward with @tienifr's proposal.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @luacmartins is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[fedirjh]

Update: Awaiting @luacmartins to sign off the proposal

[slafortune]

There is a PR draft in review. I'll be out until June 4th - so adding another BZ in the meantime. I'll take this back on the 4th if needed.

[melvin-bot]

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[jliexpensify]

@fedirjh any regressions here?

[fedirjh]

any regressions here?

There was a regression in #42752 , and it's already fixed by @tienifr.

[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.4.77-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Clear user location cache when they denied location permission #42509
• [CP Staging] fix: pending map in IOU confirmation step #42773

If no regressions arise, payment will be issued on 2024-06-06. 🎊

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

• @fedirjh requires payment automatic offer (Reviewer)
• @tienifr requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@fedirjh] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jliexpensify]

Ok, is this a correct Summary?

• C: @tienifr $250
• C+: @fedirjh $250 - please accept the offer!

Upwork job

Just waiting on the checklist (if applicable)

[melvin-bot]

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

• [CP Staging] fix: pending map in IOU confirmation step #42773

If no regressions arise, payment will be issued on 2024-06-11. 🎊

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

• @fedirjh requires payment automatic offer (Reviewer)
• @tienifr requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@fedirjh] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: