Fix issue with onRequestPermissionsResult not being called in Nearby …

…map and list (#1424) * Update changelog.md * Versioning for v2.7.0 * Call fragment method, not activity's * Initialize directUpload and controller in onCreate * Add logs * Change requestcodes in DirectUpload and NearbyMapFragment * Chain to super in default case (where request codes don't match activity's) * Controller must be initialized before directUpload * Fix whitespace, add comments * Alter request codes for Nearby List as well * Make permission rationales more specific * Add comments
commons-app · Apr 5, 2018 · f416689 · f416689
1 parent 2150d7f
commit f416689
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 36 deletions.
diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/DirectUpload.java b/app/src/main/java/fr/free/nrw/commons/nearby/DirectUpload.java
@@ -8,6 +8,7 @@
 
 import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.contributions.ContributionController;
+import timber.log.Timber;
 
 import static android.Manifest.permission.READ_EXTERNAL_STORAGE;
 import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE;
@@ -23,53 +24,56 @@ class DirectUpload {
         this.controller = controller;
     }
 
-    void initiateCameraUpload() {
+    // These permission requests will be handled by the Fragments.
+    // Do not use requestCode 1 as it will conflict with NearbyActivity's requestCodes
+    void initiateGalleryUpload() {
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-            if (ContextCompat.checkSelfPermission(fragment.getActivity(), WRITE_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
-                if (fragment.getActivity().shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) {
+            if (ContextCompat.checkSelfPermission(fragment.getActivity(), READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
+                if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) {
                     new AlertDialog.Builder(fragment.getActivity())
-                            .setMessage(fragment.getActivity().getString(R.string.write_storage_permission_rationale))
+                            .setMessage(fragment.getActivity().getString(R.string.read_storage_permission_rationale))
                             .setPositiveButton("OK", (dialog, which) -> {
-                                fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 3);
+                                Timber.d("Requesting permissions for read external storage");
+                                fragment.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, 4);
                                 dialog.dismiss();
                             })
                             .setNegativeButton("Cancel", null)
                             .create()
                             .show();
                 } else {
-                    fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 3);
+                    fragment.requestPermissions(new String[]{READ_EXTERNAL_STORAGE},
+                            4);
                 }
             } else {
-                controller.startCameraCapture();
+                controller.startGalleryPick();
             }
-        } else {
-            controller.startCameraCapture();
+        }
+        else {
+            controller.startGalleryPick();
         }
     }
 
-    void initiateGalleryUpload() {
+    void initiateCameraUpload() {
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-            if (ContextCompat.checkSelfPermission(fragment.getActivity(), READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
-                if (fragment.getActivity().shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) {
+            if (ContextCompat.checkSelfPermission(fragment.getActivity(), WRITE_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
+                if (fragment.shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) {
                     new AlertDialog.Builder(fragment.getActivity())
-                            .setMessage(fragment.getActivity().getString(R.string.read_storage_permission_rationale))
+                            .setMessage(fragment.getActivity().getString(R.string.write_storage_permission_rationale))
                             .setPositiveButton("OK", (dialog, which) -> {
-                                fragment.getActivity().requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, 1);
+                                fragment.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 5);
                                 dialog.dismiss();
                             })
                             .setNegativeButton("Cancel", null)
                             .create()
                             .show();
                 } else {
-                    fragment.getActivity().requestPermissions(new String[]{READ_EXTERNAL_STORAGE},
-                            1);
+                    fragment.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 5);
                 }
             } else {
-                controller.startGalleryPick();
+                controller.startCameraCapture();
             }
-        }
-        else {
-            controller.startGalleryPick();
+        } else {
+            controller.startCameraCapture();
         }
     }
 }
diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java
@@ -165,6 +165,11 @@ public void onRequestPermissionsResult(int requestCode, @NonNull String[] permis
                     showLocationPermissionDeniedErrorDialog();
                 }
             }
+            break;
+
+            default:
+                // This is needed to allow the request codes from the Fragments to be routed appropriately
+                super.onRequestPermissionsResult(requestCode, permissions, grantResults);
         }
     }
 

diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyListFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyListFragment.java
@@ -106,17 +106,17 @@ public void onRequestPermissionsResult(int requestCode, @NonNull String[] permis
         Timber.d("onRequestPermissionsResult: req code = " + " perm = " + permissions + " grant =" + grantResults);
 
         switch (requestCode) {
-            // 1 = "Read external storage" allowed when gallery selected
-            case 1: {
+            // 4 = "Read external storage" allowed when gallery selected
+            case 4: {
                 if (grantResults.length > 0 && grantResults[0] == PERMISSION_GRANTED) {
                     Timber.d("Call controller.startGalleryPick()");
                     controller.startGalleryPick();
                 }
             }
             break;
 
-            // 3 = "Write external storage" allowed when camera selected
-            case 3: {
+            // 5 = "Write external storage" allowed when camera selected
+            case 5: {
                 if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
                     Timber.d("Call controller.startCameraCapture()");
                     controller.startCameraCapture();

diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java
@@ -98,6 +98,7 @@ public class NearbyMapFragment extends DaggerFragment {
     private Animation fab_open;
     private Animation rotate_forward;
     private ContributionController controller;
+    private DirectUpload directUpload;
 
     private Place place;
     private Marker selected;
@@ -122,6 +123,10 @@ public NearbyMapFragment() {
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
+
+        controller = new ContributionController(this);
+        directUpload = new DirectUpload(this, controller);
+
         Bundle bundle = this.getArguments();
         Gson gson = new GsonBuilder()
                 .registerTypeAdapter(Uri.class, new UriDeserializer())
@@ -680,9 +685,6 @@ private void passInfoToSheet(Place place) {
         fabCamera.setOnClickListener(view -> {
             if (fabCamera.isShown()) {
                 Timber.d("Camera button tapped. Image title: " + place.getName() + "Image desc: " + place.getLongDescription());
-                controller = new ContributionController(this);
-
-                DirectUpload directUpload = new DirectUpload(this, controller);
                 storeSharedPrefs();
                 directUpload.initiateCameraUpload();
             }
@@ -691,9 +693,6 @@ private void passInfoToSheet(Place place) {
         fabGallery.setOnClickListener(view -> {
             if (fabGallery.isShown()) {
                 Timber.d("Gallery button tapped. Image title: " + place.getName() + "Image desc: " + place.getLongDescription());
-                controller = new ContributionController(this);
-
-                DirectUpload directUpload = new DirectUpload(this, controller);
                 storeSharedPrefs();
                 directUpload.initiateGalleryUpload();
             }
@@ -712,18 +711,19 @@ void storeSharedPrefs() {
     public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
         Timber.d("onRequestPermissionsResult: req code = " + " perm = " + permissions + " grant =" + grantResults);
 
+        // Do not use requestCode 1 as it will conflict with NearbyActivity's requestCodes
         switch (requestCode) {
-            // 1 = "Read external storage" allowed when gallery selected
-            case 1: {
+            // 4 = "Read external storage" allowed when gallery selected
+            case 4: {
                 if (grantResults.length > 0 && grantResults[0] == PERMISSION_GRANTED) {
                     Timber.d("Call controller.startGalleryPick()");
                     controller.startGalleryPick();
                 }
             }
             break;
 
-            // 3 = "Write external storage" allowed when camera selected
-            case 3: {
+            // 5 = "Write external storage" allowed when camera selected
+            case 5: {
                 if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
                     Timber.d("Call controller.startCameraCapture()");
                     controller.startCameraCapture();

diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
@@ -154,8 +154,8 @@
   <string name="detail_description_empty">No description</string>
   <string name="detail_license_empty">Unknown license</string>
   <string name="menu_refresh">Refresh</string>
-  <string name="read_storage_permission_rationale">Required permission: Read external storage. App cannot function without this.</string>
-  <string name="write_storage_permission_rationale">Required permission: Write external storage. App cannot function without this.</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="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>