From 80657251a5e7865285db3e396b2f003869052ae4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 12 Jul 2021 23:03:16 +0200 Subject: [PATCH] Fix some misunderstanding about the permissions request - step 1 --- .../app/features/debug/DebugMenuActivity.kt | 7 +- .../features/debug/DebugPermissionActivity.kt | 46 +++++-- .../vector/app/core/utils/PermissionsTools.kt | 126 +++++++++--------- .../app/features/call/VectorCallActivity.kt | 2 +- .../createdirect/CreateDirectRoomActivity.kt | 8 +- .../invite/InviteUsersToRoomActivity.kt | 7 +- 6 files changed, 106 insertions(+), 90 deletions(-) diff --git a/vector/src/debug/java/im/vector/app/features/debug/DebugMenuActivity.kt b/vector/src/debug/java/im/vector/app/features/debug/DebugMenuActivity.kt index 5d94f312b2..2c9b81b1c0 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/DebugMenuActivity.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/DebugMenuActivity.kt @@ -20,20 +20,15 @@ import android.app.Activity import android.app.NotificationChannel import android.app.NotificationManager import android.content.Intent -import android.content.pm.PackageManager import android.os.Build -import androidx.core.app.ActivityCompat import androidx.core.app.NotificationCompat import androidx.core.app.Person -import androidx.core.content.ContextCompat import androidx.core.content.getSystemService -import com.google.android.material.dialog.MaterialAlertDialogBuilder import im.vector.app.R import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.core.di.ScreenComponent import im.vector.app.core.extensions.registerStartForActivityResult import im.vector.app.core.platform.VectorBaseActivity -import im.vector.app.core.utils.PERMISSIONS_ALL import im.vector.app.core.utils.PERMISSIONS_FOR_TAKING_PHOTO import im.vector.app.core.utils.checkPermissions import im.vector.app.core.utils.registerForPermissionsResult @@ -228,7 +223,7 @@ class DebugMenuActivity : VectorBaseActivity() { } } - private val permissionCamera = registerForPermissionsResult { allGranted -> + private val permissionCamera = registerForPermissionsResult { allGranted, _ -> if (allGranted) { doScanQRCode() } diff --git a/vector/src/debug/java/im/vector/app/features/debug/DebugPermissionActivity.kt b/vector/src/debug/java/im/vector/app/features/debug/DebugPermissionActivity.kt index 660e6c4312..3eca2b49be 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/DebugPermissionActivity.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/DebugPermissionActivity.kt @@ -16,6 +16,7 @@ package im.vector.app.features.debug +import android.Manifest import android.content.pm.PackageManager import android.os.Build import android.widget.Toast @@ -33,27 +34,48 @@ class DebugPermissionActivity : VectorBaseActivity() + override fun initUiAndData() { views.status.setOnClickListener { refresh() } - listOf( - views.audio, - views.camera, - views.write, - views.read, - views.contact - ).forEach { button -> - button.setOnClickListener { - checkPermissions(listOf(button.text.toString()), this, launcher, R.string.debug_rationale) - } + views.camera.setOnClickListener { + lastPermissions = listOf(Manifest.permission.CAMERA) + checkPerm() + } + views.audio.setOnClickListener { + lastPermissions = listOf(Manifest.permission.RECORD_AUDIO) + checkPerm() + } + views.write.setOnClickListener { + lastPermissions = listOf(Manifest.permission.WRITE_EXTERNAL_STORAGE) + checkPerm() + } + views.read.setOnClickListener { + lastPermissions = listOf(Manifest.permission.READ_EXTERNAL_STORAGE) + checkPerm() + } + views.contact.setOnClickListener { + lastPermissions = listOf(Manifest.permission.READ_CONTACTS) + checkPerm() } } - private val launcher = registerForPermissionsResult { allGranted -> + private fun checkPerm() { + if (checkPermissions(lastPermissions, this, launcher, R.string.debug_rationale)) { + Toast.makeText(this, "Already granted, sync call", Toast.LENGTH_SHORT).show() + } + } + + private val launcher = registerForPermissionsResult { allGranted, deniedPermanently -> if (allGranted) { Toast.makeText(this, "All granted", Toast.LENGTH_SHORT).show() } else { - Toast.makeText(this, "Denied", Toast.LENGTH_SHORT).show() + if (deniedPermanently) { + Toast.makeText(this, "Denied forever", Toast.LENGTH_SHORT).show() + } else { + Toast.makeText(this, "Denied", Toast.LENGTH_SHORT).show() + } } } diff --git a/vector/src/main/java/im/vector/app/core/utils/PermissionsTools.kt b/vector/src/main/java/im/vector/app/core/utils/PermissionsTools.kt index 54823c0750..3901e63eb0 100644 --- a/vector/src/main/java/im/vector/app/core/utils/PermissionsTools.kt +++ b/vector/src/main/java/im/vector/app/core/utils/PermissionsTools.kt @@ -19,8 +19,6 @@ package im.vector.app.core.utils import android.Manifest import android.app.Activity import android.content.pm.PackageManager -import android.os.Build -import android.widget.Toast import androidx.activity.ComponentActivity import androidx.activity.result.ActivityResultLauncher import androidx.activity.result.contract.ActivityResultContracts @@ -31,7 +29,6 @@ import androidx.fragment.app.Fragment import com.google.android.material.dialog.MaterialAlertDialogBuilder import im.vector.app.R import im.vector.app.core.platform.VectorBaseActivity -import timber.log.Timber // Permissions sets val PERMISSIONS_FOR_AUDIO_IP_CALL = listOf(Manifest.permission.RECORD_AUDIO) @@ -52,9 +49,32 @@ val PERMISSIONS_ALL = listOf( Manifest.permission.READ_EXTERNAL_STORAGE, Manifest.permission.READ_CONTACTS) -fun ComponentActivity.registerForPermissionsResult(allGranted: (Boolean) -> Unit): ActivityResultLauncher> { +// This is not ideal to store the value like that, but it works +private var permissionDialogDisplayed = false + +/** + * First boolean is true if all permissions have been granted + * Second boolean is true if the permission is denied forever AND the permission request has not been displayed. + * So when the user does not grant the permission and check the box do not ask again, this boolean will be false. + * Only useful if the first boolean is false + */ +fun ComponentActivity.registerForPermissionsResult(lambda: (allGranted: Boolean, deniedPermanently: Boolean) -> Unit) + : ActivityResultLauncher> { return registerForActivityResult(ActivityResultContracts.RequestMultiplePermissions()) { result -> - allGranted.invoke(result.keys.all { result[it] == true }) + if (result.keys.all { result[it] == true }) { + lambda(true, /* not used */ false) + } else { + if (permissionDialogDisplayed) { + // A permission dialog has been displayed, so even if the user has checked the do not ask again button, we do + // not tell the user to open the app settings + lambda(false, false) + } else { + // No dialog has been displayed, so tell the user to go to the system setting + lambda(false, true) + } + } + // Reset + permissionDialogDisplayed = false } } @@ -80,54 +100,62 @@ fun Fragment.registerForPermissionsResult(allGranted: (Boolean) -> Unit): Activi * @param permissionsToBeGranted the permissions to be granted * @param activity the calling Activity that is requesting the permissions (or fragment parent) * @param activityResultLauncher from the calling fragment/Activity that is requesting the permissions + * @param rationaleMessage message to be displayed BEFORE requesting for the permission * @return true if the permissions are granted (synchronous flow), false otherwise (asynchronous flow) */ fun checkPermissions(permissionsToBeGranted: List, activity: Activity, activityResultLauncher: ActivityResultLauncher>, @StringRes rationaleMessage: Int = 0): Boolean { - var isPermissionGranted = false + // retrieve the permissions to be granted according to the permission list + val missingPermissions = permissionsToBeGranted.filter { permission -> + ContextCompat.checkSelfPermission(activity.applicationContext, permission) == PackageManager.PERMISSION_DENIED + } - // sanity check - if (permissionsToBeGranted.isEmpty()) { - isPermissionGranted = true - } else { - val permissionListAlreadyDenied = mutableListOf() - val permissionsListToBeGranted = mutableListOf() - var isRequestPermissionRequired = false + return if (missingPermissions.isNotEmpty()) { + permissionDialogDisplayed = !permissionsDeniedPermanently(missingPermissions, activity) - // retrieve the permissions to be granted according to the permission list - permissionsToBeGranted.forEach { permission -> - if (updatePermissionsToBeGranted(activity, permissionListAlreadyDenied, permissionsListToBeGranted, permission)) { - isRequestPermissionRequired = true - } - } - - // if some permissions were already denied: display a dialog to the user before asking again. - if (permissionListAlreadyDenied.isNotEmpty() && rationaleMessage != 0) { - // display the dialog with the info text + if (rationaleMessage != 0 && permissionDialogDisplayed) { + // display the dialog with the info text. Do not do it if no system dialog will + // be displayed MaterialAlertDialogBuilder(activity) .setTitle(R.string.permissions_rationale_popup_title) .setMessage(rationaleMessage) - .setOnCancelListener { Toast.makeText(activity, R.string.missing_permissions_warning, Toast.LENGTH_SHORT).show() } + .setCancelable(false) .setPositiveButton(R.string.ok) { _, _ -> - if (permissionsListToBeGranted.isNotEmpty()) { - activityResultLauncher.launch(permissionsListToBeGranted.toTypedArray()) - } + activityResultLauncher.launch(missingPermissions.toTypedArray()) } .show() } else { // some permissions are not granted, ask permissions - if (isRequestPermissionRequired) { - activityResultLauncher.launch(permissionsListToBeGranted.toTypedArray()) - } else { - // permissions were granted, start now. - isPermissionGranted = true - } + activityResultLauncher.launch(missingPermissions.toTypedArray()) } + false + } else { + // permissions were granted, start now. + true } +} - return isPermissionGranted +/** + * To be call after the permission request + * + * @param permissionsToBeGranted the permissions to be granted + * @param activity the calling Activity that is requesting the permissions (or fragment parent) + * + * @return true if one of the permission has been denied and the user check the do not ask again checkbox + */ +private fun permissionsDeniedPermanently(permissionsToBeGranted: List, + activity: Activity): Boolean { + return permissionsToBeGranted + .filter { permission -> + ContextCompat.checkSelfPermission(activity.applicationContext, permission) == PackageManager.PERMISSION_DENIED + } + .any { permission -> + // If shouldShowRequestPermissionRationale() returns true, it means that the user as denied the permission, but not permanently. + // If it return false, it mean that the user as denied permanently the permission + ActivityCompat.shouldShowRequestPermissionRationale(activity, permission).not() + } } fun VectorBaseActivity<*>.onPermissionDeniedSnackbar(@StringRes rationaleMessage: Int) { @@ -135,33 +163,3 @@ fun VectorBaseActivity<*>.onPermissionDeniedSnackbar(@StringRes rationaleMessage openAppSettingsPage(this) } } - -/** - * Helper method used in [.checkPermissions] to populate the list of the - * permissions to be granted (permissionsListToBeGrantedOut) and the list of the permissions already denied (permissionAlreadyDeniedListOut). - * - * @param activity calling activity - * @param permissionAlreadyDeniedListOut list to be updated with the permissions already denied by the user - * @param permissionsListToBeGrantedOut list to be updated with the permissions to be granted - * @param permissionType the permission to be checked - * @return true if the permission requires to be granted, false otherwise - */ -private fun updatePermissionsToBeGranted(activity: Activity, - permissionAlreadyDeniedListOut: MutableList, - permissionsListToBeGrantedOut: MutableList, - permissionType: String): Boolean { - var isRequestPermissionRequested = false - - // add permission to be granted - permissionsListToBeGrantedOut.add(permissionType) - - if (PackageManager.PERMISSION_GRANTED != ContextCompat.checkSelfPermission(activity.applicationContext, permissionType)) { - isRequestPermissionRequested = true - - // add permission to the ones that were already asked to the user - if (ActivityCompat.shouldShowRequestPermissionRationale(activity, permissionType)) { - permissionAlreadyDeniedListOut.add(permissionType) - } - } - return isRequestPermissionRequested -} diff --git a/vector/src/main/java/im/vector/app/features/call/VectorCallActivity.kt b/vector/src/main/java/im/vector/app/features/call/VectorCallActivity.kt index cbe2733134..7e84811102 100644 --- a/vector/src/main/java/im/vector/app/features/call/VectorCallActivity.kt +++ b/vector/src/main/java/im/vector/app/features/call/VectorCallActivity.kt @@ -298,7 +298,7 @@ class VectorCallActivity : VectorBaseActivity(), CallContro } } - private val permissionCameraLauncher = registerForPermissionsResult { allGranted -> + private val permissionCameraLauncher = registerForPermissionsResult { allGranted, _ -> if (allGranted) { start() } else { diff --git a/vector/src/main/java/im/vector/app/features/createdirect/CreateDirectRoomActivity.kt b/vector/src/main/java/im/vector/app/features/createdirect/CreateDirectRoomActivity.kt index c2d323ff64..faae9102e3 100644 --- a/vector/src/main/java/im/vector/app/features/createdirect/CreateDirectRoomActivity.kt +++ b/vector/src/main/java/im/vector/app/features/createdirect/CreateDirectRoomActivity.kt @@ -120,18 +120,18 @@ class CreateDirectRoomActivity : SimpleFragmentActivity(), UserListViewModel.Fac } } - private val permissionReadContactLauncher = registerForPermissionsResult { allGranted -> + private val permissionReadContactLauncher = registerForPermissionsResult { allGranted, deniedPermanently -> if (allGranted) { doOnPostResume { addFragmentToBackstack(R.id.container, ContactsBookFragment::class.java) } - } else { + } else if (deniedPermanently) { onPermissionDeniedSnackbar(R.string.permissions_denied_add_contact) } } - private val permissionCameraLauncher = registerForPermissionsResult { allGranted -> + private val permissionCameraLauncher = registerForPermissionsResult { allGranted, deniedPermanently -> if (allGranted) { addFragment(R.id.container, CreateDirectRoomByQrCodeFragment::class.java) - } else { + } else if (deniedPermanently) { onPermissionDeniedSnackbar(R.string.permissions_denied_qr_code) } } diff --git a/vector/src/main/java/im/vector/app/features/invite/InviteUsersToRoomActivity.kt b/vector/src/main/java/im/vector/app/features/invite/InviteUsersToRoomActivity.kt index 0f3e3c57d2..5eff1189a2 100644 --- a/vector/src/main/java/im/vector/app/features/invite/InviteUsersToRoomActivity.kt +++ b/vector/src/main/java/im/vector/app/features/invite/InviteUsersToRoomActivity.kt @@ -34,6 +34,7 @@ import im.vector.app.core.platform.SimpleFragmentActivity import im.vector.app.core.platform.WaitingViewData import im.vector.app.core.utils.PERMISSIONS_FOR_MEMBERS_SEARCH import im.vector.app.core.utils.checkPermissions +import im.vector.app.core.utils.onPermissionDeniedSnackbar import im.vector.app.core.utils.registerForPermissionsResult import im.vector.app.core.utils.toast import im.vector.app.features.contactsbook.ContactsBookFragment @@ -122,11 +123,11 @@ class InviteUsersToRoomActivity : SimpleFragmentActivity(), UserListViewModel.Fa } } - private val permissionContactLauncher = registerForPermissionsResult { allGranted -> + private val permissionContactLauncher = registerForPermissionsResult { allGranted, deniedPermanently -> if (allGranted) { doOnPostResume { addFragmentToBackstack(R.id.container, ContactsBookFragment::class.java) } - } else { - Toast.makeText(baseContext, R.string.missing_permissions_error, Toast.LENGTH_SHORT).show() + } else if (deniedPermanently) { + onPermissionDeniedSnackbar(R.string.permissions_denied_add_contact) } }