From d5717caf0ced008a9267dccd960e2026c637f197 Mon Sep 17 00:00:00 2001 From: Ashish Date: Wed, 20 Mar 2019 09:36:33 +0530 Subject: [PATCH 1/3] Bug Fix issue #2648 * Handled external storage permission before file download --- .../media/MediaDetailPagerFragment.java | 38 ++++++------ .../nrw/commons/utils/PermissionUtils.java | 58 ++++++++++++++++++- app/src/main/res/values/strings.xml | 3 +- 3 files changed, 77 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java index 1dadd05379..0254bb3d87 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java @@ -9,8 +9,6 @@ import android.os.Bundle; import android.os.Environment; import android.os.Handler; -import android.support.design.widget.Snackbar; -import android.support.v4.app.ActivityCompat; import android.support.v4.app.Fragment; import android.support.v4.app.FragmentManager; import android.support.v4.app.FragmentStatePagerAdapter; @@ -23,10 +21,6 @@ import android.view.View; import android.view.ViewGroup; import android.widget.Toast; - -import javax.inject.Inject; -import javax.inject.Named; - import butterknife.BindView; import butterknife.ButterKnife; import fr.free.nrw.commons.Media; @@ -43,10 +37,13 @@ import fr.free.nrw.commons.mwapi.MediaWikiApi; import fr.free.nrw.commons.utils.ImageUtils; import fr.free.nrw.commons.utils.NetworkUtils; +import fr.free.nrw.commons.utils.PermissionUtils; import fr.free.nrw.commons.utils.ViewUtil; +import javax.inject.Inject; +import javax.inject.Named; import timber.log.Timber; -import static android.Manifest.permission.READ_EXTERNAL_STORAGE; +import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE; import static android.content.Context.DOWNLOAD_SERVICE; import static android.content.Intent.ACTION_VIEW; import static android.content.pm.PackageManager.PERMISSION_GRANTED; @@ -238,19 +235,22 @@ private void downloadMedia(Media m) { req.allowScanningByMediaScanner(); req.setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M && - ContextCompat.checkSelfPermission(getContext(), READ_EXTERNAL_STORAGE) - != PERMISSION_GRANTED - && getView() != null) { - Snackbar.make(getView(), R.string.read_storage_permission_rationale, - Snackbar.LENGTH_INDEFINITE).setAction(R.string.ok, - view -> ActivityCompat.requestPermissions(getActivity(), - new String[]{READ_EXTERNAL_STORAGE}, 1)).show(); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M + && ContextCompat.checkSelfPermission(getContext(), WRITE_EXTERNAL_STORAGE) + != PERMISSION_GRANTED + && getView() != null) { + PermissionUtils.checkPermissionsAndPerformAction(getActivity(), WRITE_EXTERNAL_STORAGE, + () -> enqueueRequest(req), () -> Toast.makeText(getContext(),R.string.donwload_failed_we_cannot_download_the_file_without_storage_permission,Toast.LENGTH_SHORT).show(), R.string.storage_permission, R.string.write_storage_permission_rationale); } else { - DownloadManager systemService = (DownloadManager) getActivity().getSystemService(DOWNLOAD_SERVICE); - if (systemService != null) { - systemService.enqueue(req); - } + enqueueRequest(req); + } + } + + private void enqueueRequest(DownloadManager.Request req) { + DownloadManager systemService = + (DownloadManager) getActivity().getSystemService(DOWNLOAD_SERVICE); + if (systemService != null) { + systemService.enqueue(req); } } 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 index ac92f67299..8841b0343e 100644 --- a/app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java +++ b/app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java @@ -7,14 +7,12 @@ import android.provider.Settings; import android.support.annotation.StringRes; import android.support.v4.content.ContextCompat; - import com.karumi.dexter.Dexter; import com.karumi.dexter.PermissionToken; import com.karumi.dexter.listener.PermissionDeniedResponse; import com.karumi.dexter.listener.PermissionGrantedResponse; import com.karumi.dexter.listener.PermissionRequest; import com.karumi.dexter.listener.single.BasePermissionListener; - import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.R; @@ -102,4 +100,60 @@ public void onPermissionRationaleShouldBeShown(PermissionRequest permission, Per } }).check(); } + + /** + * Checks for a particular permission and runs the corresponding runnables to perform an action when the permission is granted/denied + * Also, it shows a rationale if needed + * + * Sample usage: + * + * PermissionUtils.checkPermissionsAndPerformAction(activity, + * Manifest.permission.WRITE_EXTERNAL_STORAGE, + * () -> initiateCameraUpload(activity), + * () -> showMessage(), + * R.string.storage_permission_title, + * R.string.write_storage_permission_rationale); + * + * + * @param activity activity requesting permissions + * @param permission the permission being requests + * @param onPermissionGranted the runnable to be executed when the permission is granted + * @param onPermissionDenied the runnable to be executed when the permission is denied(but not permanently) + * @param rationaleTitle rationale title to be displayed when permission was denied + * @param rationaleMessage rationale message to be displayed when permission was denied + */ + + public static void checkPermissionsAndPerformAction(Activity activity, String permission, + Runnable onPermissionGranted, Runnable onPermissionDenied, @StringRes int rationaleTitle, + @StringRes int rationaleMessage) { + Dexter.withActivity(activity) + .withPermission(permission) + .withListener(new BasePermissionListener() { + @Override public void onPermissionGranted(PermissionGrantedResponse response) { + onPermissionGranted.run(); + } + + @Override public void onPermissionDenied(PermissionDeniedResponse response) { + if (response.isPermanentlyDenied()) { + DialogUtil.showAlertDialog(activity, activity.getString(rationaleTitle), + activity.getString(rationaleMessage), + activity.getString(R.string.navigation_item_settings), null, + () -> askUserToManuallyEnablePermissionFromSettings(activity), null); + } else { + onPermissionDenied.run(); + } + } + + @Override + public void onPermissionRationaleShouldBeShown(PermissionRequest permission, + PermissionToken token) { + DialogUtil.showAlertDialog(activity, activity.getString(rationaleTitle), + activity.getString(rationaleMessage), + activity.getString(android.R.string.ok), + activity.getString(android.R.string.cancel), + token::continuePermissionRequest, token::cancelPermissionRequest); + } + }) + .check(); + } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 22a44c1ce3..567b4dfc16 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -165,7 +165,7 @@ Refresh Requesting Storage Permission Required permission: Read external storage. App cannot access your gallery without this. - Required permission: Write external storage. App cannot access your camera without this. + Required permission: Write external storage. App cannot access your camera/gallery without this. Optional permission: Get current location for category suggestions OK Nearby Places @@ -472,4 +472,5 @@ Upload your first media by touching the camera or gallery icon above. Choose Images to upload Please wait… + Download Failed!!. We cannot download the file without external storage permission. From e2e09667a286e4bd0ae7166e384f49839cd30967 Mon Sep 17 00:00:00 2001 From: Ashish Date: Wed, 20 Mar 2019 12:23:27 +0530 Subject: [PATCH 2/3] * Removed redudant check for permission in MediaDetailPagerFragment (Dexter already does that) * Removed duplicate code in PermissionUtil$checkPermissionsAndPerformAction, used the existing function with conditional extra parameters --- .../media/MediaDetailPagerFragment.java | 17 ++----- .../nrw/commons/utils/PermissionUtils.java | 46 ++++--------------- 2 files changed, 13 insertions(+), 50 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java index 0254bb3d87..9cf0e4861d 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java @@ -5,14 +5,12 @@ import android.content.Intent; import android.database.DataSetObserver; import android.net.Uri; -import android.os.Build; import android.os.Bundle; import android.os.Environment; import android.os.Handler; import android.support.v4.app.Fragment; import android.support.v4.app.FragmentManager; import android.support.v4.app.FragmentStatePagerAdapter; -import android.support.v4.content.ContextCompat; import android.support.v4.view.ViewPager; import android.view.LayoutInflater; import android.view.Menu; @@ -46,7 +44,6 @@ import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE; import static android.content.Context.DOWNLOAD_SERVICE; import static android.content.Intent.ACTION_VIEW; -import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.widget.Toast.LENGTH_SHORT; public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment implements ViewPager.OnPageChangeListener { @@ -234,16 +231,12 @@ private void downloadMedia(Media m) { // Modern Android updates the gallery automatically. Yay! req.allowScanningByMediaScanner(); req.setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED); + PermissionUtils.checkPermissionsAndPerformAction(getActivity(), WRITE_EXTERNAL_STORAGE, + () -> enqueueRequest(req), () -> Toast.makeText(getContext(), + R.string.donwload_failed_we_cannot_download_the_file_without_storage_permission, + Toast.LENGTH_SHORT).show(), R.string.storage_permission, + R.string.write_storage_permission_rationale); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M - && ContextCompat.checkSelfPermission(getContext(), WRITE_EXTERNAL_STORAGE) - != PERMISSION_GRANTED - && getView() != null) { - PermissionUtils.checkPermissionsAndPerformAction(getActivity(), WRITE_EXTERNAL_STORAGE, - () -> enqueueRequest(req), () -> Toast.makeText(getContext(),R.string.donwload_failed_we_cannot_download_the_file_without_storage_permission,Toast.LENGTH_SHORT).show(), R.string.storage_permission, R.string.write_storage_permission_rationale); - } else { - enqueueRequest(req); - } } private void enqueueRequest(DownloadManager.Request req) { 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 index 8841b0343e..798b5c9ded 100644 --- a/app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java +++ b/app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java @@ -62,43 +62,11 @@ public static boolean hasPermission(Activity activity, String permission) { * @param rationaleTitle rationale title to be displayed when permission was denied * @param rationaleMessage rationale message to be displayed when permission was denied */ - public static void checkPermissionsAndPerformAction(Activity activity, - String permission, - Runnable onPermissionGranted, - @StringRes int rationaleTitle, - @StringRes int rationaleMessage) { - Dexter.withActivity(activity) - .withPermission(permission) - .withListener(new BasePermissionListener() { - @Override - public void onPermissionGranted(PermissionGrantedResponse response) { - onPermissionGranted.run(); - } - - @Override - public void onPermissionDenied(PermissionDeniedResponse response) { - if (response.isPermanentlyDenied()) { - DialogUtil.showAlertDialog(activity, - activity.getString(rationaleTitle), - activity.getString(rationaleMessage), - activity.getString(R.string.navigation_item_settings), - null, - () -> askUserToManuallyEnablePermissionFromSettings(activity), - null); - } - } - - @Override - public void onPermissionRationaleShouldBeShown(PermissionRequest permission, PermissionToken token) { - DialogUtil.showAlertDialog(activity, - activity.getString(rationaleTitle), - activity.getString(rationaleMessage), - activity.getString(android.R.string.ok), - activity.getString(android.R.string.cancel), - token::continuePermissionRequest, - token::cancelPermissionRequest); - } - }).check(); + public static void checkPermissionsAndPerformAction(Activity activity, String permission, + Runnable onPermissionGranted, @StringRes int rationaleTitle, + @StringRes int rationaleMessage) { + checkPermissionsAndPerformAction(activity, permission, onPermissionGranted, null, + rationaleTitle, rationaleMessage); } /** @@ -140,7 +108,9 @@ public static void checkPermissionsAndPerformAction(Activity activity, String pe activity.getString(R.string.navigation_item_settings), null, () -> askUserToManuallyEnablePermissionFromSettings(activity), null); } else { - onPermissionDenied.run(); + if (null != onPermissionDenied) { + onPermissionDenied.run(); + } } } From f3cafc56461c6bbac06e700007264b3fe42ce102 Mon Sep 17 00:00:00 2001 From: Ashish Date: Wed, 20 Mar 2019 12:27:54 +0530 Subject: [PATCH 3/3] string name typo correction --- .../fr/free/nrw/commons/media/MediaDetailPagerFragment.java | 2 +- app/src/main/res/values/strings.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java index 9cf0e4861d..bae28c3a4d 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java @@ -233,7 +233,7 @@ private void downloadMedia(Media m) { req.setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED); PermissionUtils.checkPermissionsAndPerformAction(getActivity(), WRITE_EXTERNAL_STORAGE, () -> enqueueRequest(req), () -> Toast.makeText(getContext(), - R.string.donwload_failed_we_cannot_download_the_file_without_storage_permission, + R.string.download_failed_we_cannot_download_the_file_without_storage_permission, Toast.LENGTH_SHORT).show(), R.string.storage_permission, R.string.write_storage_permission_rationale); diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 567b4dfc16..c0954bd8b1 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -472,5 +472,5 @@ Upload your first media by touching the camera or gallery icon above. Choose Images to upload Please wait… - Download Failed!!. We cannot download the file without external storage permission. + Download Failed!!. We cannot download the file without external storage permission.