Skip to content

Commit

Permalink
Feature/permissions library (commons-app#1855)
Browse files Browse the repository at this point in the history
* Added permission for Dexter, the runtime permission handling library

* [Preparing fir issue commons-app#1773] Added a utility function which would take the user to app settings screen where he could manually give us the required permission

* Added an alert dialog with positive and negative callback [Preparing fir issue commons-app#1773]

* Improvements in the way External Storage Permission is handled in MultipleShareActivity[Bug fix commons-app#1697]
1. Used dexter to handle the external storage permission
2. Behaviour changes : When user tries to share(uppload) images to commons via MultipleShareActivity, following decision tree is followed
	a. If the app has permission for external storage, normal upload operation is followed
	b. If the app does not has the permission for external storage, dexter is used to ask for the same
	c. If the user gives us the required permission, normal upload flow is proceeded
	d. If the doesnot gives us the required permission a rationale dialog is shown with the appropriate message to let him know why we need the permission
	e. If he presses okay, steps a-c are followed and if he presses cancel, we close the app.
	f. If while asking for permission, the user chooses never ask again, then next time he tries to upload an image via MSA, the rational dialog follows the app setting screen where he could manually give us the required permission and the onActivityResult of same is handled

* Added a Constants class to handle request and result codes from one place and other related constants common to the all app elements

* replaced hardcoded strings ok and cancel in DialogUtil to string resources

* init permission rationale dialog in activities onCreate

* Code formatting, updated access modifiers wherever required, added javadocs for new methods created

* *shifted constants to app class
*Added JavaDocs in PermissionUtils

* removed class REQUEST_CODES from CommonsApplication and instead put the enclosing constants in the App class itself
  • Loading branch information
ashishkumar468 authored and ilgazer committed Sep 6, 2018
1 parent d455c35 commit e2aadc9
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 16 deletions.
3 changes: 3 additions & 0 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ dependencies {
debugImplementation "com.squareup.leakcanary:leakcanary-android:$LEAK_CANARY"
releaseImplementation "com.squareup.leakcanary:leakcanary-android-no-op:$LEAK_CANARY"
testImplementation "com.squareup.leakcanary:leakcanary-android-no-op:$LEAK_CANARY"
//For handling runtime permissions
implementation 'com.karumi:dexter:5.0.0'

}

android {
Expand Down
9 changes: 9 additions & 0 deletions app/src/main/java/fr/free/nrw/commons/CommonsApplication.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ public class CommonsApplication extends Application {
@Inject @Named("application_preferences") SharedPreferences applicationPrefs;
@Inject @Named("prefs") SharedPreferences otherPrefs;

/**
* Constants begin
*/
public static final int OPEN_APPLICATION_DETAIL_SETTINGS = 1001;

public static final String DEFAULT_EDIT_SUMMARY = "Uploaded using [[COM:MOA|Commons Mobile App]]";

public static final String FEEDBACK_EMAIL = "[email protected]";
Expand All @@ -66,6 +71,10 @@ public class CommonsApplication extends Application {

public static final String NOTIFICATION_CHANNEL_ID_ALL = "CommonsNotificationAll";

/**
* Constants End
*/

private RefWatcher refWatcher;


Expand Down
120 changes: 104 additions & 16 deletions app/src/main/java/fr/free/nrw/commons/upload/MultipleShareActivity.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package fr.free.nrw.commons.upload;

import android.Manifest;
import android.Manifest.permission;
import android.app.AlertDialog;
import android.app.ProgressDialog;
import android.content.ContentResolver;
import android.content.Context;
Expand All @@ -12,7 +14,6 @@
import android.os.Build;
import android.os.Bundle;
import android.os.ParcelFileDescriptor;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.v4.app.ActivityCompat;
import android.support.v4.app.FragmentManager;
Expand All @@ -23,6 +24,15 @@
import android.widget.AdapterView;
import android.widget.Toast;

import com.karumi.dexter.Dexter;
import com.karumi.dexter.DexterBuilder;
import com.karumi.dexter.listener.PermissionDeniedResponse;
import com.karumi.dexter.listener.PermissionGrantedResponse;
import com.karumi.dexter.listener.single.BasePermissionListener;
import fr.free.nrw.commons.CommonsApplication;
import fr.free.nrw.commons.utils.DialogUtil;
import fr.free.nrw.commons.utils.DialogUtil.Callback;
import fr.free.nrw.commons.utils.PermissionUtils;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.List;
Expand All @@ -41,7 +51,6 @@
import fr.free.nrw.commons.contributions.Contribution;
import fr.free.nrw.commons.media.MediaDetailPagerFragment;
import fr.free.nrw.commons.modifications.CategoryModifier;
import fr.free.nrw.commons.modifications.ModificationsContentProvider;
import fr.free.nrw.commons.modifications.ModifierSequence;
import fr.free.nrw.commons.modifications.ModifierSequenceDao;
import fr.free.nrw.commons.modifications.TemplateRemoveModifier;
Expand Down Expand Up @@ -81,6 +90,11 @@ public class MultipleShareActivity extends AuthenticatedActivity
private boolean locationPermitted = false;
private boolean isMultipleUploadsPrepared = false;
private boolean isMultipleUploadsFinalised = false; // Checks is user clicked to upload button or regret before this phase
private final String TAG="#MultipleShareActivity#";
private AlertDialog storagePermissionInfoDialog;
private DexterBuilder dexterStoragePermissionBuilder;

private PermissionDeniedResponse permissionDeniedResponse;

@Override
public Media getMediaAtPosition(int i) {
Expand Down Expand Up @@ -124,17 +138,6 @@ public void OnMultipleUploadInitiated() {
multipleUploadBegins();
}

@Override
public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
if (requestCode == 1 && grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
Timber.d("onRequestPermissionsResult external storage permission granted");
prepareMultipleUpoadList();
} else {
// Permission is not granted, close activity
finish();
}
}

private void multipleUploadBegins() {

Timber.d("Multiple upload begins");
Expand Down Expand Up @@ -216,6 +219,7 @@ protected void onCreate(Bundle savedInstanceState) {
setContentView(R.layout.activity_multiple_uploads);
ButterKnife.bind(this);
initDrawer();
initPermissionsRationaleDialog();

if (savedInstanceState != null) {
photosList = savedInstanceState.getParcelableArrayList("uploadsList");
Expand All @@ -233,6 +237,47 @@ protected void onCreate(Bundle savedInstanceState) {
}
}


/**
* We have agreed to show a dialog showing why we need a particular permission.
* This method is used to initialise the dialog which is going to show the permission's rationale.
* The dialog is initialised along with a callback for positive and negative user actions.
*/
private void initPermissionsRationaleDialog() {
if (storagePermissionInfoDialog == null) {
storagePermissionInfoDialog = DialogUtil
.getAlertDialogWithPositiveAndNegativeCallbacks(
MultipleShareActivity.this,
getString(R.string.storage_permission), getString(
R.string.write_storage_permission_rationale_for_image_share),
R.drawable.ic_launcher, new Callback() {
@Override
public void onPositiveButtonClicked() {
//If the user is willing to give us the permission
//But had somehow previously choose never ask again, we take him to app settings to manually enable permission
if(null== permissionDeniedResponse){
//Dexter returned null, lets see if this ever happens
return;
}
else if (permissionDeniedResponse.isPermanentlyDenied()) {
PermissionUtils.askUserToManuallyEnablePermissionFromSettings(MultipleShareActivity.this);
} else {
//or if we still have chance to show runtime permission dialog, we show him that.
askDexterToHandleExternalStoragePermission();
}
}

@Override
public void onNegativeButtonClicked() {
//This was the behaviour as of now, I was planning to maybe snack him with some message
//and then call finish after some time, or may be it could be associated with some action on the snack
//If the user does not want us to give the permission, even after showing rationale dialog, lets not trouble him anymore
finish();
}
});
}
}

@Override
protected void onDestroy() {
super.onDestroy();
Expand Down Expand Up @@ -275,20 +320,63 @@ protected void onAuthCookieAcquired(String authCookie) {
isMultipleUploadsPrepared = false;
mwApi.setAuthCookie(authCookie);
if (!ExternalStorageUtils.isStoragePermissionGranted(this)) {
ExternalStorageUtils.requestExternalStoragePermission(this);
//If permission is not there, handle the negative cases
askDexterToHandleExternalStoragePermission();
isMultipleUploadsPrepared = false;
return; // Postpone operation to do after gettion permission
} else {
isMultipleUploadsPrepared = true;
prepareMultipleUpoadList();
prepareMultipleUploadList();
}
}

/**
* This method initialised the Dexter's permission builder (if not already initialised). Also makes sure that the builder is initialised
* only once, otherwise we would'nt know on which instance of it, the user is working on. And after the builder is initialised, it checks for the required
* permission and then handles the permission status, thanks to Dexter's appropriate callbacks.
*/
private void askDexterToHandleExternalStoragePermission() {
Timber.d(TAG, "External storage permission is being requested");
if (null == dexterStoragePermissionBuilder) {
dexterStoragePermissionBuilder = Dexter.withActivity(this)
.withPermission(permission.WRITE_EXTERNAL_STORAGE)
.withListener(new BasePermissionListener() {
@Override
public void onPermissionGranted(PermissionGrantedResponse response) {
Timber.d(TAG,"User has granted us the permission for writing the external storage");
//If permission is granted, well and good
prepareMultipleUploadList();
}

@Override
public void onPermissionDenied(PermissionDeniedResponse response) {
Timber.d(TAG,"User has granted us the permission for writing the external storage");
//If permission is not granted in whatsoever scenario, we show him a dialog stating why we need the permission
permissionDeniedResponse=response;
if (null != storagePermissionInfoDialog && !storagePermissionInfoDialog
.isShowing()) {
storagePermissionInfoDialog.show();
}
}
});
}
dexterStoragePermissionBuilder.check();
}

@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
super.onActivityResult(requestCode, resultCode, data);
if (requestCode == CommonsApplication.OPEN_APPLICATION_DETAIL_SETTINGS) {
//OnActivity result, no matter what the result is, our function can handle that.
askDexterToHandleExternalStoragePermission();
}
}

/**
* Prepares a list from files will be uploaded. Saves these files temporarily to external
* storage. Adds them to uploads list
*/
private void prepareMultipleUpoadList() {
private void prepareMultipleUploadList() {
Intent intent = getIntent();

if (Intent.ACTION_SEND_MULTIPLE.equals(intent.getAction())) {
Expand Down
31 changes: 31 additions & 0 deletions app/src/main/java/fr/free/nrw/commons/utils/DialogUtil.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package fr.free.nrw.commons.utils;

import android.app.Activity;
import android.app.AlertDialog;
import android.app.AlertDialog.Builder;
import android.app.Dialog;
import android.content.Context;
import android.os.Build;
import android.support.annotation.Nullable;
import android.support.v4.app.DialogFragment;
import android.support.v4.app.FragmentActivity;

import fr.free.nrw.commons.R;
import timber.log.Timber;

public class DialogUtil {
Expand Down Expand Up @@ -92,4 +96,31 @@ public static void showSafely(FragmentActivity activity, DialogFragment dialog)
Timber.e(e, "Could not show dialog.");
}
}

public static AlertDialog getAlertDialogWithPositiveAndNegativeCallbacks(
Context context, String title, String message, int iconResourceId, Callback callback) {

AlertDialog alertDialog = new Builder(context)
.setTitle(title)
.setMessage(message)
.setPositiveButton(context.getString(R.string.ok), (dialog, which) -> {
callback.onPositiveButtonClicked();
dialog.dismiss();
})
.setNegativeButton(context.getString(R.string.cancel), (dialog, which) -> {
callback.onNegativeButtonClicked();
dialog.dismiss();
})
.setIcon(iconResourceId).create();

return alertDialog;

}

public interface Callback {

void onPositiveButtonClicked();

void onNegativeButtonClicked();
}
}
23 changes: 23 additions & 0 deletions app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package fr.free.nrw.commons.utils;

import android.app.Activity;
import android.content.Intent;
import android.net.Uri;
import android.provider.Settings;
import fr.free.nrw.commons.CommonsApplication;

public class PermissionUtils {

/**
* This method can be used by any activity which requires a permission which has been blocked(marked never ask again by the user)
It open the app settings from where the user can manually give us the required permission.
* @param activity
*/
public static void askUserToManuallyEnablePermissionFromSettings(
Activity activity) {
Intent intent = new Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS);
Uri uri = Uri.fromParts("package", activity.getPackageName(), null);
intent.setData(uri);
activity.startActivityForResult(intent,CommonsApplication.OPEN_APPLICATION_DETAIL_SETTINGS);
}
}
2 changes: 2 additions & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,8 @@
s <string name="notifications_channel_name_all">Commons Notification</string>

<string name="step_count">Step %1$d of %2$d</string>
<string name="storage_permission">Storage Permission</string>
<string name="write_storage_permission_rationale_for_image_share">We need your permission to access the external storage of your device in order to upload images.</string>
<string name="next">Next</string>
<string name="previous">Previous</string>
<string name="submit">Submit</string>
Expand Down

0 comments on commit e2aadc9

Please sign in to comment.