From 8db0d9105e004ea7b298b882c253c7fb75eee191 Mon Sep 17 00:00:00 2001 From: Irfan Omur Date: Mon, 27 Mar 2023 17:59:18 +0300 Subject: [PATCH 01/10] Remove WRITE_EXTERNAL_STORAGE from tests WRITE_EXTERNAL_STORAGE is not required for SDK higher than 29. --- .../java/org/wordpress/android/e2e/BlockEditorTests.kt | 6 ------ .../java/org/wordpress/android/e2e/ReaderTests.kt | 6 ------ 2 files changed, 12 deletions(-) diff --git a/WordPress/src/androidTest/java/org/wordpress/android/e2e/BlockEditorTests.kt b/WordPress/src/androidTest/java/org/wordpress/android/e2e/BlockEditorTests.kt index 06509ce796fd..5dabb0f51b89 100644 --- a/WordPress/src/androidTest/java/org/wordpress/android/e2e/BlockEditorTests.kt +++ b/WordPress/src/androidTest/java/org/wordpress/android/e2e/BlockEditorTests.kt @@ -1,11 +1,8 @@ package org.wordpress.android.e2e -import android.Manifest.permission -import androidx.test.rule.GrantPermissionRule import dagger.hilt.android.testing.HiltAndroidTest import org.junit.Before import org.junit.Ignore -import org.junit.Rule import org.junit.Test import org.wordpress.android.e2e.pages.BlockEditorPage import org.wordpress.android.e2e.pages.MySitesPage @@ -14,9 +11,6 @@ import java.time.Instant @HiltAndroidTest class BlockEditorTests : BaseTest() { - @JvmField @Rule - var mRuntimeImageAccessRule = GrantPermissionRule.grant(permission.WRITE_EXTERNAL_STORAGE) - @Before fun setUp() { logoutIfNecessary() diff --git a/WordPress/src/androidTest/java/org/wordpress/android/e2e/ReaderTests.kt b/WordPress/src/androidTest/java/org/wordpress/android/e2e/ReaderTests.kt index e353e82ae306..89d9b3f652ba 100644 --- a/WordPress/src/androidTest/java/org/wordpress/android/e2e/ReaderTests.kt +++ b/WordPress/src/androidTest/java/org/wordpress/android/e2e/ReaderTests.kt @@ -1,19 +1,13 @@ package org.wordpress.android.e2e -import android.Manifest.permission -import androidx.test.rule.GrantPermissionRule import dagger.hilt.android.testing.HiltAndroidTest import org.junit.Before -import org.junit.Rule import org.junit.Test import org.wordpress.android.e2e.pages.ReaderPage import org.wordpress.android.support.BaseTest @HiltAndroidTest class ReaderTests : BaseTest() { - @JvmField @Rule - var mRuntimeImageAccessRule = GrantPermissionRule.grant(permission.WRITE_EXTERNAL_STORAGE) - @Before fun setUp() { logoutIfNecessary() From ea209eb10563e2345c6cc81aac2aea2744072b6d Mon Sep 17 00:00:00 2001 From: Irfan Omur Date: Mon, 27 Mar 2023 19:17:41 +0300 Subject: [PATCH 02/10] Use media permissions on media picker For devices with API 33 and higher, we must request one or more of the granular media permissions instead of the READ_EXTERNAL_STORAGE permission. --- WordPress/src/debug/AndroidManifest.xml | 13 ++- WordPress/src/main/AndroidManifest.xml | 4 +- .../ui/mediapicker/MediaPickerFragment.kt | 84 ++++++++++----- .../ui/mediapicker/MediaPickerSetup.kt | 24 +++-- .../ui/mediapicker/MediaPickerViewModel.kt | 102 ++++++++++-------- .../ui/photopicker/MediaPickerLauncher.kt | 32 +++--- .../ui/photopicker/PermissionsHandler.kt | 48 ++++++++- .../ui/photopicker/PhotoPickerFragment.kt | 4 +- .../wordpress/android/ui/prefs/AppPrefs.java | 3 + .../android/util/WPPermissionUtils.java | 20 +++- .../main/res/layout/media_picker_fragment.xml | 2 +- WordPress/src/main/res/values/strings.xml | 9 +- .../mediapicker/MediaPickerViewModelTest.kt | 65 +++++++---- .../loader/MediaLoaderFactoryTest.kt | 12 ++- 14 files changed, 287 insertions(+), 135 deletions(-) diff --git a/WordPress/src/debug/AndroidManifest.xml b/WordPress/src/debug/AndroidManifest.xml index 27223a7554e1..525e3b2711e8 100644 --- a/WordPress/src/debug/AndroidManifest.xml +++ b/WordPress/src/debug/AndroidManifest.xml @@ -21,9 +21,16 @@ - - - + + + + + + - + diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerFragment.kt index ded23422bb0a..a3a5cd7c1264 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerFragment.kt @@ -5,6 +5,7 @@ import android.app.Activity import android.content.Intent.ACTION_GET_CONTENT import android.content.Intent.ACTION_OPEN_DOCUMENT import android.net.Uri +import android.os.Build import android.os.Bundle import android.os.Parcelable import android.view.LayoutInflater @@ -52,8 +53,6 @@ import org.wordpress.android.ui.mediapicker.MediaPickerViewModel.BrowseMenuUiMod import org.wordpress.android.ui.mediapicker.MediaPickerViewModel.BrowseMenuUiModel.BrowseAction.SYSTEM_PICKER import org.wordpress.android.ui.mediapicker.MediaPickerViewModel.BrowseMenuUiModel.BrowseAction.WP_MEDIA_LIBRARY import org.wordpress.android.ui.mediapicker.MediaPickerViewModel.FabUiModel -import org.wordpress.android.ui.mediapicker.MediaPickerViewModel.PermissionsRequested.CAMERA -import org.wordpress.android.ui.mediapicker.MediaPickerViewModel.PermissionsRequested.STORAGE import org.wordpress.android.ui.mediapicker.MediaPickerViewModel.PhotoListUiModel import org.wordpress.android.ui.mediapicker.MediaPickerViewModel.ProgressDialogUiModel import org.wordpress.android.ui.mediapicker.MediaPickerViewModel.ProgressDialogUiModel.Visible @@ -202,6 +201,7 @@ class MediaPickerFragment : Fragment(), MenuProvider { lateinit var uiHelpers: UiHelpers private lateinit var viewModel: MediaPickerViewModel private var binding: MediaPickerFragmentBinding? = null + private lateinit var mediaPickerSetup: MediaPickerSetup override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -226,7 +226,7 @@ class MediaPickerFragment : Fragment(), MenuProvider { super.onViewCreated(view, savedInstanceState) requireActivity().addMenuProvider(this, viewLifecycleOwner) - val mediaPickerSetup = MediaPickerSetup.fromBundle(requireArguments()) + mediaPickerSetup = MediaPickerSetup.fromBundle(requireArguments()) val site = requireArguments().getSerializableCompat(WordPress.SITE) var selectedIds: List? = null var lastTappedIcon: MediaPickerIcon? = null @@ -276,12 +276,7 @@ class MediaPickerFragment : Fragment(), MenuProvider { navigateEvent(navigationEvent) } - viewModel.onPermissionsRequested.observeEvent(viewLifecycleOwner) { - when (it) { - CAMERA -> requestCameraPermission() - STORAGE -> requestStoragePermission() - } - } + viewModel.onCameraPermissionsRequested.observeEvent(viewLifecycleOwner) { requestCameraPermission() } viewModel.onSnackbarMessage.observeEvent(viewLifecycleOwner) { messageHolder -> showSnackbar(messageHolder) } @@ -456,7 +451,7 @@ class MediaPickerFragment : Fragment(), MenuProvider { if (uiModel.isAlwaysDenied) { WPPermissionUtils.showAppSettings(requireActivity()) } else { - requestStoragePermission() + requestMediaPermission() } } @@ -621,44 +616,75 @@ class MediaPickerFragment : Fragment(), MenuProvider { override fun onResume() { super.onResume() - checkStoragePermission() + checkMediaPermission() } fun setMediaPickerListener(listener: MediaPickerListener?) { this.listener = listener } - private val isStoragePermissionAlwaysDenied: Boolean - get() = WPPermissionUtils.isPermissionAlwaysDenied( - requireActivity(), permission.WRITE_EXTERNAL_STORAGE - ) - /* * load the photos if we have the necessary permission, otherwise show the "soft ask" view * which asks the user to allow the permission */ - private fun checkStoragePermission() { + private fun checkMediaPermission() { if (!isAdded) { return } - viewModel.checkStoragePermission(isStoragePermissionAlwaysDenied) + + // Storage permission is available only for API lower than 33 + val isStoragePermissionAlwaysDenied = WPPermissionUtils.isPermissionAlwaysDenied( + requireActivity(), + permission.WRITE_EXTERNAL_STORAGE + ) + + val isPhotosVideosPermissionAlwaysDenied = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + WPPermissionUtils.isPermissionAlwaysDenied(requireActivity(), permission.READ_MEDIA_IMAGES) || + WPPermissionUtils.isPermissionAlwaysDenied(requireActivity(), permission.READ_MEDIA_VIDEO) + } else { + // For devices lower than API 33, storage permission is the equivalent of Photos and Videos permission + isStoragePermissionAlwaysDenied + } + val isMusicAudioPermissionAlwaysDenied = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + WPPermissionUtils.isPermissionAlwaysDenied( + requireActivity(), + permission.READ_MEDIA_AUDIO + ) + } else { + // For devices lower than API 33, storage permission is the equivalent of Music and Audio permission + isStoragePermissionAlwaysDenied + } + viewModel.checkMediaPermissions(isPhotosVideosPermissionAlwaysDenied, isMusicAudioPermissionAlwaysDenied) } @Suppress("DEPRECATION") - private fun requestStoragePermission() { - val permissions = arrayOf(permission.WRITE_EXTERNAL_STORAGE, permission.READ_EXTERNAL_STORAGE) - requestPermissions( - permissions, WPPermissionUtils.PHOTO_PICKER_STORAGE_PERMISSION_REQUEST_CODE - ) + private fun requestMediaPermission() { + val permissions = arrayListOf() + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + if (mediaPickerSetup.requiresPhotosVideosPermissions) { + permissions.add(permission.READ_MEDIA_IMAGES) + permissions.add(permission.READ_MEDIA_VIDEO) + } + if (mediaPickerSetup.requiresMusicAudioPermissions) { + permissions.add(permission.READ_MEDIA_AUDIO) + } + } else { + // READ_EXTERNAL_STORAGE is the equivalent of READ_MEDIA_IMAGES, READ_MEDIA_VIDEO and READ_MEDIA_AUDIO on + // devices lower than API 33. + permissions.add(permission.READ_EXTERNAL_STORAGE) + } + requestPermissions(permissions.toTypedArray(), WPPermissionUtils.PHOTO_PICKER_MEDIA_PERMISSION_REQUEST_CODE) } @Suppress("DEPRECATION") private fun requestCameraPermission() { - // in addition to CAMERA permission we also need a storage permission, to store media from the camera - val permissions = arrayOf( - permission.CAMERA, - permission.WRITE_EXTERNAL_STORAGE - ) + // For devices lower than API 30, in addition to CAMERA permission we also need a storage permission, to store + // media from the camera + val permissions = if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) { + arrayOf(permission.CAMERA, permission.WRITE_EXTERNAL_STORAGE) + } else { + arrayOf(permission.CAMERA) + } requestPermissions(permissions, WPPermissionUtils.PHOTO_PICKER_CAMERA_PERMISSION_REQUEST_CODE) } @@ -673,7 +699,7 @@ class MediaPickerFragment : Fragment(), MenuProvider { requireActivity(), requestCode, permissions, grantResults, checkForAlwaysDenied ) when (requestCode) { - WPPermissionUtils.PHOTO_PICKER_STORAGE_PERMISSION_REQUEST_CODE -> checkStoragePermission() + WPPermissionUtils.PHOTO_PICKER_MEDIA_PERMISSION_REQUEST_CODE -> checkMediaPermission() WPPermissionUtils.PHOTO_PICKER_CAMERA_PERMISSION_REQUEST_CODE -> if (allGranted) { viewModel.clickOnLastTappedIcon() } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerSetup.kt b/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerSetup.kt index 9a210d777428..5cb2ebf4c0ca 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerSetup.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerSetup.kt @@ -9,7 +9,8 @@ data class MediaPickerSetup( val primaryDataSource: DataSource, val availableDataSources: Set, val canMultiselect: Boolean, - val requiresStoragePermissions: Boolean, + val requiresPhotosVideosPermissions: Boolean, + val requiresMusicAudioPermissions: Boolean, val allowedTypes: Set, val cameraSetup: CameraSetup, val systemPickerEnabled: Boolean, @@ -31,7 +32,8 @@ data class MediaPickerSetup( bundle.putIntegerArrayList(KEY_AVAILABLE_DATA_SOURCES, ArrayList(availableDataSources.map { it.ordinal })) bundle.putIntegerArrayList(KEY_ALLOWED_TYPES, ArrayList(allowedTypes.map { it.ordinal })) bundle.putBoolean(KEY_CAN_MULTISELECT, canMultiselect) - bundle.putBoolean(KEY_REQUIRES_STORAGE_PERMISSIONS, requiresStoragePermissions) + bundle.putBoolean(KEY_REQUIRES_PHOTOS_VIDEOS_PERMISSIONS, requiresPhotosVideosPermissions) + bundle.putBoolean(KEY_REQUIRES_MUSIC_AUDIO_PERMISSIONS, requiresMusicAudioPermissions) bundle.putInt(KEY_CAMERA_SETUP, cameraSetup.ordinal) bundle.putBoolean(KEY_SYSTEM_PICKER_ENABLED, systemPickerEnabled) bundle.putBoolean(KEY_EDITING_ENABLED, editingEnabled) @@ -45,7 +47,8 @@ data class MediaPickerSetup( intent.putIntegerArrayListExtra(KEY_AVAILABLE_DATA_SOURCES, ArrayList(availableDataSources.map { it.ordinal })) intent.putIntegerArrayListExtra(KEY_ALLOWED_TYPES, ArrayList(allowedTypes.map { it.ordinal })) intent.putExtra(KEY_CAN_MULTISELECT, canMultiselect) - intent.putExtra(KEY_REQUIRES_STORAGE_PERMISSIONS, requiresStoragePermissions) + intent.putExtra(KEY_REQUIRES_PHOTOS_VIDEOS_PERMISSIONS, requiresPhotosVideosPermissions) + intent.putExtra(KEY_REQUIRES_MUSIC_AUDIO_PERMISSIONS, requiresMusicAudioPermissions) intent.putExtra(KEY_CAMERA_SETUP, cameraSetup.ordinal) intent.putExtra(KEY_SYSTEM_PICKER_ENABLED, systemPickerEnabled) intent.putExtra(KEY_EDITING_ENABLED, editingEnabled) @@ -58,7 +61,8 @@ data class MediaPickerSetup( private const val KEY_PRIMARY_DATA_SOURCE = "key_primary_data_source" private const val KEY_AVAILABLE_DATA_SOURCES = "key_available_data_sources" private const val KEY_CAN_MULTISELECT = "key_can_multiselect" - private const val KEY_REQUIRES_STORAGE_PERMISSIONS = "key_requires_storage_permissions" + private const val KEY_REQUIRES_PHOTOS_VIDEOS_PERMISSIONS = "key_requires_photos_videos_permissions" + private const val KEY_REQUIRES_MUSIC_AUDIO_PERMISSIONS = "key_requires_music_audio_permissions" private const val KEY_ALLOWED_TYPES = "key_allowed_types" private const val KEY_CAMERA_SETUP = "key_camera_setup" private const val KEY_SYSTEM_PICKER_ENABLED = "key_system_picker_enabled" @@ -77,7 +81,8 @@ data class MediaPickerSetup( }.toSet() val multipleSelectionAllowed = bundle.getBoolean(KEY_CAN_MULTISELECT) val cameraSetup = CameraSetup.values()[bundle.getInt(KEY_CAMERA_SETUP)] - val requiresStoragePermissions = bundle.getBoolean(KEY_REQUIRES_STORAGE_PERMISSIONS) + val requiresPhotosVideosPermissions = bundle.getBoolean(KEY_REQUIRES_PHOTOS_VIDEOS_PERMISSIONS) + val requiresMusicAudioPermissions = bundle.getBoolean(KEY_REQUIRES_MUSIC_AUDIO_PERMISSIONS) val systemPickerEnabled = bundle.getBoolean(KEY_SYSTEM_PICKER_ENABLED) val editingEnabled = bundle.getBoolean(KEY_EDITING_ENABLED) val queueResults = bundle.getBoolean(KEY_QUEUE_RESULTS) @@ -87,7 +92,8 @@ data class MediaPickerSetup( dataSource, availableDataSources, multipleSelectionAllowed, - requiresStoragePermissions, + requiresPhotosVideosPermissions, + requiresMusicAudioPermissions, allowedTypes, cameraSetup, systemPickerEnabled, @@ -109,7 +115,8 @@ data class MediaPickerSetup( }.toSet() val multipleSelectionAllowed = intent.getBooleanExtra(KEY_CAN_MULTISELECT, false) val cameraSetup = CameraSetup.values()[intent.getIntExtra(KEY_CAMERA_SETUP, -1)] - val requiresStoragePermissions = intent.getBooleanExtra(KEY_REQUIRES_STORAGE_PERMISSIONS, false) + val requiresPhotosVideosPermissions = intent.getBooleanExtra(KEY_REQUIRES_PHOTOS_VIDEOS_PERMISSIONS, false) + val requiresMusicAudioPermissions = intent.getBooleanExtra(KEY_REQUIRES_MUSIC_AUDIO_PERMISSIONS, false) val systemPickerEnabled = intent.getBooleanExtra(KEY_SYSTEM_PICKER_ENABLED, false) val editingEnabled = intent.getBooleanExtra(KEY_SYSTEM_PICKER_ENABLED, false) val queueResults = intent.getBooleanExtra(KEY_QUEUE_RESULTS, false) @@ -119,7 +126,8 @@ data class MediaPickerSetup( dataSource, availableDataSources, multipleSelectionAllowed, - requiresStoragePermissions, + requiresPhotosVideosPermissions, + requiresMusicAudioPermissions, allowedTypes, cameraSetup, systemPickerEnabled, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt index 243f084145ce..2cd105c10e3c 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt @@ -1,6 +1,6 @@ package org.wordpress.android.ui.mediapicker -import android.Manifest.permission +import android.os.Build import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import kotlinx.coroutines.CoroutineDispatcher @@ -64,7 +64,6 @@ import org.wordpress.android.ui.utils.UiString.UiStringText import org.wordpress.android.util.LocaleManagerWrapper import org.wordpress.android.util.MediaUtilsWrapper import org.wordpress.android.util.UriWrapper -import org.wordpress.android.util.WPPermissionUtils import org.wordpress.android.util.distinct import org.wordpress.android.util.merge import org.wordpress.android.viewmodel.Event @@ -91,7 +90,7 @@ class MediaPickerViewModel @Inject constructor( private var searchJob: Job? = null private val _domainModel = MutableLiveData() private val _selectedIds = MutableLiveData?>() - private val _onPermissionsRequested = MutableLiveData>() + private val _onCameraPermissionsRequested = MutableLiveData>() private val _softAskRequest = MutableLiveData() private val _searchExpanded = MutableLiveData() private val _showProgressDialog = MutableLiveData() @@ -101,7 +100,7 @@ class MediaPickerViewModel @Inject constructor( val onSnackbarMessage: LiveData> = _onSnackbarMessage val onNavigate = _onNavigate as LiveData> - val onPermissionsRequested: LiveData> = _onPermissionsRequested + val onCameraPermissionsRequested: LiveData> = _onCameraPermissionsRequested val uiState: LiveData = merge( _domainModel.distinct(), @@ -297,14 +296,11 @@ class MediaPickerViewModel @Inject constructor( UiStringText(String.format(resourceProvider.getString(R.string.cab_selected), numSelected)) } else -> { - val isImagePicker = mediaPickerSetup.allowedTypes.contains(IMAGE) - val isVideoPicker = mediaPickerSetup.allowedTypes.contains(VIDEO) - val isAudioPicker = mediaPickerSetup.allowedTypes.contains(AUDIO) - if (isImagePicker && isVideoPicker) { + if (isImagePicker() && isVideoPicker()) { UiStringRes(R.string.photo_picker_use_media) - } else if (isVideoPicker) { + } else if (isVideoPicker()) { UiStringRes(R.string.photo_picker_use_video) - } else if (isAudioPicker) { + } else if (isAudioPicker()) { UiStringRes(R.string.photo_picker_use_audio) } else { UiStringRes(R.string.photo_picker_use_photo) @@ -328,12 +324,17 @@ class MediaPickerViewModel @Inject constructor( ) } + private fun hasPermission() = when { + mediaPickerSetup.requiresPhotosVideosPermissions -> permissionsHandler.hasPhotosVideosPermission() + mediaPickerSetup.requiresMusicAudioPermissions -> permissionsHandler.hasMusicAudioPermission() + else -> false + } + fun refreshData(forceReload: Boolean) { - if (!permissionsHandler.hasStoragePermission()) { - return - } - launch(bgDispatcher) { - loadActions.send(LoadAction.Refresh(forceReload)) + if (hasPermission()) { + launch(bgDispatcher) { + loadActions.send(LoadAction.Refresh(forceReload)) + } } } @@ -369,7 +370,9 @@ class MediaPickerViewModel @Inject constructor( _domainModel.value = domainModel } } - if (!mediaPickerSetup.requiresStoragePermissions || permissionsHandler.hasStoragePermission()) { + if ((!mediaPickerSetup.requiresPhotosVideosPermissions && !mediaPickerSetup.requiresMusicAudioPermissions) + || hasPermission() + ) { launch(bgDispatcher) { loadActions.send(LoadAction.Start()) } @@ -499,7 +502,7 @@ class MediaPickerViewModel @Inject constructor( mediaPickerTracker.trackIconClick(icon, mediaPickerSetup) if (icon is WpStoriesCapture || icon is CapturePhoto) { if (!permissionsHandler.hasPermissionsToAccessPhotos()) { - _onPermissionsRequested.value = Event(PermissionsRequested.CAMERA) + _onCameraPermissionsRequested.value = Event(Unit) lastTappedIcon = icon return } @@ -566,11 +569,17 @@ class MediaPickerViewModel @Inject constructor( return IconClickEvent(action) } - fun checkStoragePermission(isAlwaysDenied: Boolean) { - if (!mediaPickerSetup.requiresStoragePermissions) { + fun checkMediaPermissions(isPhotosVideosAlwaysDenied: Boolean, isMusicAudioAlwaysDenied: Boolean) { + if (!mediaPickerSetup.requiresPhotosVideosPermissions && !mediaPickerSetup.requiresMusicAudioPermissions) { + // No permission is required, so there is no need to check permissions. return } - if (permissionsHandler.hasStoragePermission()) { + val isAlwaysDenied = if (mediaPickerSetup.requiresPhotosVideosPermissions) { + isPhotosVideosAlwaysDenied + } else { + isMusicAudioAlwaysDenied + } + if (hasPermission()) { _softAskRequest.value = SoftAskRequest(show = false, isAlwaysDenied = isAlwaysDenied) if (_domainModel.value?.domainItems.isNullOrEmpty()) { refreshData(false) @@ -591,34 +600,45 @@ class MediaPickerViewModel @Inject constructor( clickIcon(icon) } + private fun getRequiredPermissionName(): String { + val permissionName = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + if (mediaPickerSetup.requiresPhotosVideosPermissions) { + R.string.permission_images + } else if (mediaPickerSetup.requiresMusicAudioPermissions) { + R.string.permission_audio + } else { + R.string.unknown + } + } else { + R.string.permission_storage + } + return resourceProvider.getString(permissionName) + } + + private fun isImagePicker() = mediaPickerSetup.allowedTypes.contains(IMAGE) + private fun isVideoPicker() = mediaPickerSetup.allowedTypes.contains(VIDEO) + private fun isAudioPicker() = mediaPickerSetup.allowedTypes.contains(AUDIO) + private fun buildSoftAskView(softAskRequest: SoftAskRequest?): SoftAskViewUiModel { if (softAskRequest != null && softAskRequest.show) { mediaPickerTracker.trackShowPermissionsScreen(mediaPickerSetup, softAskRequest.isAlwaysDenied) val appName = "${resourceProvider.getString(R.string.app_name)}" val label = if (softAskRequest.isAlwaysDenied) { - val writePermission = ("${ - WPPermissionUtils.getPermissionName( - resourceProvider, - permission.WRITE_EXTERNAL_STORAGE - ) - }") - val readPermission = ("${ - WPPermissionUtils.getPermissionName( - resourceProvider, - permission.READ_EXTERNAL_STORAGE - ) - }") + val permission = ("${getRequiredPermissionName()}") String.format( - resourceProvider.getString(R.string.media_picker_soft_ask_permissions_denied), + resourceProvider.getString(R.string.media_picker_soft_ask_media_permissions_denied), appName, - writePermission, - readPermission + permission ) } else { - String.format( - resourceProvider.getString(R.string.photo_picker_soft_ask_label), - appName - ) + val description = when { + isImagePicker() && isVideoPicker() -> R.string.photo_picker_soft_ask_photos_videos_label + isImagePicker() -> R.string.photo_picker_soft_ask_photos_label + isVideoPicker() -> R.string.photo_picker_soft_ask_videos_label + isAudioPicker() -> R.string.photo_picker_soft_ask_audios_label + else -> R.string.unknown + } + String.format(resourceProvider.getString(description), appName) } val allowId = if (softAskRequest.isAlwaysDenied) { R.string.button_edit_permissions @@ -733,10 +753,6 @@ class MediaPickerViewModel @Inject constructor( } } - enum class PermissionsRequested { - CAMERA, STORAGE - } - data class SoftAskRequest(val show: Boolean, val isAlwaysDenied: Boolean) data class EditActionUiModel( diff --git a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/MediaPickerLauncher.kt b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/MediaPickerLauncher.kt index af6ea34f7f5c..2ad7d92c8e7d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/MediaPickerLauncher.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/MediaPickerLauncher.kt @@ -47,7 +47,8 @@ class MediaPickerLauncher @Inject constructor( primaryDataSource = DEVICE, availableDataSources = availableDataSources, canMultiselect = false, - requiresStoragePermissions = true, + requiresPhotosVideosPermissions = true, + requiresMusicAudioPermissions = false, allowedTypes = setOf(IMAGE), cameraSetup = ENABLED, systemPickerEnabled = true, @@ -65,14 +66,6 @@ class MediaPickerLauncher @Inject constructor( activity.startActivityForResult(intent, RequestCodes.PHOTO_PICKER) } - fun showSiteIconPicker( - activity: Activity, - site: SiteModel? - ) { - val intent = buildSitePickerIntent(activity, site) - activity.startActivityForResult(intent, RequestCodes.PHOTO_PICKER) - } - @Suppress("DEPRECATION") fun showSiteIconPicker( fragment: Fragment, @@ -90,7 +83,8 @@ class MediaPickerLauncher @Inject constructor( primaryDataSource = DEVICE, availableDataSources = setOf(WP_LIBRARY), canMultiselect = false, - requiresStoragePermissions = true, + requiresPhotosVideosPermissions = true, + requiresMusicAudioPermissions = false, allowedTypes = setOf(IMAGE), cameraSetup = ENABLED, systemPickerEnabled = true, @@ -142,7 +136,8 @@ class MediaPickerLauncher @Inject constructor( primaryDataSource = DEVICE, availableDataSources = setOf(), canMultiselect = false, - requiresStoragePermissions = true, + requiresPhotosVideosPermissions = true, + requiresMusicAudioPermissions = false, allowedTypes = setOf(IMAGE), cameraSetup = ENABLED, systemPickerEnabled = true, @@ -192,7 +187,8 @@ class MediaPickerLauncher @Inject constructor( primaryDataSource = DEVICE, availableDataSources = setOf(), canMultiselect = canMultiselect, - requiresStoragePermissions = true, + requiresPhotosVideosPermissions = false, + requiresMusicAudioPermissions = allowedTypes.contains(AUDIO), allowedTypes = allowedTypes, cameraSetup = HIDDEN, systemPickerEnabled = true, @@ -236,7 +232,8 @@ class MediaPickerLauncher @Inject constructor( primaryDataSource = STOCK_LIBRARY, availableDataSources = setOf(), canMultiselect = allowMultipleSelection, - requiresStoragePermissions = false, + requiresPhotosVideosPermissions = false, + requiresMusicAudioPermissions = false, allowedTypes = setOf(IMAGE), cameraSetup = HIDDEN, systemPickerEnabled = false, @@ -267,7 +264,8 @@ class MediaPickerLauncher @Inject constructor( primaryDataSource = GIF_LIBRARY, availableDataSources = setOf(), canMultiselect = allowMultipleSelection, - requiresStoragePermissions = false, + requiresPhotosVideosPermissions = false, + requiresMusicAudioPermissions = false, allowedTypes = setOf(IMAGE), cameraSetup = HIDDEN, systemPickerEnabled = false, @@ -303,7 +301,8 @@ class MediaPickerLauncher @Inject constructor( primaryDataSource = DEVICE, availableDataSources = if (browserType.isWPStoriesPicker) setOf(WP_LIBRARY) else setOf(), canMultiselect = browserType.canMultiselect(), - requiresStoragePermissions = true, + requiresPhotosVideosPermissions = browserType.isImagePicker || browserType.isVideoPicker, + requiresMusicAudioPermissions = browserType.isAudioPicker, allowedTypes = allowedTypes, cameraSetup = if (browserType.isWPStoriesPicker) STORIES else HIDDEN, systemPickerEnabled = true, @@ -335,7 +334,8 @@ class MediaPickerLauncher @Inject constructor( primaryDataSource = WP_LIBRARY, availableDataSources = setOf(), canMultiselect = browserType.canMultiselect(), - requiresStoragePermissions = false, + requiresPhotosVideosPermissions = false, + requiresMusicAudioPermissions = false, allowedTypes = allowedTypes, cameraSetup = if (browserType.isWPStoriesPicker) STORIES else HIDDEN, systemPickerEnabled = false, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PermissionsHandler.kt b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PermissionsHandler.kt index e1e7874cfc33..cf725de0e5fa 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PermissionsHandler.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PermissionsHandler.kt @@ -3,22 +3,62 @@ package org.wordpress.android.ui.photopicker import android.Manifest.permission import android.content.Context import android.content.pm.PackageManager +import android.os.Build +import androidx.annotation.RequiresApi import androidx.core.content.ContextCompat import javax.inject.Inject class PermissionsHandler @Inject constructor(private val context: Context) { fun hasPermissionsToAccessPhotos(): Boolean { - return hasCameraPermission() && hasStoragePermission() + return hasCameraPermission() && hasPhotosVideosPermission() } - fun hasStoragePermission(): Boolean { - return hasReadStoragePermission() && hasWriteStoragePermission() + fun hasPhotosVideosPermission(): Boolean { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + hasReadMediaImagesPermission() && hasReadMediaVideoPermission() + } else { + // For devices lower than API 33, storage permission is the equivalent of Photos and Videos permission + hasReadStoragePermission() + } + } + + fun hasMusicAudioPermission(): Boolean { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + hasReadMediaAudioPermission() + } else { + // For devices lower than API 33, storage permission is the equivalent of Music and Audio permission + hasReadStoragePermission() + } } fun hasWriteStoragePermission(): Boolean { + // WRITE_EXTERNAL_STORAGE is not required for SDK higher than 29. + return Build.VERSION.SDK_INT >= Build.VERSION_CODES.R || + ContextCompat.checkSelfPermission( + context, + permission.WRITE_EXTERNAL_STORAGE + ) == PackageManager.PERMISSION_GRANTED + } + + @RequiresApi(Build.VERSION_CODES.TIRAMISU) + private fun hasReadMediaImagesPermission(): Boolean { + return ContextCompat.checkSelfPermission( + context, permission.READ_MEDIA_IMAGES + ) == PackageManager.PERMISSION_GRANTED + } + + @RequiresApi(Build.VERSION_CODES.TIRAMISU) + private fun hasReadMediaVideoPermission(): Boolean { + return ContextCompat.checkSelfPermission( + context, permission.READ_MEDIA_VIDEO + ) == PackageManager.PERMISSION_GRANTED + } + + @RequiresApi(Build.VERSION_CODES.TIRAMISU) + private fun hasReadMediaAudioPermission(): Boolean { return ContextCompat.checkSelfPermission( - context, permission.WRITE_EXTERNAL_STORAGE + context, permission.READ_MEDIA_AUDIO ) == PackageManager.PERMISSION_GRANTED } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerFragment.kt index a49ea878679d..1eba8f88f266 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerFragment.kt @@ -440,7 +440,7 @@ class PhotoPickerFragment : Fragment(R.layout.photo_picker_fragment) { private fun requestStoragePermission() { val permissions = arrayOf(permission.WRITE_EXTERNAL_STORAGE) requestPermissions( - permissions, WPPermissionUtils.PHOTO_PICKER_STORAGE_PERMISSION_REQUEST_CODE + permissions, WPPermissionUtils.PHOTO_PICKER_MEDIA_PERMISSION_REQUEST_CODE ) } @@ -465,7 +465,7 @@ class PhotoPickerFragment : Fragment(R.layout.photo_picker_fragment) { requireActivity(), requestCode, permissions, grantResults, checkForAlwaysDenied ) when (requestCode) { - WPPermissionUtils.PHOTO_PICKER_STORAGE_PERMISSION_REQUEST_CODE -> checkStoragePermission() + WPPermissionUtils.PHOTO_PICKER_MEDIA_PERMISSION_REQUEST_CODE -> checkStoragePermission() WPPermissionUtils.PHOTO_PICKER_CAMERA_PERMISSION_REQUEST_CODE -> if (allGranted) { viewModel.clickOnLastTappedIcon() } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefs.java b/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefs.java index cf707b5afd0d..8f41a47912fe 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefs.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefs.java @@ -228,6 +228,9 @@ public enum UndeletablePrefKey implements PrefKey { // permission keys - set once a specific permission has been asked, regardless of response ASKED_PERMISSION_STORAGE_WRITE, ASKED_PERMISSION_STORAGE_READ, + ASKED_PERMISSION_IMAGES_READ, + ASKED_PERMISSION_VIDEO_READ, + ASKED_PERMISSION_AUDIO_READ, ASKED_PERMISSION_CAMERA, // Updated after WP.com themes have been fetched diff --git a/WordPress/src/main/java/org/wordpress/android/util/WPPermissionUtils.java b/WordPress/src/main/java/org/wordpress/android/util/WPPermissionUtils.java index 99544ddb1f6c..9bdbfd613a71 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/WPPermissionUtils.java +++ b/WordPress/src/main/java/org/wordpress/android/util/WPPermissionUtils.java @@ -30,7 +30,7 @@ public class WPPermissionUtils { public static final int SHARE_MEDIA_PERMISSION_REQUEST_CODE = 10; public static final int MEDIA_BROWSER_PERMISSION_REQUEST_CODE = 20; public static final int MEDIA_PREVIEW_PERMISSION_REQUEST_CODE = 30; - public static final int PHOTO_PICKER_STORAGE_PERMISSION_REQUEST_CODE = 40; + public static final int PHOTO_PICKER_MEDIA_PERMISSION_REQUEST_CODE = 40; public static final int PHOTO_PICKER_CAMERA_PERMISSION_REQUEST_CODE = 41; public static final int EDITOR_LOCATION_PERMISSION_REQUEST_CODE = 50; public static final int EDITOR_MEDIA_PERMISSION_REQUEST_CODE = 60; @@ -144,6 +144,12 @@ private static AppPrefs.PrefKey getPermissionAskedKey(@NonNull String permission return AppPrefs.UndeletablePrefKey.ASKED_PERMISSION_STORAGE_WRITE; case android.Manifest.permission.READ_EXTERNAL_STORAGE: return AppPrefs.UndeletablePrefKey.ASKED_PERMISSION_STORAGE_READ; + case android.Manifest.permission.READ_MEDIA_IMAGES: + return AppPrefs.UndeletablePrefKey.ASKED_PERMISSION_IMAGES_READ; + case android.Manifest.permission.READ_MEDIA_VIDEO: + return AppPrefs.UndeletablePrefKey.ASKED_PERMISSION_VIDEO_READ; + case android.Manifest.permission.READ_MEDIA_AUDIO: + return AppPrefs.UndeletablePrefKey.ASKED_PERMISSION_AUDIO_READ; case android.Manifest.permission.CAMERA: return AppPrefs.UndeletablePrefKey.ASKED_PERMISSION_CAMERA; default: @@ -160,6 +166,12 @@ public static String getPermissionName(@NonNull Context context, @NonNull String case android.Manifest.permission.WRITE_EXTERNAL_STORAGE: case android.Manifest.permission.READ_EXTERNAL_STORAGE: return context.getString(R.string.permission_storage); + case android.Manifest.permission.READ_MEDIA_IMAGES: + return context.getString(R.string.permission_images); + case android.Manifest.permission.READ_MEDIA_VIDEO: + return context.getString(R.string.permission_video); + case android.Manifest.permission.READ_MEDIA_AUDIO: + return context.getString(R.string.permission_audio); case android.Manifest.permission.CAMERA: return context.getString(R.string.permission_camera); case Manifest.permission.RECORD_AUDIO: @@ -178,6 +190,12 @@ public static String getPermissionName(@NonNull ResourceProvider resourceProvide case android.Manifest.permission.WRITE_EXTERNAL_STORAGE: case android.Manifest.permission.READ_EXTERNAL_STORAGE: return resourceProvider.getString(R.string.permission_storage); + case android.Manifest.permission.READ_MEDIA_IMAGES: + return resourceProvider.getString(R.string.permission_images); + case android.Manifest.permission.READ_MEDIA_VIDEO: + return resourceProvider.getString(R.string.permission_video); + case android.Manifest.permission.READ_MEDIA_AUDIO: + return resourceProvider.getString(R.string.permission_audio); case android.Manifest.permission.CAMERA: return resourceProvider.getString(R.string.permission_camera); case Manifest.permission.RECORD_AUDIO: diff --git a/WordPress/src/main/res/layout/media_picker_fragment.xml b/WordPress/src/main/res/layout/media_picker_fragment.xml index 08c3d58c0458..af4c35e2c082 100644 --- a/WordPress/src/main/res/layout/media_picker_fragment.xml +++ b/WordPress/src/main/res/layout/media_picker_fragment.xml @@ -77,7 +77,7 @@ android:visibility="gone" app:aevButton="@string/photo_picker_soft_ask_allow" app:aevImage="@drawable/img_illustration_add_media_150dp" - app:aevTitle="@string/photo_picker_soft_ask_label" + app:aevTitle="@string/photo_picker_soft_ask_photos_label" tools:visibility="visible" /> diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index c21915bf03e0..81cc4ce21b02 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -2804,6 +2804,10 @@ Choose from WordPress Media Library Choose from Tenor %s needs permissions to access your photos + %s needs permission to access your photos and videos + %s needs permission to access your photos + %s needs permission to access your videos + %s needs permission to access your audios Allow %1$s was denied access to your photos. To fix this, edit your permissions and turn on %2$s. Use this photo @@ -2825,7 +2829,7 @@ Media insert failed: %s Media insert failed. Media loading failed - %1$s was denied access to your photos. To fix this, edit your permissions and turn on %2$s and %3$s. + %1$s was denied access to your photos. To fix this, edit your permissions and turn on %2$s. Powered by Tenor @@ -2850,6 +2854,9 @@ Permissions It looks like you turned off permissions required for this feature.<br/><br/>To change this, edit your permissions and make sure <strong>%s</strong> is enabled. Storage + Photos and videos + Photos and videos + Music and audio Camera Microphone diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModelTest.kt index fa1554b0aedf..11f1cc86ea84 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModelTest.kt @@ -320,13 +320,21 @@ class MediaPickerViewModelTest : BaseUnitTest() { } @Test - fun `shows soft ask screen when storage permissions are turned off`() = test { - setupViewModel(listOf(), singleSelectMediaPickerSetup, hasStoragePermissions = false) + fun `shows soft ask screen when photos videos permissions are turned off`() = test { + setupViewModel( + listOf(), + singleSelectMediaPickerSetup, + hasPhotosVideosPermission = false + ) whenever(resourceProvider.getString(R.string.app_name)).thenReturn("WordPress") - whenever(resourceProvider.getString(R.string.photo_picker_soft_ask_label)).thenReturn("Soft ask label") + whenever(resourceProvider.getString(R.string.photo_picker_soft_ask_photos_label)) + .thenReturn("Soft ask label") val isAlwaysDenied = false - viewModel.checkStoragePermission(isAlwaysDenied = isAlwaysDenied) + viewModel.checkMediaPermissions( + isPhotosVideosAlwaysDenied = isAlwaysDenied, + isMusicAudioAlwaysDenied = isAlwaysDenied + ) assertThat(uiStates).hasSize(3) @@ -682,7 +690,7 @@ class MediaPickerViewModelTest : BaseUnitTest() { fun `empty state is emitted when no items in picker`() = test { setupViewModel(null, singleSelectMediaPickerSetup, numberOfStates = 1) - viewModel.checkStoragePermission(isAlwaysDenied = false) + viewModel.checkMediaPermissions(isPhotosVideosAlwaysDenied = false, isMusicAudioAlwaysDenied = false) assertThat(uiStates).hasSize(2) assertPhotoListUiStateEmpty() @@ -690,11 +698,15 @@ class MediaPickerViewModelTest : BaseUnitTest() { @Test fun `hidden state is emitted when when need to ask permission in picker`() = test { - setupViewModel(listOf(firstItem), singleSelectMediaPickerSetup, hasStoragePermissions = false) + setupViewModel( + listOf(firstItem), + singleSelectMediaPickerSetup, + hasPhotosVideosPermission = false + ) whenever(resourceProvider.getString(R.string.app_name)).thenReturn("WordPress") - whenever(resourceProvider.getString(R.string.photo_picker_soft_ask_label)).thenReturn("Soft ask label") + whenever(resourceProvider.getString(R.string.photo_picker_soft_ask_photos_label)).thenReturn("Soft ask label") - viewModel.checkStoragePermission(isAlwaysDenied = false) + viewModel.checkMediaPermissions(isPhotosVideosAlwaysDenied = false, isMusicAudioAlwaysDenied = false) assertThat(uiStates).hasSize(3) assertPhotoListUiStateHidden() @@ -702,7 +714,11 @@ class MediaPickerViewModelTest : BaseUnitTest() { @Test fun `data items state is emitted when items available in picker and have permissions`() = test { - setupViewModel(listOf(firstItem), singleSelectMediaPickerSetup, hasStoragePermissions = true) + setupViewModel( + listOf(firstItem), + singleSelectMediaPickerSetup, + hasPhotosVideosPermission = true + ) viewModel.refreshData(false) @@ -711,33 +727,36 @@ class MediaPickerViewModelTest : BaseUnitTest() { } @Test - fun `does not start loading without storage permissions`() = test { + fun `does not start loading without media permission`() = test { setupViewModel( listOf(firstItem), - singleSelectMediaPickerSetup.copy(requiresStoragePermissions = true), - hasStoragePermissions = false + singleSelectMediaPickerSetup.copy(requiresPhotosVideosPermissions = true), + hasPhotosVideosPermission = false ) assertThat(actions.tryReceive().getOrNull()).isNull() } @Test - fun `starts loading with storage permissions`() = test { + fun `starts loading with media permissions`() = test { setupViewModel( listOf(firstItem), - singleSelectMediaPickerSetup.copy(requiresStoragePermissions = true), - hasStoragePermissions = true + singleSelectMediaPickerSetup.copy(requiresPhotosVideosPermissions = true), + hasPhotosVideosPermission = true ) assertThat(actions.tryReceive().getOrNull()).isEqualTo(LoadAction.Start(null)) } @Test - fun `starts loading when storage permissions not necessary`() = test { + fun `starts loading when no permission necessary`() = test { setupViewModel( listOf(firstItem), - singleSelectMediaPickerSetup.copy(requiresStoragePermissions = false), - hasStoragePermissions = false + singleSelectMediaPickerSetup.copy( + requiresPhotosVideosPermissions = false, + requiresMusicAudioPermissions = false + ), + hasPhotosVideosPermission = false ) assertThat(actions.tryReceive().getOrNull()).isEqualTo(LoadAction.Start(null)) @@ -833,12 +852,12 @@ class MediaPickerViewModelTest : BaseUnitTest() { private suspend fun setupViewModel( domainModel: List?, mediaPickerSetup: MediaPickerSetup, - hasStoragePermissions: Boolean = true, + hasPhotosVideosPermission: Boolean = true, filter: String? = null, numberOfStates: Int = 2, hasMore: Boolean = false ) { - whenever(permissionsHandler.hasStoragePermission()).thenReturn(hasStoragePermissions) + whenever(permissionsHandler.hasPhotosVideosPermission()).thenReturn(hasPhotosVideosPermission) whenever(mediaLoaderFactory.build(mediaPickerSetup, site)).thenReturn(mediaLoader) doAnswer { actions = it.getArgument(0) @@ -931,12 +950,14 @@ class MediaPickerViewModelTest : BaseUnitTest() { allowedTypes: Set, cameraSetup: CameraSetup = HIDDEN, editingEnabled: Boolean = true, - requiresStoragePermissions: Boolean = true + requiresPhotosVideosPermission: Boolean = true, + requiresMusicAudioPermission: Boolean = true ) = MediaPickerSetup( primaryDataSource = DEVICE, availableDataSources = setOf(), canMultiselect = canMultiselect, - requiresStoragePermissions = requiresStoragePermissions, + requiresPhotosVideosPermissions = requiresPhotosVideosPermission, + requiresMusicAudioPermissions = requiresMusicAudioPermission, allowedTypes = allowedTypes, cameraSetup = cameraSetup, systemPickerEnabled = true, diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/loader/MediaLoaderFactoryTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/loader/MediaLoaderFactoryTest.kt index 2be0e469f6af..4b024e5899e0 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/loader/MediaLoaderFactoryTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/loader/MediaLoaderFactoryTest.kt @@ -58,7 +58,8 @@ class MediaLoaderFactoryTest { DEVICE, availableDataSources = setOf(), canMultiselect = true, - requiresStoragePermissions = true, + requiresPhotosVideosPermissions = true, + requiresMusicAudioPermissions = true, allowedTypes = setOf(), cameraSetup = HIDDEN, systemPickerEnabled = true, @@ -79,7 +80,8 @@ class MediaLoaderFactoryTest { WP_LIBRARY, availableDataSources = setOf(), canMultiselect = true, - requiresStoragePermissions = false, + requiresPhotosVideosPermissions = false, + requiresMusicAudioPermissions = false, allowedTypes = setOf(), cameraSetup = HIDDEN, systemPickerEnabled = false, @@ -101,7 +103,8 @@ class MediaLoaderFactoryTest { STOCK_LIBRARY, availableDataSources = setOf(), canMultiselect = true, - requiresStoragePermissions = false, + requiresPhotosVideosPermissions = false, + requiresMusicAudioPermissions = false, allowedTypes = setOf(), cameraSetup = HIDDEN, systemPickerEnabled = false, @@ -122,7 +125,8 @@ class MediaLoaderFactoryTest { GIF_LIBRARY, availableDataSources = setOf(), canMultiselect = true, - requiresStoragePermissions = false, + requiresPhotosVideosPermissions = false, + requiresMusicAudioPermissions = false, allowedTypes = setOf(), cameraSetup = HIDDEN, systemPickerEnabled = false, From 324c4c0ddfec4f81cb01330fade0f2b462773c60 Mon Sep 17 00:00:00 2001 From: Irfan Omur Date: Mon, 27 Mar 2023 21:21:24 +0300 Subject: [PATCH 03/10] Use media permissions on media browser For devices with API 33 and higher, we must request one or more of the granular media permissions instead of the READ_EXTERNAL_STORAGE permission. --- .../android/ui/media/MediaBrowserActivity.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/media/MediaBrowserActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/media/MediaBrowserActivity.java index 8396fc0ceb84..34b8dd8e3eba 100755 --- a/WordPress/src/main/java/org/wordpress/android/ui/media/MediaBrowserActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/media/MediaBrowserActivity.java @@ -10,6 +10,7 @@ import android.content.ServiceConnection; import android.net.ConnectivityManager; import android.net.Uri; +import android.os.Build; import android.os.Bundle; import android.os.IBinder; import android.text.TextUtils; @@ -1002,13 +1003,17 @@ private void doAddMediaItemClicked(@NonNull AddMenuItem item) { // stock photos item requires no permission, all other items do if (item != AddMenuItem.ITEM_CHOOSE_STOCK_MEDIA) { - String[] permissions; + String[] permissions = null; if (item == AddMenuItem.ITEM_CAPTURE_PHOTO || item == AddMenuItem.ITEM_CAPTURE_VIDEO) { - permissions = new String[]{Manifest.permission.CAMERA, Manifest.permission.WRITE_EXTERNAL_STORAGE}; - } else { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { + permissions = new String[]{Manifest.permission.CAMERA}; + } else { + permissions = new String[]{Manifest.permission.CAMERA, Manifest.permission.WRITE_EXTERNAL_STORAGE}; + } + } else if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) { permissions = new String[]{Manifest.permission.WRITE_EXTERNAL_STORAGE}; } - if (!PermissionUtils.checkAndRequestPermissions( + if (permissions != null && !PermissionUtils.checkAndRequestPermissions( this, WPPermissionUtils.MEDIA_BROWSER_PERMISSION_REQUEST_CODE, permissions)) { return; } From bb26d70d882088d1db51027ee65a18d2173f2640 Mon Sep 17 00:00:00 2001 From: Irfan Omur Date: Mon, 27 Mar 2023 21:27:25 +0300 Subject: [PATCH 04/10] Update WRITE_EXTERNAL_STORAGE usage for newer APIs on advertiseImageOptimization --- .../android/ui/media/MediaSettingsActivity.java | 17 +++++++++-------- .../wordpress/android/util/WPMediaUtils.java | 6 ++++-- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/media/MediaSettingsActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/media/MediaSettingsActivity.java index 2dec39e84ab3..350b96532dd8 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/media/MediaSettingsActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/media/MediaSettingsActivity.java @@ -14,6 +14,7 @@ import android.graphics.Bitmap; import android.graphics.drawable.Drawable; import android.net.Uri; +import android.os.Build; import android.os.Bundle; import android.os.Environment; import android.os.Handler; @@ -987,14 +988,14 @@ private void updateImageSizeParameters() { * saves the media to the local device using the Android DownloadManager */ private void saveMediaToDevice() { - // must request permissions even though they're already defined in the manifest - String[] permissionList = { - Manifest.permission.READ_EXTERNAL_STORAGE, - Manifest.permission.WRITE_EXTERNAL_STORAGE - }; - if (!PermissionUtils.checkAndRequestPermissions(this, WPPermissionUtils.MEDIA_PREVIEW_PERMISSION_REQUEST_CODE, - permissionList)) { - return; + // must request the permission even though it's already defined in the manifest + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) { + String[] permissionList = {Manifest.permission.WRITE_EXTERNAL_STORAGE}; + if (!PermissionUtils.checkAndRequestPermissions(this, + WPPermissionUtils.MEDIA_PREVIEW_PERMISSION_REQUEST_CODE, + permissionList)) { + return; + } } if (!NetworkUtils.checkConnection(this)) { diff --git a/WordPress/src/main/java/org/wordpress/android/util/WPMediaUtils.java b/WordPress/src/main/java/org/wordpress/android/util/WPMediaUtils.java index 40bcf329a9b1..87408847a19c 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/WPMediaUtils.java +++ b/WordPress/src/main/java/org/wordpress/android/util/WPMediaUtils.java @@ -9,6 +9,7 @@ import android.content.pm.PackageManager; import android.media.MediaScannerConnection; import android.net.Uri; +import android.os.Build; import android.os.Environment; import android.provider.MediaStore; import android.view.ViewConfiguration; @@ -121,8 +122,9 @@ public static boolean shouldAdvertiseImageOptimization(final Context context) { } // Check we can access storage before asking for optimizing image - boolean hasStoreAccess = ContextCompat.checkSelfPermission( - context, android.Manifest.permission.WRITE_EXTERNAL_STORAGE) == PackageManager.PERMISSION_GRANTED; + boolean hasStoreAccess = Build.VERSION.SDK_INT >= Build.VERSION_CODES.R + || ContextCompat.checkSelfPermission(context, + android.Manifest.permission.WRITE_EXTERNAL_STORAGE) == PackageManager.PERMISSION_GRANTED; if (!hasStoreAccess) { return false; } From e8de8bbb6c39c430bfeb2bc612ada75e7ef35fab Mon Sep 17 00:00:00 2001 From: Irfan Omur Date: Wed, 29 Mar 2023 03:53:58 +0300 Subject: [PATCH 05/10] Update Utils library reference --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index cdfaf244b9b3..24017eb3f78b 100644 --- a/build.gradle +++ b/build.gradle @@ -25,7 +25,7 @@ ext { wordPressFluxCVersion = '2.17.0' wordPressLoginVersion = '1.0.0' wordPressPersistentEditTextVersion = '1.0.2' - wordPressUtilsVersion = '3.4.0' + wordPressUtilsVersion = '126-d35e69fcbd9ce85384602fcb8532604d86749636' // debug stethoVersion = '1.6.0' From ac21e3c732fa7d3779a8bb185f1710e3535b5d0c Mon Sep 17 00:00:00 2001 From: Irfan Omur Date: Mon, 3 Apr 2023 01:21:13 +0300 Subject: [PATCH 06/10] Add all media types support to the media picker --- .../ui/mediapicker/MediaPickerViewModel.kt | 53 +++++++++---------- .../ui/photopicker/MediaPickerLauncher.kt | 2 +- WordPress/src/main/res/values/strings.xml | 4 +- .../mediapicker/MediaPickerViewModelTest.kt | 35 +++++++++--- 4 files changed, 56 insertions(+), 38 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt index 2cd105c10e3c..a261ff57ba38 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt @@ -296,7 +296,8 @@ class MediaPickerViewModel @Inject constructor( UiStringText(String.format(resourceProvider.getString(R.string.cab_selected), numSelected)) } else -> { - if (isImagePicker() && isVideoPicker()) { + if (mediaPickerSetup.allowedTypes.size > 1) { + // "image + video" picker, or "image + video + audio" picker UiStringRes(R.string.photo_picker_use_media) } else if (isVideoPicker()) { UiStringRes(R.string.photo_picker_use_video) @@ -324,14 +325,8 @@ class MediaPickerViewModel @Inject constructor( ) } - private fun hasPermission() = when { - mediaPickerSetup.requiresPhotosVideosPermissions -> permissionsHandler.hasPhotosVideosPermission() - mediaPickerSetup.requiresMusicAudioPermissions -> permissionsHandler.hasMusicAudioPermission() - else -> false - } - fun refreshData(forceReload: Boolean) { - if (hasPermission()) { + if (!needPhotosVideoPermission() && !needMusicAudioPermission()) { launch(bgDispatcher) { loadActions.send(LoadAction.Refresh(forceReload)) } @@ -370,9 +365,7 @@ class MediaPickerViewModel @Inject constructor( _domainModel.value = domainModel } } - if ((!mediaPickerSetup.requiresPhotosVideosPermissions && !mediaPickerSetup.requiresMusicAudioPermissions) - || hasPermission() - ) { + if (!needPhotosVideoPermission() && !needMusicAudioPermission()) { launch(bgDispatcher) { loadActions.send(LoadAction.Start()) } @@ -574,12 +567,9 @@ class MediaPickerViewModel @Inject constructor( // No permission is required, so there is no need to check permissions. return } - val isAlwaysDenied = if (mediaPickerSetup.requiresPhotosVideosPermissions) { - isPhotosVideosAlwaysDenied - } else { - isMusicAudioAlwaysDenied - } - if (hasPermission()) { + val isAlwaysDenied = (mediaPickerSetup.requiresPhotosVideosPermissions && isPhotosVideosAlwaysDenied) || + (mediaPickerSetup.requiresMusicAudioPermissions && isMusicAudioAlwaysDenied) + if (!needPhotosVideoPermission() && !needMusicAudioPermission()) { _softAskRequest.value = SoftAskRequest(show = false, isAlwaysDenied = isAlwaysDenied) if (_domainModel.value?.domainItems.isNullOrEmpty()) { refreshData(false) @@ -600,17 +590,13 @@ class MediaPickerViewModel @Inject constructor( clickIcon(icon) } - private fun getRequiredPermissionName(): String { - val permissionName = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { - if (mediaPickerSetup.requiresPhotosVideosPermissions) { - R.string.permission_images - } else if (mediaPickerSetup.requiresMusicAudioPermissions) { - R.string.permission_audio - } else { - R.string.unknown - } - } else { - R.string.permission_storage + private fun getRequiredPermissionsNames(): String { + val permissionName = when { + Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU -> R.string.permission_storage + needPhotosVideoPermission() && needMusicAudioPermission() -> R.string.permission_images_video_audio + needPhotosVideoPermission() -> R.string.permission_images + needMusicAudioPermission() -> R.string.permission_audio + else -> R.string.unknown } return resourceProvider.getString(permissionName) } @@ -619,12 +605,18 @@ class MediaPickerViewModel @Inject constructor( private fun isVideoPicker() = mediaPickerSetup.allowedTypes.contains(VIDEO) private fun isAudioPicker() = mediaPickerSetup.allowedTypes.contains(AUDIO) + private fun needPhotosVideoPermission() = + mediaPickerSetup.requiresPhotosVideosPermissions && !permissionsHandler.hasPhotosVideosPermission() + + private fun needMusicAudioPermission() = + mediaPickerSetup.requiresMusicAudioPermissions && !permissionsHandler.hasMusicAudioPermission() + private fun buildSoftAskView(softAskRequest: SoftAskRequest?): SoftAskViewUiModel { if (softAskRequest != null && softAskRequest.show) { mediaPickerTracker.trackShowPermissionsScreen(mediaPickerSetup, softAskRequest.isAlwaysDenied) val appName = "${resourceProvider.getString(R.string.app_name)}" val label = if (softAskRequest.isAlwaysDenied) { - val permission = ("${getRequiredPermissionName()}") + val permission = ("${getRequiredPermissionsNames()}") String.format( resourceProvider.getString(R.string.media_picker_soft_ask_media_permissions_denied), appName, @@ -632,6 +624,9 @@ class MediaPickerViewModel @Inject constructor( ) } else { val description = when { + isImagePicker() && isVideoPicker() && isAudioPicker() -> { + R.string.photo_picker_soft_ask_photos_videos_audio_label + } isImagePicker() && isVideoPicker() -> R.string.photo_picker_soft_ask_photos_videos_label isImagePicker() -> R.string.photo_picker_soft_ask_photos_label isVideoPicker() -> R.string.photo_picker_soft_ask_videos_label diff --git a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/MediaPickerLauncher.kt b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/MediaPickerLauncher.kt index 2ad7d92c8e7d..5170e3b31ab1 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/MediaPickerLauncher.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/MediaPickerLauncher.kt @@ -187,7 +187,7 @@ class MediaPickerLauncher @Inject constructor( primaryDataSource = DEVICE, availableDataSources = setOf(), canMultiselect = canMultiselect, - requiresPhotosVideosPermissions = false, + requiresPhotosVideosPermissions = allowedTypes.contains(IMAGE) || allowedTypes.contains(VIDEO), requiresMusicAudioPermissions = allowedTypes.contains(AUDIO), allowedTypes = allowedTypes, cameraSetup = HIDDEN, diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index 81cc4ce21b02..ba05d567f1a9 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -2804,6 +2804,7 @@ Choose from WordPress Media Library Choose from Tenor %s needs permissions to access your photos + %s needs permission to access your music, audio, photos and videos %s needs permission to access your photos and videos %s needs permission to access your photos %s needs permission to access your videos @@ -2829,7 +2830,7 @@ Media insert failed: %s Media insert failed. Media loading failed - %1$s was denied access to your photos. To fix this, edit your permissions and turn on %2$s. + %1$s was denied access to your media files. To fix this, edit your permissions and turn on %2$s. Powered by Tenor @@ -2857,6 +2858,7 @@ Photos and videos Photos and videos Music and audio + Photos and videos & Music and audio Camera Microphone diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModelTest.kt index 11f1cc86ea84..345ac5a9038e 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModelTest.kt @@ -116,11 +116,32 @@ class MediaPickerViewModelTest : BaseUnitTest() { private var uiStates = mutableListOf() private lateinit var actions: Channel private var navigateEvents = mutableListOf>() - private val singleSelectMediaPickerSetup = buildMediaPickerSetup(false, setOf(IMAGE)) - private val multiSelectMediaPickerSetup = buildMediaPickerSetup(true, setOf(IMAGE, VIDEO)) - private val singleSelectVideoPickerSetup = buildMediaPickerSetup(false, setOf(VIDEO)) - private val singleSelectAudioPickerSetup = buildMediaPickerSetup(false, setOf(AUDIO)) - private val multiSelectFilePickerSetup = buildMediaPickerSetup(true, setOf(IMAGE, VIDEO, AUDIO, DOCUMENT)) + private val singleSelectMediaPickerSetup = buildMediaPickerSetup( + false, + setOf(IMAGE), + requiresPhotosVideosPermission = true + ) + private val multiSelectMediaPickerSetup = buildMediaPickerSetup( + true, + setOf(IMAGE, VIDEO), + requiresPhotosVideosPermission = true + ) + private val singleSelectVideoPickerSetup = buildMediaPickerSetup( + false, + setOf(VIDEO), + requiresPhotosVideosPermission = true + ) + private val singleSelectAudioPickerSetup = buildMediaPickerSetup( + false, + setOf(AUDIO), + requiresMusicAudioPermission = true + ) + private val multiSelectFilePickerSetup = buildMediaPickerSetup( + true, + setOf(IMAGE, VIDEO, AUDIO, DOCUMENT), + requiresPhotosVideosPermission = true, + requiresMusicAudioPermission = true + ) private val site = SiteModel() private lateinit var firstItem: MediaItem private lateinit var secondItem: MediaItem @@ -950,8 +971,8 @@ class MediaPickerViewModelTest : BaseUnitTest() { allowedTypes: Set, cameraSetup: CameraSetup = HIDDEN, editingEnabled: Boolean = true, - requiresPhotosVideosPermission: Boolean = true, - requiresMusicAudioPermission: Boolean = true + requiresPhotosVideosPermission: Boolean = false, + requiresMusicAudioPermission: Boolean = false ) = MediaPickerSetup( primaryDataSource = DEVICE, availableDataSources = setOf(), From 898b117a086de961e7e9fbcc3de0014b1d3cb77c Mon Sep 17 00:00:00 2001 From: Irfan Omur Date: Tue, 4 Apr 2023 00:27:09 +0300 Subject: [PATCH 07/10] Migrate permissions in photo picker to Android 13 --- .../ui/mediapicker/MediaPickerViewModel.kt | 2 +- .../ui/photopicker/PermissionsHandler.kt | 4 +- .../ui/photopicker/PhotoPickerFragment.kt | 86 ++++++++++++------- .../ui/photopicker/PhotoPickerViewModel.kt | 85 +++++++++++------- WordPress/src/main/res/values/strings.xml | 1 - .../photopicker/PhotoPickerViewModelTest.kt | 12 +-- 6 files changed, 118 insertions(+), 72 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt index a261ff57ba38..ae18dc31d0b4 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModel.kt @@ -494,7 +494,7 @@ class MediaPickerViewModel @Inject constructor( private fun clickIcon(icon: MediaPickerIcon) { mediaPickerTracker.trackIconClick(icon, mediaPickerSetup) if (icon is WpStoriesCapture || icon is CapturePhoto) { - if (!permissionsHandler.hasPermissionsToAccessPhotos()) { + if (!permissionsHandler.hasPermissionsToTakePhoto()) { _onCameraPermissionsRequested.value = Event(Unit) lastTappedIcon = icon return diff --git a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PermissionsHandler.kt b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PermissionsHandler.kt index cf725de0e5fa..87421224653b 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PermissionsHandler.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PermissionsHandler.kt @@ -10,8 +10,8 @@ import javax.inject.Inject class PermissionsHandler @Inject constructor(private val context: Context) { - fun hasPermissionsToAccessPhotos(): Boolean { - return hasCameraPermission() && hasPhotosVideosPermission() + fun hasPermissionsToTakePhoto(): Boolean { + return hasCameraPermission() && (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R || hasWriteStoragePermission()) } fun hasPhotosVideosPermission(): Boolean { diff --git a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerFragment.kt index 1eba8f88f266..5f5326b917a8 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerFragment.kt @@ -2,6 +2,7 @@ package org.wordpress.android.ui.photopicker import android.Manifest.permission import android.net.Uri +import android.os.Build import android.os.Bundle import android.os.Parcelable import android.view.View @@ -81,6 +82,7 @@ class PhotoPickerFragment : Fragment(R.layout.photo_picker_fragment) { @Suppress("DEPRECATION") private lateinit var viewModel: PhotoPickerViewModel private var binding: PhotoPickerFragmentBinding? = null + private lateinit var browserType: MediaBrowserType @Suppress("DEPRECATION") override fun onCreate(savedInstanceState: Bundle?) { @@ -92,7 +94,7 @@ class PhotoPickerFragment : Fragment(R.layout.photo_picker_fragment) { override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - val browserType = requireNotNull( + browserType = requireNotNull( arguments?.getSerializableCompat(MediaBrowserActivity.ARG_BROWSER_TYPE) ) val site = requireArguments().getSerializableCompat(WordPress.SITE) @@ -131,7 +133,7 @@ class PhotoPickerFragment : Fragment(R.layout.photo_picker_fragment) { observeOnShowPopupMenu() - observeOnPermissionsRequested() + observeOnCameraPermissionsRequested() setupProgressDialog() @@ -140,13 +142,8 @@ class PhotoPickerFragment : Fragment(R.layout.photo_picker_fragment) { } @Suppress("DEPRECATION") - private fun observeOnPermissionsRequested() { - viewModel.onPermissionsRequested.observeEvent(viewLifecycleOwner) { - when (it) { - PhotoPickerViewModel.PermissionsRequested.CAMERA -> requestCameraPermission() - PhotoPickerViewModel.PermissionsRequested.STORAGE -> requestStoragePermission() - } - } + private fun observeOnCameraPermissionsRequested() { + viewModel.onCameraPermissionsRequested.observeEvent(viewLifecycleOwner) { requestCameraPermission() } } private fun observeOnShowPopupMenu() { @@ -225,7 +222,7 @@ class PhotoPickerFragment : Fragment(R.layout.photo_picker_fragment) { if (uiModel.isAlwaysDenied) { WPPermissionUtils.showAppSettings(requireActivity()) } else { - requestStoragePermission() + requestMediaPermission() } } softAskView.visibility = View.VISIBLE @@ -375,7 +372,7 @@ class PhotoPickerFragment : Fragment(R.layout.photo_picker_fragment) { override fun onResume() { super.onResume() - checkStoragePermission() + checkMediaPermission() } fun performActionOrShowPopup(view: View) { @@ -420,37 +417,68 @@ class PhotoPickerFragment : Fragment(R.layout.photo_picker_fragment) { viewModel.refreshData(false) } - private val isStoragePermissionAlwaysDenied: Boolean - get() = WPPermissionUtils.isPermissionAlwaysDenied( - requireActivity(), permission.WRITE_EXTERNAL_STORAGE - ) - /* * load the photos if we have the necessary permission, otherwise show the "soft ask" view * which asks the user to allow the permission */ - private fun checkStoragePermission() { + private fun checkMediaPermission() { if (!isAdded) { return } - viewModel.checkStoragePermission(isStoragePermissionAlwaysDenied) + + // Storage permission is available only for API lower than 33 + val isStoragePermissionAlwaysDenied = WPPermissionUtils.isPermissionAlwaysDenied( + requireActivity(), + permission.WRITE_EXTERNAL_STORAGE + ) + + val isPhotosVideosPermissionAlwaysDenied = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + WPPermissionUtils.isPermissionAlwaysDenied(requireActivity(), permission.READ_MEDIA_IMAGES) || + WPPermissionUtils.isPermissionAlwaysDenied(requireActivity(), permission.READ_MEDIA_VIDEO) + } else { + // For devices lower than API 33, storage permission is the equivalent of Photos and Videos permission + isStoragePermissionAlwaysDenied + } + val isMusicAudioPermissionAlwaysDenied = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + WPPermissionUtils.isPermissionAlwaysDenied( + requireActivity(), + permission.READ_MEDIA_AUDIO + ) + } else { + // For devices lower than API 33, storage permission is the equivalent of Music and Audio permission + isStoragePermissionAlwaysDenied + } + viewModel.checkMediaPermissions(isPhotosVideosPermissionAlwaysDenied, isMusicAudioPermissionAlwaysDenied) } @Suppress("DEPRECATION") - private fun requestStoragePermission() { - val permissions = arrayOf(permission.WRITE_EXTERNAL_STORAGE) - requestPermissions( - permissions, WPPermissionUtils.PHOTO_PICKER_MEDIA_PERMISSION_REQUEST_CODE - ) + private fun requestMediaPermission() { + val permissions = arrayListOf() + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + if (browserType.isImagePicker || browserType.isVideoPicker) { + permissions.add(permission.READ_MEDIA_IMAGES) + permissions.add(permission.READ_MEDIA_VIDEO) + } + if (browserType.isAudioPicker) { + permissions.add(permission.READ_MEDIA_AUDIO) + } + } else { + // READ_EXTERNAL_STORAGE is the equivalent of READ_MEDIA_IMAGES, READ_MEDIA_VIDEO and READ_MEDIA_AUDIO on + // devices lower than API 33. + permissions.add(permission.READ_EXTERNAL_STORAGE) + } + requestPermissions(permissions.toTypedArray(), WPPermissionUtils.PHOTO_PICKER_MEDIA_PERMISSION_REQUEST_CODE) } @Suppress("DEPRECATION") private fun requestCameraPermission() { - // in addition to CAMERA permission we also need a storage permission, to store media from the camera - val permissions = arrayOf( - permission.CAMERA, - permission.WRITE_EXTERNAL_STORAGE - ) + // For devices lower than API 30, in addition to CAMERA permission we also need a storage permission, to store + // media from the camera + val permissions = if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) { + arrayOf(permission.CAMERA, permission.WRITE_EXTERNAL_STORAGE) + } else { + arrayOf(permission.CAMERA) + } requestPermissions(permissions, WPPermissionUtils.PHOTO_PICKER_CAMERA_PERMISSION_REQUEST_CODE) } @@ -465,7 +493,7 @@ class PhotoPickerFragment : Fragment(R.layout.photo_picker_fragment) { requireActivity(), requestCode, permissions, grantResults, checkForAlwaysDenied ) when (requestCode) { - WPPermissionUtils.PHOTO_PICKER_MEDIA_PERMISSION_REQUEST_CODE -> checkStoragePermission() + WPPermissionUtils.PHOTO_PICKER_MEDIA_PERMISSION_REQUEST_CODE -> checkMediaPermission() WPPermissionUtils.PHOTO_PICKER_CAMERA_PERMISSION_REQUEST_CODE -> if (allGranted) { viewModel.clickOnLastTappedIcon() } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerViewModel.kt index 6ea2dd6fc531..01b2ff1674bb 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/photopicker/PhotoPickerViewModel.kt @@ -1,7 +1,7 @@ package org.wordpress.android.ui.photopicker -import android.Manifest.permission import android.net.Uri +import android.os.Build import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import kotlinx.coroutines.CoroutineDispatcher @@ -34,7 +34,6 @@ import org.wordpress.android.util.AppLog.T.MEDIA import org.wordpress.android.util.MediaUtils import org.wordpress.android.util.UriWrapper import org.wordpress.android.util.ViewWrapper -import org.wordpress.android.util.WPPermissionUtils import org.wordpress.android.util.analytics.AnalyticsTrackerWrapper import org.wordpress.android.util.analytics.AnalyticsUtilsWrapper import org.wordpress.android.util.distinct @@ -68,7 +67,7 @@ class PhotoPickerViewModel @Inject constructor( private val _photoPickerItems = MutableLiveData>() private val _selectedIds = MutableLiveData?>() private val _onIconClicked = MutableLiveData>() - private val _onPermissionsRequested = MutableLiveData>() + private val _onCameraPermissionsRequested = MutableLiveData>() private val _softAskRequest = MutableLiveData() private val _showProgressDialog = MutableLiveData() @@ -77,7 +76,7 @@ class PhotoPickerViewModel @Inject constructor( val onIconClicked: LiveData> = _onIconClicked val onShowPopupMenu: LiveData> = _showPopupMenu - val onPermissionsRequested: LiveData> = _onPermissionsRequested + val onCameraPermissionsRequested: LiveData> = _onCameraPermissionsRequested val selectedIds: LiveData?> = _selectedIds @@ -219,14 +218,13 @@ class PhotoPickerViewModel @Inject constructor( } fun refreshData(forceReload: Boolean) { - if (!permissionsHandler.hasWriteStoragePermission()) { - return - } - launch(bgDispatcher) { - val result = deviceMediaListBuilder.buildDeviceMedia(browserType) - val currentItems = _photoPickerItems.value ?: listOf() - if (forceReload || currentItems != result) { - _photoPickerItems.postValue(result) + if (!needPhotosVideoPermission() && !needMusicAudioPermission()) { + launch(bgDispatcher) { + val result = deviceMediaListBuilder.buildDeviceMedia(browserType) + val currentItems = _photoPickerItems.value ?: listOf() + if (forceReload || currentItems != result) { + _photoPickerItems.postValue(result) + } } } } @@ -318,8 +316,8 @@ class PhotoPickerViewModel @Inject constructor( icon == PhotoPickerFragment.PhotoPickerIcon.ANDROID_CAPTURE_VIDEO || icon == PhotoPickerFragment.PhotoPickerIcon.WP_STORIES_CAPTURE ) { - if (!permissionsHandler.hasPermissionsToAccessPhotos()) { - _onPermissionsRequested.value = Event(PermissionsRequested.CAMERA) + if (!permissionsHandler.hasPermissionsToTakePhoto()) { + _onCameraPermissionsRequested.value = Event(Unit) lastTappedIcon = icon return } @@ -417,8 +415,11 @@ class PhotoPickerViewModel @Inject constructor( } } - fun checkStoragePermission(isAlwaysDenied: Boolean) { - if (permissionsHandler.hasWriteStoragePermission()) { + fun checkMediaPermissions(isPhotosVideosAlwaysDenied: Boolean, isMusicAudioAlwaysDenied: Boolean) { + val isAlwaysDenied = ((browserType.isImagePicker || browserType.isVideoPicker) && isPhotosVideosAlwaysDenied) || + (browserType.isAudioPicker && isMusicAudioAlwaysDenied) + + if (!needPhotosVideoPermission() && !needMusicAudioPermission()) { _softAskRequest.value = SoftAskRequest(show = false, isAlwaysDenied = isAlwaysDenied) if (_photoPickerItems.value.isNullOrEmpty()) { refreshData(false) @@ -428,25 +429,47 @@ class PhotoPickerViewModel @Inject constructor( } } + private fun getRequiredPermissionsNames(): String { + val permissionName = when { + Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU -> R.string.permission_storage + needPhotosVideoPermission() && needMusicAudioPermission() -> R.string.permission_images_video_audio + needPhotosVideoPermission() -> R.string.permission_images + needMusicAudioPermission() -> R.string.permission_audio + else -> R.string.unknown + } + return resourceProvider.getString(permissionName) + } + + private fun needPhotosVideoPermission() = + (browserType.isImagePicker || browserType.isVideoPicker) && !permissionsHandler.hasPhotosVideosPermission() + + private fun needMusicAudioPermission() = + browserType.isAudioPicker && !permissionsHandler.hasMusicAudioPermission() + private fun buildSoftAskView(softAskRequest: SoftAskRequest?): SoftAskViewUiModel { if (softAskRequest != null && softAskRequest.show) { val appName = "${resourceProvider.getString(R.string.app_name)}" val label = if (softAskRequest.isAlwaysDenied) { - val permissionName = ("${ - WPPermissionUtils.getPermissionName( - resourceProvider, - permission.WRITE_EXTERNAL_STORAGE - ) - }") + val permission = ("${getRequiredPermissionsNames()}") String.format( - resourceProvider.getString(R.string.photo_picker_soft_ask_permissions_denied), appName, - permissionName + resourceProvider.getString(R.string.media_picker_soft_ask_media_permissions_denied), + appName, + permission ) } else { - String.format( - resourceProvider.getString(R.string.photo_picker_soft_ask_label), - appName - ) + val description = when { + browserType.isImagePicker && browserType.isVideoPicker && browserType.isAudioPicker -> { + R.string.photo_picker_soft_ask_photos_videos_audio_label + } + browserType.isImagePicker && browserType.isVideoPicker -> { + R.string.photo_picker_soft_ask_photos_videos_label + } + browserType.isImagePicker -> R.string.photo_picker_soft_ask_photos_label + browserType.isVideoPicker -> R.string.photo_picker_soft_ask_videos_label + browserType.isAudioPicker -> R.string.photo_picker_soft_ask_audios_label + else -> R.string.unknown + } + String.format(resourceProvider.getString(description), appName) } val allowId = if (softAskRequest.isAlwaysDenied) { R.string.button_edit_permissions @@ -471,7 +494,7 @@ class PhotoPickerViewModel @Inject constructor( } } - fun copySelectedUrisLocally(uris: List) { + private fun copySelectedUrisLocally(uris: List) { launch { _showProgressDialog.value = ProgressDialogUiModel.Visible(R.string.uploading_title) { _showProgressDialog.postValue(ProgressDialogUiModel.Hidden) @@ -540,10 +563,6 @@ class PhotoPickerViewModel @Inject constructor( val allowMultipleSelection: Boolean ) - enum class PermissionsRequested { - CAMERA, STORAGE - } - data class PopupMenuUiModel(val view: ViewWrapper, val items: List) { data class PopupMenuItem(val title: UiStringRes, val action: () -> Unit) } diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index ba05d567f1a9..cf8980ec84b6 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -2810,7 +2810,6 @@ %s needs permission to access your videos %s needs permission to access your audios Allow - %1$s was denied access to your photos. To fix this, edit your permissions and turn on %2$s. Use this photo Use this video Use this audio diff --git a/WordPress/src/test/java/org/wordpress/android/ui/photopicker/PhotoPickerViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/photopicker/PhotoPickerViewModelTest.kt index d096eac556ae..6d3fc187f49f 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/photopicker/PhotoPickerViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/photopicker/PhotoPickerViewModelTest.kt @@ -245,12 +245,12 @@ class PhotoPickerViewModelTest : BaseUnitTest() { } @Test - fun `shows soft ask screen when storage permissions are turned off`() = test { - setupViewModel(listOf(), singleSelectBrowserType, hasStoragePermissions = false) + fun `shows soft ask screen when photos videos permissions are turned off`() = test { + setupViewModel(listOf(), singleSelectBrowserType, hasPhotosVideosPermissions = false) whenever(resourceProvider.getString(R.string.app_name)).thenReturn("WordPress") - whenever(resourceProvider.getString(R.string.photo_picker_soft_ask_label)).thenReturn("Soft ask label") + whenever(resourceProvider.getString(R.string.photo_picker_soft_ask_photos_label)).thenReturn("Soft ask label") - viewModel.checkStoragePermission(isAlwaysDenied = false) + viewModel.checkMediaPermissions(isPhotosVideosAlwaysDenied = false, isMusicAudioAlwaysDenied = false) assertThat(uiStates).hasSize(2) @@ -380,9 +380,9 @@ class PhotoPickerViewModelTest : BaseUnitTest() { private suspend fun setupViewModel( domainModel: List, browserType: MediaBrowserType, - hasStoragePermissions: Boolean = true + hasPhotosVideosPermissions: Boolean = true ) { - whenever(permissionsHandler.hasWriteStoragePermission()).thenReturn(hasStoragePermissions) + whenever(permissionsHandler.hasPhotosVideosPermission()).thenReturn(hasPhotosVideosPermissions) viewModel.start(listOf(), browserType, null, site) whenever(deviceMediaListBuilder.buildDeviceMedia(browserType)).thenReturn(domainModel) viewModel.uiState.observeForever { From 21f997592b97b6302889dfc549f21efaa9e25b7b Mon Sep 17 00:00:00 2001 From: Irfan Omur Date: Thu, 6 Apr 2023 18:35:03 +0300 Subject: [PATCH 08/10] Update permission_video string resource --- WordPress/src/main/res/values/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index 5173ccfa2553..9b0fbf68d7f3 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -2855,7 +2855,7 @@ It looks like you turned off permissions required for this feature.<br/><br/>To change this, edit your permissions and make sure <strong>%s</strong> is enabled. Storage Photos and videos - Photos and videos + @string/permission_images Music and audio Photos and videos & Music and audio Camera From d14b738e31df3a0fdf4095b7fe2a801a8bfa4612 Mon Sep 17 00:00:00 2001 From: Irfan Omur Date: Thu, 6 Apr 2023 18:36:56 +0300 Subject: [PATCH 09/10] Update RELEASE-NOTES --- RELEASE-NOTES.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 4be2eef3bf60..93c286d173b3 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,5 +1,8 @@ *** PLEASE FOLLOW THIS FORMAT: [] [] +22.2 +[***] [internal] Adds media permissions support for Android 13 [https://github.com/wordpress-mobile/WordPress-Android/pull/18183] + 22.1 ----- * [**] [WordPress-only] Warns user about sites with only individual plugins not supporting core app features and offers the option to switch to the Jetpack app. [https://github.com/wordpress-mobile/WordPress-Android/pull/18199] From 8b50e67c5fcfb76b8b675d744f4e102e45a8230e Mon Sep 17 00:00:00 2001 From: Irfan Omur Date: Thu, 6 Apr 2023 22:06:46 +0300 Subject: [PATCH 10/10] Resolve "translatable="false" warning on a string --- WordPress/src/main/res/values/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index 9b0fbf68d7f3..5abf76904a30 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -2855,7 +2855,7 @@ It looks like you turned off permissions required for this feature.<br/><br/>To change this, edit your permissions and make sure <strong>%s</strong> is enabled. Storage Photos and videos - @string/permission_images + @string/permission_images Music and audio Photos and videos & Music and audio Camera