Skip to content

Commit

Permalink
5196: Fix in-app camera location loss (#5249)
Browse files Browse the repository at this point in the history
Merging as this is a great improvement, additional issues/bugs can be filed as GitHub issues.

* fix in-app camera location loss

* fix failing unit tests

* UploadMediaDetailFragmentUnitTest: modify testOnActivityResultAddLocationDialog to have null location

* reintroduce removed variable

* enable prePopulateCategoriesAndDepictionsBy for current user location

* add relevant comment and fix failing test

* modify dialog and disable location tag redaction from EXIF

* modify in-app camera dialog flow and change location to inAppPictureLocation

* change location to inAppPictureLocation

* fix location flow

* preferences.xml: remove redundant default value

* inform users about location loss happening for first upload

* FileProcessor.kt: remove commented-out code

* prevent user location from getting attached to images with no EXIF location in normal and custom selector

* handle onPermissionDenied for location permission

* remove last location when the user turns the GPS off

* disable photo picker and in app camera preferences in settings for logged-out users

* remove debug statements and add toast inside runnables
  • Loading branch information
RitikaPahwa4444 authored Sep 1, 2023
1 parent 1cab938 commit 5073ca0
Show file tree
Hide file tree
Showing 21 changed files with 537 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,22 @@
import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.widget.Toast;
import androidx.annotation.NonNull;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.filepicker.DefaultCallback;
import fr.free.nrw.commons.filepicker.FilePicker;
import fr.free.nrw.commons.filepicker.FilePicker.ImageSource;
import fr.free.nrw.commons.filepicker.UploadableFile;
import fr.free.nrw.commons.kvstore.JsonKvStore;
import fr.free.nrw.commons.location.LatLng;
import fr.free.nrw.commons.location.LocationPermissionsHelper;
import fr.free.nrw.commons.location.LocationPermissionsHelper.Dialog;
import fr.free.nrw.commons.location.LocationPermissionsHelper.LocationPermissionCallback;
import fr.free.nrw.commons.location.LocationServiceManager;
import fr.free.nrw.commons.nearby.Place;
import fr.free.nrw.commons.upload.UploadActivity;
import fr.free.nrw.commons.utils.DialogUtil;
import fr.free.nrw.commons.utils.PermissionUtils;
import fr.free.nrw.commons.utils.ViewUtil;
import java.util.ArrayList;
Expand All @@ -28,7 +35,11 @@ public class ContributionController {

public static final String ACTION_INTERNAL_UPLOADS = "internalImageUploads";
private final JsonKvStore defaultKvStore;
private LatLng locationBeforeImageCapture;
private boolean isInAppCameraUpload;

@Inject
LocationServiceManager locationManager;
@Inject
public ContributionController(@Named("default_preferences") JsonKvStore defaultKvStore) {
this.defaultKvStore = defaultKvStore;
Expand All @@ -46,11 +57,94 @@ public void initiateCameraPick(Activity activity) {

PermissionUtils.checkPermissionsAndPerformAction(activity,
Manifest.permission.WRITE_EXTERNAL_STORAGE,
() -> initiateCameraUpload(activity),
() -> {
if (defaultKvStore.getBoolean("inAppCameraFirstRun")) {
defaultKvStore.putBoolean("inAppCameraFirstRun", false);
askUserToAllowLocationAccess(activity);
} else if(defaultKvStore.getBoolean("inAppCameraLocationPref")) {
createDialogsAndHandleLocationPermissions(activity);
} else {
initiateCameraUpload(activity);
}
},
R.string.storage_permission_title,
R.string.write_storage_permission_rationale);
}

/**
* Asks users to provide location access
*
* @param activity
*/
private void createDialogsAndHandleLocationPermissions(Activity activity) {
LocationPermissionsHelper.Dialog locationAccessDialog = new Dialog(
R.string.location_permission_title,
R.string.in_app_camera_location_permission_rationale
);

LocationPermissionsHelper.Dialog locationOffDialog = new Dialog(
R.string.ask_to_turn_location_on,
R.string.in_app_camera_needs_location
);
LocationPermissionsHelper locationPermissionsHelper = new LocationPermissionsHelper(
activity, locationManager,
new LocationPermissionCallback() {
@Override
public void onLocationPermissionDenied() {
initiateCameraUpload(activity);
}

@Override
public void onLocationPermissionGranted() {
initiateCameraUpload(activity);
}
}
);
locationPermissionsHelper.handleLocationPermissions(
locationAccessDialog,
locationOffDialog
);
}

/**
* Suggest user to attach location information with pictures.
* If the user selects "Yes", then:
*
* Location is taken from the EXIF if the default camera application
* does not redact location tags.
*
* Otherwise, if the EXIF metadata does not have location information,
* then location captured by the app is used
*
* @param activity
*/
private void askUserToAllowLocationAccess(Activity activity) {
DialogUtil.showAlertDialog(activity,
activity.getString(R.string.in_app_camera_location_permission_title),
activity.getString(R.string.in_app_camera_location_access_explanation),
activity.getString(R.string.option_allow),
activity.getString(R.string.option_dismiss),
()-> {
defaultKvStore.putBoolean("inAppCameraLocationPref", true);
createDialogsAndHandleLocationPermissions(activity);
},
() -> {
defaultKvStore.putBoolean("inAppCameraLocationPref", false);
initiateCameraUpload(activity);
},
null,
true);
}

/**
* Check if apps have access to location even after having individual access
*
* @return
*/
private boolean isLocationAccessToAppsTurnedOn() {
return (locationManager.isNetworkProviderEnabled() || locationManager.isGPSProviderEnabled());
}

/**
* Initiate gallery picker
*/
Expand All @@ -66,9 +160,7 @@ public void initiateCustomGalleryPickWithPermission(final Activity activity) {

PermissionUtils.checkPermissionsAndPerformAction(activity,
Manifest.permission.WRITE_EXTERNAL_STORAGE,
() -> {
FilePicker.openCustomSelector(activity, 0);
},
() -> FilePicker.openCustomSelector(activity, 0),
R.string.storage_permission_title,
R.string.write_storage_permission_rationale);
}
Expand Down Expand Up @@ -99,6 +191,10 @@ private void setPickerConfiguration(Activity activity,
*/
private void initiateCameraUpload(Activity activity) {
setPickerConfiguration(activity, false);
if (defaultKvStore.getBoolean("inAppCameraLocationPref", false)) {
locationBeforeImageCapture = locationManager.getLastLocation();
}
isInAppCameraUpload = true;
FilePicker.openCameraForImage(activity, 0);
}

Expand Down Expand Up @@ -134,7 +230,8 @@ public List<UploadableFile> handleExternalImagesPicked(Activity activity,

/**
* Returns intent to be passed to upload activity
* Attaches place object for nearby uploads
* Attaches place object for nearby uploads and
* location before image capture if in-app camera is used
*/
private Intent handleImagesPicked(Context context,
List<UploadableFile> imagesFiles) {
Expand All @@ -148,6 +245,17 @@ private Intent handleImagesPicked(Context context,
shareIntent.putExtra(PLACE_OBJECT, place);
}

if (locationBeforeImageCapture != null) {
shareIntent.putExtra(
UploadActivity.LOCATION_BEFORE_IMAGE_CAPTURE,
locationBeforeImageCapture);
}

shareIntent.putExtra(
UploadActivity.IN_APP_CAMERA_UPLOAD,
isInAppCameraUpload
);
isInAppCameraUpload = false; // reset the flag for next use
return shareIntent;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ protected void onResume() {

if ((applicationKvStore.getBoolean("firstrun", true)) &&
(!applicationKvStore.getBoolean("login_skipped"))) {
defaultKvStore.putBoolean("inAppCameraFirstRun", true);
WelcomeActivity.startYourself(this);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package fr.free.nrw.commons.location;

import android.Manifest.permission;
import android.app.Activity;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.provider.Settings;
import android.widget.Toast;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.utils.DialogUtil;
import fr.free.nrw.commons.utils.PermissionUtils;

/**
* Helper class to handle location permissions
*/
public class LocationPermissionsHelper {
Activity activity;
LocationServiceManager locationManager;
LocationPermissionCallback callback;
public LocationPermissionsHelper(Activity activity, LocationServiceManager locationManager,
LocationPermissionCallback callback) {
this.activity = activity;
this.locationManager = locationManager;
this.callback = callback;
}
public static class Dialog {
int dialogTitleResource;
int dialogTextResource;

public Dialog(int dialogTitle, int dialogText) {
dialogTitleResource = dialogTitle;
dialogTextResource = dialogText;
}
}

/**
* Handles the entire location permissions flow
*
* @param locationAccessDialog
* @param locationOffDialog
*/
public void handleLocationPermissions(Dialog locationAccessDialog,
Dialog locationOffDialog) {
requestForLocationAccess(locationAccessDialog, locationOffDialog);
}

/**
* Ask for location permission if the user agrees on attaching location with pictures
* and the app does not have the access to location
*
* @param locationAccessDialog
* @param locationOffDialog
*/
private void requestForLocationAccess(
Dialog locationAccessDialog,
Dialog locationOffDialog
) {
PermissionUtils.checkPermissionsAndPerformAction(activity,
permission.ACCESS_FINE_LOCATION,
() -> {
if(!isLocationAccessToAppsTurnedOn()) {
showLocationOffDialog(locationOffDialog);
} else {
if (callback != null) {
callback.onLocationPermissionGranted();
}
}
},
() -> {
if (callback != null) {
Toast.makeText(
activity,
R.string.in_app_camera_location_permission_denied,
Toast.LENGTH_LONG
).show();
callback.onLocationPermissionDenied();
}
},
locationAccessDialog.dialogTitleResource,
locationAccessDialog.dialogTextResource);
}

/**
* Check if apps have access to location even after having individual access
*
* @return
*/
public boolean isLocationAccessToAppsTurnedOn() {
return (locationManager.isNetworkProviderEnabled() || locationManager.isGPSProviderEnabled());
}

/**
* Ask user to grant location access to apps
*
*/

private void showLocationOffDialog(Dialog locationOffDialog) {
DialogUtil
.showAlertDialog(activity,
activity.getString(locationOffDialog.dialogTitleResource),
activity.getString(locationOffDialog.dialogTextResource),
activity.getString(R.string.title_app_shortcut_setting),
activity.getString(R.string.cancel),
() -> openLocationSettings(),
() -> {
Toast.makeText(
activity,
R.string.in_app_camera_location_unavailable,
Toast.LENGTH_LONG
).show();
callback.onLocationPermissionDenied();
});
}

/**
* Open location source settings so that apps with location access can access it
*
* TODO: modify it to fix https://github.com/commons-app/apps-android-commons/issues/5255
*/

private void openLocationSettings() {
final Intent intent = new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS);
final PackageManager packageManager = activity.getPackageManager();

if (intent.resolveActivity(packageManager)!= null) {
activity.startActivity(intent);
}
}

/**
* Handle onPermissionDenied within individual classes based on the requirements
*/
public interface LocationPermissionCallback {
void onLocationPermissionDenied();
void onLocationPermissionGranted();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@ public int getCount() {
* @return
*/
public Observable<UploadItem> preProcessImage(UploadableFile uploadableFile, Place place,
SimilarImageInterface similarImageInterface) {
SimilarImageInterface similarImageInterface, LatLng inAppPictureLocation) {
return uploadModel.preProcessImage(uploadableFile, place,
similarImageInterface);
similarImageInterface, inAppPictureLocation);
}

/**
Expand All @@ -199,8 +199,8 @@ public Observable<UploadItem> preProcessImage(UploadableFile uploadableFile, Pla
* @param uploadItem
* @return
*/
public Single<Integer> getImageQuality(UploadItem uploadItem) {
return uploadModel.getImageQuality(uploadItem);
public Single<Integer> getImageQuality(UploadItem uploadItem, LatLng location) {
return uploadModel.getImageQuality(uploadItem, location);
}

/**
Expand Down
Loading

0 comments on commit 5073ca0

Please sign in to comment.