Distance - Map centers on users location after permission to access location is denied

lanitochka17 commented 2 weeks ago

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: 9.0.5-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: https://expensify.testrail.io/index.php?/tests/view/4700585 Issue reported by: Applause - Internal Team

Action Performed:

Navigate to staging.new.expensify.com
Disallow expensify to access user's location and go to troubleshoot and clear cache and restart
Go to workspace chat and start a distance expense flow
Observe the prompt to allow access to user's location and observe the map being centered on San Francisco
Allow expensify to access user's location
Start a distance expense flow and observe the map now centers on user's location
Disallow expensify to access user's location again
Start a distance expense flow and observe the map

Expected Result:

Map should center on San Francisco

Actual Result:

Map centers on user's current location based on the saved cache but if user clears cache and restarts then map centers on San Francisco

Workaround:

Unknown

Platforms:

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

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/87a55c1f-1fb6-4e55-8206-88f212f16e76

View all open jobs on GitHub

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @anmurali (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.

lanitochka17 commented 2 weeks ago

@anmurali FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

jacobkim9881 commented 2 weeks ago

Clearing cache doesn't seem to reset the users location. Users location is saved in Onyx(local database) I think. Let me know the answer.

dominictb commented 2 weeks ago

Proposal

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

Map centers on user's current location based on the saved cache but if user clears cache and restarts then map centers on San Francisco

What is the root cause of that problem?

We do not have a logic to clear the userLocation (stored in onyx) when the location permission is not granted in MapView component.

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

We should add a logic to clear the userLocation if the location permission is not granted to this file:

    useEffect(() => {
        const checkPermission = async () => {
            const permissionStatus = await navigator.permissions.query({name: 'geolocation'});

            if (permissionStatus.state !== 'granted') {
                UserLocation.clearUserLocation();
            }
        };
        checkPermission();
    }, []);

In above, we can use permissionStatus.state === 'prompt' instead of permissionStatus.state !== 'granted'.
We should apply the fix in native platform as well:

const checkPermission = async () => {
    try {
        let permissionStatus;

        if (Platform.OS === 'ios') {
            permissionStatus = await check(PERMISSIONS.IOS.LOCATION_WHEN_IN_USE);
        } else if (Platform.OS === 'android') {
            permissionStatus = await PermissionsAndroid.check(PermissionsAndroid.PERMISSIONS.ACCESS_FINE_LOCATION);
        }

        if (permissionStatus !== RESULTS.GRANTED) {
            UserLocation.clearUserLocation();
        }
    } catch (err) {
        console.warn(err);
    }
};

useEffect(() => {
        checkPermission();
    }, []);

What alternative solutions did you explore? (Optional)

We can create an event listener to the permission location. Once it changes to not granted, call UserLocation.clearUserLocation(). Below is pseudo code:

useEffect(() => {
    let permissionStatus;

    const checkPermission = async () => {
        try {
            permissionStatus = await navigator.permissions.query({ name: 'geolocation' });

            if (permissionStatus.state !== 'granted') {
                UserLocation.clearUserLocation();
            }
        } catch (err) {
            console.error(err);
        }
    };

    // Initial permission check
    checkPermission();

    // Event listener for permission changes
    const handlePermissionChange = () => {
        if (permissionStatus.state !== 'granted') {
            UserLocation.clearUserLocation();
        }
    };

    permissionStatus.addEventListener('change', handlePermissionChange);

    // Clean up event listener on component unmount
    return () => {
        if (permissionStatus) {
            permissionStatus.removeEventListener('change', handlePermissionChange);
        }
    };
}, []);

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 1 week ago

@anmurali Still overdue 6 days?! Let's take care of this!

anmurali commented 1 week ago

Hmm I cannot reproduce. Map does go back to centering in SF. I am also not sure why this is an issue? It doesn't seem like a problem if it doesn't actually behave this way.

dominictb commented 1 week ago

@anmurali

I can reproduce this behavior:

https://github.com/user-attachments/assets/bd881af2-f60a-48fe-9a25-d86c10a4a086

Below are the steps from my side:
1. Sign in new Incognito tab.
2. Go to self DM, choose track expense then choose distance.
3. Observe the prompt to allow access to user's location and observe the map being centered on San Francisco.
4. Allow expensify to access the user's location
5. Observe the map now centers on the user's location
6. Disallow expensify to access the user's location again
7. Go to self DM again, choose track expense then choose distance again.
8. Start a distance expense flow and observe the map
9. Observe map centers on user's current location.

Expensify / App