From 9575ca94eeca6c00fe5eeb091d43389b697d790f Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:51:33 -0800 Subject: [PATCH 1/7] Update startGetLocation to only run if location is shared --- .../location/internal/LocationManager.kt | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt index de25bbead8..25863ef7e9 100644 --- a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt +++ b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt @@ -179,13 +179,18 @@ internal class LocationManager( // Started from this class or PermissionActivity private suspend fun startGetLocation() { - Logging.debug("LocationManager.startGetLocation()") // with lastLocation: " + lastLocation) - try { - if (!_locationController.start()) { - Logging.warn("LocationManager.startGetLocation: not possible, no location dependency found") + if (isShared) { + Logging.debug("LocationManager.startGetLocation()") // with lastLocation: " + lastLocation) + try { + if (!_locationController.start()) { + Logging.warn("LocationManager.startGetLocation: not possible, no location dependency found") + } + } catch (t: Throwable) { + Logging.warn( + "LocationManager.startGetLocation: Location permission exists but there was an error initializing: ", + t + ) } - } catch (t: Throwable) { - Logging.warn("LocationManager.startGetLocation: Location permission exists but there was an error initializing: ", t) } } } From 271143d19d62d647b72536424982b6e48b4a4bc4 Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Wed, 13 Dec 2023 14:11:00 -0800 Subject: [PATCH 2/7] Fix linting error --- .../java/com/onesignal/location/internal/LocationManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt index 25863ef7e9..a40327fd07 100644 --- a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt +++ b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt @@ -188,7 +188,7 @@ internal class LocationManager( } catch (t: Throwable) { Logging.warn( "LocationManager.startGetLocation: Location permission exists but there was an error initializing: ", - t + t, ) } } From b1f4e8eb077a9d76d784d46257e80ea0557ccb5d Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Wed, 13 Dec 2023 14:30:14 -0800 Subject: [PATCH 3/7] Update startGetLocation to return early if isShared is false --- .../location/internal/LocationManager.kt | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt index a40327fd07..6259262a01 100644 --- a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt +++ b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt @@ -179,18 +179,20 @@ internal class LocationManager( // Started from this class or PermissionActivity private suspend fun startGetLocation() { - if (isShared) { - Logging.debug("LocationManager.startGetLocation()") // with lastLocation: " + lastLocation) - try { - if (!_locationController.start()) { - Logging.warn("LocationManager.startGetLocation: not possible, no location dependency found") - } - } catch (t: Throwable) { - Logging.warn( - "LocationManager.startGetLocation: Location permission exists but there was an error initializing: ", - t, - ) + if (!isShared) { + return + } + + Logging.debug("LocationManager.startGetLocation()") // with lastLocation: " + lastLocation) + try { + if (!_locationController.start()) { + Logging.warn("LocationManager.startGetLocation: not possible, no location dependency found") } + } catch (t: Throwable) { + Logging.warn( + "LocationManager.startGetLocation: Location permission exists but there was an error initializing: ", + t, + ) } } } From b51cbdbfdbf5deed417665614a05300c57096114 Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Tue, 19 Dec 2023 16:24:42 -0800 Subject: [PATCH 4/7] Update isShared default to false --- .../core/internal/preferences/IPreferencesService.kt | 5 +++++ .../java/com/onesignal/location/internal/LocationManager.kt | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/IPreferencesService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/IPreferencesService.kt index e53714c15c..ac0265b278 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/IPreferencesService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/IPreferencesService.kt @@ -194,6 +194,11 @@ object PreferenceOneSignalKeys { */ const val PREFS_OS_LAST_LOCATION_TIME = "OS_LAST_LOCATION_TIME" + /** + * (Boolean) Whether location should be shared with OneSignal. + */ + const val PREFS_OS_LOCATION_SHARED = "OS_LOCATION_SHARED" + // Permissions /** diff --git a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt index 6259262a01..a40e28f5b8 100644 --- a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt +++ b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt @@ -4,6 +4,7 @@ import android.os.Build import com.onesignal.common.AndroidUtils import com.onesignal.common.threading.suspendifyOnThread import com.onesignal.core.internal.application.IApplicationService +import com.onesignal.core.internal.preferences.IPreferencesService import com.onesignal.core.internal.startup.IStartableService import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging @@ -22,12 +23,14 @@ internal class LocationManager( private val _capturer: ILocationCapturer, private val _locationController: ILocationController, private val _locationPermissionController: LocationPermissionController, + private val _prefs: IPreferencesService, ) : ILocationManager, IStartableService, ILocationPermissionChangedHandler { - private var _isShared: Boolean = false + private var _isShared: Boolean = _prefs.getBool("OneSignal", "PREFS_OS_LOCATION_SHARED", false)!! override var isShared get() = _isShared set(value) { Logging.debug("LocationManager.setIsShared(value: $value)") + _prefs.saveBool("OneSignal", "PREFS_OS_LOCATION_SHARED", value) _isShared = value } From f773b653eeb05f400c156a8306068e4732374c62 Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Tue, 19 Dec 2023 16:25:26 -0800 Subject: [PATCH 5/7] Allow for mid-session update of location permission --- .../java/com/onesignal/location/internal/LocationManager.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt index a40e28f5b8..c6cc77a839 100644 --- a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt +++ b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt @@ -32,6 +32,8 @@ internal class LocationManager( Logging.debug("LocationManager.setIsShared(value: $value)") _prefs.saveBool("OneSignal", "PREFS_OS_LOCATION_SHARED", value) _isShared = value + + onLocationPermissionChanged(value) } override fun start() { From defa69c35a32948b3e2cb254d4a705b1f99f48a1 Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Tue, 26 Dec 2023 12:46:56 -0800 Subject: [PATCH 6/7] Remove early return for requestPermission Since isShared now defaults to false, remove early return and add logging to update isShared to true. --- .../java/com/onesignal/location/internal/LocationManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt index c6cc77a839..96d8c0c545 100644 --- a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt +++ b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt @@ -76,7 +76,7 @@ internal class LocationManager( var result = false withContext(Dispatchers.Main) { if (!isShared) { - return@withContext false + Logging.error("Location permissions must be granted by setting isShared to true") } val hasFinePermissionGranted = From 747f85b2af8bddf3d815a7b6c7b4816ec213df80 Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Wed, 27 Dec 2023 09:56:32 -0800 Subject: [PATCH 7/7] Update LocationManager - Change log to warn vs. error - Access and save to constant PREFS_OS_LOCATION_SHARED --- .../com/onesignal/location/internal/LocationManager.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt index 96d8c0c545..903183d369 100644 --- a/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt +++ b/OneSignalSDK/onesignal/location/src/main/java/com/onesignal/location/internal/LocationManager.kt @@ -5,6 +5,8 @@ import com.onesignal.common.AndroidUtils import com.onesignal.common.threading.suspendifyOnThread import com.onesignal.core.internal.application.IApplicationService import com.onesignal.core.internal.preferences.IPreferencesService +import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys +import com.onesignal.core.internal.preferences.PreferenceStores import com.onesignal.core.internal.startup.IStartableService import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging @@ -25,12 +27,12 @@ internal class LocationManager( private val _locationPermissionController: LocationPermissionController, private val _prefs: IPreferencesService, ) : ILocationManager, IStartableService, ILocationPermissionChangedHandler { - private var _isShared: Boolean = _prefs.getBool("OneSignal", "PREFS_OS_LOCATION_SHARED", false)!! + private var _isShared: Boolean = _prefs.getBool(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_OS_LOCATION_SHARED, false)!! override var isShared get() = _isShared set(value) { Logging.debug("LocationManager.setIsShared(value: $value)") - _prefs.saveBool("OneSignal", "PREFS_OS_LOCATION_SHARED", value) + _prefs.saveBool(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_OS_LOCATION_SHARED, value) _isShared = value onLocationPermissionChanged(value) @@ -76,7 +78,7 @@ internal class LocationManager( var result = false withContext(Dispatchers.Main) { if (!isShared) { - Logging.error("Location permissions must be granted by setting isShared to true") + Logging.warn("Requesting location permission, but location sharing must also be enabled by setting isShared to true") } val hasFinePermissionGranted =