From 899076e67afb47e522b52e29ebffbcbb28a8e425 Mon Sep 17 00:00:00 2001 From: Ken-ichi Date: Mon, 29 Jul 2024 11:51:14 -0700 Subject: [PATCH] Only ask for permission to add photos from cameras (#1869) * Patch camera roll to not ask for readwrite after addonly granted * Only request add permission from cameras --- ios/Podfile | 62 ++++++++++--------- ios/Podfile.lock | 4 +- ...native-camera-roll+camera-roll+7.5.2.patch | 31 ++++++++++ src/components/Camera/CameraWithDevice.tsx | 4 +- .../hooks/usePrepareStoreAndNavigate.js | 30 +++++++-- .../PermissionGateContainer.tsx | 28 +++++++-- src/stores/createObservationFlowSlice.js | 7 ++- .../hooks/userPrepareStoreAndNavigate.test.js | 4 +- 8 files changed, 123 insertions(+), 47 deletions(-) create mode 100644 patches/@react-native-camera-roll+camera-roll+7.5.2.patch diff --git a/ios/Podfile b/ios/Podfile index d2af0c616..4c8424f7a 100644 --- a/ios/Podfile +++ b/ios/Podfile @@ -1,43 +1,47 @@ # frozen_string_literal: true # setup instructions from https://www.npmjs.com/package/react-native-permissions -def node_require(script) +def node_require( script ) # Resolve script with node to allow for hoisting - require Pod::Executable.execute_command('node', ['-p', - "require.resolve( - '#{script}', - {paths: [process.argv[1]]}, - )", __dir__]).strip + require Pod::Executable.execute_command( + "node", + [ + "-p", "require.resolve('#{script}', {paths: [process.argv[1]]})", + __dir__ + ] + ).strip end -node_require('react-native/scripts/react_native_pods.rb') -node_require('react-native-permissions/scripts/setup.rb') +node_require( "react-native/scripts/react_native_pods.rb" ) +node_require( "react-native-permissions/scripts/setup.rb" ) platform :ios, min_ios_version_supported prepare_react_native_project! # ⬇️ uncomment wanted permissions -setup_permissions([ - # 'AppTrackingTransparency', - # 'BluetoothPeripheral', - # 'Calendars', - 'Camera', - # 'Contacts', - # 'FaceID', - 'LocationAccuracy', - 'LocationAlways', - 'LocationWhenInUse', - 'MediaLibrary', - 'Microphone', - # 'Motion', - # 'Notifications', - 'PhotoLibrary', - # 'PhotoLibraryAddOnly', - # 'Reminders', - # 'Siri', - # 'SpeechRecognition', - # 'StoreKit', -]) +setup_permissions( + [ + # 'AppTrackingTransparency', + # 'BluetoothPeripheral', + # 'Calendars', + "Camera", + # 'Contacts', + # 'FaceID', + "LocationAccuracy", + "LocationAlways", + "LocationWhenInUse", + "MediaLibrary", + "Microphone", + # 'Motion', + # 'Notifications', + "PhotoLibraryAddOnly", + "PhotoLibrary" + # 'Reminders', + # 'Siri', + # 'SpeechRecognition', + # 'StoreKit', + ] +) # If you are using a `react-native-flipper` your iOS build will fail when `NO_FLIPPER=1` is set. # because `react-native-flipper` depends on (FlipperKit,...) that will be excluded diff --git a/ios/Podfile.lock b/ios/Podfile.lock index 596c41d48..a34efbb1e 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -1562,7 +1562,7 @@ SPEC CHECKSUMS: RNFS: 4ac0f0ea233904cb798630b3c077808c06931688 RNGestureHandler: bc2cdb2dc42facdf34992ae364b8a728e19a3686 RNLocalize: e8694475db034bf601e17bd3dfa8986565e769eb - RNPermissions: a123c47480a5f5d7a04d40637ad1f7360a41b465 + RNPermissions: b3d6efca086546e29a2920cd649a0ab04ca77794 RNReanimated: 6cfa556540186ce7ae7a0b048f369236b1d86ebb RNScreens: b6b64d956af3715adbfe84808694ae82d3fec74f RNShareMenu: cb9dac548c8bf147d06f0bf07296ad51ea9f5fc3 @@ -1574,6 +1574,6 @@ SPEC CHECKSUMS: VisionCameraPluginInatVision: 8480b3955bc608e913135d3bebaa57939911fb82 Yoga: c716aea2ee01df6258550c7505fa61b248145ced -PODFILE CHECKSUM: ebb6b37cf92e00a96e3123d4db14c5658b4e5929 +PODFILE CHECKSUM: c4b9a5123afaeedac9558dd67c0e10f6cf9706b0 COCOAPODS: 1.15.2 diff --git a/patches/@react-native-camera-roll+camera-roll+7.5.2.patch b/patches/@react-native-camera-roll+camera-roll+7.5.2.patch new file mode 100644 index 000000000..46231728f --- /dev/null +++ b/patches/@react-native-camera-roll+camera-roll+7.5.2.patch @@ -0,0 +1,31 @@ +diff --git a/node_modules/@react-native-camera-roll/camera-roll/ios/RNCCameraRoll.mm b/node_modules/@react-native-camera-roll/camera-roll/ios/RNCCameraRoll.mm +index b8f2aa2..aa0df68 100644 +--- a/node_modules/@react-native-camera-roll/camera-roll/ios/RNCCameraRoll.mm ++++ b/node_modules/@react-native-camera-roll/camera-roll/ios/RNCCameraRoll.mm +@@ -207,6 +207,26 @@ RCT_EXPORT_METHOD(saveToCameraRoll:(NSURLRequest *)request + } + } completionHandler:^(BOOL success, NSError *error) { + if (success) { ++ // If the write succeeded but we don't have readwrite permission, that ++ // means we have addonly permission and we cannot read the file we ++ // just created to construct a response ++ if (@available(iOS 14, *)) { ++ PHAuthorizationStatus readWriteAuthStatus = [PHPhotoLibrary authorizationStatusForAccessLevel:PHAccessLevelReadWrite]; ++ if (readWriteAuthStatus != PHAuthorizationStatusAuthorized) { ++ NSDictionary *addOnlyResponse = @{ ++ @"node": @{ ++ @"id": placeholder.localIdentifier, ++ @"type": options[@"type"], ++ @"image": @{ ++ @"uri": @"placeholder/readWritePermissionNotGranted" ++ } ++ } ++ }; ++ resolve(addOnlyResponse); ++ return; ++ } ++ } ++ + PHFetchOptions *options = [PHFetchOptions new]; + options.includeHiddenAssets = YES; + options.includeAllBurstAssets = YES; diff --git a/src/components/Camera/CameraWithDevice.tsx b/src/components/Camera/CameraWithDevice.tsx index 638137900..46feebe04 100644 --- a/src/components/Camera/CameraWithDevice.tsx +++ b/src/components/Camera/CameraWithDevice.tsx @@ -1,6 +1,6 @@ import { useIsFocused, useNavigation } from "@react-navigation/native"; import PermissionGateContainer, { - READ_WRITE_MEDIA_PERMISSIONS + WRITE_MEDIA_PERMISSIONS } from "components/SharedComponents/PermissionGateContainer.tsx"; import { View } from "components/styledComponents"; import React, { @@ -166,7 +166,7 @@ const CameraWithDevice = ( { testID="CameraWithDevice" > { // logger.info( "saving rotated original camera photo: ", uri ); try { - const savedPhotoUri = await CameraRoll.save( uri, { - type: "photo", - album: "iNaturalist Next" - } ); + let savedPhotoUri; + // One quirk of CameraRoll is that if you want to write ton album, you + // need readwrite permission, but we don't want to ask for that here + // b/c it might come immediately after asking for *add only* + // permission, so we're checking to see if we have that permission + // and skipping the album if we don't + if ( readWritePermissionResult === RESULTS.GRANTED ) { + savedPhotoUri = await CameraRoll.save( uri, { + type: "photo", + album: "iNaturalist Next" + } ); + } else { + savedPhotoUri = await CameraRoll.save( uri ); + } + // logger.info( "saved to camera roll: ", savedPhotoUri ); // Save these camera roll URIs, so later on observation editor can update // the EXIF metadata of these photos, once we retrieve a location. @@ -105,7 +123,7 @@ const usePrepareStoreAndNavigate = ( options: Options ): Function => { local: true } ); setObservations( [newObservation] ); - if ( addPhotoPermissionResult !== "granted" ) return Promise.resolve( ); + if ( addPhotoPermissionResult !== RESULTS.GRANTED ) return Promise.resolve( ); return savePhotosToCameraGallery( cameraUris, addCameraRollUri ); }, [ addCameraRollUri, diff --git a/src/components/SharedComponents/PermissionGateContainer.tsx b/src/components/SharedComponents/PermissionGateContainer.tsx index c2d70e9e0..fc511c124 100644 --- a/src/components/SharedComponents/PermissionGateContainer.tsx +++ b/src/components/SharedComponents/PermissionGateContainer.tsx @@ -1,7 +1,13 @@ import { useNavigation } from "@react-navigation/native"; import Modal from "components/SharedComponents/Modal.tsx"; import _ from "lodash"; -import React, { useCallback, useEffect, useState } from "react"; +import React, { + JSX, + PropsWithChildren, + useCallback, + useEffect, + useState +} from "react"; import { Platform } from "react-native"; import { AndroidPermission, @@ -38,6 +44,15 @@ if ( usesAndroid10Permissions ) { ]; } +// TODO does this really work for Android above 10? +let androidWritePermissions: AndroidPermission[] = []; +if ( usesAndroid10Permissions ) { + androidWritePermissions = [ + ...androidWritePermissions, + PERMISSIONS.ANDROID.WRITE_EXTERNAL_STORAGE + ]; +} + const androidCameraPermissions = usesAndroid10Permissions ? [PERMISSIONS.ANDROID.CAMERA, PERMISSIONS.ANDROID.WRITE_EXTERNAL_STORAGE] : [PERMISSIONS.ANDROID.CAMERA]; @@ -54,16 +69,19 @@ export const READ_WRITE_MEDIA_PERMISSIONS = Platform.OS === "ios" ? [PERMISSIONS.IOS.PHOTO_LIBRARY] : androidReadWritePermissions; +export const WRITE_MEDIA_PERMISSIONS = Platform.OS === "ios" + ? [PERMISSIONS.IOS.PHOTO_LIBRARY_ADD_ONLY] + : androidWritePermissions; + export const LOCATION_PERMISSIONS = Platform.OS === "ios" ? [PERMISSIONS.IOS.LOCATION_WHEN_IN_USE] : [PERMISSIONS.ANDROID.ACCESS_FINE_LOCATION]; -type Props = { +interface Props extends PropsWithChildren { blockedPrompt?: string; body?: string; body2?: string; buttonText?: string; - children?: React.ReactNode, icon: string; image?: number; onModalHide?: () => void; @@ -76,7 +94,7 @@ type Props = { title?: string; titleDenied?: string; withoutNavigation?: boolean; -}; +} interface MultiResult { [permission: string]: PermissionStatus; @@ -125,7 +143,7 @@ const PermissionGateContainer = ( { title, titleDenied, withoutNavigation -}: Props ) => { +}: Props ): JSX.Element | null => { const [result, setResult] = useState( null ); const [modalShown, setModalShown] = useState( false ); diff --git a/src/stores/createObservationFlowSlice.js b/src/stores/createObservationFlowSlice.js index cffbbe56d..563f79de3 100644 --- a/src/stores/createObservationFlowSlice.js +++ b/src/stores/createObservationFlowSlice.js @@ -123,7 +123,12 @@ const createObservationFlowSlice = ( set, get ) => ( { resetObservationFlowSlice: ( ) => set( DEFAULT_STATE ), addCameraRollUri: uri => set( state => { const savedUris = state.cameraRollUris; - savedUris.push( uri ); + // A placeholder uri means we don't know the real URI, probably b/c we + // only had write permission so we were able to write the photo to the + // camera roll but not read anything about it. Keep in mind this is just + // a hack around a bug in CameraRoll. See + // patches/@react-native-camera-roll+camera-roll+7.5.2.patch + if ( uri && !uri.match( /placeholder/ ) ) savedUris.push( uri ); return ( { cameraRollUris: savedUris, savingPhoto: false diff --git a/tests/unit/components/Camera/hooks/userPrepareStoreAndNavigate.test.js b/tests/unit/components/Camera/hooks/userPrepareStoreAndNavigate.test.js index 516a18d45..a681b7772 100644 --- a/tests/unit/components/Camera/hooks/userPrepareStoreAndNavigate.test.js +++ b/tests/unit/components/Camera/hooks/userPrepareStoreAndNavigate.test.js @@ -4,14 +4,14 @@ import faker from "tests/helpers/faker"; describe( "userPrepareStoreAndNavigate", ( ) => { describe( "savePhotosToCameraGallery", ( ) => { - it( "should call CameraRoll.save three times when given three uris", ( ) => { + it( "should call CameraRoll.save three times when given three uris", async ( ) => { const uris = [ faker.system.filePath( ), faker.system.filePath( ), faker.system.filePath( ) ]; const mockOnEachSuccess = jest.fn( ); - savePhotosToCameraGallery( uris, mockOnEachSuccess ); + await savePhotosToCameraGallery( uris, mockOnEachSuccess ); // This should test that CameraRoll.save was called once for each of the // uris AND that it was called in the order of the uris array // https://jestjs.io/docs/mock-functions#custom-matchers