[MRG] Add the location permission for android 9. #286

huangyz0918 · 2019-07-28T02:25:50Z

Closes #284

What has been done to verify that this works as intended?

Tested in my Nexus 6P (android 9.0) with success.

Why is this the best possible solution? Were any other approaches considered?

This fix the android 9 permission issue with bluetooth, and after this issue was resolved, the bluetooth feature can work well in android 9 devices I think.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

N/A

Before submitting this PR, please make sure you have:

run ./gradlew checkCode and confirmed all checks still pass OR confirm CircleCI build passes
verified that any code or assets from external sources are properly credited in comments and/or in the about file.

skunkworks_crow/src/main/java/org/odk/share/views/ui/bluetooth/BtReceiverActivity.java

lakshyagupta21 · 2019-07-28T11:55:20Z

skunkworks_crow/src/main/java/org/odk/share/views/ui/bluetooth/BtSenderActivity.java

@@ -73,6 +75,11 @@ protected void onCreate(Bundle savedInstanceState) {
        setTitle(getString(R.string.send_instance_title));
        setSupportActionBar(toolbar);

+        //checking the location permission for bluetooth for android 9.


I think it would be better to ask for this permission before opening the activity(When bluetooth option is choosen). Because both the permission asking dialog and scanning dialog appears at the same time .

Can we grant this permission when we are entering the main page? If we choose bluetooth then popup the permission dialog, that page still has the method choosing dialog appears at the same time.

If we ask for location permission on start, then what if the user never chooses Bluetooth as the transfer mode then it would not be of no use.
I'm suggesting to open a dialog asking for user permission when user taps on Bluetooth as the transfer mode(Close the dialog asking for the mode of transfer then only show the dialog for the permission) and if user allows then send to BluetoothReceiver/Sender Screen otherwise again show the dialog saying that Location permission is needed for Bluetooth transfer with two button options Cancel(negative) and Access permission(positive). Access permission will ask user again for the permission and if he has selected the checkbox Never ask again when permission was asked then you won't be able to request for permission again, in that case, you have to navigate user to our apps' settings so that he can manually go and give permission to us.
I know its pretty long process and but I think this is the way to tackle this issue.

This is how generally other apps tackle this, in case you've some other option in mind please let me know.

Cool, I'll follow your suggestion. But can I use some run-time android permission library? So that I can remove the PermissionUtil.java and make the code more flexible, something like https://github.com/Karumi/Dexter, it can also help me to build a dialog.

Sure, It totally depends on you what you want to use, and but don't forget to add it in our Open Source Licenses section, its always to good to give the credits. And yes if you're doing it for Location permission then do it for every dangerous permission like read and write storage. Or you can open a different PR for that.

huangyz0918 · 2019-07-29T03:41:12Z

@lakshyagupta21 I got a better idea, you can have a review of the code now.

lakshyagupta21 · 2019-07-29T08:53:57Z

This also looks good to me.

skunkworks_crow/src/main/res/values/strings.xml

skunkworks_crow/src/main/java/org/odk/share/views/ui/bluetooth/BtSenderActivity.java

skunkworks_crow/src/main/java/org/odk/share/views/ui/bluetooth/BtReceiverActivity.java

huangyz0918 · 2019-07-30T03:17:56Z

@lakshyagupta21 thanks for your detailed review! I think the code is ready now.

skunkworks_crow/build.gradle

updated the open source license.

skunkworks_crow/src/main/assets/open_source_licenses.html

huangyz0918 · 2019-08-03T09:20:44Z

I think it's ready for a review @lakshyagupta21 .

ajay-prabhakar · 2019-08-03T17:01:07Z

@huangyz0918 I just checked in android 6(Tablet) I am unable to send the forms it is exiting app automatically
In logs, I can't see any crash message or error message I can't understand why it happens

huangyz0918 · 2019-08-04T02:59:21Z

@Chromicle This PR is not related to android 6, can you work normally in master?

skunkworks_crow/src/main/res/values/strings.xml

lakshyagupta21 · 2019-08-04T09:17:45Z

skunkworks_crow/src/main/java/org/odk/share/utilities/PermissionUtils.java

+            intent.setAction(Settings.ACTION_APPLICATION_DETAILS_SETTINGS);
+            Uri uri = Uri.fromParts(SCHEME, packageName, null);
+            intent.setData(uri);
+            targetActivity.startActivity(intent);


There is an issue over here when the user moves to app settings, and no matter what he does there and after doing all the activity on app settings when he presses back he goes back to the Send data screen with empty view.
You should check whenever an activity is resumed, whether user actually left application to actually play with the app settings, if the answer is yes then check if we've enough permission to do the transmission, if not then again display the same dialog.

Fixed, I tested in android 9 and non-android 9 devices, no issues, but I think it still needs more tests.

skunkworks_crow/src/main/res/values/strings.xml

lakshyagupta21 · 2019-08-06T02:52:14Z

skunkworks_crow/src/main/java/org/odk/share/views/ui/bluetooth/BtReceiverActivity.java

+    @OnPermissionDenied({Manifest.permission.ACCESS_FINE_LOCATION,
+            Manifest.permission.ACCESS_COARSE_LOCATION})
+    void showDeniedForLocation() {
+        Toast.makeText(this, R.string.permission_location_denied, Toast.LENGTH_SHORT).show();


Change the toast length to LONG, everywhere

lakshyagupta21 · 2019-08-06T03:15:02Z

I think we've to enable location permission for all device because in Android 7.1 I've to go and enable the location permission from the settings then only it starts showing the bluetooth devices when I start scanning it.

huangyz0918 · 2019-08-06T03:18:18Z

I think we've to enable location permission for all device because in Android 7.1 I've to go and enable the location permission from the settings then only it starts showing the bluetooth devices when I start scanning it.

So you mean I should remove the android 9 checks?

lakshyagupta21 · 2019-08-06T03:21:43Z

Can you check this once in any of your Android 6 and Android 7 device, don't forget to check for location permission (verify when enabled/disabled)

huangyz0918 · 2019-08-06T03:22:40Z

Ok, will do that and update this PR.

huangyz0918 · 2019-08-06T09:39:38Z

Can you check this once in any of your Android 6 and Android 7 device, don't forget to check for location permission (verify when enabled/disabled)

You can checkout this, from Android 6, the ACCESS_COARSE_LOCATION and ACCESS_FINE_LOCATION need to be obtained from run time. So I think we should remove the API checks and apply that to all devices. @lakshyagupta21 I updated this PR and please review.

lakshyagupta21 · 2019-08-07T02:51:42Z

ACCESS_COARSE_LOCATION and ACCESS_FINE_LOCATION need to be obtained from run time

I know that these permissions are required to be asked at runtime, but my doubt was why do we need to ask it I mean who requires this permission.
We found this problem for Android 9, then how was it working previously for other versions if location permission was not given to any of them.

huangyz0918 · 2019-08-10T01:51:01Z

I know that these permissions are required to be asked at runtime, but my doubt was why do we need to ask it I mean who requires this permission.

I have done some tests on different devices using both real devices and virtual devices.

Device Name	Android Version	Needs Location Permission for Bluetooth
Nexus 6P	9.0.0	Yes
Huawei P10	7.1.0	Yes
Nexus 5X	6.0.1	Yes
Samsung Galaxy Note3	5.0.0	No

The conclusion is, in versions of Android prior to Marshmallow, the user could use Bluetooth without location services enabled, we need the location permission in all the devices with an API level > 6.0. @lakshyagupta21

skunkworks_crow/src/main/java/org/odk/share/views/ui/bluetooth/BtSenderActivity.java

huangyz0918 added 2 commits July 28, 2019 10:15

✨ feat: add the permission checks.

6966f93

🚑 quickfix:

1975315

lakshyagupta21 reviewed Jul 28, 2019

View reviewed changes

skunkworks_crow/src/main/java/org/odk/share/views/ui/bluetooth/BtReceiverActivity.java Outdated Show resolved Hide resolved

huangyz0918 added 2 commits July 28, 2019 18:06

Merge branch 'master' into issue#284-location-permission

361e8d5

🚑 quickfix: added the api checks.

c3e21b0

lakshyagupta21 approved these changes Jul 28, 2019

View reviewed changes

lakshyagupta21 suggested changes Jul 28, 2019

View reviewed changes

huangyz0918 added 3 commits July 28, 2019 22:20

🚑 quickfix: request the necessary permission at the main page.

4f5e282

✨ feat: added permission check to receiver.

9a4539e

✨ feat: added permission check to bluetooth sender.

b76c7ac

lakshyagupta21 suggested changes Jul 30, 2019

View reviewed changes

🚑 quickfix: quick fixes according to @lakshyagupta21

ab2da8f

lakshyagupta21 reviewed Jul 30, 2019

View reviewed changes

skunkworks_crow/build.gradle Show resolved Hide resolved

🚑 quickfix:

cc35274

updated the open source license.

lakshyagupta21 reviewed Jul 30, 2019

View reviewed changes

skunkworks_crow/src/main/assets/open_source_licenses.html Outdated Show resolved Hide resolved

huangyz0918 added 3 commits July 30, 2019 11:28

🚑 quickfix: removed the mokito, etc.

0da1cdd

🚑 quickfix: added MIT license section.

71a325a

✏️ typo: fixed a typo.

a9317af

lakshyagupta21 suggested changes Aug 4, 2019

View reviewed changes

huangyz0918 added 2 commits August 4, 2019 17:57

🚑 quickfix: fixed the strings.

8a73001

🚑 quickfix: improved the permission checks.

936a4d6

lakshyagupta21 reviewed Aug 6, 2019

View reviewed changes

huangyz0918 added 2 commits August 6, 2019 17:37

🚑 quickfix: apply the permission checks to all APIs.

cf6da3d

🚑 quickfix: fixed the toasts.

ddf3864

ajay-prabhakar mentioned this pull request Aug 8, 2019

App crashes when pressing send button #294

Closed

lakshyagupta21 reviewed Aug 10, 2019

View reviewed changes

skunkworks_crow/src/main/java/org/odk/share/views/ui/bluetooth/BtSenderActivity.java Outdated Show resolved Hide resolved

🚑 quickfix: removed the comment.

bbabe14

lakshyagupta21 approved these changes Aug 10, 2019

View reviewed changes

lakshyagupta21 merged commit ae84efc into getodk:master Aug 10, 2019

This was referenced Aug 11, 2019

App not starting if permission is denied #295

Closed

[MRG] Fix the result presenting #281

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] Add the location permission for android 9. #286

[MRG] Add the location permission for android 9. #286

huangyz0918 commented Jul 28, 2019 •

edited

Loading

lakshyagupta21 Jul 28, 2019

huangyz0918 Jul 28, 2019

lakshyagupta21 Jul 28, 2019

lakshyagupta21 Jul 28, 2019

huangyz0918 Jul 28, 2019

lakshyagupta21 Jul 28, 2019

huangyz0918 commented Jul 29, 2019

lakshyagupta21 commented Jul 29, 2019

huangyz0918 commented Jul 30, 2019

huangyz0918 commented Aug 3, 2019

ajay-prabhakar commented Aug 3, 2019 •

edited

Loading

huangyz0918 commented Aug 4, 2019

lakshyagupta21 Aug 4, 2019

huangyz0918 Aug 4, 2019

lakshyagupta21 Aug 6, 2019

lakshyagupta21 commented Aug 6, 2019

huangyz0918 commented Aug 6, 2019

lakshyagupta21 commented Aug 6, 2019

huangyz0918 commented Aug 6, 2019

huangyz0918 commented Aug 6, 2019

lakshyagupta21 commented Aug 7, 2019

huangyz0918 commented Aug 10, 2019

[MRG] Add the location permission for android 9. #286

[MRG] Add the location permission for android 9. #286

Conversation

huangyz0918 commented Jul 28, 2019 • edited Loading

What has been done to verify that this works as intended?

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Before submitting this PR, please make sure you have:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huangyz0918 commented Jul 29, 2019

lakshyagupta21 commented Jul 29, 2019

huangyz0918 commented Jul 30, 2019

huangyz0918 commented Aug 3, 2019

ajay-prabhakar commented Aug 3, 2019 • edited Loading

huangyz0918 commented Aug 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lakshyagupta21 commented Aug 6, 2019

huangyz0918 commented Aug 6, 2019

lakshyagupta21 commented Aug 6, 2019

huangyz0918 commented Aug 6, 2019

huangyz0918 commented Aug 6, 2019

lakshyagupta21 commented Aug 7, 2019

huangyz0918 commented Aug 10, 2019

huangyz0918 commented Jul 28, 2019 •

edited

Loading

ajay-prabhakar commented Aug 3, 2019 •

edited

Loading