From f4166899ca33eebb2205b661e4091af4a563fff3 Mon Sep 17 00:00:00 2001 From: Josephine Lim Date: Thu, 5 Apr 2018 23:31:18 +1000 Subject: [PATCH] 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 --- .../free/nrw/commons/nearby/DirectUpload.java | 60 ++++++++++--------- .../nrw/commons/nearby/NearbyActivity.java | 5 ++ .../commons/nearby/NearbyListFragment.java | 8 +-- .../nrw/commons/nearby/NearbyMapFragment.java | 20 +++---- app/src/main/res/values/strings.xml | 4 +- 5 files changed, 53 insertions(+), 44 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 index b58afa82a..7ab427b2d 100644 --- 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 android.support.v7.app.AlertDialog; 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,46 +24,25 @@ class DirectUpload { this.controller = controller; } - void initiateCameraUpload() { - 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)) { - new AlertDialog.Builder(fragment.getActivity()) - .setMessage(fragment.getActivity().getString(R.string.write_storage_permission_rationale)) - .setPositiveButton("OK", (dialog, which) -> { - fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 3); - dialog.dismiss(); - }) - .setNegativeButton("Cancel", null) - .create() - .show(); - } else { - fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 3); - } - } else { - controller.startCameraCapture(); - } - } else { - controller.startCameraCapture(); - } - } - + // 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(), READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) { - if (fragment.getActivity().shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) { + if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) { new AlertDialog.Builder(fragment.getActivity()) .setMessage(fragment.getActivity().getString(R.string.read_storage_permission_rationale)) .setPositiveButton("OK", (dialog, which) -> { - fragment.getActivity().requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, 1); + 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[]{READ_EXTERNAL_STORAGE}, - 1); + fragment.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, + 4); } } else { controller.startGalleryPick(); @@ -72,4 +52,28 @@ class DirectUpload { controller.startGalleryPick(); } } + + void initiateCameraUpload() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + 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.write_storage_permission_rationale)) + .setPositiveButton("OK", (dialog, which) -> { + fragment.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 5); + dialog.dismiss(); + }) + .setNegativeButton("Cancel", null) + .create() + .show(); + } else { + fragment.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 5); + } + } else { + controller.startCameraCapture(); + } + } 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 index 59a1d3ebc..36dee44a9 100644 --- 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 class NearbyActivity extends NavigationBaseActivity implements LocationUp 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 index dcc7f5e24..e46bd76e7 100644 --- 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,8 +106,8 @@ public class NearbyListFragment extends DaggerFragment { 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(); @@ -115,8 +115,8 @@ public class NearbyListFragment extends DaggerFragment { } 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 index e3e066ef5..9834f1a6c 100644 --- 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 class NearbyMapFragment extends DaggerFragment { @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 @@ public class NearbyMapFragment extends DaggerFragment { 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 @@ public class NearbyMapFragment extends DaggerFragment { 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,9 +711,10 @@ public class NearbyMapFragment extends DaggerFragment { 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(); @@ -722,8 +722,8 @@ public class NearbyMapFragment extends DaggerFragment { } 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 index 7b3931624..3a57a591a 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -154,8 +154,8 @@ No description Unknown license Refresh - Required permission: Read external storage. App cannot function without this. - Required permission: Write external storage. App cannot function without this. + Required permission: Read external storage. App cannot access your gallery without this. + Required permission: Write external storage. App cannot access your camera without this. Optional permission: Get current location for category suggestions OK Nearby Places