Skip to content

Commit f416689

Browse files
misaochanneslihanturan
misaochan
authored and
neslihanturan
committed
Fix issue with onRequestPermissionsResult not being called in Nearby map and list (commons-app#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
1 parent 2150d7f commit f416689

File tree

5 files changed

+45
-36
lines changed

5 files changed

+45
-36
lines changed

‎app/src/main/java/fr/free/nrw/commons/nearby/DirectUpload.java‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import fr.free.nrw.commons.R;
1010
import fr.free.nrw.commons.contributions.ContributionController;
11+
import timber.log.Timber;
1112

1213
import static android.Manifest.permission.READ_EXTERNAL_STORAGE;
1314
import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE;
@@ -23,53 +24,56 @@ class DirectUpload {
2324
this.controller = controller;
2425
}
2526

26-
void initiateCameraUpload() {
27+
// These permission requests will be handled by the Fragments.
28+
// Do not use requestCode 1 as it will conflict with NearbyActivity's requestCodes
29+
void initiateGalleryUpload() {
2730
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
28-
if (ContextCompat.checkSelfPermission(fragment.getActivity(), WRITE_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
29-
if (fragment.getActivity().shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) {
31+
if (ContextCompat.checkSelfPermission(fragment.getActivity(), READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
32+
if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) {
3033
new AlertDialog.Builder(fragment.getActivity())
31-
.setMessage(fragment.getActivity().getString(R.string.write_storage_permission_rationale))
34+
.setMessage(fragment.getActivity().getString(R.string.read_storage_permission_rationale))
3235
.setPositiveButton("OK", (dialog, which) -> {
33-
fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 3);
36+
Timber.d("Requesting permissions for read external storage");
37+
fragment.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, 4);
3438
dialog.dismiss();
3539
})
3640
.setNegativeButton("Cancel", null)
3741
.create()
3842
.show();
3943
} else {
40-
fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 3);
44+
fragment.requestPermissions(new String[]{READ_EXTERNAL_STORAGE},
45+
4);
4146
}
4247
} else {
43-
controller.startCameraCapture();
48+
controller.startGalleryPick();
4449
}
45-
} else {
46-
controller.startCameraCapture();
50+
}
51+
else {
52+
controller.startGalleryPick();
4753
}
4854
}
4955

50-
void initiateGalleryUpload() {
56+
void initiateCameraUpload() {
5157
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
52-
if (ContextCompat.checkSelfPermission(fragment.getActivity(), READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
53-
if (fragment.getActivity().shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) {
58+
if (ContextCompat.checkSelfPermission(fragment.getActivity(), WRITE_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
59+
if (fragment.shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) {
5460
new AlertDialog.Builder(fragment.getActivity())
55-
.setMessage(fragment.getActivity().getString(R.string.read_storage_permission_rationale))
61+
.setMessage(fragment.getActivity().getString(R.string.write_storage_permission_rationale))
5662
.setPositiveButton("OK", (dialog, which) -> {
57-
fragment.getActivity().requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, 1);
63+
fragment.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 5);
5864
dialog.dismiss();
5965
})
6066
.setNegativeButton("Cancel", null)
6167
.create()
6268
.show();
6369
} else {
64-
fragment.getActivity().requestPermissions(new String[]{READ_EXTERNAL_STORAGE},
65-
1);
70+
fragment.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, 5);
6671
}
6772
} else {
68-
controller.startGalleryPick();
73+
controller.startCameraCapture();
6974
}
70-
}
71-
else {
72-
controller.startGalleryPick();
75+
} else {
76+
controller.startCameraCapture();
7377
}
7478
}
7579
}

‎app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,11 @@ public void onRequestPermissionsResult(int requestCode, @NonNull String[] permis
165165
showLocationPermissionDeniedErrorDialog();
166166
}
167167
}
168+
break;
169+
170+
default:
171+
// This is needed to allow the request codes from the Fragments to be routed appropriately
172+
super.onRequestPermissionsResult(requestCode, permissions, grantResults);
168173
}
169174
}
170175

‎app/src/main/java/fr/free/nrw/commons/nearby/NearbyListFragment.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,17 +106,17 @@ public void onRequestPermissionsResult(int requestCode, @NonNull String[] permis
106106
Timber.d("onRequestPermissionsResult: req code = " + " perm = " + permissions + " grant =" + grantResults);
107107

108108
switch (requestCode) {
109-
// 1 = "Read external storage" allowed when gallery selected
110-
case 1: {
109+
// 4 = "Read external storage" allowed when gallery selected
110+
case 4: {
111111
if (grantResults.length > 0 && grantResults[0] == PERMISSION_GRANTED) {
112112
Timber.d("Call controller.startGalleryPick()");
113113
controller.startGalleryPick();
114114
}
115115
}
116116
break;
117117

118-
// 3 = "Write external storage" allowed when camera selected
119-
case 3: {
118+
// 5 = "Write external storage" allowed when camera selected
119+
case 5: {
120120
if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
121121
Timber.d("Call controller.startCameraCapture()");
122122
controller.startCameraCapture();

‎app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ public class NearbyMapFragment extends DaggerFragment {
9898
private Animation fab_open;
9999
private Animation rotate_forward;
100100
private ContributionController controller;
101+
private DirectUpload directUpload;
101102

102103
private Place place;
103104
private Marker selected;
@@ -122,6 +123,10 @@ public NearbyMapFragment() {
122123
@Override
123124
public void onCreate(Bundle savedInstanceState) {
124125
super.onCreate(savedInstanceState);
126+
127+
controller = new ContributionController(this);
128+
directUpload = new DirectUpload(this, controller);
129+
125130
Bundle bundle = this.getArguments();
126131
Gson gson = new GsonBuilder()
127132
.registerTypeAdapter(Uri.class, new UriDeserializer())
@@ -680,9 +685,6 @@ private void passInfoToSheet(Place place) {
680685
fabCamera.setOnClickListener(view -> {
681686
if (fabCamera.isShown()) {
682687
Timber.d("Camera button tapped. Image title: " + place.getName() + "Image desc: " + place.getLongDescription());
683-
controller = new ContributionController(this);
684-
685-
DirectUpload directUpload = new DirectUpload(this, controller);
686688
storeSharedPrefs();
687689
directUpload.initiateCameraUpload();
688690
}
@@ -691,9 +693,6 @@ private void passInfoToSheet(Place place) {
691693
fabGallery.setOnClickListener(view -> {
692694
if (fabGallery.isShown()) {
693695
Timber.d("Gallery button tapped. Image title: " + place.getName() + "Image desc: " + place.getLongDescription());
694-
controller = new ContributionController(this);
695-
696-
DirectUpload directUpload = new DirectUpload(this, controller);
697696
storeSharedPrefs();
698697
directUpload.initiateGalleryUpload();
699698
}
@@ -712,18 +711,19 @@ void storeSharedPrefs() {
712711
public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
713712
Timber.d("onRequestPermissionsResult: req code = " + " perm = " + permissions + " grant =" + grantResults);
714713

714+
// Do not use requestCode 1 as it will conflict with NearbyActivity's requestCodes
715715
switch (requestCode) {
716-
// 1 = "Read external storage" allowed when gallery selected
717-
case 1: {
716+
// 4 = "Read external storage" allowed when gallery selected
717+
case 4: {
718718
if (grantResults.length > 0 && grantResults[0] == PERMISSION_GRANTED) {
719719
Timber.d("Call controller.startGalleryPick()");
720720
controller.startGalleryPick();
721721
}
722722
}
723723
break;
724724

725-
// 3 = "Write external storage" allowed when camera selected
726-
case 3: {
725+
// 5 = "Write external storage" allowed when camera selected
726+
case 5: {
727727
if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
728728
Timber.d("Call controller.startCameraCapture()");
729729
controller.startCameraCapture();

‎app/src/main/res/values/strings.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@
154154
<string name="detail_description_empty">No description</string>
155155
<string name="detail_license_empty">Unknown license</string>
156156
<string name="menu_refresh">Refresh</string>
157-
<string name="read_storage_permission_rationale">Required permission: Read external storage. App cannot function without this.</string>
158-
<string name="write_storage_permission_rationale">Required permission: Write external storage. App cannot function without this.</string>
157+
<string name="read_storage_permission_rationale">Required permission: Read external storage. App cannot access your gallery without this.</string>
158+
<string name="write_storage_permission_rationale">Required permission: Write external storage. App cannot access your camera without this.</string>
159159
<string name="location_permission_rationale">Optional permission: Get current location for category suggestions</string>
160160
<string name="ok">OK</string>
161161
<string name="title_activity_nearby">Nearby Places</string>

0 commit comments

Comments
 (0)