Merge pull request #3616 from inaturalist/mob-1321-minimal-2

MOB-1321 minimal

android/app/src/main/AndroidManifest.xml

@@ -9,6 +9,7 @@
     <uses-permission android:name="android.permission.CAMERA" />
     <uses-permission android:name="android.permission.RECORD_AUDIO" />
     <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />
+    <uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION" />
     <uses-permission android:name="android.permission.ACCESS_MEDIA_LOCATION" />
     <!-- the following permission replaces READ_EXTERNAL_STORAGE in Android 13 -->
     <uses-permission android:name="android.permission.READ_MEDIA_IMAGES"/>

src/components/Match/MatchContainer.tsx

@@ -27,9 +27,10 @@ import fetchPlaceName from "sharedHelpers/fetchPlaceName";
 import saveObservation from "sharedHelpers/saveObservation";
 import shouldFetchObservationLocation from "sharedHelpers/shouldFetchObservationLocation";
 import {
-  useExitObservationFlow, useLocationPermission, useSuggestions, useWatchPosition,
+  useExitObservationFlow, useLocationPermission, useSuggestions,
 } from "sharedHooks";
 import useDebugMode from "sharedHooks/useDebugMode";
+import useObservationLocation from "sharedHooks/useObservationLocation";
 import {
   internalUseSuggestionsInitialSuggestions,
 } from "sharedHooks/useSuggestions/filterSuggestions";
@@ -289,7 +290,7 @@ const MatchContainer = ( ) => {
     stopWatch,
     subscriptionId,
     userLocation,
-  } = useWatchPosition( { shouldFetchLocation } );
+  } = useObservationLocation( { shouldFetchLocation } );
 
   const navToLocationPicker = useCallback( ( ) => {
     stopWatch( subscriptionId );
@@ -318,11 +319,14 @@ const MatchContainer = ( ) => {
     }
   }, [currentUserLocation, updateObservationKeys] );
 
-  const handleRefetchSuggestions = useCallback( () => {
+  const handleRefetchSuggestions = useCallback( ( location: {
+    latitude?: number;
+    longitude?: number;
+  } ) => {
     const newScoreImageParams = {
       ...scoreImageParams,
-      lat: currentUserLocation?.latitude,
-      lng: currentUserLocation?.longitude,
+      lat: location?.latitude,
+      lng: location?.longitude,
     };
     dispatch( {
       type: "SET_LOCATION",
@@ -337,8 +341,6 @@ const MatchContainer = ( ) => {
     refetchSuggestions,
     scoreImageParams,
     scrollToTop,
-    currentUserLocation?.latitude,
-    currentUserLocation?.longitude,
   ] );
 
   useEffect( () => {
@@ -352,7 +354,7 @@ const MatchContainer = ( ) => {
     getCurrentUserPlaceName();
 
     if ( !hasRefetchedSuggestions && suggestions ) {
-      handleRefetchSuggestions();
+      handleRefetchSuggestions( userLocation );
     }
   }, [
     userLocation,

src/components/ObsEdit/ObsEdit.js

@@ -11,8 +11,8 @@ import shouldFetchObservationLocation from "sharedHelpers/shouldFetchObservation
 import {
   useCurrentUser,
   useLocationPermission,
-  useWatchPosition,
 } from "sharedHooks";
+import useObservationLocation from "sharedHooks/useObservationLocation";
 import useStore from "stores/useStore";
 import { getShadow } from "styles/global";
 
@@ -73,7 +73,7 @@ const ObsEdit = ( ): Node => {
     stopWatch,
     subscriptionId,
     userLocation,
-  } = useWatchPosition( { shouldFetchLocation } );
+  } = useObservationLocation( { shouldFetchLocation } );
 
   useEffect( ( ) => {
     if ( userLocation?.latitude ) {

src/components/SharedComponents/PermissionGateContainer.tsx

@@ -69,7 +69,10 @@ export const WRITE_MEDIA_PERMISSIONS = Platform.OS === "ios"
 
 export const LOCATION_PERMISSIONS = Platform.OS === "ios"
   ? [PERMISSIONS.IOS.LOCATION_WHEN_IN_USE]
-  : [PERMISSIONS.ANDROID.ACCESS_FINE_LOCATION];
+  : [
+    PERMISSIONS.ANDROID.ACCESS_FINE_LOCATION,
+    PERMISSIONS.ANDROID.ACCESS_COARSE_LOCATION,
+  ];
 
 interface Props extends PropsWithChildren {
   blockedPrompt?: string;
@@ -91,9 +94,10 @@ interface Props extends PropsWithChildren {
   withoutNavigation?: boolean;
 }
 
-interface MultiResult {
+export interface MultiResult {
   [permission: string]: PermissionStatus;
 }
+
 export function permissionResultFromMultiple( multiResults: MultiResult ) {
   if ( typeof ( multiResults ) !== "object" ) {
     throw new Error(
@@ -101,6 +105,26 @@ export function permissionResultFromMultiple( multiResults: MultiResult ) {
       + "Make sure you're using it with checkMultiple and not check",
     );
   }
+  // On Android 12+, the user may grant only approximate (coarse) location,
+  // leaving fine location denied. If ANY location permission is granted,
+  // the overall result is GRANTED.
+  const coarseKey = PERMISSIONS.ANDROID.ACCESS_COARSE_LOCATION;
+  if ( coarseKey in multiResults ) {
+    if ( find( multiResults, permResult => permResult === RESULTS.GRANTED ) ) {
+      return RESULTS.GRANTED;
+    }
+    if ( find( multiResults, permResult => permResult === RESULTS.LIMITED ) ) {
+      return RESULTS.LIMITED;
+    }
+    if ( find( multiResults, permResult => permResult === RESULTS.BLOCKED ) ) {
+      return RESULTS.BLOCKED;
+    }
+    if ( find( multiResults, permResult => permResult === RESULTS.DENIED ) ) {
+      return RESULTS.DENIED;
+    }
+    return RESULTS.UNAVAILABLE;
+  }
+  // All non-android location permissions use this path
   if ( find( multiResults, ( permResult, _perm ) => permResult === RESULTS.BLOCKED ) ) {
     return RESULTS.BLOCKED;
   }
@@ -116,6 +140,15 @@ export function permissionResultFromMultiple( multiResults: MultiResult ) {
   return RESULTS.GRANTED;
 }
 
+export async function hasOnlyCoarseLocation(): Promise<boolean> {
+  if ( Platform.OS !== "android" ) return false;
+  const results = await checkMultiple( LOCATION_PERMISSIONS );
+  return (
+    results[PERMISSIONS.ANDROID.ACCESS_COARSE_LOCATION] === RESULTS.GRANTED
+    && results[PERMISSIONS.ANDROID.ACCESS_FINE_LOCATION] !== RESULTS.GRANTED
+  );
+}
+
 export async function hasWriteMediaPermission( ) {
   // WRITE_MEDIA_PERMISSIONS is empty on android 11+ because we don't need to request permissions
   if ( WRITE_MEDIA_PERMISSIONS.length === 0 ) return true;

src/sharedHooks/useObservationLocation.ts

@@ -0,0 +1,74 @@
+import { useNavigation } from "@react-navigation/native";
+import { hasOnlyCoarseLocation } from "components/SharedComponents/PermissionGateContainer";
+import { useEffect, useRef, useState } from "react";
+
+import fetchCoarseUserLocation from "../sharedHelpers/fetchCoarseUserLocation";
+import type { UserLocation } from "./useWatchPosition";
+import useWatchPosition from "./useWatchPosition";
+
+const useObservationLocation = ( options: {
+  shouldFetchLocation: boolean;
+} ) => {
+  const navigation = useNavigation( );
+  const { shouldFetchLocation } = options;
+
+  const [isCoarseOnly, setIsCoarseOnly] = useState<boolean | null>( null );
+  const [coarseLocation, setCoarseLocation] = useState<UserLocation | null>( null );
+  const [isFetchingCoarse, setIsFetchingCoarse] = useState( false );
+  const cancelledRef = useRef( false );
+
+  useEffect( ( ) => {
+    if ( !shouldFetchLocation ) return ( ) => undefined;
+    cancelledRef.current = false;
+
+    ( async ( ) => {
+      setIsFetchingCoarse( true );
+      try {
+        const coarseOnly = await hasOnlyCoarseLocation( );
+        if ( cancelledRef.current ) return;
+        setIsCoarseOnly( coarseOnly );
+
+        if ( coarseOnly ) {
+          const location = await fetchCoarseUserLocation( );
+          if ( cancelledRef.current ) return;
+          if ( location ) setCoarseLocation( location );
+        }
+      } finally {
+        if ( !cancelledRef.current ) setIsFetchingCoarse( false );
+      }
+    } )( );
+
+    return ( ) => { cancelledRef.current = true; };
+  }, [shouldFetchLocation] );
+
+  useEffect( ( ) => {
+    const unsubscribe = navigation.addListener( "blur", ( ) => {
+      cancelledRef.current = true;
+      setIsFetchingCoarse( false );
+      setCoarseLocation( null );
+      setIsCoarseOnly( null );
+    } );
+    return unsubscribe;
+  }, [navigation] );
+
+  const shouldWatchFine = shouldFetchLocation && isCoarseOnly === false;
+
+  const {
+    isFetchingLocation: isFetchingFine,
+    stopWatch,
+    subscriptionId,
+    userLocation: fineLocation,
+  } = useWatchPosition( { shouldFetchLocation: shouldWatchFine } );
+
+  return {
+    isFetchingLocation: isFetchingFine
+      || isFetchingCoarse
+      // cover first frame where shouldFetchLocation = true but both isFetching values are false
+      || ( shouldFetchLocation && isCoarseOnly === null ),
+    stopWatch,
+    subscriptionId,
+    userLocation: coarseLocation ?? fineLocation,
+  };
+};
+
+export default useObservationLocation;

tests/unit/components/ObsEdit/ObsEdit.test.js

@@ -16,7 +16,7 @@ jest.mock( "@react-navigation/elements", () => ( {
     .mockImplementation( ( ) => mockHeaderBackButton ),
 } ) );
 
-jest.mock( "sharedHooks/useWatchPosition", () => ( {
+jest.mock( "sharedHooks/useObservationLocation", () => ( {
   __esModule: true,
   default: ( ) => ( {
     hasLocation: true,

tests/unit/sharedHooks/useObservationLocation.test.js

@@ -0,0 +1,134 @@
+import { renderHook, waitFor } from "@testing-library/react-native";
+import useObservationLocation from "sharedHooks/useObservationLocation";
+
+const mockUseWatchPosition = jest.fn( );
+jest.mock( "sharedHooks/useWatchPosition", () => ( {
+  __esModule: true,
+  default: ( ...args ) => mockUseWatchPosition( ...args ),
+} ) );
+
+const mockHasOnlyCoarseLocation = jest.fn( );
+jest.mock( "components/SharedComponents/PermissionGateContainer", () => ( {
+  hasOnlyCoarseLocation: ( ...args ) => mockHasOnlyCoarseLocation( ...args ),
+} ) );
+
+const mockFetchCoarseUserLocation = jest.fn( );
+jest.mock( "sharedHelpers/fetchCoarseUserLocation", () => ( {
+  __esModule: true,
+  default: ( ...args ) => mockFetchCoarseUserLocation( ...args ),
+} ) );
+
+const mockCoarseLocation = {
+  latitude: 44.95,
+  longitude: -93.27,
+  positional_accuracy: 2000,
+  altitude: null,
+  altitudinal_accuracy: null,
+};
+
+const mockFineLocation = {
+  latitude: 44.9537,
+  longitude: -93.2690,
+  positional_accuracy: 8,
+  altitude: 250,
+  altitudinal_accuracy: 3,
+};
+
+const defaultWatchResult = {
+  isFetchingLocation: false,
+  stopWatch: jest.fn( ),
+  subscriptionId: null,
+  userLocation: null,
+};
+
+beforeEach( ( ) => {
+  jest.clearAllMocks( );
+  mockUseWatchPosition.mockReturnValue( defaultWatchResult );
+} );
+
+describe( "useObservationLocation", ( ) => {
+  describe( "when only coarse location permission is granted", ( ) => {
+    beforeEach( ( ) => {
+      mockHasOnlyCoarseLocation.mockResolvedValue( true );
+    } );
+
+    it( "fetches coarse location and returns it as userLocation", async ( ) => {
+      mockFetchCoarseUserLocation.mockResolvedValue( mockCoarseLocation );
+
+      const { result } = renderHook( ( ) => useObservationLocation( {
+        shouldFetchLocation: true,
+      } ) );
+
+      await waitFor( ( ) => {
+        expect( result.current.userLocation ).toEqual( mockCoarseLocation );
+      } );
+      expect( result.current.isFetchingLocation ).toBe( false );
+      expect( mockFetchCoarseUserLocation ).toHaveBeenCalledTimes( 1 );
+    } );
+
+    it( "does not pass shouldFetchLocation to useWatchPosition", async ( ) => {
+      mockFetchCoarseUserLocation.mockResolvedValue( mockCoarseLocation );
+
+      renderHook( ( ) => useObservationLocation( {
+        shouldFetchLocation: true,
+      } ) );
+
+      await waitFor( ( ) => {
+        expect( mockHasOnlyCoarseLocation ).toHaveBeenCalled( );
+      } );
+      const allCalls = mockUseWatchPosition.mock.calls;
+      const lastCall = allCalls[allCalls.length - 1];
+      expect( lastCall[0].shouldFetchLocation ).toBe( false );
+    } );
+  } );
+
+  describe( "when fine location permission is granted", ( ) => {
+    beforeEach( ( ) => {
+      mockHasOnlyCoarseLocation.mockResolvedValue( false );
+    } );
+
+    it( "delegates to useWatchPosition and returns its location", async ( ) => {
+      mockUseWatchPosition.mockReturnValue( {
+        ...defaultWatchResult,
+        isFetchingLocation: false,
+        userLocation: mockFineLocation,
+      } );
+
+      const { result } = renderHook( ( ) => useObservationLocation( {
+        shouldFetchLocation: true,
+      } ) );
+
+      await waitFor( ( ) => {
+        expect( result.current.userLocation ).toEqual( mockFineLocation );
+      } );
+      expect( mockFetchCoarseUserLocation ).not.toHaveBeenCalled( );
+    } );
+
+    it( "passes shouldFetchLocation: true to useWatchPosition", async ( ) => {
+      renderHook( ( ) => useObservationLocation( {
+        shouldFetchLocation: true,
+      } ) );
+
+      await waitFor( ( ) => {
+        const allCalls = mockUseWatchPosition.mock.calls;
+        const lastCall = allCalls[allCalls.length - 1];
+        expect( lastCall[0].shouldFetchLocation ).toBe( true );
+      } );
+    } );
+
+    it( "reports isFetchingLocation from useWatchPosition", async ( ) => {
+      mockUseWatchPosition.mockReturnValue( {
+        ...defaultWatchResult,
+        isFetchingLocation: true,
+      } );
+
+      const { result } = renderHook( ( ) => useObservationLocation( {
+        shouldFetchLocation: true,
+      } ) );
+
+      await waitFor( ( ) => {
+        expect( result.current.isFetchingLocation ).toBe( true );
+      } );
+    } );
+  } );
+} );