Bug Fix issue #2648 (#2678)

* Bug Fix issue #2648
* Handled external storage permission before file download

* * 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

* string name typo correction

app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java

@@ -5,16 +5,12 @@ import android.app.DownloadManager;
 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.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;
-import android.support.v4.content.ContextCompat;
 import android.support.v4.view.ViewPager;
 import android.view.LayoutInflater;
 import android.view.Menu;
@@ -23,10 +19,6 @@ import android.view.MenuItem;
 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,13 +35,15 @@ import fr.free.nrw.commons.kvstore.BasicKvStore;
 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;
 import static android.widget.Toast.LENGTH_SHORT;
 
 public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment implements ViewPager.OnPageChangeListener {
@@ -237,20 +231,19 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple
         // 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.download_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(), 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();
-        } else {
-            DownloadManager systemService = (DownloadManager) getActivity().getSystemService(DOWNLOAD_SERVICE);
-            if (systemService != null) {
-                systemService.enqueue(req);
-            }
+    }
+
+    private void enqueueRequest(DownloadManager.Request req) {
+        DownloadManager systemService =
+            (DownloadManager) getActivity().getSystemService(DOWNLOAD_SERVICE);
+        if (systemService != null) {
+            systemService.enqueue(req);
         }
     }
 

app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java

@@ -7,14 +7,12 @@ import android.net.Uri;
 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;
 
@@ -64,42 +62,68 @@ public class PermissionUtils {
      * @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();
-                    }
+    public static void checkPermissionsAndPerformAction(Activity activity, String permission,
+        Runnable onPermissionGranted, @StringRes int rationaleTitle,
+        @StringRes int rationaleMessage) {
+        checkPermissionsAndPerformAction(activity, permission, onPermissionGranted, null,
+            rationaleTitle, rationaleMessage);
+    }
 
-                    @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);
+    /**
+     * 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 {
+                        if (null != onPermissionDenied) {
+                            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();
+                @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();
     }
 }

app/src/main/res/values/strings.xml

@@ -165,7 +165,7 @@
   <string name="menu_refresh">Refresh</string>
   <string name="storage_permission_title">Requesting Storage Permission</string>
   <string name="read_storage_permission_rationale">Required permission: Read external storage. App cannot access your gallery without this.</string>
-  <string name="write_storage_permission_rationale">Required permission: Write external storage. App cannot access your camera without this.</string>
+  <string name="write_storage_permission_rationale">Required permission: Write external storage. App cannot access your camera/gallery without this.</string>
   <string name="location_permission_rationale">Optional permission: Get current location for category suggestions</string>
   <string name="ok">OK</string>
   <string name="title_activity_nearby">Nearby Places</string>
@@ -472,4 +472,5 @@ Upload your first media by touching the camera or gallery icon above.</string>
   <string name="image_chooser_title">Choose Images to upload</string>
 
   <string name="please_wait">Please wait…</string>
+  <string name="download_failed_we_cannot_download_the_file_without_storage_permission">Download Failed!!. We cannot download the file without external storage permission.</string>
 </resources>