Display warning in toast if background location is requested but location is turned off in settings

[lognaturel]

Let the enumerator know if no location provider is available for a background location request. Forum user request.

BackgroundLocationManager.locationPermissionGranted does this for audit location requests but not for background location requests. This was not done for background location requests because locationClient.isLocationAvailable docs say Must be called <b>after</b> Location updates have been requested. In the case of background location requests, the setgeopoint action manages its own location client so BackgroundLocationManager doesn't request location updates. However, I don't think that the order is strictly necessary and we could call locationClient.isLocationAvailable. I believe the reason that requirement is there is to ensure the priority level is set before. It may not be relevant in this case.

[neilong31]

Hello @lognaturel ! I'd love to try and work on this issue.

[lognaturel]

That would be wonderful, @neilong31! I'd recommend starting by making sure you understand the behavior of isLocationAvailable if it's called before location updates are requested. Is that actually a reasonable thing to do or not? You may also have ideas for alternate ways to find out whether location is available or not.

[neilong31]

Thank you for a starting point! I'm looking through and trying to understand the behavior. I'll submit a PR detailing my logic.

[chidauri]

Hello @lognaturel , I'd like to work on this issue, is this already being worked on?

I found something strange!
https://github.com/opendatakit/collect/blob/17f9ddeda041759a8f46ede52214e064b773aa68/collect_app/src/main/java/org/odk/collect/android/activities/FormEntryActivity.java#L2125-L2128

This block should only run when user has changed the location settings, right? In my case this runs even if i don't change settings, i checked with a Toast message and it is being displayed every time i open up the form entry.

[lognaturel]

Please go ahead and claim it, @chidauri.

locationPermissionsPreviouslyGranted is a field on the activity so it is set to false each time the activity is rebuilt. So indeed there will always be a permissions check on activity open. I believe that’s the intended flow but haven’t gone back to look at the details. You can verify that this makes sense and offer an alternative if you think it doesn’t. The field represents something more like “permissions known to have been granted during this form edit session” and perhaps you have an alternate name to suggest.

[chidauri]

@lognaturel Hi, can you please point me to where are start-geopoint location requests handled?
Ps. I have made some changes regarding this issue and i tested it with disabling location myself and it shows a Toast message(which was expected), is there another way i can test this? should i open a PR?
Thanks :)

[chidauri]

@opendatakit-bot claim

[seadowg]

Was just looking through #3575. I'm a little confused around the issue description:

BackgroundLocationManager.locationPermissionGranted does this for audit location requests but not for background location requests.

Is that referring to the dialog that's shown when we return a BackgroundLocationMessage.PROVIDERS_DISABLED from locationPermissionGranted. That happens here in displayUIFor in FormEntryActivity as far as I can see:

if (backgroundLocationMessage == BackgroundLocationManager.BackgroundLocationMessage.PROVIDERS_DISABLED) {
    new LocationProvidersDisabledDialog().show(getSupportFragmentManager(), LocationProvidersDisabledDialog.LOCATION_PROVIDERS_DISABLED_DIALOG_TAG);
    return;
}

If that's right? If so should we be showing a dialog rather than a toast to keep it consistent? I wouldn't be surprised if I've massively misinterpreted all of this...

[lognaturel]

@seadowg Thanks for taking a step back to discuss the desired behavior! You're correct that for the audit a similar notification is shown in the dialog. This gives the user an opportunity to jump directly to adjust their settings.

My reasoning for using a toast in this case was in the forum thread. When designing the background geopoint feature, we got the feedback that there could be legitimate reasons for users to turn off their location. That made me think that perhaps access to background location should be less strongly encouraged than access to location for audit purposes. In either case, if an organization wants to guarantee that location settings are on and can't trust their enumerators, they should put some Android-level assurances in place.

A toast feels like the "poke" that the requesting user was requesting without being as strong as a dialog. However, I don't feel super strongly about this. What do you think, @seadowg, @chidauri?

Ultimately it comes down to whether we think that location settings are more likely to be off because of a choice the enumerator made in which case a dialog is annoying or because of a mistake/oversight in which case a dialog is helpful. One approach would be to start with the most annoying (and consistent) option and to walk it back if users complain.

[chidauri]

@lognaturel yep in case it is a mistake/oversight, a dialog is a better choice and if the location is turned off knowingly, it should not hurt enumerator to go through an extra step of denying that dialog(or what organisation wants).

[seadowg]

I think a Banner might be the right middleground here as it would also let us link the enumerator to location settings without blocking them. However, I wouldn't want to look at adding new Material components to the FormEntryActivity so I thinkn a toast is probably the right thing to do for the moment.

[lognaturel]

Thanks for the feedback, @chidauri and @seadowg. @seadowg I think you're absolutely right that a banner would be ideal. So perhaps after it's been introduced for the scoped storage change we can come back to this.

In the mean time, I think @chidauri is right that it's not a huge deal to exit out of the dialog. And looking at the code, it's annoying to have a special case and a separate message string for background geopoint. So @chidauri, could you please simplify it so it just uses the same dialog as in the audit case? Thanks very much!