From 1e8dc771a03c313cf9bf482f639e35892b51d31d Mon Sep 17 00:00:00 2001 From: sepeterson <10458078+sepeterson@users.noreply.github.com> Date: Tue, 12 May 2026 13:15:10 -0500 Subject: [PATCH] Reapply "MOB-1321 minimal" This reverts commit 24e1f1afb56a36917ae3ce261b936417a4ddf165. --- android/app/src/main/AndroidManifest.xml | 1 + src/components/Match/MatchContainer.tsx | 18 +-- src/components/ObsEdit/ObsEdit.js | 4 +- .../PermissionGateContainer.tsx | 37 ++++- src/sharedHooks/useObservationLocation.ts | 74 ++++++++++ tests/unit/components/ObsEdit/ObsEdit.test.js | 2 +- .../useObservationLocation.test.js | 134 ++++++++++++++++++ 7 files changed, 257 insertions(+), 13 deletions(-) create mode 100644 src/sharedHooks/useObservationLocation.ts create mode 100644 tests/unit/sharedHooks/useObservationLocation.test.js diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index c2f035406..8f6da3bce 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -9,6 +9,7 @@ + diff --git a/src/components/Match/MatchContainer.tsx b/src/components/Match/MatchContainer.tsx index 73bc44b57..0b674004d 100644 --- a/src/components/Match/MatchContainer.tsx +++ b/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, diff --git a/src/components/ObsEdit/ObsEdit.js b/src/components/ObsEdit/ObsEdit.js index e11120f19..bb4418d1a 100644 --- a/src/components/ObsEdit/ObsEdit.js +++ b/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 ) { diff --git a/src/components/SharedComponents/PermissionGateContainer.tsx b/src/components/SharedComponents/PermissionGateContainer.tsx index 816d30137..13728755c 100644 --- a/src/components/SharedComponents/PermissionGateContainer.tsx +++ b/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 { + 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; diff --git a/src/sharedHooks/useObservationLocation.ts b/src/sharedHooks/useObservationLocation.ts new file mode 100644 index 000000000..7e6bfb0ad --- /dev/null +++ b/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( null ); + const [coarseLocation, setCoarseLocation] = useState( 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; diff --git a/tests/unit/components/ObsEdit/ObsEdit.test.js b/tests/unit/components/ObsEdit/ObsEdit.test.js index 48e964922..a4f514447 100644 --- a/tests/unit/components/ObsEdit/ObsEdit.test.js +++ b/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, diff --git a/tests/unit/sharedHooks/useObservationLocation.test.js b/tests/unit/sharedHooks/useObservationLocation.test.js new file mode 100644 index 000000000..78b3f9df3 --- /dev/null +++ b/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 ); + } ); + } ); + } ); +} );