From 70099a9014c7b66ef37b3b1b84294d5f0618f8d7 Mon Sep 17 00:00:00 2001 From: Ashish Kumar Date: Thu, 6 Sep 2018 22:27:07 +0530 Subject: [PATCH] Feature/permissions library (#1855) * Added permission for Dexter, the runtime permission handling library * [Preparing fir issue #1773] Added a utility function which would take the user to app settings screen where he could manually give us the required permission * Added an alert dialog with positive and negative callback [Preparing fir issue #1773] * Improvements in the way External Storage Permission is handled in MultipleShareActivity[Bug fix #1697] 1. Used dexter to handle the external storage permission 2. Behaviour changes : When user tries to share(uppload) images to commons via MultipleShareActivity, following decision tree is followed a. If the app has permission for external storage, normal upload operation is followed b. If the app does not has the permission for external storage, dexter is used to ask for the same c. If the user gives us the required permission, normal upload flow is proceeded d. If the doesnot gives us the required permission a rationale dialog is shown with the appropriate message to let him know why we need the permission e. If he presses okay, steps a-c are followed and if he presses cancel, we close the app. f. If while asking for permission, the user chooses never ask again, then next time he tries to upload an image via MSA, the rational dialog follows the app setting screen where he could manually give us the required permission and the onActivityResult of same is handled * Added a Constants class to handle request and result codes from one place and other related constants common to the all app elements * replaced hardcoded strings ok and cancel in DialogUtil to string resources * init permission rationale dialog in activities onCreate * Code formatting, updated access modifiers wherever required, added javadocs for new methods created * *shifted constants to app class *Added JavaDocs in PermissionUtils * removed class REQUEST_CODES from CommonsApplication and instead put the enclosing constants in the App class itself --- app/build.gradle | 3 + .../free/nrw/commons/CommonsApplication.java | 9 ++ .../commons/upload/MultipleShareActivity.java | 120 +++++++++++++++--- .../fr/free/nrw/commons/utils/DialogUtil.java | 31 +++++ .../nrw/commons/utils/PermissionUtils.java | 23 ++++ app/src/main/res/values/strings.xml | 2 + 6 files changed, 172 insertions(+), 16 deletions(-) create mode 100644 app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java diff --git a/app/build.gradle b/app/build.gradle index 5ab6b479e..4a395787c 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -70,6 +70,9 @@ dependencies { releaseImplementation "com.squareup.leakcanary:leakcanary-android-no-op:$LEAK_CANARY" testImplementation "com.squareup.leakcanary:leakcanary-android-no-op:$LEAK_CANARY" + //For handling runtime permissions + implementation 'com.karumi:dexter:5.0.0' + } android { diff --git a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java index 51b8b0b0f..2ee5f305f 100644 --- a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java +++ b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java @@ -54,6 +54,11 @@ public class CommonsApplication extends Application { @Inject @Named("application_preferences") SharedPreferences applicationPrefs; @Inject @Named("prefs") SharedPreferences otherPrefs; + /** + * Constants begin + */ + public static final int OPEN_APPLICATION_DETAIL_SETTINGS = 1001; + public static final String DEFAULT_EDIT_SUMMARY = "Uploaded using [[COM:MOA|Commons Mobile App]]"; public static final String FEEDBACK_EMAIL = "commons-app-android@googlegroups.com"; @@ -66,6 +71,10 @@ public class CommonsApplication extends Application { public static final String NOTIFICATION_CHANNEL_ID_ALL = "CommonsNotificationAll"; + /** + * Constants End + */ + private RefWatcher refWatcher; diff --git a/app/src/main/java/fr/free/nrw/commons/upload/MultipleShareActivity.java b/app/src/main/java/fr/free/nrw/commons/upload/MultipleShareActivity.java index e429c3ee8..a35eb46ee 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/MultipleShareActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/MultipleShareActivity.java @@ -1,6 +1,8 @@ package fr.free.nrw.commons.upload; import android.Manifest; +import android.Manifest.permission; +import android.app.AlertDialog; import android.app.ProgressDialog; import android.content.ContentResolver; import android.content.Context; @@ -12,7 +14,6 @@ import android.net.Uri; import android.os.Build; import android.os.Bundle; import android.os.ParcelFileDescriptor; -import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.v4.app.ActivityCompat; import android.support.v4.app.FragmentManager; @@ -23,6 +24,15 @@ import android.view.inputmethod.InputMethodManager; import android.widget.AdapterView; import android.widget.Toast; +import com.karumi.dexter.Dexter; +import com.karumi.dexter.DexterBuilder; +import com.karumi.dexter.listener.PermissionDeniedResponse; +import com.karumi.dexter.listener.PermissionGrantedResponse; +import com.karumi.dexter.listener.single.BasePermissionListener; +import fr.free.nrw.commons.CommonsApplication; +import fr.free.nrw.commons.utils.DialogUtil; +import fr.free.nrw.commons.utils.DialogUtil.Callback; +import fr.free.nrw.commons.utils.PermissionUtils; import java.io.FileNotFoundException; import java.util.ArrayList; import java.util.List; @@ -41,7 +51,6 @@ import fr.free.nrw.commons.category.OnCategoriesSaveHandler; import fr.free.nrw.commons.contributions.Contribution; import fr.free.nrw.commons.media.MediaDetailPagerFragment; import fr.free.nrw.commons.modifications.CategoryModifier; -import fr.free.nrw.commons.modifications.ModificationsContentProvider; import fr.free.nrw.commons.modifications.ModifierSequence; import fr.free.nrw.commons.modifications.ModifierSequenceDao; import fr.free.nrw.commons.modifications.TemplateRemoveModifier; @@ -81,6 +90,11 @@ public class MultipleShareActivity extends AuthenticatedActivity private boolean locationPermitted = false; private boolean isMultipleUploadsPrepared = false; private boolean isMultipleUploadsFinalised = false; // Checks is user clicked to upload button or regret before this phase + private final String TAG="#MultipleShareActivity#"; + private AlertDialog storagePermissionInfoDialog; + private DexterBuilder dexterStoragePermissionBuilder; + + private PermissionDeniedResponse permissionDeniedResponse; @Override public Media getMediaAtPosition(int i) { @@ -124,17 +138,6 @@ public class MultipleShareActivity extends AuthenticatedActivity multipleUploadBegins(); } - @Override - public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { - if (requestCode == 1 && grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) { - Timber.d("onRequestPermissionsResult external storage permission granted"); - prepareMultipleUpoadList(); - } else { - // Permission is not granted, close activity - finish(); - } - } - private void multipleUploadBegins() { Timber.d("Multiple upload begins"); @@ -216,6 +219,7 @@ public class MultipleShareActivity extends AuthenticatedActivity setContentView(R.layout.activity_multiple_uploads); ButterKnife.bind(this); initDrawer(); + initPermissionsRationaleDialog(); if (savedInstanceState != null) { photosList = savedInstanceState.getParcelableArrayList("uploadsList"); @@ -233,6 +237,47 @@ public class MultipleShareActivity extends AuthenticatedActivity } } + + /** + * We have agreed to show a dialog showing why we need a particular permission. + * This method is used to initialise the dialog which is going to show the permission's rationale. + * The dialog is initialised along with a callback for positive and negative user actions. + */ + private void initPermissionsRationaleDialog() { + if (storagePermissionInfoDialog == null) { + storagePermissionInfoDialog = DialogUtil + .getAlertDialogWithPositiveAndNegativeCallbacks( + MultipleShareActivity.this, + getString(R.string.storage_permission), getString( + R.string.write_storage_permission_rationale_for_image_share), + R.drawable.ic_launcher, new Callback() { + @Override + public void onPositiveButtonClicked() { + //If the user is willing to give us the permission + //But had somehow previously choose never ask again, we take him to app settings to manually enable permission + if(null== permissionDeniedResponse){ + //Dexter returned null, lets see if this ever happens + return; + } + else if (permissionDeniedResponse.isPermanentlyDenied()) { + PermissionUtils.askUserToManuallyEnablePermissionFromSettings(MultipleShareActivity.this); + } else { + //or if we still have chance to show runtime permission dialog, we show him that. + askDexterToHandleExternalStoragePermission(); + } + } + + @Override + public void onNegativeButtonClicked() { + //This was the behaviour as of now, I was planning to maybe snack him with some message + //and then call finish after some time, or may be it could be associated with some action on the snack + //If the user does not want us to give the permission, even after showing rationale dialog, lets not trouble him anymore + finish(); + } + }); + } + } + @Override protected void onDestroy() { super.onDestroy(); @@ -275,12 +320,55 @@ public class MultipleShareActivity extends AuthenticatedActivity isMultipleUploadsPrepared = false; mwApi.setAuthCookie(authCookie); if (!ExternalStorageUtils.isStoragePermissionGranted(this)) { - ExternalStorageUtils.requestExternalStoragePermission(this); + //If permission is not there, handle the negative cases + askDexterToHandleExternalStoragePermission(); isMultipleUploadsPrepared = false; return; // Postpone operation to do after gettion permission } else { isMultipleUploadsPrepared = true; - prepareMultipleUpoadList(); + prepareMultipleUploadList(); + } + } + + /** + * This method initialised the Dexter's permission builder (if not already initialised). Also makes sure that the builder is initialised + * only once, otherwise we would'nt know on which instance of it, the user is working on. And after the builder is initialised, it checks for the required + * permission and then handles the permission status, thanks to Dexter's appropriate callbacks. + */ + private void askDexterToHandleExternalStoragePermission() { + Timber.d(TAG, "External storage permission is being requested"); + if (null == dexterStoragePermissionBuilder) { + dexterStoragePermissionBuilder = Dexter.withActivity(this) + .withPermission(permission.WRITE_EXTERNAL_STORAGE) + .withListener(new BasePermissionListener() { + @Override + public void onPermissionGranted(PermissionGrantedResponse response) { + Timber.d(TAG,"User has granted us the permission for writing the external storage"); + //If permission is granted, well and good + prepareMultipleUploadList(); + } + + @Override + public void onPermissionDenied(PermissionDeniedResponse response) { + Timber.d(TAG,"User has granted us the permission for writing the external storage"); + //If permission is not granted in whatsoever scenario, we show him a dialog stating why we need the permission + permissionDeniedResponse=response; + if (null != storagePermissionInfoDialog && !storagePermissionInfoDialog + .isShowing()) { + storagePermissionInfoDialog.show(); + } + } + }); + } + dexterStoragePermissionBuilder.check(); + } + + @Override + protected void onActivityResult(int requestCode, int resultCode, Intent data) { + super.onActivityResult(requestCode, resultCode, data); + if (requestCode == CommonsApplication.OPEN_APPLICATION_DETAIL_SETTINGS) { + //OnActivity result, no matter what the result is, our function can handle that. + askDexterToHandleExternalStoragePermission(); } } @@ -288,7 +376,7 @@ public class MultipleShareActivity extends AuthenticatedActivity * Prepares a list from files will be uploaded. Saves these files temporarily to external * storage. Adds them to uploads list */ - private void prepareMultipleUpoadList() { + private void prepareMultipleUploadList() { Intent intent = getIntent(); if (Intent.ACTION_SEND_MULTIPLE.equals(intent.getAction())) { diff --git a/app/src/main/java/fr/free/nrw/commons/utils/DialogUtil.java b/app/src/main/java/fr/free/nrw/commons/utils/DialogUtil.java index 78c1ca155..2e4592e40 100644 --- a/app/src/main/java/fr/free/nrw/commons/utils/DialogUtil.java +++ b/app/src/main/java/fr/free/nrw/commons/utils/DialogUtil.java @@ -1,12 +1,16 @@ package fr.free.nrw.commons.utils; import android.app.Activity; +import android.app.AlertDialog; +import android.app.AlertDialog.Builder; import android.app.Dialog; +import android.content.Context; import android.os.Build; import android.support.annotation.Nullable; import android.support.v4.app.DialogFragment; import android.support.v4.app.FragmentActivity; +import fr.free.nrw.commons.R; import timber.log.Timber; public class DialogUtil { @@ -92,4 +96,31 @@ public class DialogUtil { Timber.e(e, "Could not show dialog."); } } + + public static AlertDialog getAlertDialogWithPositiveAndNegativeCallbacks( + Context context, String title, String message, int iconResourceId, Callback callback) { + + AlertDialog alertDialog = new Builder(context) + .setTitle(title) + .setMessage(message) + .setPositiveButton(context.getString(R.string.ok), (dialog, which) -> { + callback.onPositiveButtonClicked(); + dialog.dismiss(); + }) + .setNegativeButton(context.getString(R.string.cancel), (dialog, which) -> { + callback.onNegativeButtonClicked(); + dialog.dismiss(); + }) + .setIcon(iconResourceId).create(); + + return alertDialog; + + } + + public interface Callback { + + void onPositiveButtonClicked(); + + void onNegativeButtonClicked(); + } } diff --git a/app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java b/app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java new file mode 100644 index 000000000..ecdc01511 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java @@ -0,0 +1,23 @@ +package fr.free.nrw.commons.utils; + +import android.app.Activity; +import android.content.Intent; +import android.net.Uri; +import android.provider.Settings; +import fr.free.nrw.commons.CommonsApplication; + +public class PermissionUtils { + + /** + * This method can be used by any activity which requires a permission which has been blocked(marked never ask again by the user) + It open the app settings from where the user can manually give us the required permission. + * @param activity + */ + public static void askUserToManuallyEnablePermissionFromSettings( + Activity activity) { + Intent intent = new Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS); + Uri uri = Uri.fromParts("package", activity.getPackageName(), null); + intent.setData(uri); + activity.startActivityForResult(intent,CommonsApplication.OPEN_APPLICATION_DETAIL_SETTINGS); + } +} diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 14dff9500..7f5f8f4a2 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -352,4 +352,6 @@ The percentage of images you have uploaded to Commons that were not deleted The number of images you have uploaded to Commons that were used in Wikimedia articles Commons Notification + Storage Permission + We need your permission to access the external storage of your device in order to upload images.