diff --git a/.flowconfig b/.flowconfig index 2ee26bc45..e408c9a58 100644 --- a/.flowconfig +++ b/.flowconfig @@ -27,6 +27,7 @@ node_modules/react-native/Libraries/polyfills/.* .*/node_modules/@react-native/community-cli-plugin/dist/commands/ram-bundle/index.js.flow .*/node_modules/@react-native/community-cli-plugin/dist/commands/bundle/index.js.flow .*/node_modules/@react-native/community-cli-plugin/dist/commands/bundle/buildBundle.js.flow +.*/node_modules/metro/src/lib/JsonReporter.js.flow [untyped] .*/node_modules/@react-native-community/cli/.*/.* diff --git a/src/components/App.js b/src/components/App.js index f7fcf995f..2b789bf6a 100644 --- a/src/components/App.js +++ b/src/components/App.js @@ -1,5 +1,6 @@ // @flow +import Geolocation from "@react-native-community/geolocation"; import RootDrawerNavigator from "navigation/rootDrawerNavigator"; import { RealmContext } from "providers/contexts"; import type { Node } from "react"; @@ -33,6 +34,10 @@ Realm.setLogLevel( "warn" ); // https://stackoverflow.com/questions/69538962 LogBox.ignoreLogs( ["new NativeEventEmitter"] ); +const geolocationConfig = { + skipPermissionRequests: true +}; + type Props = { // $FlowIgnore children?: unknown, @@ -69,6 +74,10 @@ const App = ( { children }: Props ): Node => { logger.info( "pickup" ); }, [] ); + // skipping location permissions here since we're manually showing + // permission gates and don't want to pop up the native notification + Geolocation.setRNConfiguration( geolocationConfig ); + // this children prop is here for the sake of testing with jest // normally we would never do this in code return children || ; diff --git a/src/components/Camera/hooks/usePrepareStoreAndNavigate.js b/src/components/Camera/hooks/usePrepareStoreAndNavigate.js index 08fa77c0b..2f7dfb306 100644 --- a/src/components/Camera/hooks/usePrepareStoreAndNavigate.js +++ b/src/components/Camera/hooks/usePrepareStoreAndNavigate.js @@ -8,7 +8,7 @@ import { import Observation from "realmModels/Observation"; import ObservationPhoto from "realmModels/ObservationPhoto"; import { log } from "sharedHelpers/logger"; -import { useUserLocation } from "sharedHooks"; +import { useWatchPosition } from "sharedHooks"; import useStore from "stores/useStore"; const logger = log.extend( "usePrepareStoreAndNavigate" ); @@ -82,18 +82,22 @@ const usePrepareStoreAndNavigate = ( options: Options ): Function => { const observations = useStore( state => state.observations ); const setSavingPhoto = useStore( state => state.setSavingPhoto ); const setCameraState = useStore( state => state.setCameraState ); - const { userLocation } = useUserLocation( { untilAcc: 0, enabled: !!shouldFetchLocation } ); + const { userLocation } = useWatchPosition( { + shouldFetchLocation + } ); const numOfObsPhotos = currentObservation?.observationPhotos?.length || 0; const createObsWithCameraPhotos = useCallback( async ( localFilePaths, visionResult ) => { const newObservation = await Observation.new( ); - // location is needed for fetching online Suggestions on the next screen + // 20240709 amanda - this is temporary since we'll want to move this code to + // Suggestions after the changes to permissions github issue is complete, and + // we'll be able to updateObservationKeys on the observation there if ( userLocation?.latitude ) { newObservation.latitude = userLocation?.latitude; newObservation.longitude = userLocation?.longitude; - newObservation.positional_accuracy = userLocation?.accuracy; + newObservation.positional_accuracy = userLocation?.positional_accuracy; } newObservation.observationPhotos = await ObservationPhoto .createObsPhotosWithPosition( localFilePaths, { @@ -116,9 +120,7 @@ const usePrepareStoreAndNavigate = ( options: Options ): Function => { addPhotoPermissionResult, cameraUris, setObservations, - userLocation?.accuracy, - userLocation?.latitude, - userLocation?.longitude + userLocation ] ); const updateObsWithCameraPhotos = useCallback( async ( ) => { diff --git a/src/components/ObsEdit/EvidenceSection.js b/src/components/ObsEdit/EvidenceSection.js index 6ddc526c1..3997e8e02 100644 --- a/src/components/ObsEdit/EvidenceSection.js +++ b/src/components/ObsEdit/EvidenceSection.js @@ -1,6 +1,5 @@ // @flow -import { useNavigation } from "@react-navigation/native"; import classnames from "classnames"; import { MAX_PHOTOS_ALLOWED } from "components/Camera/StandardCamera/StandardCamera"; import { @@ -10,7 +9,7 @@ import { import { MAX_SOUNDS_ALLOWED } from "components/SoundRecorder/SoundRecorder"; import { Pressable, View } from "components/styledComponents"; import type { Node } from "react"; -import React, { useCallback } from "react"; +import React from "react"; import { useTheme } from "react-native-paper"; import useTranslation from "sharedHooks/useTranslation"; @@ -33,10 +32,8 @@ type Props = { }, uuid: string }>, - updateObservationKeys: Function, - renderPermissionsGate: Function, - requestPermissions: Function, - hasPermissions: boolean + onLocationPress: ( ) => void, + updateObservationKeys: Function } const EvidenceSection = ( { @@ -47,10 +44,8 @@ const EvidenceSection = ( { setShowAddEvidenceSheet, showAddEvidenceSheet, observationSounds, - updateObservationKeys, - renderPermissionsGate, - requestPermissions, - hasPermissions + onLocationPress, + updateObservationKeys }: Props ): Node => { const { t } = useTranslation( ); const theme = useTheme( ); @@ -60,21 +55,6 @@ const EvidenceSection = ( { // the API const obsPhotos = currentObservation?.observationPhotos || currentObservation?.observation_photos; const obsSounds = currentObservation?.observationSounds || currentObservation?.observation_sounds; - const navigation = useNavigation( ); - - const navToLocationPicker = useCallback( ( ) => { - navigation.navigate( "LocationPicker", { goBackOnSave: true } ); - }, [navigation] ); - - const onLocationPress = ( ) => { - // If we have location permissions, navigate to the location picker - if ( hasPermissions ) { - navToLocationPicker(); - } else { - // If we don't have location permissions, request them - requestPermissions( ); - } - }; const latitude = currentObservation?.latitude; const longitude = currentObservation?.longitude; @@ -176,15 +156,6 @@ const EvidenceSection = ( { } - {renderPermissionsGate( { - // If the user does not give location permissions in any form, - // navigate to the location picker (if granted we just continue fetching the location) - onRequestDenied: navToLocationPicker, - onRequestBlocked: navToLocationPicker, - onModalHide: ( ) => { - if ( !hasPermissions ) navToLocationPicker(); - } - } )} void, passesEvidenceTest: boolean, setPassesEvidenceTest: Function, - currentObservation: Object, updateObservationKeys: Function } const EvidenceSectionContainer = ( { - setPassesEvidenceTest, - passesEvidenceTest, currentObservation, + isFetchingLocation, + onLocationPress, + passesEvidenceTest, + setPassesEvidenceTest, updateObservationKeys }: Props ): Node => { const cameraRollUris = useStore( state => state.cameraRollUris ); @@ -42,55 +43,13 @@ const EvidenceSectionContainer = ( { const hasImportedPhotos = hasPhotos && cameraRollUris.length === 0; const observationPhotos = currentObservation?.observationPhotos || []; const observationSounds = currentObservation?.observationSounds || []; - const mountedRef = useRef( true ); const [showAddEvidenceSheet, setShowAddEvidenceSheet] = useState( false ); const [currentPlaceGuess, setCurrentPlaceGuess] = useState( ); - const [ - shouldRetryCurrentObservationLocation, - setShouldRetryCurrentObservationLocation - ] = useState( true ); - const { hasPermissions, renderPermissionsGate, requestPermissions } = useLocationPermission( ); - - // Hook version of componentWillUnmount. We use a ref to track mounted - // state (not useState, which might get frozen in a closure for other - // useEffects), and set it to false in the cleanup cleanup function. The - // effect has an empty dependency array so it should only run when the - // component mounts and when it unmounts, unlike in the cleanup effects of - // other hooks, which will run when any of there dependency values change, - // and maybe even before other hooks execute. If we ever need to do this - // again we could probably wrap this into its own hook, like useMounted - // ( ). - useEffect( ( ) => { - mountedRef.current = true; - return function cleanup( ) { - mountedRef.current = false; - }; - }, [mountedRef] ); - - const { - hasLocation, - isFetchingLocation - } = useCurrentObservationLocation( - mountedRef, - currentObservation, - updateObservationKeys, - hasPermissions, - { - retry: hasPermissions && shouldRetryCurrentObservationLocation - } - ); - const latitude = currentObservation?.latitude; const longitude = currentObservation?.longitude; - useEffect( ( ) => { - if ( latitude ) { - setShouldRetryCurrentObservationLocation( false ); - } - }, [latitude] ); - const hasPhotoOrSound = useMemo( ( ) => { if ( currentObservation?.observationPhotos?.length > 0 || currentObservation?.observationSounds?.length > 0 ) { @@ -114,9 +73,7 @@ const EvidenceSectionContainer = ( { validPositionalAccuracy = true; } - if ( - hasLocation - && coordinatesExist + if ( coordinatesExist && latitudeInRange && longitudeInRange && validPositionalAccuracy @@ -128,7 +85,6 @@ const EvidenceSectionContainer = ( { currentObservation, longitude, latitude, - hasLocation, isNewObs, hasImportedPhotos ] ); @@ -210,17 +166,15 @@ const EvidenceSectionContainer = ( { return ( ); }; diff --git a/src/components/ObsEdit/ObsEdit.js b/src/components/ObsEdit/ObsEdit.js index 8fbc90c42..dce9afacb 100644 --- a/src/components/ObsEdit/ObsEdit.js +++ b/src/components/ObsEdit/ObsEdit.js @@ -1,12 +1,13 @@ // @flow -import { useIsFocused } from "@react-navigation/native"; +import { useIsFocused, useNavigation } from "@react-navigation/native"; import { ViewWrapper } from "components/SharedComponents"; import { View } from "components/styledComponents"; import type { Node } from "react"; -import React, { useState } from "react"; +import React, { useCallback, useEffect, useState } from "react"; import { KeyboardAwareScrollView } from "react-native-keyboard-aware-scroll-view"; -import { useCurrentUser } from "sharedHooks"; +import shouldFetchObservationLocation from "sharedHelpers/shouldFetchObservationLocation.ts"; +import { useCurrentUser, useLocationPermission, useWatchPosition } from "sharedHooks"; import useStore from "stores/useStore"; import { getShadowForColor } from "styles/global"; import colors from "styles/tailwindColors"; @@ -23,6 +24,7 @@ const DROP_SHADOW = getShadowForColor( colors.black, { } ); const ObsEdit = ( ): Node => { + const navigation = useNavigation( ); const currentObservation = useStore( state => state.currentObservation ); const currentObservationIndex = useStore( state => state.currentObservationIndex ); const observations = useStore( state => state.observations ); @@ -33,6 +35,51 @@ const ObsEdit = ( ): Node => { const [resetScreen, setResetScreen] = useState( false ); const isFocused = useIsFocused( ); const currentUser = useCurrentUser( ); + const [shouldFetchLocation, setShouldFetchLocation] = useState( false ); + const { hasPermissions, renderPermissionsGate, requestPermissions } = useLocationPermission( ); + + const { + isFetchingLocation, + userLocation + } = useWatchPosition( { + shouldFetchLocation + } ); + + useEffect( ( ) => { + if ( shouldFetchObservationLocation( currentObservation ) ) { + setShouldFetchLocation( true ); + } + }, [currentObservation] ); + + useEffect( ( ) => { + if ( userLocation?.latitude ) { + updateObservationKeys( userLocation ); + } + }, [userLocation, updateObservationKeys] ); + + useEffect( ( ) => { + // this is needed to make sure watchPosition is called when a user + // navigates to ObsEdit a second time during an app session, + // otherwise, state will indicate that fetching is not needed + const unsubscribe = navigation.addListener( "blur", ( ) => { + setShouldFetchLocation( false ); + } ); + return unsubscribe; + }, [navigation] ); + + const navToLocationPicker = useCallback( ( ) => { + navigation.navigate( "LocationPicker", { goBackOnSave: true } ); + }, [navigation] ); + + const onLocationPress = ( ) => { + // If we have location permissions, navigate to the location picker + if ( hasPermissions ) { + navToLocationPicker(); + } else { + // If we don't have location permissions, request them + requestPermissions( ); + } + }; if ( !isFocused ) return null; @@ -67,6 +114,8 @@ const ObsEdit = ( ): Node => { )} { passesIdentificationTest={passesIdentificationTest} setCurrentObservationIndex={setCurrentObservationIndex} /> + {renderPermissionsGate( { + // If the user does not give location permissions in any form, + // navigate to the location picker (if granted we just continue fetching the location) + onRequestDenied: navToLocationPicker, + onRequestBlocked: navToLocationPicker, + onModalHide: ( ) => { + if ( !hasPermissions ) navToLocationPicker(); + } + } )} ); }; diff --git a/src/components/Projects/ProjectsContainer.tsx b/src/components/Projects/ProjectsContainer.tsx index 721af1c3a..9f51dd7dc 100644 --- a/src/components/Projects/ProjectsContainer.tsx +++ b/src/components/Projects/ProjectsContainer.tsx @@ -5,7 +5,7 @@ import { useAuthenticatedQuery, useCurrentUser, useTranslation, - useUserLocation + useWatchPosition } from "sharedHooks"; import useLocationPermission from "sharedHooks/useLocationPermission.tsx"; @@ -27,10 +27,9 @@ const ProjectsContainer = ( ) => { const { t } = useTranslation( ); const [apiParams, setApiParams] = useState( { } ); const [currentTabId, setCurrentTabId] = useState( TAB_ID.JOINED ); - const { hasPermissions, renderPermissionsGate, requestPermissions } = useLocationPermission(); - const { userLocation } = useUserLocation( { - skipName: true, - permissionsGranted: hasPermissions + const { hasPermissions, renderPermissionsGate, requestPermissions } = useLocationPermission( ); + const { userLocation } = useWatchPosition( { + shouldFetchLocation: hasPermissions } ); const { @@ -52,7 +51,8 @@ const ProjectsContainer = ( ) => { } else if ( currentTabId === TAB_ID.NEARBY && userLocation ) { setApiParams( { lat: userLocation.latitude, - lng: userLocation.longitude + lng: userLocation.longitude, + radius: 50 } ); } }, [ @@ -109,7 +109,7 @@ const ProjectsContainer = ( ) => { hasPermissions={hasPermissions} requestPermissions={requestPermissions} /> - {renderPermissionsGate()} + {renderPermissionsGate( )} ); }; diff --git a/src/sharedHelpers/shouldFetchObservationLocation.ts b/src/sharedHelpers/shouldFetchObservationLocation.ts new file mode 100644 index 000000000..ef0578839 --- /dev/null +++ b/src/sharedHelpers/shouldFetchObservationLocation.ts @@ -0,0 +1,35 @@ +import { galleryPhotosPath } from "appConstants/paths.ts"; +import RNFS from "react-native-fs"; +import { RealmObservation } from "realmModels/types.d.ts"; +import { + TARGET_POSITIONAL_ACCURACY +} from "sharedHooks/useWatchPosition.ts"; + +const shouldFetchObservationLocation = ( observation: RealmObservation ) => { + const latitude = observation?.latitude; + const longitude = observation?.longitude; + const hasLocation = !!( latitude && longitude ); + const originalPhotoUri = observation?.observationPhotos + && observation?.observationPhotos[0]?.originalPhotoUri; + const isGalleryPhoto = originalPhotoUri?.includes( galleryPhotosPath ); + // Shared photo paths will look something like Shared/AppGroup/sdgsdgsdgk + const isSharedPhoto = ( + originalPhotoUri && !originalPhotoUri.includes( RNFS.DocumentDirectoryPath ) + ); + const isNewObservation = ( + !observation?._created_at + && !observation?._synced_at + ); + const accGoodEnough = ( + observation?.positional_accuracy + && observation.positional_accuracy <= TARGET_POSITIONAL_ACCURACY + ); + + return observation + && isNewObservation + && ( !hasLocation || !accGoodEnough ) + && !isGalleryPhoto + && !isSharedPhoto; +}; + +export default shouldFetchObservationLocation; diff --git a/src/sharedHooks/index.js b/src/sharedHooks/index.js index 9e2309df4..551b56e38 100644 --- a/src/sharedHooks/index.js +++ b/src/sharedHooks/index.js @@ -1,6 +1,5 @@ export { default as useAuthenticatedMutation } from "./useAuthenticatedMutation"; export { default as useAuthenticatedQuery } from "./useAuthenticatedQuery"; -export { default as useCurrentObservationLocation } from "./useCurrentObservationLocation"; export { default as useCurrentUser } from "./useCurrentUser"; export { default as useDebugMode } from "./useDebugMode"; export { default as useDeviceOrientation } from "./useDeviceOrientation"; @@ -12,6 +11,7 @@ export { default as useInterval } from "./useInterval"; export { default as useIsConnected } from "./useIsConnected"; export { default as useLocalObservation } from "./useLocalObservation"; export { default as useLocalObservations } from "./useLocalObservations"; +export { default as useLocationPermission } from "./useLocationPermission"; export { default as useObservationsUpdates } from "./useObservationsUpdates"; export { default as useObservationUpdatesWhenFocused } from "./useObservationUpdatesWhenFocused"; export { default as useRemoteObservation } from "./useRemoteObservation"; @@ -20,5 +20,5 @@ export { default as useStoredLayout } from "./useStoredLayout"; export { default as useTaxon } from "./useTaxon"; export { default as useTaxonSearch } from "./useTaxonSearch"; export { default as useTranslation } from "./useTranslation"; -export { default as useUserLocation } from "./useUserLocation"; export { default as useUserMe } from "./useUserMe"; +export { default as useWatchPosition } from "./useWatchPosition"; diff --git a/src/sharedHooks/useCurrentObservationLocation.js b/src/sharedHooks/useCurrentObservationLocation.js deleted file mode 100644 index d82086ef3..000000000 --- a/src/sharedHooks/useCurrentObservationLocation.js +++ /dev/null @@ -1,184 +0,0 @@ -// @flow - -import { galleryPhotosPath } from "appConstants/paths.ts"; -import { - useEffect, useRef, - useState -} from "react"; -import RNFS from "react-native-fs"; - -// Please don't change this to an aliased path or the e2e mock will not get -// used in our e2e tests on Github Actions -import fetchUserLocation from "../sharedHelpers/fetchUserLocation"; - -const INITIAL_POSITIONAL_ACCURACY = 99999; -const TARGET_POSITIONAL_ACCURACY = 10; -export const LOCATION_FETCH_INTERVAL = 1000; - -// Primarily fetches the current location for a new observation and returns -// isFetchingLocation to tell the consumer whether this process is happening. -// If currentObservation is not new, it will not fetch location and return -// information about the current observation's location -const useCurrentObservationLocation = ( - // $FlowIgnore - mountedRef: unknown, - currentObservation: Object, - updateObservationKeys: Function, - hasPermissions: boolean, - options: Object = { } -): Object => { - const latitude = currentObservation?.latitude; - const longitude = currentObservation?.longitude; - const hasLocation = !!( latitude && longitude ); - const originalPhotoUri = currentObservation?.observationPhotos - && currentObservation?.observationPhotos[0]?.originalPhotoUri; - const isGalleryPhoto = originalPhotoUri?.includes( galleryPhotosPath ); - // Shared photo paths will look something like Shared/AppGroup/sdgsdgsdgk - const isSharedPhoto = ( - originalPhotoUri && !originalPhotoUri.includes( RNFS.DocumentDirectoryPath ) - ); - const isNewObservation = ( - !currentObservation?._created_at - && !currentObservation?._synced_at - ); - const accGoodEnough = ( - currentObservation?.positional_accuracy - && currentObservation.positional_accuracy <= TARGET_POSITIONAL_ACCURACY - ); - const locationNotSetYet = useRef( true ); - const prevObservation = useRef( currentObservation ); - - const [shouldFetchLocation, setShouldFetchLocation] = useState( - currentObservation - && isNewObservation - && ( !hasLocation || !accGoodEnough ) - && !isGalleryPhoto - && !isSharedPhoto - ); - - const [numLocationFetches, setNumLocationFetches] = useState( 0 ); - const [fetchingLocation, setFetchingLocation] = useState( false ); - const [positionalAccuracy, setPositionalAccuracy] = useState( INITIAL_POSITIONAL_ACCURACY ); - const [lastLocationFetchTime, setLastLocationFetchTime] = useState( 0 ); - const [currentLocation, setCurrentLocation] = useState( null ); - - useEffect( () => { - if ( locationNotSetYet.current ) { - // Don't run if it's the first render - locationNotSetYet.current = false; - return; - } if ( prevObservation.current !== currentObservation ) { - // Don't run if observation was changed (only when location was changed) - prevObservation.current = currentObservation; - return; - } - - updateObservationKeys( { - latitude: currentLocation?.latitude, - longitude: currentLocation?.longitude, - positional_accuracy: currentLocation?.positional_accuracy - } ); - - prevObservation.current = currentObservation; - }, [currentLocation, currentObservation, updateObservationKeys] ); - - useEffect( ( ) => { - if ( !currentObservation ) return; - if ( !shouldFetchLocation ) return; - if ( fetchingLocation ) return; - - const fetchLocation = async () => { - // If the component is gone, you won't be able to updated it - if ( !mountedRef.current ) return; - if ( !shouldFetchLocation ) return; - - if ( !hasPermissions ) { - setFetchingLocation( false ); - setShouldFetchLocation( false ); - return; - } - const location = await fetchUserLocation( ); - - // If we're still receiving location updates and location is blank, - // then we don't know where we are any more and the obs should update - // to reflect that - if ( - location?.latitude !== currentObservation.latitude - || location?.longitude !== currentObservation.longitude - || location?.positional_accuracy !== currentObservation.positional_accuracy - ) { - // Cannot call updateObservationKeys directly from here, since fetchUserLocation might take - // a while to return, in the meantime the current copy of the observation might have - // changed, so we update the observation from useEffect of currentLocation, so it will - // always have the latest copy of the current observation (see GH issue #584) - setCurrentLocation( location ); - } - - setFetchingLocation( false ); - - // The local state version of positionalAccuracy needs to be a number, - // so don't set it to - const newPositionalAccuracy = location?.positional_accuracy || INITIAL_POSITIONAL_ACCURACY; - setPositionalAccuracy( newPositionalAccuracy ); - if ( newPositionalAccuracy > TARGET_POSITIONAL_ACCURACY ) { - // This is just here to make absolutely sure the effect runs again in a second - setTimeout( - ( ) => setNumLocationFetches( numFetches => numFetches + 1 ), - LOCATION_FETCH_INTERVAL - ); - } - }; - - if ( - // If we're already fetching we don't need to fetch again - !fetchingLocation - // We only need to fetch when we're above the target - && positionalAccuracy > TARGET_POSITIONAL_ACCURACY - // Don't fetch location more than once a second - && Date.now() - lastLocationFetchTime >= LOCATION_FETCH_INTERVAL - ) { - setFetchingLocation( true ); - setLastLocationFetchTime( Date.now() ); - fetchLocation( ); - } else if ( positionalAccuracy <= TARGET_POSITIONAL_ACCURACY ) { - setShouldFetchLocation( false ); - } - }, [ - currentObservation, - fetchingLocation, - hasPermissions, - lastLocationFetchTime, - mountedRef, - numLocationFetches, - positionalAccuracy, - setFetchingLocation, - shouldFetchLocation, - updateObservationKeys - ] ); - - useEffect( ( ) => { - if ( options.retry && !shouldFetchLocation ) { - setTimeout( ( ) => { - setShouldFetchLocation( true ); - }, LOCATION_FETCH_INTERVAL + 1 ); - } - }, [ - options.retry, - setShouldFetchLocation, - shouldFetchLocation - ] ); - - return { - latitude, - longitude, - positionalAccuracy: currentObservation?.positional_accuracy, - hasLocation, - // Internally we're tracking isFetching when one of potentially many - // location requests is in flight, but this tells the external consumer - // whether the overall location fetching process is happening - isFetchingLocation: shouldFetchLocation, - numLocationFetches - }; -}; - -export default useCurrentObservationLocation; diff --git a/src/sharedHooks/useUserLocation.ts b/src/sharedHooks/useUserLocation.ts deleted file mode 100644 index 3164bb68f..000000000 --- a/src/sharedHooks/useUserLocation.ts +++ /dev/null @@ -1,168 +0,0 @@ -import Geolocation, { - GeolocationError, - GeolocationResponse -} from "@react-native-community/geolocation"; -import { - LOCATION_PERMISSIONS, - permissionResultFromMultiple -} from "components/SharedComponents/PermissionGateContainer.tsx"; -import { useEffect, useRef, useState } from "react"; -import { - checkMultiple, - RESULTS -} from "react-native-permissions"; -import fetchPlaceName from "sharedHelpers/fetchPlaceName"; -// import { log } from "sharedHelpers/logger"; - -// const logger = log.extend( "userUserLocation" ); - -// Max time to wait while fetching current location -const CURRENT_LOCATION_TIMEOUT_MS = 30000; - -interface UserLocation { - name?: string, - latitude: number, - longitude: number, - accuracy?: number -} - -interface UserLocationResponse { - userLocation?: UserLocation, - isLoading: boolean -} - -function useUserLocation( - options?: { - enabled?: boolean, - skipName?: boolean, - permissionsGranted?: boolean, - untilAcc?: number | undefined - } -): UserLocationResponse { - const { - enabled: enabledOpt, - skipName = false, - permissionsGranted: permissionsGrantedProp = false, - untilAcc - } = options || {}; - const enabled = enabledOpt !== false; - const [userLocation, setUserLocation] = useState( undefined ); - // logger.debug( `userLocation?.latitude: ${userLocation?.latitude}` ); - const [isLoading, setIsLoading] = useState( true ); - const [permissionsGranted, setPermissionsGranted] = useState( permissionsGrantedProp ); - const [permissionsChecked, setPermissionsChecked] = useState( false ); - const fetchingLocation = useRef( false ); - const [accGoodEnough, setAccGoodEnough] = useState( false ); - - useEffect( ( ) => { - if ( permissionsGrantedProp === true && permissionsGranted === false ) { - setPermissionsGranted( true ); - } - }, [permissionsGranted, permissionsGrantedProp] ); - - useEffect( ( ) => { - async function checkPermissions() { - const permissionsResult = permissionResultFromMultiple( - await checkMultiple( LOCATION_PERMISSIONS ) - ); - if ( permissionsResult === RESULTS.GRANTED ) { - setPermissionsGranted( true ); - } else { - console.warn( - "Location permissions have not been granted. You probably need to use a PermissionGate" - ); - } - setPermissionsChecked( true ); - } - checkPermissions( ); - }, [] ); - - useEffect( ( ) => { - const fetchLocation = async ( ) => { - fetchingLocation.current = true; - setIsLoading( true ); - - const success = async ( position: GeolocationResponse ) => { - // logger.debug( - // `getCurrentPosition success, position?.coords?.latitude: ${position?.coords?.latitude}` - // ); - const { coords } = position; - let locationName; - if ( !skipName ) { - locationName = await fetchPlaceName( coords.latitude, coords.longitude ); - } - setUserLocation( { - name: locationName || undefined, - latitude: coords.latitude, - longitude: coords.longitude, - accuracy: coords.accuracy - } ); - setIsLoading( false ); - fetchingLocation.current = false; - setAccGoodEnough( true ); - if ( typeof ( untilAcc ) === "number" && coords?.accuracy && coords?.accuracy > untilAcc ) { - setTimeout( ( ) => setAccGoodEnough( false ), 1000 ); - } - }; - - // TODO: set geolocation fetch error - const failure = ( error: GeolocationError ) => { - console.warn( `useUserLocation: ${error.message} (${error.code})` ); - setIsLoading( false ); - fetchingLocation.current = false; - setAccGoodEnough( true ); - }; - - const gcpOptions = { - enableHighAccuracy: true, - maximumAge: 0, - timeout: CURRENT_LOCATION_TIMEOUT_MS - }; - - // TODO refactor to use fetchUserLocation, which is promise-ified and - // mockable in an e2e context - Geolocation.getCurrentPosition( success, failure, gcpOptions ); - }; - - if ( permissionsChecked && !permissionsGranted ) { - setIsLoading( false ); - fetchingLocation.current = false; - } else if ( - // we have permission - permissionsGranted - // and we're not already fetching - && !fetchingLocation.current - // and we're not waiting OR we are and acc is above threshold - && !accGoodEnough - && enabled - ) { - fetchingLocation.current = true; - fetchLocation( ); - } - }, [ - accGoodEnough, - enabled, - permissionsChecked, - permissionsGranted, - skipName, - untilAcc - ] ); - - // When the consumer tells us we no longer need to fetch location, reset the - // user location so it's not stale the next time we need to fetch - useEffect( ( ) => { - // logger.debug( "enabled effect" ); - if ( enabled === false ) { - // logger.debug( "enabled effect, disabled resetting" ); - setUserLocation( undefined ); - setAccGoodEnough( false ); - } - }, [enabled] ); - - return { - userLocation, - isLoading - }; -} - -export default useUserLocation; diff --git a/src/sharedHooks/useWatchPosition.ts b/src/sharedHooks/useWatchPosition.ts new file mode 100644 index 000000000..61a1030cc --- /dev/null +++ b/src/sharedHooks/useWatchPosition.ts @@ -0,0 +1,102 @@ +import Geolocation, { + GeolocationError, + GeolocationResponse +} from "@react-native-community/geolocation"; +import { useNavigation } from "@react-navigation/native"; +import { useCallback, useEffect, useState } from "react"; + +export const TARGET_POSITIONAL_ACCURACY = 10; + +export const TIMEOUT = 2000; + +interface UserLocation { + latitude: number, + longitude: number, + positional_accuracy: number +} + +const geolocationOptions = { + distanceFilter: 0, + enableHighAccuracy: true, + maximumAge: 0, + timeout: TIMEOUT +}; + +const useWatchPosition = ( options: { + shouldFetchLocation: boolean +} ) => { + const navigation = useNavigation( ); + const [currentPosition, setCurrentPosition] = useState( null ); + const [subscriptionId, setSubscriptionId] = useState( null ); + const [userLocation, setUserLocation] = useState( null ); + const { shouldFetchLocation } = options; + + const [isFetchingLocation, setIsFetchingLocation] = useState( + shouldFetchLocation + ); + + const watchPosition = ( ) => { + const success = ( position: GeolocationResponse ) => { + setCurrentPosition( position ); + }; + + const failure = ( error: GeolocationError ) => { + console.warn( error, ": useWatchPosition error" ); + setIsFetchingLocation( false ); + }; + + try { + const watchID = Geolocation.watchPosition( + success, + failure, + geolocationOptions + ); + setSubscriptionId( watchID ); + } catch ( error ) { + failure( error ); + } + }; + + const stopWatch = useCallback( id => { + Geolocation.clearWatch( id ); + setSubscriptionId( null ); + setCurrentPosition( null ); + setIsFetchingLocation( false ); + }, [] ); + + useEffect( ( ) => { + if ( !currentPosition ) { return; } + const newLocation = { + latitude: currentPosition?.coords?.latitude, + longitude: currentPosition?.coords?.longitude, + positional_accuracy: currentPosition?.coords?.accuracy + }; + setUserLocation( newLocation ); + if ( currentPosition?.coords?.accuracy < TARGET_POSITIONAL_ACCURACY ) { + stopWatch( subscriptionId ); + } + }, [currentPosition, stopWatch, subscriptionId] ); + + useEffect( ( ) => { + if ( shouldFetchLocation ) { + watchPosition( ); + } + }, [shouldFetchLocation] ); + + useEffect( ( ) => { + const unsubscribe = navigation.addListener( "blur", ( ) => { + setSubscriptionId( null ); + setCurrentPosition( null ); + setIsFetchingLocation( false ); + setUserLocation( null ); + } ); + return unsubscribe; + }, [navigation] ); + + return { + isFetchingLocation, + userLocation + }; +}; + +export default useWatchPosition; diff --git a/tests/integration/ObsEditOffline.test.js b/tests/integration/ObsEditOffline.test.js index 0b7e004a1..56ea603bb 100644 --- a/tests/integration/ObsEditOffline.test.js +++ b/tests/integration/ObsEditOffline.test.js @@ -4,7 +4,7 @@ import { screen, waitFor } from "@testing-library/react-native"; import ObsEdit from "components/ObsEdit/ObsEdit"; import fetchMock from "jest-fetch-mock"; import React from "react"; -import { LOCATION_FETCH_INTERVAL } from "sharedHooks/useCurrentObservationLocation"; +import ReactNativePermissions from "react-native-permissions"; import useStore from "stores/useStore"; import factory from "tests/factory"; import faker from "tests/helpers/faker"; @@ -44,6 +44,12 @@ beforeEach( async ( ) => { locale: "en" } ); await signIn( mockUser, { realm: global.mockRealms[__filename] } ); + const mockedPermissions = { + "ios.permission.LOCATION": "granted" + }; + + jest.spyOn( ReactNativePermissions, "checkMultiple" ) + .mockResolvedValueOnce( mockedPermissions ); } ); afterEach( ( ) => { @@ -70,25 +76,33 @@ describe( "ObsEdit offline", ( ) => { describe( "creation", ( ) => { it( "should fetch coordinates", async ( ) => { - const mockGetCurrentPosition = jest.fn( ( success, _error, _options ) => success( { + const mockWatchPosition = jest.fn( ( success, _error, _options ) => success( { coords: { latitude: 1, longitude: 1, - accuracy: 10, + accuracy: 9, timestamp: Date.now( ) } } ) ); - Geolocation.getCurrentPosition.mockImplementation( mockGetCurrentPosition ); + Geolocation.watchPosition.mockImplementation( mockWatchPosition ); const observation = factory( "LocalObservation", { observationPhotos: [] } ); - useStore.setState( { observations: [observation] } ); + useStore.setState( { + observations: [observation], + currentObservation: observation + } ); renderAppWithComponent( ); + // removing the next lines since location fetch is fast enough that the location indicator + // won't show up in this scenario + // expect( + // screen.getByTestId( "EvidenceSection.fetchingLocationIndicator" ) + // ).toBeTruthy( ); await waitFor( ( ) => { - expect( mockGetCurrentPosition ).toHaveBeenCalled( ); - }, { timeout: LOCATION_FETCH_INTERVAL * 2 } ); + expect( mockWatchPosition ).toHaveBeenCalled( ); + } ); const coords = await screen.findByText( /Lat:/ ); expect( coords ).toBeTruthy( ); expect( screen.queryByText( "Finding location..." ) ).toBeFalsy( ); diff --git a/tests/integration/ObsEditWithoutProvider.test.js b/tests/integration/ObsEditOnline.test.js similarity index 74% rename from tests/integration/ObsEditWithoutProvider.test.js rename to tests/integration/ObsEditOnline.test.js index 2f51a1208..b26750a1a 100644 --- a/tests/integration/ObsEditWithoutProvider.test.js +++ b/tests/integration/ObsEditOnline.test.js @@ -1,7 +1,7 @@ +import Geolocation from "@react-native-community/geolocation"; import { screen, waitFor } from "@testing-library/react-native"; import ObsEdit from "components/ObsEdit/ObsEdit"; import React from "react"; -import { LOCATION_FETCH_INTERVAL } from "sharedHooks/useCurrentObservationLocation"; import useStore from "stores/useStore"; import factory from "tests/factory"; import faker from "tests/helpers/faker"; @@ -11,30 +11,8 @@ const initialStoreState = useStore.getState( ); const mockLocationName = "San Francisco, CA"; -// import { checkMultiple, RESULTS } from "react-native-permissions"; -// jest.mock( "react-native-permissions", ( ) => { -// const actual = jest.requireActual( "react-native-permissions" ); -// return { -// ...actual, -// checkMultiple: permissions => permissions.reduce( -// ( memo, permission ) => { -// memo[permission] = actual.RESULTS.GRANTED; -// return memo; -// }, -// {} -// ) -// }; -// } ); -// jest.mock('react-native-permissions', () => require('react-native-permissions/mock')); - const mockCurrentUser = factory( "LocalUser" ); -const mockFetchUserLocation = jest.fn( () => ( { latitude: 37, longitude: 34 } ) ); -jest.mock( "sharedHelpers/fetchUserLocation", () => ( { - __esModule: true, - default: () => mockFetchUserLocation() -} ) ); - const renderObsEdit = ( ) => renderComponent( ); const mockTaxon = factory( "RemoteTaxon", { @@ -93,7 +71,7 @@ describe( "location fetching", () => { beforeEach( () => { // resets mock back to original state - mockFetchUserLocation.mockReset(); + Geolocation.watchPosition.mockReset(); } ); test( "should fetch location when new observation hasn't saved", async ( ) => { @@ -104,14 +82,12 @@ describe( "location fetching", () => { observations, currentObservation: observations[0] } ); - renderObsEdit( ); - expect( mockFetchUserLocation ).not.toHaveBeenCalled(); renderObsEdit( ); await waitFor( () => { - expect( mockFetchUserLocation ).toHaveBeenCalled(); - }, { timeout: LOCATION_FETCH_INTERVAL * 2 } ); + expect( Geolocation.watchPosition ).toHaveBeenCalled(); + } ); // Note: it would be nice to look for an update in the UI } ); @@ -137,9 +113,9 @@ describe( "location fetching", () => { // Location may not fetch immediately, so wait for twice the default fetch // interval before testing whether the mock was called - await waitFor( () => undefined, { timeout: LOCATION_FETCH_INTERVAL * 2 } ); + await waitFor( () => undefined ); - expect( mockFetchUserLocation ).not.toHaveBeenCalled(); + expect( Geolocation.watchPosition ).not.toHaveBeenCalled(); } ); test( "shouldn't fetch location for existing observation created elsewhere", async () => { @@ -163,8 +139,8 @@ describe( "location fetching", () => { screen.getByText( new RegExp( `Lat: ${observation.latitude}` ) ) ).toBeTruthy(); - await waitFor( () => undefined, { timeout: LOCATION_FETCH_INTERVAL * 2 } ); + await waitFor( () => undefined ); - expect( mockFetchUserLocation ).not.toHaveBeenCalled(); + expect( Geolocation.watchPosition ).not.toHaveBeenCalled(); } ); } ); diff --git a/tests/integration/PhotoDeletion.test.js b/tests/integration/PhotoDeletion.test.js index 2910c9493..77e7c7ccf 100644 --- a/tests/integration/PhotoDeletion.test.js +++ b/tests/integration/PhotoDeletion.test.js @@ -27,13 +27,14 @@ getPredictionsForImage.mockImplementation( async ( ) => ( mockModelResult ) ); -const mockGetCurrentPosition = jest.fn( ( success, _error, _options ) => success( { +const mockWatchPosition = jest.fn( ( success, _error, _options ) => success( { coords: { latitude: 56, - longitude: 9 + longitude: 9, + accuracy: 8 } } ) ); -Geolocation.getCurrentPosition.mockImplementation( mockGetCurrentPosition ); +Geolocation.watchPosition.mockImplementation( mockWatchPosition ); // UNIQUE REALM SETUP const mockRealmIdentifier = __filename; diff --git a/tests/integration/navigation/AICamera.test.js b/tests/integration/navigation/AICamera.test.js index c93b5b58b..769cb2f58 100644 --- a/tests/integration/navigation/AICamera.test.js +++ b/tests/integration/navigation/AICamera.test.js @@ -79,13 +79,14 @@ describe( "AICamera navigation with advanced user layout", ( ) => { describe( "to Suggestions", ( ) => { beforeEach( ( ) => { - const mockGetCurrentPosition = jest.fn( ( success, _error, _options ) => success( { + const mockWatchPosition = jest.fn( ( success, _error, _options ) => success( { coords: { latitude: 56, - longitude: 9 + longitude: 9, + accuracy: 8 } } ) ); - Geolocation.getCurrentPosition.mockImplementation( mockGetCurrentPosition ); + Geolocation.watchPosition.mockImplementation( mockWatchPosition ); } ); it( "should advance to suggestions screen", async ( ) => { diff --git a/tests/integration/navigation/StandardCamera.test.js b/tests/integration/navigation/StandardCamera.test.js index 1fc40be9f..7d93b8242 100644 --- a/tests/integration/navigation/StandardCamera.test.js +++ b/tests/integration/navigation/StandardCamera.test.js @@ -62,13 +62,14 @@ describe( "StandardCamera navigation with advanced user layout", ( ) => { } ); it( "should advance to Suggestions when photo taken and checkmark tapped", async ( ) => { - const mockGetCurrentPosition = jest.fn( ( success, _error, _options ) => success( { + const mockWatchPosition = jest.fn( ( success, _error, _options ) => success( { coords: { latitude: 56, - longitude: 9 + longitude: 9, + accuracy: 8 } } ) ); - Geolocation.getCurrentPosition.mockImplementation( mockGetCurrentPosition ); + Geolocation.watchPosition.mockImplementation( mockWatchPosition ); renderApp( ); expect( await screen.findByText( /Log in to contribute/ ) ).toBeVisible( ); const tabBar = await screen.findByTestId( "CustomTabBar" ); diff --git a/tests/jest.setup.js b/tests/jest.setup.js index 58c3a4134..20939d9e6 100644 --- a/tests/jest.setup.js +++ b/tests/jest.setup.js @@ -233,7 +233,12 @@ jest.mock( "react-native-permissions", () => require( "react-native-permissions/ // mocking globally since this currently affects a handful of unit and integration tests jest.mock( "@react-native-community/geolocation", ( ) => ( { - getCurrentPosition: jest.fn( ) + getCurrentPosition: jest.fn( ), + watchPosition: jest.fn( ), + clearWatch: jest.fn( ), + setRNConfiguration: jest.fn( ( ) => ( { + skipPermissionRequests: true + } ) ) } ) ); require( "react-native" ).NativeModules.RNCGeolocation = { }; diff --git a/tests/unit/components/ObsEdit/ObsEdit.test.js b/tests/unit/components/ObsEdit/ObsEdit.test.js index b70ca9c0d..72dfbe4ad 100644 --- a/tests/unit/components/ObsEdit/ObsEdit.test.js +++ b/tests/unit/components/ObsEdit/ObsEdit.test.js @@ -5,12 +5,11 @@ import useStore from "stores/useStore"; import factory from "tests/factory"; import { renderComponent } from "tests/helpers/render"; -jest.mock( "sharedHooks/useCurrentObservationLocation", () => ( { +jest.mock( "sharedHooks/useWatchPosition", () => ( { __esModule: true, default: ( ) => ( { hasLocation: true, - isFetchingLocation: false, - permissionResult: "granted" + isFetchingLocation: false } ) } ) ); diff --git a/tests/unit/components/Projects/Projects.test.js b/tests/unit/components/Projects/Projects.test.js index 5efca1bad..ea622f009 100644 --- a/tests/unit/components/Projects/Projects.test.js +++ b/tests/unit/components/Projects/Projects.test.js @@ -24,7 +24,8 @@ jest.mock( "@react-navigation/native", ( ) => { ...actualNav, useNavigation: () => ( { navigate: mockedNavigate, - setOptions: jest.fn( ) + setOptions: jest.fn( ), + addListener: jest.fn( ) } ), useRoute: () => ( {} ) }; diff --git a/tests/unit/sharedHooks/useUserLocation.test.js b/tests/unit/sharedHooks/useUserLocation.test.js deleted file mode 100644 index 0cd6ce27e..000000000 --- a/tests/unit/sharedHooks/useUserLocation.test.js +++ /dev/null @@ -1,75 +0,0 @@ -import Geolocation from "@react-native-community/geolocation"; -import { renderHook, waitFor } from "@testing-library/react-native"; -import useUserLocation from "sharedHooks/useUserLocation.ts"; - -const mockPositions = [ - { - coords: { - latitude: 1, - longitude: 1, - accuracy: 100 - } - }, - { - coords: { - latitude: 2, - longitude: 2, - accuracy: 20 - } - }, - { - coords: { - latitude: 3, - longitude: 3, - accuracy: 8 - } - } -]; - -describe( "useUserLocation", ( ) => { - beforeEach( ( ) => { - Geolocation.getCurrentPosition.mockReset( ); - // Mock so success gets called immediately and so that three subsequent - // calls succeed with changing coordinates and improving accuracy - Geolocation.getCurrentPosition - .mockImplementationOnce( success => success( mockPositions[0] ) ) - .mockImplementationOnce( success => success( mockPositions[1] ) ) - .mockImplementationOnce( success => success( mockPositions[2] ) ); - } ); - - // Geolocation.getCurrentPosition should have been called and that roughly - // marks the end of async effects, so hopefull this prevents "outside of - // act" warnings - afterEach( ( ) => waitFor( ( ) => { - expect( Geolocation.getCurrentPosition ).toHaveBeenCalled( ); - } ) ); - - it( "should be loading by default", async ( ) => { - const { result } = renderHook( ( ) => useUserLocation( ) ); - await waitFor( ( ) => { - expect( result.current.isLoading ).toBeTruthy( ); - } ); - } ); - - it( "should return a user location", async ( ) => { - const { result } = renderHook( ( ) => useUserLocation( ) ); - await waitFor( ( ) => { - expect( result.current.userLocation ).toBeDefined( ); - } ); - expect( result?.current?.userLocation?.longitude ) - .toEqual( mockPositions[0].coords.latitude ); - } ); - - describe( "untilAcc", ( ) => { - it( "should fetch coordinates until target accuracy reached", async ( ) => { - const { result } = renderHook( ( ) => useUserLocation( { - untilAcc: 10 - } ) ); - await waitFor( ( ) => { - expect( result.current.userLocation?.accuracy ) - .toEqual( mockPositions[2].coords.accuracy ); - }, { timeout: 4000, interval: 1000 } ); - expect( Geolocation.getCurrentPosition ).toHaveBeenCalledTimes( 3 ); - } ); - } ); -} ); diff --git a/tests/unit/sharedHooks/useWatchPosition.test.js b/tests/unit/sharedHooks/useWatchPosition.test.js new file mode 100644 index 000000000..e5a4c5785 --- /dev/null +++ b/tests/unit/sharedHooks/useWatchPosition.test.js @@ -0,0 +1,90 @@ +import Geolocation from "@react-native-community/geolocation"; +import { renderHook, waitFor } from "@testing-library/react-native"; +import { useWatchPosition } from "sharedHooks"; + +const mockPositions = [ + { + coords: { + latitude: 1, + longitude: 1, + accuracy: 100 + } + }, + { + coords: { + latitude: 2, + longitude: 2, + accuracy: 20 + } + }, + { + coords: { + latitude: 3, + longitude: 3, + accuracy: 8 + } + } +]; + +describe( "useWatchPosition with inaccurate location", ( ) => { + beforeEach( ( ) => { + Geolocation.watchPosition.mockReset( ); + // Mock so success gets called immediately and so that three subsequent + // calls succeed with changing coordinates and improving accuracy + Geolocation.watchPosition.mockImplementationOnce( success => success( mockPositions[0] ) ); + } ); + + // Geolocation.watchPosition should have been called and that roughly + // marks the end of async effects, so hopefull this prevents "outside of + // act" warnings + afterEach( ( ) => waitFor( ( ) => { + expect( Geolocation.watchPosition ).toHaveBeenCalled( ); + } ) ); + + it( "should be loading by default", async ( ) => { + const { result } = renderHook( ( ) => useWatchPosition( { shouldFetchLocation: true } ) ); + await waitFor( ( ) => { + expect( result.current.isFetchingLocation ).toBeTruthy( ); + } ); + } ); + + it( "should return a user location", async ( ) => { + const { result } = renderHook( ( ) => useWatchPosition( { shouldFetchLocation: true } ) ); + await waitFor( ( ) => { + expect( result.current.userLocation ).toBeDefined( ); + } ); + expect( result?.current?.userLocation?.latitude ) + .toEqual( mockPositions[0].coords.latitude ); + } ); +} ); + +describe( "useWatchPosition with accurate location", ( ) => { + beforeEach( ( ) => { + Geolocation.watchPosition.mockReset( ); + + Geolocation.watchPosition + .mockImplementationOnce( success => success( mockPositions[2] ) ); + } ); + + it( "should stop watching position when target accuracy reached", async ( ) => { + const { result } = renderHook( ( ) => useWatchPosition( { shouldFetchLocation: true } ) ); + await waitFor( ( ) => { + expect( result.current.userLocation?.positional_accuracy ) + .toEqual( mockPositions[2].coords.accuracy ); + } ); + expect( Geolocation.clearWatch ).toHaveBeenCalledTimes( 1 ); + } ); +} ); + +describe( "useWatchPosition when shouldn't fetch", ( ) => { + beforeEach( ( ) => { + Geolocation.watchPosition.mockReset( ); + } ); + + it( "should not watch position when shouldFetchLocation is false", async ( ) => { + renderHook( ( ) => useWatchPosition( { shouldFetchLocation: false } ) ); + await waitFor( ( ) => { + expect( Geolocation.watchPosition ).not.toHaveBeenCalled( ); + } ); + } ); +} );