Skip to content

Commit

Permalink
feat(permission): be more explicit about asking for background locati…
Browse files Browse the repository at this point in the history
…on permission

Hopefully fixes #1759
  • Loading branch information
growse committed Jul 30, 2024
1 parent 2e4fd25 commit 04df53f
Show file tree
Hide file tree
Showing 19 changed files with 288 additions and 47 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Version 2.5.1

### New features

- The background location permission is explicitly asked for in the welcome activity. It's also prompted if missing (but foreground location permissions are present) in the map activity, to catch people upgrading from <2.5.0.

### Bug fixes

- Re-added `tst` from Lwt MQTT message type that was accidentally dropped in 2.5.0 (#1766)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.platform.app.InstrumentationRegistry.getInstrumentation
import androidx.test.runner.lifecycle.ActivityLifecycleMonitorRegistry
import androidx.test.runner.lifecycle.Stage
import com.adevinta.android.barista.interaction.BaristaDialogInteractions.clickDialogNegativeButton
import com.adevinta.android.barista.interaction.BaristaDialogInteractions.clickDialogPositiveButton
import com.adevinta.android.barista.interaction.BaristaDrawerInteractions.openDrawer
import com.adevinta.android.barista.interaction.BaristaEditTextInteractions
Expand Down Expand Up @@ -198,14 +199,19 @@ fun enableDeviceLocation() {
}
}

/** Who knows what order these will appear in. */
fun grantMapActivityPermissions() {
fun grantNotificationAndForegroundPermissions() {
PermissionGranter.allowPermissionsIfNeeded(Manifest.permission.POST_NOTIFICATIONS)
PermissionGranter.allowPermissionsIfNeeded(Manifest.permission.ACCESS_FINE_LOCATION)
PermissionGranter.allowPermissionsIfNeeded(Manifest.permission.POST_NOTIFICATIONS)
PermissionGranter.allowPermissionsIfNeeded(Manifest.permission.ACCESS_FINE_LOCATION)
}

/** Who knows what order these will appear in. */
fun grantMapActivityPermissions() {
grantNotificationAndForegroundPermissions()
clickDialogNegativeButton()
}

/**
* Write file to device
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import com.adevinta.android.barista.interaction.BaristaClickInteractions.clickOn
import com.adevinta.android.barista.interaction.BaristaDrawerInteractions.openDrawer
import com.adevinta.android.barista.interaction.BaristaEditTextInteractions.writeTo
import com.fasterxml.jackson.databind.ObjectMapper
import org.hamcrest.Matchers.not
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Before
Expand All @@ -25,7 +24,7 @@ import org.owntracks.android.testutils.TestWithAnActivity
import org.owntracks.android.testutils.TestWithAnMQTTBroker
import org.owntracks.android.testutils.TestWithAnMQTTBrokerImpl
import org.owntracks.android.testutils.getText
import org.owntracks.android.testutils.grantMapActivityPermissions
import org.owntracks.android.testutils.grantNotificationAndForegroundPermissions
import org.owntracks.android.ui.waypoints.WaypointsActivity

@OptIn(ExperimentalUnsignedTypes::class)
Expand All @@ -36,7 +35,7 @@ class WaypointsActivityTests :
TestWithAnMQTTBroker by TestWithAnMQTTBrokerImpl() {
@Before
fun grantPermissions() {
grantMapActivityPermissions()
grantNotificationAndForegroundPermissions()
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,112 @@ package org.owntracks.android.ui
import android.Manifest
import androidx.test.espresso.Espresso.pressBack
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.MediumTest
import androidx.test.filters.SdkSuppress
import com.adevinta.android.barista.assertion.BaristaVisibilityAssertions.assertDisplayed
import com.adevinta.android.barista.assertion.BaristaVisibilityAssertions.assertNotDisplayed
import com.adevinta.android.barista.interaction.BaristaClickInteractions.clickOn
import com.adevinta.android.barista.interaction.PermissionGranter.allowPermissionsIfNeeded
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.owntracks.android.R
import org.owntracks.android.testutils.TestWithAnActivity
import org.owntracks.android.testutils.doIfViewNotVisible
import org.owntracks.android.testutils.getCurrentActivity
import org.owntracks.android.ui.map.MapActivity
import org.owntracks.android.ui.welcome.WelcomeActivity

@RunWith(AndroidJUnit4::class)
@MediumTest
class WelcomeActivityTests : TestWithAnActivity<WelcomeActivity>(WelcomeActivity::class.java) {

@Test
fun welcomeActivityStartsWithIntroFragment() {
assertDisplayed(R.string.welcome_heading)
assertDisplayed(R.string.welcome_description)
assertDisplayed(R.id.btn_next)
}

@Test
fun welcomeActivityShowsConnectionSetupDetails() {
// Intro fragment
clickOn(R.id.btn_next)
// Connection setup fragment
assertDisplayed(R.string.welcome_connection_setup_title)
assertDisplayed(R.string.welcome_connection_setup_description)
assertDisplayed(R.id.btn_next)
}

@SdkSuppress(minSdkVersion = 34)
@Test
fun welcomeActivityStartsTheMapActivityWhenDone() {
// Intro fragment
clickOn(R.id.btn_next)
// Connection setup fragment
clickOn(R.id.btn_next)
// Location Permission fragment
doIfViewNotVisible(R.id.btn_next) {
R.id.ui_fragment_welcome_location_permissions_request.run {
assertDisplayed(this)
clickOn(this)
}
allowPermissionsIfNeeded(Manifest.permission.ACCESS_FINE_LOCATION)
}
clickOn(R.id.btn_next)
// Notification permission fragment
doIfViewNotVisible(R.id.btn_next) {
R.id.ui_fragment_welcome_notification_permissions_request.run {
assertDisplayed(this)
clickOn(this)
}
allowPermissionsIfNeeded(Manifest.permission.POST_NOTIFICATIONS)
}
clickOn(R.id.btn_next)
clickOn(R.id.btn_done)
assertTrue(getCurrentActivity() is MapActivity)
}

@SdkSuppress(minSdkVersion = 29)
@Test
fun welcomeActivityPromptsForBackgroundLocationPermission() {
clickOn(R.id.btn_next)
clickOn(R.id.btn_next)

// Location permissions fragment
assertDisplayed(R.string.welcome_location_permission_description)
doIfViewNotVisible(R.id.btn_next) {
R.id.ui_fragment_welcome_location_permissions_request.run {
assertDisplayed(this)
clickOn(this)
}
allowPermissionsIfNeeded(Manifest.permission.ACCESS_FINE_LOCATION)
}

assertDisplayed(R.id.btn_next)
assertDisplayed(R.id.ui_fragment_welcome_location_background_permissions_request)
}

@SdkSuppress(maxSdkVersion = 28)
@Test
fun welcomeActivityDoesntPromptForBackgroundLocationPermission() {
clickOn(R.id.btn_next)
clickOn(R.id.btn_next)

// Location permissions fragment
assertDisplayed(R.string.welcome_location_permission_description)
doIfViewNotVisible(R.id.btn_next) {
R.id.ui_fragment_welcome_location_permissions_request.run {
assertDisplayed(this)
clickOn(this)
}
allowPermissionsIfNeeded(Manifest.permission.ACCESS_FINE_LOCATION)
}

assertDisplayed(R.id.btn_next)
assertNotDisplayed(R.id.ui_fragment_welcome_location_background_permissions_request)
}

@SdkSuppress(minSdkVersion = 34)
@Test
fun welcomeActivityDisplaysCorrectFragmentsWithNotificationPermissions() {
Expand All @@ -36,7 +128,7 @@ class WelcomeActivityTests : TestWithAnActivity<WelcomeActivity>(WelcomeActivity
}

// Location permissions fragment
assertDisplayed(R.id.ui_fragment_welcome_location_permissions_message)
assertDisplayed(R.string.welcome_location_permission_description)
doIfViewNotVisible(R.id.btn_next) {
R.id.ui_fragment_welcome_location_permissions_request.run {
assertDisplayed(this)
Expand All @@ -46,11 +138,12 @@ class WelcomeActivityTests : TestWithAnActivity<WelcomeActivity>(WelcomeActivity
}
R.id.btn_next.run {
assertDisplayed(this)
assertDisplayed(R.id.ui_fragment_welcome_location_background_permissions_request)
clickOn(this)
}

// Notification permissions fragment
assertDisplayed(R.id.ui_fragment_welcome_notification_permissions_message)
assertDisplayed(R.string.welcome_notification_permission_description)
doIfViewNotVisible(R.id.btn_next) {
R.id.ui_fragment_welcome_notification_permissions_request.run {
assertDisplayed(this)
Expand Down Expand Up @@ -90,7 +183,7 @@ class WelcomeActivityTests : TestWithAnActivity<WelcomeActivity>(WelcomeActivity
}

// Location permissions fragment
assertDisplayed(R.id.ui_fragment_welcome_location_permissions_message)
assertDisplayed(R.string.welcome_location_permission_description)
doIfViewNotVisible(R.id.btn_next) {
R.id.ui_fragment_welcome_location_permissions_request.run {
assertDisplayed(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ interface DefaultsProvider {
StringMaxTwoAlphaNumericChars(preferences.deviceId.takeLast(2).ifEmpty { "na" })
Preferences::url -> ""
Preferences::userDeclinedEnableLocationPermissions -> false
Preferences::userDeclinedEnableBackgroundLocationPermissions -> false
Preferences::userDeclinedEnableLocationServices -> false
Preferences::userDeclinedEnableNotificationPermissions -> false
Preferences::username -> ""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,9 @@ constructor(
@Preference(exportModeMqtt = false, exportModeHttp = false)
var userDeclinedEnableLocationPermissions: Boolean by preferencesStore

@Preference(exportModeMqtt = false, exportModeHttp = false)
var userDeclinedEnableBackgroundLocationPermissions: Boolean by preferencesStore

@Preference(exportModeMqtt = false, exportModeHttp = false)
var userDeclinedEnableLocationServices: Boolean by preferencesStore

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ import org.owntracks.android.preferences.types.MonitoringMode
import org.owntracks.android.preferences.types.MonitoringMode.Companion.getByValue
import org.owntracks.android.services.worker.Scheduler
import org.owntracks.android.support.DateFormatter.formatDate
import org.owntracks.android.support.RequirementsChecker
import org.owntracks.android.support.RunThingsOnOtherThreads
import org.owntracks.android.test.SimpleIdlingResource
import org.owntracks.android.ui.map.MapActivity
Expand Down Expand Up @@ -114,6 +115,8 @@ class BackgroundService : LifecycleService(), Preferences.OnPreferenceChangeList

@Inject lateinit var locationProviderClient: LocationProviderClient

@Inject lateinit var requirementsChecker: RequirementsChecker

@Inject
@Named("contactsClearedIdlingResource")
lateinit var contactsClearedIdlingResource: SimpleIdlingResource
Expand Down Expand Up @@ -296,13 +299,8 @@ class BackgroundService : LifecycleService(), Preferences.OnPreferenceChangeList
INTENT_ACTION_BOOT_COMPLETED,
INTENT_ACTION_PACKAGE_REPLACED -> {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
val backgroundLocationPermissionDenied =
ActivityCompat.checkSelfPermission(
this, Manifest.permission.ACCESS_BACKGROUND_LOCATION) ==
PackageManager.PERMISSION_DENIED
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R &&
!hasBeenStartedExplicitly &&
backgroundLocationPermissionDenied) {
if (!requirementsChecker.hasBackgroundLocationPermission() &&
!hasBeenStartedExplicitly) {
notifyUserOfBackgroundLocationRestriction()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ open class OSSRequirementsChecker @Inject constructor(open val context: Context)
ContextCompat.checkSelfPermission(context, Manifest.permission.ACCESS_COARSE_LOCATION) ==
PackageManager.PERMISSION_GRANTED

override fun hasBackgroundLocationPermission(): Boolean =
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
ContextCompat.checkSelfPermission(
context, Manifest.permission.ACCESS_BACKGROUND_LOCATION) ==
PackageManager.PERMISSION_GRANTED
} else {
true
}

override fun isLocationServiceEnabled(): Boolean =
(context.getSystemService(Context.LOCATION_SERVICE) as LocationManager?)?.run {
LocationManagerCompat.isLocationEnabled(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package org.owntracks.android.support
interface RequirementsChecker {
fun hasLocationPermissions(): Boolean

fun hasBackgroundLocationPermission(): Boolean

fun isLocationServiceEnabled(): Boolean

fun isPlayServicesCheckPassed(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import android.hardware.Sensor
import android.hardware.SensorManager
import android.hardware.SensorManager.SENSOR_DELAY_UI
import android.net.Uri
import android.os.Build
import android.os.Bundle
import android.os.IBinder
import android.provider.Settings
Expand Down Expand Up @@ -61,6 +62,7 @@ import org.owntracks.android.support.RequirementsChecker
import org.owntracks.android.test.CountingIdlingResourceShim
import org.owntracks.android.test.SimpleIdlingResource
import org.owntracks.android.ui.NotificationsStash
import org.owntracks.android.ui.mixins.BackgroundLocationPermissionRequester
import org.owntracks.android.ui.mixins.LocationPermissionRequester
import org.owntracks.android.ui.mixins.NotificationPermissionRequester
import org.owntracks.android.ui.mixins.ServiceStarter
Expand All @@ -81,6 +83,9 @@ class MapActivity :
this, ::notificationPermissionGranted, ::notificationPermissionDenied)
private val locationPermissionRequester =
LocationPermissionRequester(this, ::locationPermissionGranted, ::locationPermissionDenied)
private val backgroundLocationPermissionRequester =
BackgroundLocationPermissionRequester(
this, ::backgroundLocationPermissionGranted, ::backgroundLocationPermissionDenied)
private var service: BackgroundService? = null
private var bottomSheetBehavior: BottomSheetBehavior<LinearLayoutCompat>? = null
private var menu: Menu? = null
Expand Down Expand Up @@ -392,7 +397,7 @@ class MapActivity :
}

/**
* User has declined notification permissions. Log this in the preferences so we don't keep askin
* User has declined notification permissions. Log this in the preferences so we don't keep asking
* them
*/
private fun notificationPermissionDenied() {
Expand Down Expand Up @@ -437,6 +442,24 @@ class MapActivity :
.show()
}

/**
* User has declined to enable background location permissions. Log this in the preferences so we
* don't keep asking
*/
private fun backgroundLocationPermissionGranted() {
Timber.d("Background location permission granted")
preferences.userDeclinedEnableBackgroundLocationPermissions = false
}

/**
* User has declined to enable background location permissions. Log this in the preferences so we
* don't keep asking
*/
private fun backgroundLocationPermissionDenied() {
Timber.d("Background location permission denied")
preferences.userDeclinedEnableBackgroundLocationPermissions = true
}

enum class CheckPermissionsResult {
HAS_PERMISSIONS,
NO_PERMISSIONS_LAUNCHED_REQUEST,
Expand Down Expand Up @@ -521,9 +544,11 @@ class MapActivity :
updateMonitoringModeMenu()
viewModel.updateMyLocationStatus()

// Request Notification permissions
when (checkAndRequestNotificationPermissions()) {
CheckPermissionsResult.HAS_PERMISSIONS,
CheckPermissionsResult.NO_PERMISSIONS_NOT_LAUNCHED_REQUEST -> {
// Request Location permissions
when (checkAndRequestLocationPermissions(false)) {
CheckPermissionsResult.NO_PERMISSIONS_LAUNCHED_REQUEST -> {
Timber.d("Launched location permission request")
Expand All @@ -535,6 +560,12 @@ class MapActivity :
Timber.d("Has location permissions")
if (checkAndRequestLocationServicesEnabled(false)) {
viewModel.requestLocationUpdatesForBlueDot()
// Request background location permissions if needed
if (!requirementsChecker.hasBackgroundLocationPermission() &&
Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q &&
!preferences.userDeclinedEnableBackgroundLocationPermissions) {
backgroundLocationPermissionRequester.requestLocationPermissions(this) { true }
}
}
}
}
Expand Down
Loading

0 comments on commit 04df53f

Please sign in to comment.