From 11c3772dd003287cd93ba0d35201e7b6cd759ff5 Mon Sep 17 00:00:00 2001 From: Vivek Maskara Date: Fri, 17 Aug 2018 22:01:10 +0530 Subject: [PATCH 01/21] Add Traceur for getting meaningful RxJava stack traces (#1832) --- app/build.gradle | 1 + app/src/main/java/fr/free/nrw/commons/CommonsApplication.java | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/app/build.gradle b/app/build.gradle index d6bdede1b0..1802eba338 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -62,6 +62,7 @@ dependencies { testImplementation 'com.squareup.okhttp3:mockwebserver:3.8.1' implementation 'com.dinuscxj:circleprogressbar:1.1.1' + implementation 'com.tspoon.traceur:traceur:1.0.1' implementation 'com.caverock:androidsvg:1.2.1' implementation 'com.github.bumptech.glide:glide:4.7.1' kapt 'com.github.bumptech.glide:compiler:4.7.1' 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 cb47f75e9a..2ceb22d06a 100644 --- a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java +++ b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java @@ -10,6 +10,8 @@ import com.facebook.stetho.Stetho; import com.squareup.leakcanary.LeakCanary; import com.squareup.leakcanary.RefWatcher; +import com.tspoon.traceur.Traceur; +import com.tspoon.traceur.TraceurConfig; import org.acra.ACRA; import org.acra.ReportingInteractionMode; @@ -69,6 +71,8 @@ public class CommonsApplication extends MultiDexApplication { @Override public void onCreate() { super.onCreate(); + Traceur.enableLogging(); + ApplicationlessInjection .getInstance(this) .getCommonsApplicationComponent() From 4c476e7a064102599daad0b59ce538de752ec77a Mon Sep 17 00:00:00 2001 From: neslihanturan Date: Mon, 20 Aug 2018 11:32:45 +0300 Subject: [PATCH 02/21] Hotfix for overwrite issue in 2.8.0 (#1838) * This solution is an hotfix for overrite issue came back on 2.8.0 version. What I did is checking the extension, and if it is null, adding .jpg suffix. Because commons files always have suffixes, and we should compare file names after adding suffixes. Othervise overrides are possible. * Check if file title includes an extension already, by checking if is there any dot in it. * Fix logic error * Add uncovered tests * Remove unecessary line breaks * Make Javadocs more explicit --- app/src/main/java/fr/free/nrw/commons/Utils.java | 11 ++++++++++- .../fr/free/nrw/commons/UtilsFixExtensionTest.kt | 10 ++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/fr/free/nrw/commons/Utils.java b/app/src/main/java/fr/free/nrw/commons/Utils.java index 4e4b46b01f..5f829abfce 100644 --- a/app/src/main/java/fr/free/nrw/commons/Utils.java +++ b/app/src/main/java/fr/free/nrw/commons/Utils.java @@ -111,7 +111,7 @@ public static int licenseNameFor(String license) { } /** - * Fixing incorrect extension + * Adds extension to filename. Converts to .jpg if system provides .jpeg, adds .jpg if no extension detected * @param title File name * @param extension Correct extension * @return File with correct extension @@ -128,6 +128,15 @@ public static String fixExtension(String title, String extension) { .endsWith("." + extension.toLowerCase(Locale.ENGLISH))) { title += "." + extension; } + + // If extension is still null, make it jpg. (Hotfix for https://github.com/commons-app/apps-android-commons/issues/228) + // If title has an extension in it, if won't be true + // FIXME: .png uploads fail when uploaded via Share + if (extension == null && title.lastIndexOf(".")<=0) { + extension = "jpg"; + title += "." + extension; + } + return title; } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/UtilsFixExtensionTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/UtilsFixExtensionTest.kt index 0082b9d284..1f3b127fdc 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/UtilsFixExtensionTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/UtilsFixExtensionTest.kt @@ -65,4 +65,14 @@ class UtilsFixExtensionTest { fun inWordJpegToJpgResultsInJpg() { assertEquals("X.jpeg.SAMPLE.jpg", fixExtension("X.jpeg.SAMPLE", "jpg")) } + + @Test + fun noExtensionShouldResultInJpg() { + assertEquals("Sample.jpg", fixExtension("Sample", null)) + } + + @Test + fun extensionAlreadyInTitleShouldRemain() { + assertEquals("Sample.jpg", fixExtension("Sample.jpg", null)) + } } From b743d021f079b02581b24f698d898317b034f43c Mon Sep 17 00:00:00 2001 From: Josephine Lim Date: Mon, 20 Aug 2018 20:07:29 +1000 Subject: [PATCH 03/21] Versioning and changelog for v2.8.2 (#1842) * Versioning for v2.8.2 * Changelog for v2.8.2 --- CHANGELOG.md | 3 +++ app/build.gradle | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60562d7519..b3ecfa8af5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Wikimedia Commons for Android +## v2.8.2 +- Fixed bug with uploads sent via Share being given .jpeg extensions and overwriting files of the same name + ## v2.8.1 - Fixed bug with category edits not being sent to server diff --git a/app/build.gradle b/app/build.gradle index 1802eba338..16dad0e6bd 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -90,8 +90,8 @@ android { defaultConfig { applicationId 'fr.free.nrw.commons' - versionCode 88 - versionName '2.8.1' + versionCode 89 + versionName '2.8.2' setProperty("archivesBaseName", "app-commons-v$versionName-" + getBranchName()) minSdkVersion project.minSdkVersion From 9c890ecd1df10a58b608143d9f96002031382345 Mon Sep 17 00:00:00 2001 From: misaochan Date: Sat, 8 Dec 2018 23:00:53 +1000 Subject: [PATCH 04/21] Delete unused MaterialShowcase class --- .../nearby/NearbyMaterialShowcaseSequence.java | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java deleted file mode 100644 index c6e46611d9..0000000000 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java +++ /dev/null @@ -1,18 +0,0 @@ -package fr.free.nrw.commons.nearby; - -import android.app.Activity; - -import uk.co.deanwild.materialshowcaseview.MaterialShowcaseSequence; -import uk.co.deanwild.materialshowcaseview.ShowcaseConfig; - - -public class NearbyMaterialShowcaseSequence extends MaterialShowcaseSequence { - - public NearbyMaterialShowcaseSequence(Activity activity, String sequenceID) { - super(activity, sequenceID); - ShowcaseConfig config = new ShowcaseConfig(); - config.setDelay(500); // half second between each showcase view - this.setConfig(config); - this.singleUse(sequenceID); // Display tutorial only once - } -} From 656a46c81b4a688663ace435d123735ccff8f533 Mon Sep 17 00:00:00 2001 From: misaochan Date: Sat, 8 Dec 2018 23:42:58 +1000 Subject: [PATCH 05/21] Add Javadocs and fix lint errors for DirectUpload.java --- .../free/nrw/commons/nearby/DirectUpload.java | 87 +++++++++++-------- 1 file changed, 51 insertions(+), 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 index 5d82b5492b..592d79e527 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 @@ -1,5 +1,6 @@ package fr.free.nrw.commons.nearby; +import android.app.Activity; import android.os.Build; import android.support.v4.app.Fragment; import android.support.v4.content.ContextCompat; @@ -15,6 +16,9 @@ import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE; import static android.content.pm.PackageManager.PERMISSION_GRANTED; +/** + * This class handles the uploads made from a Nearby Place + */ class DirectUpload { private ContributionController controller; @@ -25,59 +29,70 @@ class DirectUpload { this.controller = controller; } - // These permission requests will be handled by the Fragments. - // Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes + /** + * Initiates the upload if user selects the Gallery FAB. + * The permission requests will be handled by the Fragments. + * Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes. + */ void initiateGalleryUpload() { - Log.d("deneme7","initiateGalleryUpload"); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - 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.read_storage_permission_rationale)) - .setPositiveButton(android.R.string.ok, (dialog, which) -> { - Timber.d("Requesting permissions for read external storage"); - fragment.getActivity().requestPermissions - (new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); - dialog.dismiss(); - }) - .setNegativeButton(android.R.string.cancel, null) - .create() - .show(); + Activity parentActivity = fragment.getActivity(); + if (parentActivity != null) { + if (ContextCompat.checkSelfPermission(parentActivity, READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) { + if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) { + new AlertDialog.Builder(fragment.getActivity()) + .setMessage(fragment.getActivity().getString(R.string.read_storage_permission_rationale)) + .setPositiveButton(android.R.string.ok, (dialog, which) -> { + Timber.d("Requesting permissions for read external storage"); + fragment.getActivity().requestPermissions + (new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); + dialog.dismiss(); + }) + .setNegativeButton(android.R.string.cancel, null) + .create() + .show(); + } else { + fragment.getActivity().requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); + } } else { - fragment.getActivity().requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); + controller.startSingleGalleryPick(); } } else { controller.startSingleGalleryPick(); } } - else { - controller.startSingleGalleryPick(); - } } + /** + * Initiates the upload if user selects the Camera FAB. + * The permission requests will be handled by the Fragments. + * Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes. + */ void initiateCameraUpload() { - Log.d("deneme7","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(android.R.string.ok, (dialog, which) -> { - fragment.getActivity().requestPermissions - (new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); - dialog.dismiss(); - }) - .setNegativeButton(android.R.string.cancel, null) - .create() - .show(); + Activity parentActivity = fragment.getActivity(); + if (parentActivity != null) { + if (ContextCompat.checkSelfPermission(parentActivity, 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(android.R.string.ok, (dialog, which) -> { + fragment.getActivity().requestPermissions + (new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); + dialog.dismiss(); + }) + .setNegativeButton(android.R.string.cancel, null) + .create() + .show(); + } else { + fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); + } } else { - fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); + controller.startCameraCapture(); } } else { controller.startCameraCapture(); } - } else { - controller.startCameraCapture(); } } } From bb7983e16cbc793981b9512ea394d2777f7671fa Mon Sep 17 00:00:00 2001 From: misaochan Date: Sat, 8 Dec 2018 23:44:08 +1000 Subject: [PATCH 06/21] Fix whitespace and add docs --- .../java/fr/free/nrw/commons/nearby/DirectUpload.java | 8 +++----- 1 file changed, 3 insertions(+), 5 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 592d79e527..ce37c548ad 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 @@ -17,7 +17,7 @@ import static android.content.pm.PackageManager.PERMISSION_GRANTED; /** - * This class handles the uploads made from a Nearby Place + * This class handles the uploads made from a Nearby Place, in both the list and map views. */ class DirectUpload { @@ -44,8 +44,7 @@ void initiateGalleryUpload() { .setMessage(fragment.getActivity().getString(R.string.read_storage_permission_rationale)) .setPositiveButton(android.R.string.ok, (dialog, which) -> { Timber.d("Requesting permissions for read external storage"); - fragment.getActivity().requestPermissions - (new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); + fragment.getActivity().requestPermissions (new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); dialog.dismiss(); }) .setNegativeButton(android.R.string.cancel, null) @@ -77,8 +76,7 @@ void initiateCameraUpload() { new AlertDialog.Builder(fragment.getActivity()) .setMessage(fragment.getActivity().getString(R.string.write_storage_permission_rationale)) .setPositiveButton(android.R.string.ok, (dialog, which) -> { - fragment.getActivity().requestPermissions - (new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); + fragment.getActivity().requestPermissions (new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); dialog.dismiss(); }) .setNegativeButton(android.R.string.cancel, null) From 5b38f9c13764b45df25ed2bd6c2070618930fcb8 Mon Sep 17 00:00:00 2001 From: misaochan Date: Sat, 8 Dec 2018 23:49:11 +1000 Subject: [PATCH 07/21] Replace fragment.getActivity() with the parentActivity var --- .../fr/free/nrw/commons/nearby/DirectUpload.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 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 ce37c548ad..3ca0603e86 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 @@ -40,18 +40,18 @@ void initiateGalleryUpload() { if (parentActivity != null) { if (ContextCompat.checkSelfPermission(parentActivity, READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) { if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) { - new AlertDialog.Builder(fragment.getActivity()) - .setMessage(fragment.getActivity().getString(R.string.read_storage_permission_rationale)) + new AlertDialog.Builder(parentActivity) + .setMessage(parentActivity.getString(R.string.read_storage_permission_rationale)) .setPositiveButton(android.R.string.ok, (dialog, which) -> { Timber.d("Requesting permissions for read external storage"); - fragment.getActivity().requestPermissions (new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); + parentActivity.requestPermissions (new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); dialog.dismiss(); }) .setNegativeButton(android.R.string.cancel, null) .create() .show(); } else { - fragment.getActivity().requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); + parentActivity.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); } } else { controller.startSingleGalleryPick(); @@ -73,17 +73,17 @@ void initiateCameraUpload() { if (parentActivity != null) { if (ContextCompat.checkSelfPermission(parentActivity, 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)) + new AlertDialog.Builder(parentActivity) + .setMessage(parentActivity.getString(R.string.write_storage_permission_rationale)) .setPositiveButton(android.R.string.ok, (dialog, which) -> { - fragment.getActivity().requestPermissions (new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); + parentActivity.requestPermissions (new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); dialog.dismiss(); }) .setNegativeButton(android.R.string.cancel, null) .create() .show(); } else { - fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); + parentActivity.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); } } else { controller.startCameraCapture(); From 238114ea1ebd4ddcff822d7cedb65cfac29bfd3a Mon Sep 17 00:00:00 2001 From: misaochan Date: Sun, 9 Dec 2018 00:02:41 +1000 Subject: [PATCH 08/21] Rename unnecessarily-overloaded method getFromWikidataQuery(), add Javadocs --- .../nrw/commons/nearby/NearbyController.java | 3 +-- .../free/nrw/commons/nearby/NearbyPlaces.java | 22 +++++++++++-------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java index abf02362f2..539763b5ea 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java @@ -5,7 +5,6 @@ import android.content.res.Resources; import android.graphics.Bitmap; import android.support.graphics.drawable.VectorDrawableCompat; -import android.util.Log; import com.mapbox.mapboxsdk.annotations.IconFactory; @@ -61,7 +60,7 @@ public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng lat Timber.d("Loading attractions neari, but curLatLng is null"); return null; } - List places = nearbyPlaces.getFromWikidataQuery(latLangToSearchAround, Locale.getDefault().getLanguage(), returnClosestResult); + List places = nearbyPlaces.radiusExpander(latLangToSearchAround, Locale.getDefault().getLanguage(), returnClosestResult); if (null != places && places.size() > 0) { LatLng[] boundaryCoordinates = {places.get(0).location, // south diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java index 74af35de58..56ed9f0292 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java @@ -41,13 +41,19 @@ public NearbyPlaces() { } } - List getFromWikidataQuery(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException { + /** + * Expands the radius as needed for the Wikidata query + * @param curLatLng user's location + * @param lang user's language + * @param returnClosestResult true if only the nearest point is desired + * @return list of places obtained + * @throws IOException if query fails + */ + List radiusExpander(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException { List places = Collections.emptyList(); - /** - * If returnClosestResult is true, then this means that we are trying to get closest point - * to use in cardView above contributions list - */ + // If returnClosestResult is true, then this means that we are trying to get closest point + // to use in cardView in Contributions fragment if (returnClosestResult) { MIN_RESULTS = 1; // Return closest nearby place MAX_RADIUS = 5; // Return places only in 5 km area @@ -58,7 +64,7 @@ List getFromWikidataQuery(LatLng curLatLng, String lang, boolean returnCl radius = INITIAL_RADIUS; } - // increase the radius gradually to find a satisfactory number of nearby places + // Increase the radius gradually to find a satisfactory number of nearby places while (radius <= MAX_RADIUS) { try { places = getFromWikidataQuery(curLatLng, lang, radius); @@ -81,9 +87,7 @@ List getFromWikidataQuery(LatLng curLatLng, String lang, boolean returnCl return places; } - private List getFromWikidataQuery(LatLng cur, - String lang, - double radius) + private List getFromWikidataQuery(LatLng cur, String lang, double radius) throws IOException { List places = new ArrayList<>(); From c32e8d20f068c28223c35af7c51b0d88f860e067 Mon Sep 17 00:00:00 2001 From: misaochan Date: Sun, 9 Dec 2018 00:06:52 +1000 Subject: [PATCH 09/21] Javadocs and whitespaces for NearbyPlaces.java --- .../fr/free/nrw/commons/nearby/NearbyPlaces.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java index 56ed9f0292..5e88fd0b0f 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java @@ -43,7 +43,7 @@ public NearbyPlaces() { /** * Expands the radius as needed for the Wikidata query - * @param curLatLng user's location + * @param curLatLng coordinates of search location * @param lang user's language * @param returnClosestResult true if only the nearest point is desired * @return list of places obtained @@ -83,12 +83,18 @@ List radiusExpander(LatLng curLatLng, String lang, boolean returnClosestR if (radius > MAX_RADIUS) { radius = MAX_RADIUS; } - return places; } - private List getFromWikidataQuery(LatLng cur, String lang, double radius) - throws IOException { + /** + * Runs the Wikidata query to populate the Places around search location + * @param cur coordinates of search location + * @param lang user's language + * @param radius radius for search, as determined by radiusExpander() + * @return list of places obtained + * @throws IOException if query fails + */ + private List getFromWikidataQuery(LatLng cur, String lang, double radius) throws IOException { List places = new ArrayList<>(); String query = wikidataQuery @@ -168,5 +174,4 @@ private List getFromWikidataQuery(LatLng cur, String lang, double radius) return places; } - } From 8ebe9b9235634ab855e3f10497a2843b42129a78 Mon Sep 17 00:00:00 2001 From: misaochan Date: Sun, 9 Dec 2018 00:10:33 +1000 Subject: [PATCH 10/21] Use local vars where possible instead of class fields. Non-constants should not be in all caps --- .../free/nrw/commons/nearby/NearbyPlaces.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java index 5e88fd0b0f..cbca81f042 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java @@ -23,7 +23,6 @@ public class NearbyPlaces { - private static int MIN_RESULTS = 40; private static final double INITIAL_RADIUS = 1.0; // in kilometers private static double MAX_RADIUS = 300.0; // in kilometers private static final double RADIUS_MULTIPLIER = 1.618; @@ -50,22 +49,25 @@ public NearbyPlaces() { * @throws IOException if query fails */ List radiusExpander(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException { + + int minResults; + double maxRadius; List places = Collections.emptyList(); // If returnClosestResult is true, then this means that we are trying to get closest point // to use in cardView in Contributions fragment if (returnClosestResult) { - MIN_RESULTS = 1; // Return closest nearby place - MAX_RADIUS = 5; // Return places only in 5 km area + minResults = 1; // Return closest nearby place + maxRadius = 5; // Return places only in 5 km area radius = INITIAL_RADIUS; // refresh radius again, otherwise increased radius is grater than MAX_RADIUS, thus returns null } else { - MIN_RESULTS = 40; - MAX_RADIUS = 300.0; // in kilometers + minResults = 40; + maxRadius = 300.0; // in kilometers radius = INITIAL_RADIUS; } // Increase the radius gradually to find a satisfactory number of nearby places - while (radius <= MAX_RADIUS) { + while (radius <= maxRadius) { try { places = getFromWikidataQuery(curLatLng, lang, radius); } catch (InterruptedIOException e) { @@ -73,15 +75,15 @@ List radiusExpander(LatLng curLatLng, String lang, boolean returnClosestR return places; } Timber.d("%d results at radius: %f", places.size(), radius); - if (places.size() >= MIN_RESULTS) { + if (places.size() >= minResults) { break; } else { radius *= RADIUS_MULTIPLIER; } } // make sure we will be able to send at least one request next time - if (radius > MAX_RADIUS) { - radius = MAX_RADIUS; + if (radius > maxRadius) { + radius = maxRadius; } return places; } From 959b53eceb53dd66025bdf1fbd7aaa3f41c0bf6c Mon Sep 17 00:00:00 2001 From: misaochan Date: Sun, 9 Dec 2018 00:11:16 +1000 Subject: [PATCH 11/21] Missed one unnecessary class field --- app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java index cbca81f042..6dd92f226e 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java @@ -24,7 +24,6 @@ public class NearbyPlaces { private static final double INITIAL_RADIUS = 1.0; // in kilometers - private static double MAX_RADIUS = 300.0; // in kilometers private static final double RADIUS_MULTIPLIER = 1.618; private static final Uri WIKIDATA_QUERY_URL = Uri.parse("https://query.wikidata.org/sparql"); private static final Uri WIKIDATA_QUERY_UI_URL = Uri.parse("https://query.wikidata.org/"); From a8bfd4a97f493aa47626b7f8664fdfea0752ff2e Mon Sep 17 00:00:00 2001 From: misaochan Date: Sun, 9 Dec 2018 00:12:04 +1000 Subject: [PATCH 12/21] Remove unnecessary whitespaces that don't improve readability --- .../main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java index 6dd92f226e..64ac885632 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java @@ -108,8 +108,7 @@ private List getFromWikidataQuery(LatLng cur, String lang, double radius) // format as a URL Timber.d(WIKIDATA_QUERY_UI_URL.buildUpon().fragment(query).build().toString()); - String url = WIKIDATA_QUERY_URL.buildUpon() - .appendQueryParameter("query", query).build().toString(); + String url = WIKIDATA_QUERY_URL.buildUpon().appendQueryParameter("query", query).build().toString(); URLConnection conn = new URL(url).openConnection(); conn.setRequestProperty("Accept", "text/tab-separated-values"); BufferedReader in = new BufferedReader(new InputStreamReader(conn.getInputStream())); @@ -145,8 +144,7 @@ private List getFromWikidataQuery(LatLng cur, String lang, double radius) double latitude; double longitude; - Matcher matcher = - Pattern.compile("Point\\(([^ ]+) ([^ ]+)\\)").matcher(point); + Matcher matcher = Pattern.compile("Point\\(([^ ]+) ([^ ]+)\\)").matcher(point); if (!matcher.find()) { continue; } From 9f826003feb1d14485e6b5b21e23f81a2f103df1 Mon Sep 17 00:00:00 2001 From: misaochan Date: Sun, 9 Dec 2018 00:13:11 +1000 Subject: [PATCH 13/21] Add class summary --- app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java index 64ac885632..d2de16ca12 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java @@ -21,6 +21,9 @@ import fr.free.nrw.commons.upload.FileUtils; import timber.log.Timber; +/** + * Handles the Wikidata query to obtain Places around search location + */ public class NearbyPlaces { private static final double INITIAL_RADIUS = 1.0; // in kilometers From 0e6faa73275d3827274346eda9cfc72c1ddad3e6 Mon Sep 17 00:00:00 2001 From: misaochan Date: Sun, 9 Dec 2018 00:13:41 +1000 Subject: [PATCH 14/21] Optimize imports --- app/src/main/java/fr/free/nrw/commons/nearby/DirectUpload.java | 1 - 1 file changed, 1 deletion(-) 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 3ca0603e86..9b692963b7 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 @@ -5,7 +5,6 @@ import android.support.v4.app.Fragment; import android.support.v4.content.ContextCompat; import android.support.v7.app.AlertDialog; -import android.util.Log; import fr.free.nrw.commons.R; import fr.free.nrw.commons.contributions.ContributionController; From c75911565363ab0958dd0df47e19e602c5137fb1 Mon Sep 17 00:00:00 2001 From: misaochan Date: Sun, 9 Dec 2018 00:18:13 +1000 Subject: [PATCH 15/21] Fix access modifiers in Place.java --- .../main/java/fr/free/nrw/commons/nearby/Place.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/Place.java b/app/src/main/java/fr/free/nrw/commons/nearby/Place.java index 7635ac06f1..a7aa31c089 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/Place.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/Place.java @@ -22,7 +22,7 @@ public class Place { private final String category; public Bitmap image; - public Bitmap secondaryImage; + private Bitmap secondaryImage; public String distance; public final Sitelinks siteLinks; @@ -59,7 +59,7 @@ public void setDistance(String distance) { * @return returns the entity id if wikidata link exists */ @Nullable - public String getWikiDataEntityId() { + String getWikiDataEntityId() { if (!hasWikidataLink()) { Timber.d("Wikidata entity ID is null for place with sitelink %s", siteLinks.toString()); return null; @@ -70,15 +70,15 @@ public String getWikiDataEntityId() { return wikiDataLink.replace("http://www.wikidata.org/entity/", ""); } - public boolean hasWikipediaLink() { + boolean hasWikipediaLink() { return !(siteLinks == null || Uri.EMPTY.equals(siteLinks.getWikipediaLink())); } - public boolean hasWikidataLink() { + boolean hasWikidataLink() { return !(siteLinks == null || Uri.EMPTY.equals(siteLinks.getWikidataLink())); } - public boolean hasCommonsLink() { + boolean hasCommonsLink() { return !(siteLinks == null || Uri.EMPTY.equals(siteLinks.getCommonsLink())); } From 7aac64e919fa43afa9773fa20de0b693ba142ae6 Mon Sep 17 00:00:00 2001 From: misaochan Date: Sun, 9 Dec 2018 00:33:08 +1000 Subject: [PATCH 16/21] Clearer Javadocs --- app/src/main/java/fr/free/nrw/commons/nearby/DirectUpload.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9b692963b7..a04c068807 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 @@ -16,7 +16,7 @@ import static android.content.pm.PackageManager.PERMISSION_GRANTED; /** - * This class handles the uploads made from a Nearby Place, in both the list and map views. + * Initiates the uploads made from a Nearby Place, in both the list and map views. */ class DirectUpload { From 3a7fe2d6961100213252eabbdf1048241514e500 Mon Sep 17 00:00:00 2001 From: misaochan Date: Sun, 16 Dec 2018 23:32:20 +1000 Subject: [PATCH 17/21] Add Javadocs to Place.java --- .../fr/free/nrw/commons/nearby/Place.java | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/Place.java b/app/src/main/java/fr/free/nrw/commons/nearby/Place.java index a7aa31c089..d3eca04650 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/Place.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/Place.java @@ -12,6 +12,9 @@ import fr.free.nrw.commons.location.LatLng; import timber.log.Timber; +/** + * A single geolocated Wikidata item + */ public class Place { public final String name; @@ -38,20 +41,44 @@ public Place(String name, Label label, String longDescription, this.siteLinks = siteLinks; } + /** + * Gets the name of the place + * @return name + */ public String getName() { return name; } + /** Gets the label of the place + * e.g. "building", "city", etc + * @return label + */ public Label getLabel() { return label; } + /** + * Gets the long description of the place + * @return long description + */ public String getLongDescription() { return longDescription; } + /** + * Gets the Commons category of the place + * @return Commons category + */ public String getCategory() {return category; } + /** + * Sets the distance of the place from the user's location + * @param distance distance of place from user's location + */ public void setDistance(String distance) { this.distance = distance; } + /** + * Gets the secondary image url for bookmarks + * @return secondary image url + */ public Uri getSecondaryImageUrl() { return this.secondaryImageUrl; } /** @@ -70,18 +97,35 @@ String getWikiDataEntityId() { return wikiDataLink.replace("http://www.wikidata.org/entity/", ""); } + /** + * Checks if the Wikidata item has a Wikipedia page associated with it + * @return true if there is a Wikipedia link + */ boolean hasWikipediaLink() { return !(siteLinks == null || Uri.EMPTY.equals(siteLinks.getWikipediaLink())); } + /** + * Checks if the Wikidata item has a Wikidata page associated with it + * @return true if there is a Wikidata link + */ boolean hasWikidataLink() { return !(siteLinks == null || Uri.EMPTY.equals(siteLinks.getWikidataLink())); } + /** + * Checks if the Wikidata item has a Commons page associated with it + * @return true if there is a Commons link + */ boolean hasCommonsLink() { return !(siteLinks == null || Uri.EMPTY.equals(siteLinks.getCommonsLink())); } + /** + * Check if we already have the exact same Place + * @param o Place being tested + * @return true if name and location of Place is exactly the same + */ @Override public boolean equals(Object o) { if (o instanceof Place) { From bc11ad8504f3e74de247ecfbf36dbc477ab4ce78 Mon Sep 17 00:00:00 2001 From: misaochan Date: Thu, 20 Dec 2018 20:11:24 +1000 Subject: [PATCH 18/21] Remove residual conflict --- app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java index e31a601c1a..9251490936 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java @@ -96,7 +96,6 @@ List radiusExpander(LatLng curLatLng, String lang, boolean returnClosestR } /** -<<<<<<< HEAD * Runs the Wikidata query to populate the Places around search location * @param cur coordinates of search location * @param lang user's language From aca60c8db17d23fc73e6093ea1642dec33b688cd Mon Sep 17 00:00:00 2001 From: misaochan Date: Thu, 20 Dec 2018 20:12:35 +1000 Subject: [PATCH 19/21] Fix lint issues in Sitelinks --- app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java b/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java index f4925af0e5..9db852963d 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java @@ -11,7 +11,7 @@ public class Sitelinks implements Parcelable { private final String wikidataLink; - protected Sitelinks(Parcel in) { + private Sitelinks(Parcel in) { wikipediaLink = in.readString(); commonsLink = in.readString(); wikidataLink = in.readString(); From 0dcedf0b191324bba63ae0e364c26b7c5a869324 Mon Sep 17 00:00:00 2001 From: misaochan Date: Thu, 20 Dec 2018 20:39:44 +1000 Subject: [PATCH 20/21] Javadocs for Sitelinks.java --- .../fr/free/nrw/commons/nearby/Sitelinks.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java b/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java index 9db852963d..e67136286c 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java @@ -5,6 +5,9 @@ import android.os.Parcelable; import android.support.annotation.Nullable; +/** + * Handles the links to Wikipedia, Commons, and Wikidata that are displayed for a Place + */ public class Sitelinks implements Parcelable { private final String wikipediaLink; private final String commonsLink; @@ -29,6 +32,9 @@ public int describeContents() { return 0; } + /** + * Creates sitelinks from the parcel + */ public static final Creator CREATOR = new Creator() { @Override public Sitelinks createFromParcel(Parcel in) { @@ -41,18 +47,35 @@ public Sitelinks[] newArray(int size) { } }; + /** + * Gets the Wikipedia link for a Place + * @return Wikipedia link + */ public Uri getWikipediaLink() { return sanitiseString(wikipediaLink); } + /** + * Gets the Commons link for a Place + * @return Commons link + */ public Uri getCommonsLink() { return sanitiseString(commonsLink); } + /** + * Gets the Wikidata link for a Place + * @return Wikidata link + */ public Uri getWikidataLink() { return sanitiseString(wikidataLink); } + /** + * Sanitises and parses the links before using them + * @param stringUrl unsanitised link + * @return sanitised and parsed link + */ private static Uri sanitiseString(String stringUrl) { String sanitisedStringUrl = stringUrl.replaceAll("[<>\n\r]", "").trim(); return Uri.parse(sanitisedStringUrl); @@ -73,6 +96,9 @@ private Sitelinks(Sitelinks.Builder builder) { this.commonsLink = builder.commonsLink; } + /** + * Builds a list of Sitelinks for a Place + */ public static class Builder { private String wikidataLink; private String commonsLink; From f25b2679caf379a01b671dc9aacf8be2a07d7452 Mon Sep 17 00:00:00 2001 From: Adam Jones Date: Thu, 20 Dec 2018 12:48:50 +0000 Subject: [PATCH 21/21] DirectUpload: Replace nested conditionals with guard clauses --- .../free/nrw/commons/nearby/DirectUpload.java | 108 ++++++++++-------- 1 file changed, 62 insertions(+), 46 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 a04c068807..85439145e9 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 @@ -9,7 +9,6 @@ import fr.free.nrw.commons.R; import fr.free.nrw.commons.contributions.ContributionController; import fr.free.nrw.commons.utils.PermissionUtils; -import timber.log.Timber; import static android.Manifest.permission.READ_EXTERNAL_STORAGE; import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE; @@ -34,31 +33,39 @@ class DirectUpload { * Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes. */ void initiateGalleryUpload() { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - Activity parentActivity = fragment.getActivity(); - if (parentActivity != null) { - if (ContextCompat.checkSelfPermission(parentActivity, READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) { - if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) { - new AlertDialog.Builder(parentActivity) - .setMessage(parentActivity.getString(R.string.read_storage_permission_rationale)) - .setPositiveButton(android.R.string.ok, (dialog, which) -> { - Timber.d("Requesting permissions for read external storage"); - parentActivity.requestPermissions (new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); - dialog.dismiss(); - }) - .setNegativeButton(android.R.string.cancel, null) - .create() - .show(); - } else { + // Only need to handle permissions for Marshmallow and above + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) { + return; + } + + Activity parentActivity = fragment.getActivity(); + if (parentActivity == null) { + controller.startSingleGalleryPick(); + return; + } + + // If we have permission, go ahead + if (ContextCompat.checkSelfPermission(parentActivity, READ_EXTERNAL_STORAGE) == PERMISSION_GRANTED) { + controller.startSingleGalleryPick(); + return; + } + + // If we don't have permission, and we need to show the rationale, show the rationale + if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) { + new AlertDialog.Builder(parentActivity) + .setMessage(parentActivity.getString(R.string.read_storage_permission_rationale)) + .setPositiveButton(android.R.string.ok, (dialog, which) -> { parentActivity.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); - } - } else { - controller.startSingleGalleryPick(); - } - } else { - controller.startSingleGalleryPick(); - } + dialog.dismiss(); + }) + .setNegativeButton(android.R.string.cancel, null) + .create() + .show(); + return; } + + // If we don't have permission, and we don't need to show rationale just request permission + parentActivity.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP); } /** @@ -67,29 +74,38 @@ void initiateGalleryUpload() { * Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes. */ void initiateCameraUpload() { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - Activity parentActivity = fragment.getActivity(); - if (parentActivity != null) { - if (ContextCompat.checkSelfPermission(parentActivity, WRITE_EXTERNAL_STORAGE) != PERMISSION_GRANTED) { - if (fragment.shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) { - new AlertDialog.Builder(parentActivity) - .setMessage(parentActivity.getString(R.string.write_storage_permission_rationale)) - .setPositiveButton(android.R.string.ok, (dialog, which) -> { - parentActivity.requestPermissions (new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); - dialog.dismiss(); - }) - .setNegativeButton(android.R.string.cancel, null) - .create() - .show(); - } else { + // Only need to handle permissions for Marshmallow and above + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) { + return; + } + + Activity parentActivity = fragment.getActivity(); + if (parentActivity == null) { + controller.startCameraCapture(); + return; + } + + // If we have permission, go ahead + if (ContextCompat.checkSelfPermission(parentActivity, WRITE_EXTERNAL_STORAGE) == PERMISSION_GRANTED) { + controller.startCameraCapture(); + return; + } + + // If we don't have permission, and we need to show the rationale, show the rationale + if (fragment.shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) { + new AlertDialog.Builder(parentActivity) + .setMessage(parentActivity.getString(R.string.write_storage_permission_rationale)) + .setPositiveButton(android.R.string.ok, (dialog, which) -> { parentActivity.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); - } - } else { - controller.startCameraCapture(); - } - } else { - controller.startCameraCapture(); - } + dialog.dismiss(); + }) + .setNegativeButton(android.R.string.cancel, null) + .create() + .show(); + return; } + + // If we don't have permission, and we don't need to show rationale just request permission + parentActivity.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); } }