From 14c7ce755836cea3db273cdb37fa3adf882e1b23 Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Wed, 16 Sep 2026 14:26:50 -0700 Subject: [PATCH] feat(settings): profile location from the device or by hand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the web's "Profile location" section to Android Settings: the optional latitude/longitude pair on the account, which the help centre documents as "Optional location for your profile, used by the Weather widget and similar location-aware features". It sits between Message settings and Permissions, the order /help/settings itself uses. Coordinates can be captured from the device or typed in: - Capture asks for ACCESS_COARSE_LOCATION only — the stored pair drives city-level surfaces, not navigation — and always shows the app's own rationale *before* the system dialog, since afterwards the decision has already been made. The reading is one-shot, taken only from a press of "Use my location", and rounded to about a kilometre before it is saved. - Refusing costs nothing. Any denial, including a permanent one, leaves manual entry and the rest of Settings working; the section says so, in plain text and not in the error colour. - Manual entry is validated against -90..90 and -180..180 and refused before any request is made. A saved location cannot be removed, and the UI says so instead of offering a button that would fail. PATCH /api/user/update validates `latitude` as a required number whenever the key is present, so a JSON null, an empty string and the string "null" are all answered with 400 "latitude must be a number between -90 and 90"; omitting the key leaves the value untouched, and 0,0 is a real position in the Gulf of Guinea rather than an absence. So "Clear location" is rendered disabled with the reason beside it, the view model can only build LocationUpdate.Set, and the rationale dialog says up front that a location can later be replaced but not removed. The three-state LocationUpdate / JsonElement plumbing stays, with the probe results recorded on LocationUpdate.Clear and its request shape pinned by tests, so re-enabling it when the API allows a null is a one-liner. This is an API gap, not an app limitation. Nothing is applied optimistically here: unlike a page size, a row claiming the account stores a position it does not would be a privacy claim the app cannot back up, so the displayed value is only ever what the server confirmed. Closes #37 --- app/src/main/AndroidManifest.xml | 8 + feature/profile/build.gradle.kts | 5 +- .../profile/ui/ProfileLocationSectionTest.kt | 212 ++++++++ .../data/location/DeviceLocationSource.kt | 44 ++ .../location/SystemDeviceLocationSource.kt | 112 +++++ .../profile/data/mapper/SettingsMappers.kt | 33 +- .../data/remote/dto/ProfileRequests.kt | 13 +- .../feature/profile/di/ProfileModule.kt | 15 + .../feature/profile/domain/ProfileLocation.kt | 106 ++++ .../feature/profile/domain/UserSettings.kt | 12 +- .../ui/settings/ProfileLocationSection.kt | 463 ++++++++++++++++++ .../ui/settings/ProfileLocationViewModel.kt | 222 +++++++++ .../profile/ui/settings/SettingsScreen.kt | 17 +- .../data/DefaultSettingsRepositoryTest.kt | 111 ++++- .../profile/data/SettingsMappersTest.kt | 42 +- .../profile/ui/FakeDeviceLocationSource.kt | 25 + .../profile/ui/FakeSettingsRepository.kt | 13 + .../profile/ui/ProfileLocationInputTest.kt | 84 ++++ .../ui/ProfileLocationViewModelTest.kt | 388 +++++++++++++++ 19 files changed, 1910 insertions(+), 15 deletions(-) create mode 100644 feature/profile/src/androidTest/kotlin/com/interlinedlist/android/feature/profile/ui/ProfileLocationSectionTest.kt create mode 100644 feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/location/DeviceLocationSource.kt create mode 100644 feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/location/SystemDeviceLocationSource.kt create mode 100644 feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/ProfileLocation.kt create mode 100644 feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/ProfileLocationSection.kt create mode 100644 feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/ProfileLocationViewModel.kt create mode 100644 feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeDeviceLocationSource.kt create mode 100644 feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/ProfileLocationInputTest.kt create mode 100644 feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/ProfileLocationViewModelTest.kt diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 03d93a3..6269e51 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -5,6 +5,14 @@ + + Unit = {}, + onSaveLocation: (Double, Double) -> Unit = { _, _ -> }, + onDismissNotice: () -> Unit = {}, + ) { + composeRule.setContent { + InterlinedListTheme { + ProfileLocationGroup( + state = state, + onUseDeviceLocation = onUseDeviceLocation, + onSaveLocation = onSaveLocation, + onDismissNotice = onDismissNotice, + ) + } + } + } + + @Test + fun storedLocation_isShownWithWhatHappensToIt() { + setContent(ProfileLocationUiState(coordinates = Coordinates(47.6062, -122.3321))) + + composeRule.onNodeWithTag(ProfileLocationTestTags.GROUP).assertIsDisplayed() + composeRule.onNodeWithTag(ProfileLocationTestTags.VALUE) + .assertTextContains("Saved location: 47.6062, -122.3321") + // The privacy note is not optional decoration: it says where this ends up. + composeRule.onNodeWithTag(ProfileLocationTestTags.PRIVACY_NOTE).assertIsDisplayed() + } + + @Test + fun storedLocation_explainsThatItCannotBeRemoved() { + // The API refuses every way of unsetting a location, so the button that would + // do it must never look pressable — and the reason has to be on screen, at the + // control, rather than left for the user to discover by failing. + setContent(ProfileLocationUiState(coordinates = Coordinates(47.6062, -122.3321))) + + composeRule.onNodeWithTag(ProfileLocationTestTags.CLEAR).assertIsNotEnabled() + composeRule.onNodeWithTag(ProfileLocationTestTags.CANNOT_REMOVE).assertIsDisplayed() + } + + @Test + fun storedLocation_canBeReplacedByTypingAnother() { + // Replacing is the only supported way to change a saved location. + var saved: Pair? = null + setContent( + state = ProfileLocationUiState(coordinates = Coordinates(47.6062, -122.3321)), + onSaveLocation = { latitude, longitude -> saved = latitude to longitude }, + ) + + composeRule.onNodeWithTag(ProfileLocationTestTags.LATITUDE).performTextClearance() + composeRule.onNodeWithTag(ProfileLocationTestTags.LATITUDE).performTextInput("45.52") + composeRule.onNodeWithTag(ProfileLocationTestTags.LONGITUDE).performTextClearance() + composeRule.onNodeWithTag(ProfileLocationTestTags.LONGITUDE).performTextInput("-122.68") + composeRule.onNodeWithTag(ProfileLocationTestTags.SAVE).performClick() + + assertThat(saved).isEqualTo(45.52 to -122.68) + } + + @Test + fun noStoredLocation_saysSoAndOffersNothingToClear() { + setContent(ProfileLocationUiState(coordinates = null)) + + composeRule.onNodeWithTag(ProfileLocationTestTags.VALUE) + .assertTextContains("No location saved.") + composeRule.onNodeWithTag(ProfileLocationTestTags.CLEAR).assertIsNotEnabled() + composeRule.onNodeWithTag(ProfileLocationTestTags.SAVE).assertIsNotEnabled() + // Nothing to remove, so nothing to explain about removal. + composeRule.onNodeWithTag(ProfileLocationTestTags.CANNOT_REMOVE).assertDoesNotExist() + } + + @Test + fun typingCoordinates_andSaving_reportsThem() { + var saved: Pair? = null + setContent( + state = ProfileLocationUiState(coordinates = null), + onSaveLocation = { latitude, longitude -> saved = latitude to longitude }, + ) + + composeRule.onNodeWithTag(ProfileLocationTestTags.LATITUDE).performTextInput("47.6062") + composeRule.onNodeWithTag(ProfileLocationTestTags.LONGITUDE).performTextInput("-122.3321") + composeRule.onNodeWithTag(ProfileLocationTestTags.SAVE).performClick() + + assertThat(saved).isEqualTo(47.6062 to -122.3321) + } + + @Test + fun outOfRangeEntry_cannotBeSavedAndSaysWhy() { + var saved: Pair? = null + setContent( + state = ProfileLocationUiState(coordinates = null), + onSaveLocation = { latitude, longitude -> saved = latitude to longitude }, + ) + + composeRule.onNodeWithTag(ProfileLocationTestTags.LATITUDE).performTextInput("91") + composeRule.onNodeWithTag(ProfileLocationTestTags.LONGITUDE).performTextInput("0") + + composeRule.onNodeWithTag(ProfileLocationTestTags.ENTRY_ERROR).assertIsDisplayed() + composeRule.onNodeWithTag(ProfileLocationTestTags.SAVE).assertIsNotEnabled() + composeRule.onNodeWithTag(ProfileLocationTestTags.SAVE).performClick() + assertThat(saved).isNull() + } + + @Test + fun editingBackIntoRange_reEnablesSaving() { + setContent(ProfileLocationUiState(coordinates = null)) + + composeRule.onNodeWithTag(ProfileLocationTestTags.LATITUDE).performTextInput("991") + composeRule.onNodeWithTag(ProfileLocationTestTags.LONGITUDE).performTextInput("10") + composeRule.onNodeWithTag(ProfileLocationTestTags.SAVE).assertIsNotEnabled() + + composeRule.onNodeWithTag(ProfileLocationTestTags.LATITUDE).performTextClearance() + composeRule.onNodeWithTag(ProfileLocationTestTags.LATITUDE).performTextInput("47.6") + + composeRule.onNodeWithTag(ProfileLocationTestTags.SAVE).assertIsEnabled() + } + + @Test + fun useMyLocation_reportsTheRequest() { + var asked = false + setContent( + state = ProfileLocationUiState(coordinates = null), + onUseDeviceLocation = { asked = true }, + ) + + composeRule.onNodeWithTag(ProfileLocationTestTags.USE_DEVICE).performClick() + + assertThat(asked).isTrue() + } + + @Test + fun whileReadingTheDevice_theButtonIsBusy() { + setContent(ProfileLocationUiState(coordinates = null, isReadingDevice = true)) + + composeRule.onNodeWithTag(ProfileLocationTestTags.USE_DEVICE).assertIsNotEnabled() + } + + @Test + fun aRefusedPermission_isShownAndTheFieldsStayUsable() { + var saved: Pair? = null + setContent( + state = ProfileLocationUiState( + coordinates = null, + notice = LocationNotice("Location permission wasn't granted."), + ), + onSaveLocation = { latitude, longitude -> saved = latitude to longitude }, + ) + + composeRule.onNodeWithTag(ProfileLocationTestTags.NOTICE).assertIsDisplayed() + // Manual entry is the whole point of the fallback, so it must still work. + composeRule.onNodeWithTag(ProfileLocationTestTags.LATITUDE).performTextInput("47.6") + composeRule.onNodeWithTag(ProfileLocationTestTags.LONGITUDE).performTextInput("-122.3") + composeRule.onNodeWithTag(ProfileLocationTestTags.SAVE).performClick() + + assertThat(saved).isEqualTo(47.6 to -122.3) + } + + @Test + fun rationale_explainsBeforeAsking() { + var continued = false + var dismissed = false + composeRule.setContent { + InterlinedListTheme { + LocationPermissionRationaleDialog( + onContinue = { continued = true }, + onDismiss = { dismissed = true }, + ) + } + } + + composeRule.onNodeWithTag(ProfileLocationTestTags.RATIONALE).assertIsDisplayed() + composeRule.onNodeWithTag(ProfileLocationTestTags.RATIONALE_DISMISS).performClick() + assertThat(dismissed).isTrue() + assertThat(continued).isFalse() + + composeRule.onNodeWithTag(ProfileLocationTestTags.RATIONALE_CONTINUE).performClick() + assertThat(continued).isTrue() + } +} diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/location/DeviceLocationSource.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/location/DeviceLocationSource.kt new file mode 100644 index 0000000..bf7551e --- /dev/null +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/location/DeviceLocationSource.kt @@ -0,0 +1,44 @@ +package com.interlinedlist.android.feature.profile.data.location + +import com.interlinedlist.android.feature.profile.domain.Coordinates + +/** The outcome of one attempt to read this device's position. */ +sealed interface DeviceLocationResult { + + /** A fix was obtained. */ + data class Available(val coordinates: Coordinates) : DeviceLocationResult + + /** + * `ACCESS_COARSE_LOCATION` is not held, so nothing was read. Setting a location + * stays entirely optional, so this is an ordinary outcome and not an error. + */ + data object PermissionMissing : DeviceLocationResult + + /** + * The permission is held but the device produced no fix — location services off, + * no usable provider, or nothing reported inside the time budget. + */ + data object Unavailable : DeviceLocationResult +} + +/** + * Reads this device's approximate position, so Settings can offer "use my location" + * as an alternative to typing coordinates in. + * + * Abstracted (DIP) for two reasons: the view model stays unit-testable with no Android + * runtime, and there is exactly one place in the app that can touch the device's + * position — [currentCoordinates] — which is called only from an explicit user action. + * Nothing observes location; there is no background reader and no subscription. + */ +interface DeviceLocationSource { + + /** True when `ACCESS_COARSE_LOCATION` is currently granted to the app. */ + fun hasCoarsePermission(): Boolean + + /** + * Takes a single reading. Answers [DeviceLocationResult.PermissionMissing] rather + * than throwing when the permission is absent, so a caller can never capture a + * position it was not granted. + */ + suspend fun currentCoordinates(): DeviceLocationResult +} diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/location/SystemDeviceLocationSource.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/location/SystemDeviceLocationSource.kt new file mode 100644 index 0000000..53335a0 --- /dev/null +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/location/SystemDeviceLocationSource.kt @@ -0,0 +1,112 @@ +package com.interlinedlist.android.feature.profile.data.location + +import android.Manifest +import android.annotation.SuppressLint +import android.content.Context +import android.content.pm.PackageManager +import android.location.Location +import android.location.LocationManager +import androidx.core.content.ContextCompat +import androidx.core.location.LocationManagerCompat +import androidx.core.os.CancellationSignal +import com.interlinedlist.android.core.common.dispatcher.DispatcherProvider +import com.interlinedlist.android.feature.profile.domain.Coordinates +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.suspendCancellableCoroutine +import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeoutOrNull +import javax.inject.Inject +import javax.inject.Singleton +import kotlin.coroutines.resume + +/** + * Platform [DeviceLocationSource], backed by the framework's own [LocationManager] — + * no Play Services dependency, so the feature works on any device the app runs on. + * + * Only **coarse** providers are read. `GPS_PROVIDER` is deliberately absent: it needs + * `ACCESS_FINE_LOCATION`, which this app does not ask for and does not need, because + * the stored coordinates drive city-level surfaces (the weather and location widgets) + * rather than navigation. + * + * A reading is a one-shot with a time budget and no subscription: the app never + * watches the user's position, it answers a button press once and stops. + */ +@Singleton +class SystemDeviceLocationSource @Inject constructor( + @ApplicationContext private val context: Context, + private val dispatchers: DispatcherProvider, +) : DeviceLocationSource { + + override fun hasCoarsePermission(): Boolean = + ContextCompat.checkSelfPermission(context, COARSE_LOCATION) == + PackageManager.PERMISSION_GRANTED + + override suspend fun currentCoordinates(): DeviceLocationResult = + withContext(dispatchers.io) { + // Re-checked here and not only in the UI: this is the single point that can + // read a position, so the guarantee belongs with it. + if (!hasCoarsePermission()) return@withContext DeviceLocationResult.PermissionMissing + + val manager = context.getSystemService(Context.LOCATION_SERVICE) as? LocationManager + ?: return@withContext DeviceLocationResult.Unavailable + val provider = manager.coarseProvider() + ?: return@withContext DeviceLocationResult.Unavailable + + // A fresh fix when the device can manage one inside the budget; otherwise + // whatever it already knows, which is plenty for a city-level location. + val fix = withTimeoutOrNull(FIX_TIMEOUT_MILLIS) { manager.awaitLocation(provider) } + ?: manager.lastKnown(provider) + + fix?.let { DeviceLocationResult.Available(Coordinates(it.latitude, it.longitude)) } + ?: DeviceLocationResult.Unavailable + } + + /** + * The coarse provider to read from, or null when the device has none enabled. + * `fused` (the platform's own, from Android 12) is preferred, then the network + * provider, then the passive one — never GPS. + */ + private fun LocationManager.coarseProvider(): String? = COARSE_PROVIDERS.firstOrNull { + it in allProviders && runCatching { isProviderEnabled(it) }.getOrDefault(false) + } + + /** One fix from [provider], or null if the provider reports none. */ + // Permission is verified in currentCoordinates() immediately before this runs. + @SuppressLint("MissingPermission") + private suspend fun LocationManager.awaitLocation(provider: String): Location? = + suspendCancellableCoroutine { continuation -> + val signal = CancellationSignal() + continuation.invokeOnCancellation { signal.cancel() } + LocationManagerCompat.getCurrentLocation( + this, + provider, + signal, + ContextCompat.getMainExecutor(context), + ) { location: Location? -> + if (continuation.isActive) continuation.resume(location) + } + } + + /** The provider's last known fix, if it has one and still permits reading it. */ + @SuppressLint("MissingPermission") + private fun LocationManager.lastKnown(provider: String): Location? = + runCatching { getLastKnownLocation(provider) }.getOrNull() + + private companion object { + val COARSE_LOCATION: String = Manifest.permission.ACCESS_COARSE_LOCATION + + /** + * `LocationManager.FUSED_PROVIDER` is API 31, so its value is spelled out to + * keep this compiling against minSdk 26; the provider list is checked for it + * anyway, so an older device simply falls through to the next entry. + */ + val COARSE_PROVIDERS = listOf( + "fused", + LocationManager.NETWORK_PROVIDER, + LocationManager.PASSIVE_PROVIDER, + ) + + /** How long a press of "Use my location" may wait before giving up. */ + const val FIX_TIMEOUT_MILLIS = 10_000L + } +} diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/mapper/SettingsMappers.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/mapper/SettingsMappers.kt index 1ea22aa..b229c62 100644 --- a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/mapper/SettingsMappers.kt +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/mapper/SettingsMappers.kt @@ -2,9 +2,13 @@ package com.interlinedlist.android.feature.profile.data.mapper import com.interlinedlist.android.feature.profile.data.remote.dto.ProfileUserDto import com.interlinedlist.android.feature.profile.data.remote.dto.UpdateProfileRequest +import com.interlinedlist.android.feature.profile.domain.LocationUpdate import com.interlinedlist.android.feature.profile.domain.UserSettings import com.interlinedlist.android.feature.profile.domain.UserSettingsUpdate import com.interlinedlist.android.feature.profile.domain.ViewingPreference +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonNull +import kotlinx.serialization.json.JsonPrimitive /** * Maps the wire user onto the settings the Settings screen and the feed read. @@ -30,6 +34,9 @@ fun ProfileUserDto.toUserSettings(): UserSettings = UserSettings( /** * Maps a partial settings update onto the PATCH body. Untouched (null) fields stay * null and are dropped from the JSON, so the request only carries what changed. + * + * The location is the one field with three states rather than two: absent (left + * alone), set, or cleared. See [latitudeJson] for how a clear is put on the wire. */ fun UserSettingsUpdate.toRequest(): UpdateProfileRequest = UpdateProfileRequest( displayName = displayName, @@ -42,9 +49,31 @@ fun UserSettingsUpdate.toRequest(): UpdateProfileRequest = UpdateProfileRequest( viewingPreference = viewingPreference?.wire, showPreviews = showPreviews, showAdvancedPostSettings = showAdvancedPostSettings, - latitude = latitude, - longitude = longitude, + latitude = location?.latitudeJson, + longitude = location?.longitudeJson, isPrivateAccount = isPrivateAccount, githubDefaultRepo = githubDefaultRepo, notificationTrayLimit = notificationTrayLimit, ) + +/** + * The JSON to send for `latitude`: the number when setting a location, an explicit + * `null` when clearing one. A Kotlin null is never used here — the serializer drops + * those, which would silently turn a clear into a no-op request. + * + * The clear shape is correct but currently unreachable: the live endpoint rejects a + * null coordinate outright (400 `bad_request`), so no caller builds a + * [LocationUpdate.Clear]. See that type for the evidence. + */ +private val LocationUpdate.latitudeJson: JsonElement + get() = when (this) { + is LocationUpdate.Set -> JsonPrimitive(coordinates.latitude) + LocationUpdate.Clear -> JsonNull + } + +/** The JSON to send for `longitude`; see [latitudeJson]. */ +private val LocationUpdate.longitudeJson: JsonElement + get() = when (this) { + is LocationUpdate.Set -> JsonPrimitive(coordinates.longitude) + LocationUpdate.Clear -> JsonNull + } diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/remote/dto/ProfileRequests.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/remote/dto/ProfileRequests.kt index 1250cf9..2e3c5ba 100644 --- a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/remote/dto/ProfileRequests.kt +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/remote/dto/ProfileRequests.kt @@ -1,6 +1,7 @@ package com.interlinedlist.android.feature.profile.data.remote.dto import kotlinx.serialization.Serializable +import kotlinx.serialization.json.JsonElement /** * Body for `PATCH /api/user/update`. Every field the endpoint accepts is modelled @@ -12,6 +13,14 @@ import kotlinx.serialization.Serializable * matching the types the same fields come back with on `UserWire`. The generated * spec types several of them as `string` because the route handler coerces its * input, which native types satisfy too. + * + * [latitude] and [longitude] are the exception: they are typed as [JsonElement] so + * the profile location can be **cleared**. With `explicitNulls = false` a Kotlin + * `null` is dropped from the body, which is how "leave this field alone" is said, so + * there would otherwise be no way to say "unset it". A `JsonNull` is not a Kotlin + * null, so it survives serialisation and goes out as a literal `null`; a + * `JsonPrimitive(47.6062)` goes out as the number the live API returns. Nothing else + * needs this, so nothing else pays for it. */ @Serializable data class UpdateProfileRequest( @@ -25,8 +34,8 @@ data class UpdateProfileRequest( val viewingPreference: String? = null, val showPreviews: Boolean? = null, val showAdvancedPostSettings: Boolean? = null, - val latitude: Double? = null, - val longitude: Double? = null, + val latitude: JsonElement? = null, + val longitude: JsonElement? = null, val isPrivateAccount: Boolean? = null, val githubDefaultRepo: String? = null, val notificationTrayLimit: Int? = null, diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/di/ProfileModule.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/di/ProfileModule.kt index 9f11b93..6d2a1eb 100644 --- a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/di/ProfileModule.kt +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/di/ProfileModule.kt @@ -8,6 +8,8 @@ import com.interlinedlist.android.feature.profile.data.ProfileRepository import com.interlinedlist.android.feature.profile.data.SettingsRepository import com.interlinedlist.android.feature.profile.data.local.ProfileDao import com.interlinedlist.android.feature.profile.data.local.ProfileDatabase +import com.interlinedlist.android.feature.profile.data.location.DeviceLocationSource +import com.interlinedlist.android.feature.profile.data.location.SystemDeviceLocationSource import com.interlinedlist.android.feature.profile.data.remote.ProfileApi import dagger.Binds import dagger.Module @@ -33,6 +35,19 @@ abstract class ProfileRepositoryModule { abstract fun bindSettingsRepository(impl: DefaultSettingsRepository): SettingsRepository } +/** + * Binds the one component allowed to read the device's position, used by the profile + * location setting. See [DeviceLocationSource] for why it is an interface. + */ +@Module +@InstallIn(SingletonComponent::class) +abstract class ProfileLocationModule { + + @Binds + @Singleton + abstract fun bindDeviceLocationSource(impl: SystemDeviceLocationSource): DeviceLocationSource +} + /** Provides this feature's API, its own Room database, and DAO. */ @Module @InstallIn(SingletonComponent::class) diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/ProfileLocation.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/ProfileLocation.kt new file mode 100644 index 0000000..3ef7a02 --- /dev/null +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/ProfileLocation.kt @@ -0,0 +1,106 @@ +package com.interlinedlist.android.feature.profile.domain + +/** + * The account's profile location — the `latitude`/`longitude` pair carried by + * `GET /api/user` and accepted by `PATCH /api/user/update` (live-verified as JSON + * floats: `47.6062` / `-122.3321`). + * + * The two numbers are modelled as one value because they only mean anything together: + * half a coordinate would put the account somewhere its owner has never been. Every + * write therefore sends both, and a stored location exists only when the API returned + * both (see [UserSettings.coordinates]). + * + * The help centre describes the field as "Optional location for your profile, used by + * the Weather widget and similar location-aware features" (`/help/settings`, *Profile + * location*) — so nothing in the app may depend on it being set. + */ +data class Coordinates(val latitude: Double, val longitude: Double) + +/** + * The range each coordinate may take. These are the definition of latitude and + * longitude, not a server limit — the API publishes none — so they are enforced + * client-side and an entry outside them is refused before any request is made. + */ +object CoordinateBounds { + + /** Degrees north (+) or south (−) of the equator. */ + val LATITUDE: ClosedFloatingPointRange = -90.0..90.0 + + /** Degrees east (+) or west (−) of the prime meridian. */ + val LONGITUDE: ClosedFloatingPointRange = -180.0..180.0 +} + +/** + * True when both numbers are real coordinates. `NaN` and the infinities fall outside + * every range, so they are rejected here too rather than reaching the API. + */ +val Coordinates.isValid: Boolean + get() = latitude in CoordinateBounds.LATITUDE && longitude in CoordinateBounds.LONGITUDE + +/** + * The location stored on the account, or null when it has none. + * + * A half-populated pair reads as "no location": showing one coordinate on its own + * would imply a position the account has not actually stored. + */ +val UserSettings.coordinates: Coordinates? + get() { + val lat = latitude ?: return null + val lon = longitude ?: return null + return Coordinates(lat, lon) + } + +/** + * What a `PATCH /api/user/update` should do to the account's coordinates: set them, or + * clear them. Absence (a null [UserSettingsUpdate.location]) means "leave them alone". + * + * Clearing has to be its own case because the two are *not* the same request. The + * module's JSON config drops null properties (`explicitNulls = false`), so a Kotlin + * null would be omitted from the body — which is exactly how "leave it alone" is + * expressed. [Clear] is serialised as an explicit JSON `null` instead; see + * `SettingsMappers`. + */ +sealed interface LocationUpdate { + + /** Store [coordinates] on the account. */ + data class Set(val coordinates: Coordinates) : LocationUpdate + + /** + * Remove the account's stored location. + * + * **The live API does not support this, so nothing in the app sends it.** Probed + * against a real account: whenever the `latitude` key is present the endpoint + * validates it as a required number, so every way of saying "no value" is refused + * with `400 {"error":"latitude must be a number between -90 and 90", + * "code":"bad_request"}` — an explicit JSON `null`, an empty string, and the + * string `"null"` alike. Omitting the key leaves the stored value untouched, and + * `0,0` is accepted but is Null Island, a real position off the coast of Africa, + * not an absence. + * + * The case is kept because the modelling is right and the serialisation is + * already correct (see `SettingsMappers` and its tests): the day the endpoint + * accepts a null, wiring this back up is a one-liner. Until then the UI offers no + * way to reach it — see `ProfileLocationViewModel`, which can only ever send + * [Set]. This is an API gap, not an app one. + */ + data object Clear : LocationUpdate +} + +/** + * The same position rounded to about a kilometre. + * + * Applied to readings taken **from the device**, never to what the user typed. The app + * asks for `ACCESS_COARSE_LOCATION`, whose fix is only accurate to roughly that anyway, + * and the stored value exists to drive city-level surfaces (the weather and location + * widgets the help centre describes). Writing a street-level position to the account + * would record more about the user than the feature can use, so it is rounded off + * before it is ever sent. A coordinate the user enters by hand is left exactly as + * typed — that one is their own choice, to whatever precision they chose. + */ +fun Coordinates.coarsened(): Coordinates = Coordinates( + latitude = latitude.roundToCoarse(), + longitude = longitude.roundToCoarse(), +) + +/** Two decimal places ≈ 1.1 km — the granularity a coarse fix actually carries. */ +private fun Double.roundToCoarse(): Double = kotlin.math.round(this * 100.0) / 100.0 diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/UserSettings.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/UserSettings.kt index 86488f0..b27e987 100644 --- a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/UserSettings.kt +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/UserSettings.kt @@ -72,6 +72,7 @@ data class UserSettings( val viewingPreference: ViewingPreference = ViewingPreference.DEFAULT, val showPreviews: Boolean = true, val showAdvancedPostSettings: Boolean? = null, + /** The profile location, present only when the account has one set. */ val latitude: Double? = null, val longitude: Double? = null, val isPrivateAccount: Boolean? = null, @@ -84,9 +85,10 @@ data class UserSettings( * all optional. A null field means "leave it alone" — it is omitted from the request * body entirely, so changing one preference can never clobber another. * - * (Consequence to be aware of: a nullable server field such as `latitude` cannot be - * *cleared* through this type. Nothing needs that yet; whichever settings group does - * will have to model the clear explicitly.) + * A consequence of that rule is that a nullable server field cannot be *cleared* by + * setting it to null here — null already means "leave it alone". A field that has to + * be clearable therefore models the clear explicitly, which is what [location] does + * for the coordinates (see [LocationUpdate]). */ data class UserSettingsUpdate( val displayName: String? = null, @@ -99,8 +101,8 @@ data class UserSettingsUpdate( val viewingPreference: ViewingPreference? = null, val showPreviews: Boolean? = null, val showAdvancedPostSettings: Boolean? = null, - val latitude: Double? = null, - val longitude: Double? = null, + /** Set or clear the profile location; null leaves the stored coordinates alone. */ + val location: LocationUpdate? = null, val isPrivateAccount: Boolean? = null, val githubDefaultRepo: String? = null, val notificationTrayLimit: Int? = null, diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/ProfileLocationSection.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/ProfileLocationSection.kt new file mode 100644 index 0000000..37ecb44 --- /dev/null +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/ProfileLocationSection.kt @@ -0,0 +1,463 @@ +package com.interlinedlist.android.feature.profile.ui.settings + +import android.Manifest +import android.app.Activity +import android.content.Context +import android.content.ContextWrapper +import android.content.pm.PackageManager +import androidx.activity.compose.rememberLauncherForActivityResult +import androidx.activity.result.contract.ActivityResultContracts +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width +import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.Button +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedButton +import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.text.input.ImeAction +import androidx.compose.ui.text.input.KeyboardType +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import androidx.core.app.ActivityCompat +import androidx.core.content.ContextCompat +import androidx.hilt.navigation.compose.hiltViewModel +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import com.interlinedlist.android.core.designsystem.theme.InterlinedListTheme +import com.interlinedlist.android.feature.profile.domain.CoordinateBounds +import com.interlinedlist.android.feature.profile.domain.Coordinates + +/** Stable test tags for the profile-location section and its permission rationale. */ +object ProfileLocationTestTags { + const val GROUP = "settingsGroupProfileLocation" + const val VALUE = "settingsLocationValue" + const val PRIVACY_NOTE = "settingsLocationPrivacyNote" + const val USE_DEVICE = "settingsLocationUseDevice" + const val LATITUDE = "settingsLocationLatitude" + const val LONGITUDE = "settingsLocationLongitude" + const val SAVE = "settingsLocationSave" + const val CLEAR = "settingsLocationClear" + const val NOTICE = "settingsLocationNotice" + const val CANNOT_REMOVE = "settingsLocationCannotRemove" + const val ENTRY_ERROR = "settingsLocationEntryError" + const val RATIONALE = "locationRationale" + const val RATIONALE_CONTINUE = "locationRationaleContinue" + const val RATIONALE_DISMISS = "locationRationaleDismiss" +} + +/** + * What the section says about where the coordinates end up. + * + * The first two sentences are facts: `PATCH /api/user/update` stores them on the + * account, and the public profile (`GET /api/users/{username}`, verified live) returns + * only name, bio, avatar and counts — no coordinates. The third is the honest gap: the + * help centre documents the field as "Optional location for your profile, used by the + * Weather widget and similar location-aware features" and never says who else can read + * it, so the section does not claim to know. + */ +internal const val LOCATION_PRIVACY_NOTE: String = + "This is saved on your InterlinedList account, not just on this device. Your public " + + "profile doesn't include it. The help centre doesn't say who else can see it, and " + + "a saved location can be replaced but not removed — so save only a location " + + "you're happy to keep on your account." + +/** + * Why "Clear location" is offered but cannot be pressed. + * + * `PATCH /api/user/update` validates `latitude` as a required number whenever the key + * is present, so a null, an empty string and the string `"null"` are all answered with + * `400 {"error":"latitude must be a number between -90 and 90"}`, and omitting the key + * leaves the stored value alone. There is therefore no request this app can send that + * removes a location. The button stays visible, and disabled, with this note beside it: + * the question "how do I remove this?" is answered where it gets asked, rather than by + * a control that would fail every time it was pressed. (`0, 0` is accepted, but it is a + * real position in the Gulf of Guinea, not an absence — writing it would be worse than + * doing nothing.) + */ +internal const val LOCATION_CANNOT_REMOVE_NOTE: String = + "InterlinedList can't remove a saved location: its API rejects an empty value, so " + + "there's nothing this app can send to unset it. You can replace it with different " + + "coordinates at any time." + +/** + * Parses a typed coordinate, or null when it is not a number inside [range]. + * + * The guard that keeps a bad entry off the wire: the field reports a value only when + * this returns non-null, so an empty, malformed, or impossible entry is never saved. + * The view model repeats the range check, because it also takes readings from the + * device. + */ +internal fun parseCoordinate(text: String, range: ClosedFloatingPointRange): Double? = + text.trim().toDoubleOrNull()?.takeIf { it in range } + +/** The stored pair as the section shows it, e.g. `47.6062, -122.3321`. */ +internal fun Coordinates.display(): String = "$latitude, $longitude" + +/** + * The "Profile location" group, wired to its view model. + * + * This is the only place in the app that can reach the device's position, and it does + * so along one path: the user presses **Use my location**, reads the rationale, and + * agrees to the system dialog. The rationale is shown **before** the system prompt — + * after it, the decision has already been made and an explanation is just an excuse — + * and it is shown every time the permission is not already held, so nobody is asked to + * decide without being told what the app will do with the answer. + * + * Declining costs nothing: the coordinates can still be typed in, and no other part of + * Settings changes. + */ +@Composable +fun ProfileLocationSection( + modifier: Modifier = Modifier, + viewModel: ProfileLocationViewModel = hiltViewModel(), +) { + val state by viewModel.uiState.collectAsStateWithLifecycle() + val context = LocalContext.current + var showRationale by rememberSaveable { mutableStateOf(false) } + + val requestPermission = rememberLauncherForActivityResult( + ActivityResultContracts.RequestPermission(), + ) { granted -> + if (granted) { + viewModel.useDeviceLocation() + } else { + viewModel.onPermissionDenied(permanently = !context.canAskForCoarseLocation()) + } + } + + ProfileLocationGroup( + state = state, + onUseDeviceLocation = { + // Already granted: read it. Otherwise explain first, and only then ask. + if (context.hasCoarseLocationPermission()) { + viewModel.useDeviceLocation() + } else { + showRationale = true + } + }, + onSaveLocation = viewModel::saveLocation, + onDismissNotice = viewModel::dismissNotice, + modifier = modifier, + ) + + if (showRationale) { + LocationPermissionRationaleDialog( + onContinue = { + showRationale = false + requestPermission.launch(Manifest.permission.ACCESS_COARSE_LOCATION) + }, + onDismiss = { showRationale = false }, + ) + } +} + +/** + * Stateless "Profile location" UI: what is stored, a one-press capture from the + * device, and manual entry. + * + * The stored line and the fields both come from [state], which only ever holds what + * the server last confirmed — so the section never shows a location the account does + * not actually have. Removing a location is not offered as an action because the API + * cannot do it; see [LOCATION_CANNOT_REMOVE_NOTE]. + */ +@Composable +internal fun ProfileLocationGroup( + state: ProfileLocationUiState, + onUseDeviceLocation: () -> Unit, + onSaveLocation: (Double, Double) -> Unit, + onDismissNotice: () -> Unit, + modifier: Modifier = Modifier, +) { + // Keyed on the stored pair so a save, a capture or a refresh re-seeds the fields. + var latitudeText by remember(state.coordinates) { + mutableStateOf(state.coordinates?.latitude?.toString().orEmpty()) + } + var longitudeText by remember(state.coordinates) { + mutableStateOf(state.coordinates?.longitude?.toString().orEmpty()) + } + val latitude = parseCoordinate(latitudeText, CoordinateBounds.LATITUDE) + val longitude = parseCoordinate(longitudeText, CoordinateBounds.LONGITUDE) + val entryIsInvalid = (latitudeText.isNotBlank() && latitude == null) || + (longitudeText.isNotBlank() && longitude == null) + + SettingsGroup( + title = "Profile location", + description = "Optional. The coordinates saved on your account, used by " + + "location-aware features such as the weather widget.", + modifier = modifier.testTag(ProfileLocationTestTags.GROUP), + ) { + Text( + text = state.coordinates?.let { "Saved location: ${it.display()}" } + ?: "No location saved.", + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurface, + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 24.dp, vertical = 4.dp) + .testTag(ProfileLocationTestTags.VALUE), + ) + Text( + text = LOCATION_PRIVACY_NOTE, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 24.dp) + .testTag(ProfileLocationTestTags.PRIVACY_NOTE), + ) + Spacer(Modifier.height(12.dp)) + Row( + Modifier.fillMaxWidth().padding(horizontal = 24.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + OutlinedButton( + onClick = onUseDeviceLocation, + enabled = !state.isReadingDevice && !state.isSaving, + modifier = Modifier.testTag(ProfileLocationTestTags.USE_DEVICE), + ) { + Text("Use my location") + } + if (state.isReadingDevice || state.isSaving) { + Spacer(Modifier.width(12.dp)) + CircularProgressIndicator(Modifier.size(20.dp), strokeWidth = 2.dp) + } + } + Spacer(Modifier.height(8.dp)) + Row( + Modifier.fillMaxWidth().padding(horizontal = 24.dp), + horizontalArrangement = Arrangement.spacedBy(12.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + CoordinateField( + label = "Latitude", + value = latitudeText, + isError = latitudeText.isNotBlank() && latitude == null, + onValueChange = { latitudeText = it.filterCoordinate() }, + tag = ProfileLocationTestTags.LATITUDE, + modifier = Modifier.weight(1f), + ) + CoordinateField( + label = "Longitude", + value = longitudeText, + isError = longitudeText.isNotBlank() && longitude == null, + onValueChange = { longitudeText = it.filterCoordinate() }, + tag = ProfileLocationTestTags.LONGITUDE, + modifier = Modifier.weight(1f), + ) + } + if (entryIsInvalid) { + Text( + text = "Latitude runs from -90 to 90 and longitude from -180 to 180.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + modifier = Modifier + .padding(horizontal = 24.dp) + .testTag(ProfileLocationTestTags.ENTRY_ERROR), + ) + } + Spacer(Modifier.height(8.dp)) + Row( + Modifier.fillMaxWidth().padding(horizontal = 24.dp), + horizontalArrangement = Arrangement.spacedBy(8.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Button( + onClick = { + if (latitude != null && longitude != null) { + onSaveLocation(latitude, longitude) + } + }, + enabled = latitude != null && longitude != null && !state.isSaving, + modifier = Modifier.testTag(ProfileLocationTestTags.SAVE), + ) { + Text("Save location") + } + // Shown, never pressable: the endpoint has no way to unset a location, so + // the note below says so rather than the button failing on every press. + TextButton( + onClick = {}, + enabled = false, + modifier = Modifier.testTag(ProfileLocationTestTags.CLEAR), + ) { + Text("Clear location") + } + } + if (state.coordinates != null) { + Text( + text = LOCATION_CANNOT_REMOVE_NOTE, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 24.dp, vertical = 4.dp) + .testTag(ProfileLocationTestTags.CANNOT_REMOVE), + ) + } + if (state.notice != null) { + Row( + Modifier.fillMaxWidth().padding(start = 24.dp, end = 8.dp, top = 4.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = state.notice.text, + style = MaterialTheme.typography.bodySmall, + color = if (state.notice.isError) { + MaterialTheme.colorScheme.error + } else { + MaterialTheme.colorScheme.onSurfaceVariant + }, + modifier = Modifier.weight(1f).testTag(ProfileLocationTestTags.NOTICE), + ) + TextButton(onClick = onDismissNotice) { Text("Dismiss") } + } + } + } +} + +/** One coordinate entry field: signed decimals only, saved from the button. */ +@Composable +private fun CoordinateField( + label: String, + value: String, + isError: Boolean, + onValueChange: (String) -> Unit, + tag: String, + modifier: Modifier = Modifier, +) { + OutlinedTextField( + value = value, + onValueChange = onValueChange, + label = { Text(label) }, + singleLine = true, + isError = isError, + keyboardOptions = KeyboardOptions( + keyboardType = KeyboardType.Decimal, + imeAction = ImeAction.Done, + ), + modifier = modifier.testTag(tag), + ) +} + +/** Keeps the field to the characters a signed decimal can contain. */ +private fun String.filterCoordinate(): String = + filter { it.isDigit() || it == '-' || it == '.' }.take(MAX_COORDINATE_CHARS) + +/** Enough for any coordinate, short enough that a paste cannot fill the field. */ +private const val MAX_COORDINATE_CHARS = 12 + +/** + * The rationale shown **before** Android's permission dialog. + * + * It says what will be read (approximate position, once), what will happen to it (it + * is saved to the account as the profile location), how precise it will be (rounded to + * about a kilometre), and that saying no costs nothing. Everything here is what the + * code actually does — see `SystemDeviceLocationSource` and `Coordinates.coarsened`. + */ +@Composable +internal fun LocationPermissionRationaleDialog( + onContinue: () -> Unit, + onDismiss: () -> Unit, + modifier: Modifier = Modifier, +) { + AlertDialog( + onDismissRequest = onDismiss, + modifier = modifier.testTag(ProfileLocationTestTags.RATIONALE), + title = { Text("Use this device's location?") }, + text = { + Column { + Text( + "Android will ask for permission to read your approximate location. " + + "InterlinedList reads it once, right now — never in the background.", + ) + Spacer(Modifier.height(8.dp)) + Text( + "The reading is rounded to about a kilometre and saved to your account " + + "as your profile location, which powers location-aware features such " + + "as the weather widget.", + ) + Spacer(Modifier.height(8.dp)) + Text( + "Once saved, a location can be replaced with different coordinates but " + + "not removed — InterlinedList has no way to unset it.", + ) + Spacer(Modifier.height(8.dp)) + Text("You can say no and type your coordinates in instead, or leave them unset.") + } + }, + confirmButton = { + TextButton( + onClick = onContinue, + modifier = Modifier.testTag(ProfileLocationTestTags.RATIONALE_CONTINUE), + ) { + Text("Continue") + } + }, + dismissButton = { + TextButton( + onClick = onDismiss, + modifier = Modifier.testTag(ProfileLocationTestTags.RATIONALE_DISMISS), + ) { + Text("Not now") + } + }, + ) +} + +/** True when `ACCESS_COARSE_LOCATION` is already granted to the app. */ +private fun Context.hasCoarseLocationPermission(): Boolean = + ContextCompat.checkSelfPermission(this, Manifest.permission.ACCESS_COARSE_LOCATION) == + PackageManager.PERMISSION_GRANTED + +/** + * Whether Android would still show the permission dialog. + * + * Only consulted immediately after a refusal, where `false` means the system will not + * ask again — the point at which the app has to say where the decision can be changed + * instead of offering a button that would now do nothing. + */ +private fun Context.canAskForCoarseLocation(): Boolean { + val activity = findActivity() ?: return true + return ActivityCompat.shouldShowRequestPermissionRationale( + activity, + Manifest.permission.ACCESS_COARSE_LOCATION, + ) +} + +private tailrec fun Context.findActivity(): Activity? = when (this) { + is Activity -> this + is ContextWrapper -> baseContext.findActivity() + else -> null +} + +@Preview(showBackground = true) +@Composable +private fun ProfileLocationGroupPreview() { + InterlinedListTheme { + ProfileLocationGroup( + state = ProfileLocationUiState(coordinates = Coordinates(47.6062, -122.3321)), + onUseDeviceLocation = {}, + onSaveLocation = { _, _ -> }, + onDismissNotice = {}, + ) + } +} diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/ProfileLocationViewModel.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/ProfileLocationViewModel.kt new file mode 100644 index 0000000..dd739ee --- /dev/null +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/ProfileLocationViewModel.kt @@ -0,0 +1,222 @@ +package com.interlinedlist.android.feature.profile.ui.settings + +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import com.interlinedlist.android.core.common.result.ApiResult +import com.interlinedlist.android.feature.profile.data.SettingsRepository +import com.interlinedlist.android.feature.profile.data.location.DeviceLocationResult +import com.interlinedlist.android.feature.profile.data.location.DeviceLocationSource +import com.interlinedlist.android.feature.profile.domain.Coordinates +import com.interlinedlist.android.feature.profile.domain.CoordinateBounds +import com.interlinedlist.android.feature.profile.domain.LocationUpdate +import com.interlinedlist.android.feature.profile.domain.UserSettingsUpdate +import com.interlinedlist.android.feature.profile.domain.coarsened +import com.interlinedlist.android.feature.profile.domain.coordinates +import com.interlinedlist.android.feature.profile.domain.isValid +import com.interlinedlist.android.feature.profile.ui.common.toUserMessage +import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import javax.inject.Inject + +/** + * A short line under the location controls explaining why nothing was saved, or what + * to do next. [isError] separates "something went wrong" (shown in the error colour) + * from "you said no, and that is fine" — refusing the permission is a choice the app + * respects, not a failure to report in red. + */ +data class LocationNotice(val text: String, val isError: Boolean = false) + +/** + * State of the profile-location section. + * + * [coordinates] is always what the **server** last said: nothing is applied + * optimistically here. The other settings rows can show a change before it is + * confirmed and roll it back, because a toggle that briefly lies about a page size is + * harmless; a row claiming the account stores a position it does not (or no longer + * stores one it does) would be a privacy claim the app cannot back up. + */ +data class ProfileLocationUiState( + val coordinates: Coordinates? = null, + val isSaving: Boolean = false, + val isReadingDevice: Boolean = false, + val notice: LocationNotice? = null, +) + +/** + * Drives the "Profile location" settings group: the optional `latitude`/`longitude` + * pair on the account, which the help centre describes as "Optional location for your + * profile, used by the Weather widget and similar location-aware features". + * + * It is a view model of its own rather than more methods on [SettingsViewModel] + * because it has a dependency none of the other preferences have — [DeviceLocationSource], + * the single component that may read the device's position — and a different failure + * vocabulary (permission refused, no fix). It shares the same [SettingsRepository], + * whose cache is app-wide, so a save here reaches the Settings screen's own state and + * a refresh there reaches this section. Loading is left to [SettingsViewModel]: this + * section follows the shared cache and never issues a fetch of its own. + * + * Three rules hold throughout: + * - **The position is read only from an explicit user action** ([useDeviceLocation]), + * never on load, never on a schedule. + * - **Refusing is free.** Any denial path ends with the section still usable: the + * coordinates can be typed in, and nothing else in the app changes. + * - **Only [LocationUpdate.Set] is ever sent.** The endpoint has no way to unset a + * saved location — every spelling of "no value" comes back + * `400 {"error":"latitude must be a number between -90 and 90"}` — so this class + * cannot express a clear at all, rather than offering one that always fails. See + * [LocationUpdate.Clear] for the probe results. + */ +@HiltViewModel +class ProfileLocationViewModel @Inject constructor( + private val repository: SettingsRepository, + private val deviceLocation: DeviceLocationSource, +) : ViewModel() { + + private val _uiState = MutableStateFlow(ProfileLocationUiState()) + val uiState: StateFlow = _uiState.asStateFlow() + + init { + viewModelScope.launch { + repository.observeSettings().collect { settings -> + _uiState.update { it.copy(coordinates = settings?.coordinates) } + } + } + } + + /** + * Saves coordinates the user typed in, exactly as typed. + * + * Values outside the meaning of latitude and longitude are refused here, before + * any request: an entry of 91° north does not exist, so there is nothing to ask + * the server about. + * + * This is also the only way to change a location already stored — the account + * offers no way to remove one (see [LocationUpdate.Clear]) — so a save over an + * existing pair replaces it. + */ + fun saveLocation(latitude: Double, longitude: Double) { + val entered = Coordinates(latitude, longitude) + if (!entered.isValid) { + val note = LocationNotice(outOfRangeNote(entered), isError = true) + _uiState.update { it.copy(notice = note) } + return + } + if (entered == _uiState.value.coordinates) { + _uiState.update { it.copy(notice = null) } + return + } + save(entered) + } + + /** + * Reads this device's position once and saves it, rounded to about a kilometre + * (see [coarsened]). + * + * Call this only after `ACCESS_COARSE_LOCATION` has been granted; if it has not + * been, the source answers [DeviceLocationResult.PermissionMissing] and nothing is + * read, so a mis-wired caller cannot capture a position behind the user's back. + */ + fun useDeviceLocation() { + if (_uiState.value.isReadingDevice) return + _uiState.update { it.copy(isReadingDevice = true, notice = null) } + viewModelScope.launch { + val result = deviceLocation.currentCoordinates() + _uiState.update { it.copy(isReadingDevice = false) } + when (result) { + is DeviceLocationResult.Available -> { + val coarse = result.coordinates.coarsened() + if (coarse.isValid) save(coarse) else note(NO_FIX_NOTE) + } + DeviceLocationResult.PermissionMissing -> note(PERMISSION_DENIED_NOTE) + DeviceLocationResult.Unavailable -> note(NO_FIX_NOTE) + } + } + } + + /** + * Records that the user refused the system permission dialog. + * + * Nothing is saved, nothing is retried and no other preference is touched — the + * section simply says that coordinates can still be entered by hand. When Android + * will no longer show the dialog ([permanently]), the note adds where to change + * the decision, since the button alone can no longer do it. + */ + fun onPermissionDenied(permanently: Boolean) { + _uiState.update { + it.copy( + isReadingDevice = false, + notice = LocationNotice( + if (permanently) PERMISSION_BLOCKED_NOTE else PERMISSION_DENIED_NOTE, + ), + ) + } + } + + /** Dismisses the current notice. */ + fun dismissNotice() = _uiState.update { it.copy(notice = null) } + + /** + * `PATCH /api/user/update` with the two location fields and nothing else. + * + * Takes [Coordinates] rather than a [LocationUpdate] on purpose: a clear is not + * representable here, so the one request the API refuses cannot be built by + * accident. What lands in the state afterwards is what the server echoed back. + */ + private fun save(coordinates: Coordinates) { + _uiState.update { it.copy(isSaving = true, notice = null) } + viewModelScope.launch { + val update = UserSettingsUpdate(location = LocationUpdate.Set(coordinates)) + when (val result = repository.update(update)) { + is ApiResult.Success -> _uiState.update { + it.copy(coordinates = result.data.coordinates, isSaving = false, notice = null) + } + + is ApiResult.Failure -> _uiState.update { + it.copy( + isSaving = false, + notice = LocationNotice(result.error.toUserMessage(), isError = true), + ) + } + } + } + } + + private fun note(text: String) = _uiState.update { it.copy(notice = LocationNotice(text)) } + + /** Names whichever of the two entries is not a coordinate. */ + private fun outOfRangeNote(entered: Coordinates): String { + val latitudeBad = entered.latitude !in CoordinateBounds.LATITUDE + val longitudeBad = entered.longitude !in CoordinateBounds.LONGITUDE + return when { + latitudeBad && longitudeBad -> "$LATITUDE_RANGE_NOTE $LONGITUDE_RANGE_NOTE" + latitudeBad -> LATITUDE_RANGE_NOTE + else -> LONGITUDE_RANGE_NOTE + } + } + + companion object { + /** Shown when the user declines the system dialog; the section stays usable. */ + const val PERMISSION_DENIED_NOTE: String = + "Location permission wasn't granted, so nothing was read from this device. " + + "You can still enter coordinates below, or leave your location unset." + + /** Shown when Android will not offer the dialog again. */ + const val PERMISSION_BLOCKED_NOTE: String = + "Location permission is off for InterlinedList, so nothing was read from this " + + "device. You can still enter coordinates below, or allow Location for this " + + "app in Android Settings." + + /** Permission held, but the device had nothing to give. */ + const val NO_FIX_NOTE: String = + "Couldn't get a location from this device. Check that Location is switched on, " + + "or enter coordinates below." + + const val LATITUDE_RANGE_NOTE: String = "Latitude must be between -90 and 90." + + const val LONGITUDE_RANGE_NOTE: String = "Longitude must be between -180 and 180." + } +} diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsScreen.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsScreen.kt index 3df7ead..f488dd0 100644 --- a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsScreen.kt +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsScreen.kt @@ -133,10 +133,21 @@ fun SettingsRoute( onTogglePrivateAccount = viewModel::setPrivateAccount, onDismissError = viewModel::dismissError, modifier = modifier, + // Filled in here rather than inside the stateless screen: the location section + // owns a runtime permission request, which needs an activity result registry + // that previews and Compose tests of the screen do not have. + locationSection = { ProfileLocationSection() }, ) } -/** Stateless Settings UI — one titled group per area of the web Settings page. */ +/** + * Stateless Settings UI — one titled group per area of the web Settings page. + * + * @param locationSection the "Profile location" group, passed in as a slot because it + * requests a runtime permission and therefore cannot be rendered from a preview or a + * plain Compose test. It sits between Message settings and Permissions, which is the + * order the web uses (`/help/settings`). + */ @OptIn(ExperimentalMaterial3Api::class) @Composable fun SettingsScreen( @@ -154,6 +165,7 @@ fun SettingsScreen( onTogglePrivateAccount: (Boolean) -> Unit, onDismissError: () -> Unit, modifier: Modifier = Modifier, + locationSection: @Composable () -> Unit = {}, ) { Scaffold( modifier = modifier.fillMaxSize(), @@ -197,6 +209,9 @@ fun SettingsScreen( onToggleDefaultPubliclyVisible = onToggleDefaultPubliclyVisible, onToggleShowAdvancedPostSettings = onToggleShowAdvancedPostSettings, ) + // Where the web puts it: `/help/settings` lists Profile location after + // Message settings and before Permissions. + locationSection() PermissionsGroup( settings = settings, onTogglePrivateAccount = onTogglePrivateAccount, diff --git a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepositoryTest.kt b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepositoryTest.kt index 1d02dc2..33de518 100644 --- a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepositoryTest.kt +++ b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepositoryTest.kt @@ -7,6 +7,8 @@ import com.interlinedlist.android.core.common.result.AppError import com.interlinedlist.android.core.network.api.InterlinedListApi import com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore import com.interlinedlist.android.feature.profile.data.remote.ProfileApi +import com.interlinedlist.android.feature.profile.domain.Coordinates +import com.interlinedlist.android.feature.profile.domain.LocationUpdate import com.interlinedlist.android.feature.profile.domain.UserSettingsUpdate import com.interlinedlist.android.feature.profile.domain.ViewingPreference import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFactory @@ -16,8 +18,10 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.runTest import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonNull import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.booleanOrNull +import kotlinx.serialization.json.doubleOrNull import kotlinx.serialization.json.intOrNull import kotlinx.serialization.json.jsonPrimitive import okhttp3.MediaType.Companion.toMediaType @@ -80,6 +84,8 @@ class DefaultSettingsRepositoryTest { showAdvancedPostSettings: Boolean = false, isPrivateAccount: Boolean = false, notificationTrayLimit: Int = 25, + latitude: String = "45.52", + longitude: String = "-122.68", ) = server.enqueue( MockResponse().setResponseCode(200).setBody( """ @@ -95,8 +101,8 @@ class DefaultSettingsRepositoryTest { "viewingPreference": "$viewingPreference", "showPreviews": $showPreviews, "showAdvancedPostSettings": $showAdvancedPostSettings, - "latitude": 45.52, - "longitude": -122.68, + "latitude": $latitude, + "longitude": $longitude, "isPrivateAccount": $isPrivateAccount, "githubDefaultRepo": "adron/notes", "notificationTrayLimit": $notificationTrayLimit @@ -413,4 +419,105 @@ class DefaultSettingsRepositoryTest { assertThat(trayLimitStore.current()).isEqualTo(20) } + + // --- Profile location (issue #37) ---------------------------------------- + + @Test + fun `saving a location PATCHes both coordinates and nothing else`() = runTest(testDispatcher) { + enqueueUser(latitude = "47.6062", longitude = "-122.3321") + + val result = repository.update( + UserSettingsUpdate(location = LocationUpdate.Set(Coordinates(47.6062, -122.3321))), + ) + + val body = server.takeJsonBody() + assertThat(body.keys).containsExactly("latitude", "longitude") + val latitude = body.getValue("latitude").jsonPrimitive + assertThat(latitude.isString).isFalse() + assertThat(latitude.doubleOrNull).isEqualTo(47.6062) + assertThat(body.getValue("longitude").jsonPrimitive.doubleOrNull).isEqualTo(-122.3321) + assertThat((result as ApiResult.Success).data.latitude).isEqualTo(47.6062) + assertThat(result.data.longitude).isEqualTo(-122.3321) + assertThat(repository.observeSettings().first()?.latitude).isEqualTo(47.6062) + } + + /** + * The clear path is modelled and serialised correctly but **the live API refuses + * it**, so nothing in the app sends one (see `LocationUpdate.Clear`). The two + * tests below keep that state of affairs honest: the first pins the request shape + * so re-enabling it later is a one-liner, the second pins what the server does + * with it today. + */ + @Test + fun `a clear would carry both coordinates as explicit nulls`() = runTest(testDispatcher) { + enqueueUser(latitude = "null", longitude = "null") + + repository.update(UserSettingsUpdate(location = LocationUpdate.Clear)) + + val body = server.takeJsonBody() + // The keys must be present *and* null: `explicitNulls = false` would have + // dropped a Kotlin null, leaving an empty, pointless request. + assertThat(body.keys).containsExactly("latitude", "longitude") + assertThat(body.getValue("latitude")).isEqualTo(JsonNull) + assertThat(body.getValue("longitude")).isEqualTo(JsonNull) + } + + @Test + fun `the API refuses a clear and the cached location survives it`() = + runTest(testDispatcher) { + enqueueUser(latitude = "47.6062", longitude = "-122.3321") + repository.refresh() + server.takeRequest() + + // Verbatim from the live endpoint, probed with a real account: a null + // latitude, an empty string and the string "null" all answer with this. + server.enqueue( + MockResponse().setResponseCode(400).setBody( + """ + { + "error": "latitude must be a number between -90 and 90", + "code": "bad_request" + } + """.trimIndent(), + ), + ) + val result = repository.update(UserSettingsUpdate(location = LocationUpdate.Clear)) + + assertThat(result).isInstanceOf(ApiResult.Failure::class.java) + // A 400 normalises to Unknown, which keeps the server's own wording — so a + // clear wired up by mistake would surface the reason, not a generic error. + val error = (result as ApiResult.Failure).error + assertThat(error).isInstanceOf(AppError.Unknown::class.java) + assertThat(error.message).isEqualTo("latitude must be a number between -90 and 90") + assertThat(repository.observeSettings().first()?.latitude).isEqualTo(47.6062) + assertThat(repository.observeSettings().first()?.longitude).isEqualTo(-122.3321) + } + + @Test + fun `a rejected location save leaves the cached coordinates untouched`() = + runTest(testDispatcher) { + enqueueUser(latitude = "47.6062", longitude = "-122.3321") + repository.refresh() + server.takeRequest() + + server.enqueue(MockResponse().setResponseCode(500).setBody("""{ "error": "boom" }""")) + val result = repository.update( + UserSettingsUpdate(location = LocationUpdate.Set(Coordinates(10.0, 10.0))), + ) + + assertThat(result).isInstanceOf(ApiResult.Failure::class.java) + assertThat(repository.observeSettings().first()?.latitude).isEqualTo(47.6062) + assertThat(repository.observeSettings().first()?.longitude).isEqualTo(-122.3321) + } + + @Test + fun `a user with no coordinates reads as having no location`() = runTest(testDispatcher) { + enqueueUser(latitude = "null", longitude = "null") + + val settings = (repository.refresh() as ApiResult.Success).data + + assertThat(settings.latitude).isNull() + assertThat(settings.longitude).isNull() + } + } diff --git a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/SettingsMappersTest.kt b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/SettingsMappersTest.kt index d9cdebd..53d1371 100644 --- a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/SettingsMappersTest.kt +++ b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/SettingsMappersTest.kt @@ -4,6 +4,8 @@ import com.google.common.truth.Truth.assertThat import com.interlinedlist.android.feature.profile.data.mapper.toRequest import com.interlinedlist.android.feature.profile.data.mapper.toUserSettings import com.interlinedlist.android.feature.profile.data.remote.dto.ProfileUserDto +import com.interlinedlist.android.feature.profile.domain.Coordinates +import com.interlinedlist.android.feature.profile.domain.LocationUpdate import com.interlinedlist.android.feature.profile.domain.UserSettingsUpdate import com.interlinedlist.android.feature.profile.domain.ViewingPreference import kotlinx.serialization.encodeToString @@ -82,6 +84,43 @@ class SettingsMappersTest { assertThat(body).isEqualTo("""{"showPreviews":true}""") } + // --- Profile location (issue #37) ---------------------------------------- + // Setting a location is what the app does. Clearing is modelled and serialised + // correctly — an explicit JSON null, since `explicitNulls = false` drops a Kotlin + // one — but the live endpoint refuses it (400 "latitude must be a number between + // -90 and 90"), so nothing sends it. The shape is pinned here anyway: when the + // API grows the ability, re-enabling it must not need a serialisation change too. + + @Test + fun `setting a location sends both coordinates as JSON numbers`() { + val body = json.encodeToString( + UserSettingsUpdate( + location = LocationUpdate.Set(Coordinates(47.6062, -122.3321)), + ).toRequest(), + ) + + assertThat(body).isEqualTo("""{"latitude":47.6062,"longitude":-122.3321}""") + } + + @Test + fun `a clear would send both coordinates as explicit nulls`() { + val body = json.encodeToString( + UserSettingsUpdate(location = LocationUpdate.Clear).toRequest(), + ) + + // Not `{}`: an omitted key means "leave it alone", so the keys would have to + // reach the server for it to unset anything — which it currently refuses to do. + assertThat(body).isEqualTo("""{"latitude":null,"longitude":null}""") + } + + @Test + fun `an update that does not mention the location leaves both coordinates out`() { + val body = json.encodeToString(UserSettingsUpdate(showPreviews = true).toRequest()) + + assertThat(body).doesNotContain("latitude") + assertThat(body).doesNotContain("longitude") + } + @Test fun `an update carries every field when they are all set`() { val request = UserSettingsUpdate( @@ -95,8 +134,7 @@ class SettingsMappersTest { viewingPreference = ViewingPreference.MINE, showPreviews = false, showAdvancedPostSettings = true, - latitude = 45.52, - longitude = -122.68, + location = LocationUpdate.Set(Coordinates(45.52, -122.68)), isPrivateAccount = true, githubDefaultRepo = "adron/notes", notificationTrayLimit = 25, diff --git a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeDeviceLocationSource.kt b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeDeviceLocationSource.kt new file mode 100644 index 0000000..02172d6 --- /dev/null +++ b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeDeviceLocationSource.kt @@ -0,0 +1,25 @@ +package com.interlinedlist.android.feature.profile.ui + +import com.interlinedlist.android.feature.profile.data.location.DeviceLocationResult +import com.interlinedlist.android.feature.profile.data.location.DeviceLocationSource + +/** + * In-memory [DeviceLocationSource] for view-model tests: [result] decides what a + * reading answers, and [readCount] records how many times the device was consulted — + * which is how a test proves the app never takes a position it was not asked to. + */ +class FakeDeviceLocationSource( + var result: DeviceLocationResult = DeviceLocationResult.PermissionMissing, + var granted: Boolean = false, +) : DeviceLocationSource { + + var readCount: Int = 0 + private set + + override fun hasCoarsePermission(): Boolean = granted + + override suspend fun currentCoordinates(): DeviceLocationResult { + readCount++ + return result + } +} diff --git a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeSettingsRepository.kt b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeSettingsRepository.kt index 45720f1..5ce70f4 100644 --- a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeSettingsRepository.kt +++ b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeSettingsRepository.kt @@ -3,6 +3,7 @@ package com.interlinedlist.android.feature.profile.ui import com.interlinedlist.android.core.common.result.ApiResult import com.interlinedlist.android.core.datastore.ThemeMode import com.interlinedlist.android.feature.profile.data.SettingsRepository +import com.interlinedlist.android.feature.profile.domain.LocationUpdate import com.interlinedlist.android.feature.profile.domain.UserSettings import com.interlinedlist.android.feature.profile.domain.UserSettingsUpdate import kotlinx.coroutines.flow.Flow @@ -74,6 +75,18 @@ class FakeSettingsRepository : SettingsRepository { isPrivateAccount = update.isPrivateAccount ?: current.isPrivateAccount, notificationTrayLimit = update.notificationTrayLimit ?: current.notificationTrayLimit, + // The location has three states, so it cannot collapse into an elvis: + // absent leaves the pair alone, Set replaces it, Clear removes it. + latitude = when (val location = update.location) { + null -> current.latitude + is LocationUpdate.Set -> location.coordinates.latitude + LocationUpdate.Clear -> null + }, + longitude = when (val location = update.location) { + null -> current.longitude + is LocationUpdate.Set -> location.coordinates.longitude + LocationUpdate.Clear -> null + }, ) } } diff --git a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/ProfileLocationInputTest.kt b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/ProfileLocationInputTest.kt new file mode 100644 index 0000000..2a0c962 --- /dev/null +++ b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/ProfileLocationInputTest.kt @@ -0,0 +1,84 @@ +package com.interlinedlist.android.feature.profile.ui + +import com.google.common.truth.Truth.assertThat +import com.interlinedlist.android.feature.profile.domain.CoordinateBounds +import com.interlinedlist.android.feature.profile.domain.Coordinates +import com.interlinedlist.android.feature.profile.domain.UserSettings +import com.interlinedlist.android.feature.profile.domain.coarsened +import com.interlinedlist.android.feature.profile.domain.coordinates +import com.interlinedlist.android.feature.profile.domain.isValid +import com.interlinedlist.android.feature.profile.ui.settings.display +import com.interlinedlist.android.feature.profile.ui.settings.parseCoordinate +import org.junit.Test + +/** + * The pure parts of the location section: what the entry fields accept, how a device + * reading is rounded, and when the account counts as having a location. + */ +class ProfileLocationInputTest { + + @Test + fun `a coordinate inside its range parses`() { + assertThat(parseCoordinate("47.6062", CoordinateBounds.LATITUDE)).isEqualTo(47.6062) + assertThat(parseCoordinate("-122.3321", CoordinateBounds.LONGITUDE)).isEqualTo(-122.3321) + assertThat(parseCoordinate(" 0 ", CoordinateBounds.LATITUDE)).isEqualTo(0.0) + } + + @Test + fun `the exact bounds parse and anything beyond them does not`() { + assertThat(parseCoordinate("90", CoordinateBounds.LATITUDE)).isEqualTo(90.0) + assertThat(parseCoordinate("-90", CoordinateBounds.LATITUDE)).isEqualTo(-90.0) + assertThat(parseCoordinate("90.0001", CoordinateBounds.LATITUDE)).isNull() + assertThat(parseCoordinate("180.1", CoordinateBounds.LONGITUDE)).isNull() + assertThat(parseCoordinate("-181", CoordinateBounds.LONGITUDE)).isNull() + } + + @Test + fun `an empty or malformed entry does not parse`() { + assertThat(parseCoordinate("", CoordinateBounds.LATITUDE)).isNull() + assertThat(parseCoordinate("-", CoordinateBounds.LATITUDE)).isNull() + assertThat(parseCoordinate("4-7", CoordinateBounds.LATITUDE)).isNull() + assertThat(parseCoordinate("north", CoordinateBounds.LATITUDE)).isNull() + assertThat(parseCoordinate("NaN", CoordinateBounds.LATITUDE)).isNull() + assertThat(parseCoordinate("Infinity", CoordinateBounds.LONGITUDE)).isNull() + } + + @Test + fun `a real position is valid and an impossible one is not`() { + assertThat(Coordinates(47.6062, -122.3321).isValid).isTrue() + assertThat(Coordinates(90.0, 180.0).isValid).isTrue() + assertThat(Coordinates(90.1, 0.0).isValid).isFalse() + assertThat(Coordinates(0.0, -180.1).isValid).isFalse() + assertThat(Coordinates(Double.NaN, 0.0).isValid).isFalse() + } + + @Test + fun `a device reading is rounded to about a kilometre`() { + // Coarse location is no more accurate than this, and the account has no use + // for a street-level position. + assertThat(Coordinates(47.60621, -122.33207).coarsened()) + .isEqualTo(Coordinates(47.61, -122.33)) + assertThat(Coordinates(-0.004, 0.006).coarsened()).isEqualTo(Coordinates(-0.0, 0.01)) + assertThat(Coordinates(47.6, -122.3).coarsened()).isEqualTo(Coordinates(47.6, -122.3)) + } + + @Test + fun `rounding never pushes a coordinate out of range`() { + assertThat(Coordinates(90.0, 180.0).coarsened().isValid).isTrue() + assertThat(Coordinates(-90.0, -180.0).coarsened().isValid).isTrue() + } + + @Test + fun `an account has a location only when both coordinates are present`() { + assertThat(UserSettings(latitude = 47.6062, longitude = -122.3321).coordinates) + .isEqualTo(Coordinates(47.6062, -122.3321)) + assertThat(UserSettings(latitude = 47.6062, longitude = null).coordinates).isNull() + assertThat(UserSettings(latitude = null, longitude = -122.3321).coordinates).isNull() + assertThat(UserSettings().coordinates).isNull() + } + + @Test + fun `the stored pair is shown as latitude then longitude`() { + assertThat(Coordinates(47.6062, -122.3321).display()).isEqualTo("47.6062, -122.3321") + } +} diff --git a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/ProfileLocationViewModelTest.kt b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/ProfileLocationViewModelTest.kt new file mode 100644 index 0000000..25ca68e --- /dev/null +++ b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/ProfileLocationViewModelTest.kt @@ -0,0 +1,388 @@ +package com.interlinedlist.android.feature.profile.ui + +import app.cash.turbine.test +import com.google.common.truth.Truth.assertThat +import com.interlinedlist.android.core.common.result.ApiResult +import com.interlinedlist.android.core.common.result.AppError +import com.interlinedlist.android.feature.profile.data.location.DeviceLocationResult +import com.interlinedlist.android.feature.profile.domain.Coordinates +import com.interlinedlist.android.feature.profile.domain.LocationUpdate +import com.interlinedlist.android.feature.profile.domain.UserSettings +import com.interlinedlist.android.feature.profile.ui.settings.ProfileLocationViewModel +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Before +import org.junit.Test + +/** + * The profile-location section (issue #37): the optional `latitude`/`longitude` pair + * on the account, set from the device or by hand. + * + * Two properties matter more here than in the sibling settings, and both are pinned + * down below: **the app never reads a position it was not explicitly asked to**, and + * **refusing the permission costs the user nothing** — manual entry, clearing and the + * rest of Settings all keep working. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class ProfileLocationViewModelTest { + + private val dispatcher = StandardTestDispatcher() + private lateinit var repo: FakeSettingsRepository + private lateinit var device: FakeDeviceLocationSource + + private val seattle = Coordinates(47.6062, -122.3321) + + @Before + fun setUp() { + Dispatchers.setMain(dispatcher) + repo = FakeSettingsRepository() + device = FakeDeviceLocationSource() + } + + @After + fun tearDown() = Dispatchers.resetMain() + + /** A view model following a repository that already holds [settings]. */ + private suspend fun loadedViewModel(settings: UserSettings): ProfileLocationViewModel { + repo.refreshResult = ApiResult.Success(settings) + repo.refresh() + return ProfileLocationViewModel(repo, device) + } + + // --- Manual entry --------------------------------------------------------- + + @Test + fun `saving typed coordinates PATCHes latitude and longitude alone`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings()) + + vm.uiState.test { + advanceUntilIdle() + vm.saveLocation(47.6062, -122.3321) + advanceUntilIdle() + + val sent = repo.updates.single() + assertThat(sent.location).isEqualTo(LocationUpdate.Set(seattle)) + assertThat(sent.touchedFieldNames()).containsExactly("latitude", "longitude") + val state = expectMostRecentItem() + assertThat(state.coordinates).isEqualTo(seattle) + assertThat(state.isSaving).isFalse() + assertThat(state.notice).isNull() + } + } + + @Test + fun `a typed coordinate is saved exactly as entered`() = runTest(dispatcher) { + // Rounding belongs to device readings only: what the user typed is their choice. + val vm = loadedViewModel(UserSettings()) + advanceUntilIdle() + + vm.saveLocation(47.60621, -122.33211) + advanceUntilIdle() + + assertThat(repo.updates.single().location) + .isEqualTo(LocationUpdate.Set(Coordinates(47.60621, -122.33211))) + } + + @Test + fun `an out-of-range latitude is refused without a request`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings()) + advanceUntilIdle() + + vm.saveLocation(91.0, 0.0) + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + assertThat(vm.uiState.value.notice?.text) + .isEqualTo(ProfileLocationViewModel.LATITUDE_RANGE_NOTE) + assertThat(vm.uiState.value.notice?.isError).isTrue() + assertThat(vm.uiState.value.coordinates).isNull() + } + + @Test + fun `an out-of-range longitude is refused without a request`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings()) + advanceUntilIdle() + + vm.saveLocation(0.0, -180.5) + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + assertThat(vm.uiState.value.notice?.text) + .isEqualTo(ProfileLocationViewModel.LONGITUDE_RANGE_NOTE) + } + + @Test + fun `both bounds are reported when both entries are impossible`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings()) + advanceUntilIdle() + + vm.saveLocation(-90.001, 200.0) + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + assertThat(vm.uiState.value.notice?.text) + .contains(ProfileLocationViewModel.LATITUDE_RANGE_NOTE) + assertThat(vm.uiState.value.notice?.text) + .contains(ProfileLocationViewModel.LONGITUDE_RANGE_NOTE) + } + + @Test + fun `the exact bounds are accepted`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings()) + advanceUntilIdle() + + vm.saveLocation(-90.0, 180.0) + advanceUntilIdle() + + assertThat(repo.updates.single().location) + .isEqualTo(LocationUpdate.Set(Coordinates(-90.0, 180.0))) + } + + @Test + fun `a value that is not a coordinate at all is refused without a request`() = + runTest(dispatcher) { + val vm = loadedViewModel(UserSettings()) + advanceUntilIdle() + + vm.saveLocation(Double.NaN, Double.POSITIVE_INFINITY) + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + } + + @Test + fun `saving the coordinates already stored does not call the API`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings(latitude = 47.6062, longitude = -122.3321)) + advanceUntilIdle() + + vm.saveLocation(47.6062, -122.3321) + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + assertThat(vm.uiState.value.coordinates).isEqualTo(seattle) + } + + @Test + fun `a rejected save leaves the stored location showing and reports why`() = + runTest(dispatcher) { + val vm = loadedViewModel(UserSettings(latitude = 47.6062, longitude = -122.3321)) + repo.updateResult = { ApiResult.Failure(AppError.Server("boom")) } + advanceUntilIdle() + + vm.saveLocation(10.0, 10.0) + advanceUntilIdle() + + // Never claimed, so nothing to roll back: the row still shows server truth. + assertThat(vm.uiState.value.coordinates).isEqualTo(seattle) + assertThat(vm.uiState.value.isSaving).isFalse() + assertThat(vm.uiState.value.notice?.text) + .isEqualTo("InterlinedList is having trouble right now. Try again shortly.") + assertThat(vm.uiState.value.notice?.isError).isTrue() + } + + // --- Reading the device --------------------------------------------------- + + @Test + fun `nothing reads the device until the user asks`() = runTest(dispatcher) { + device.result = DeviceLocationResult.Available(Coordinates(47.61, -122.33)) + device.granted = true + loadedViewModel(UserSettings()) + advanceUntilIdle() + + // Constructing and loading the section must not take a position. + assertThat(device.readCount).isEqualTo(0) + assertThat(repo.updates).isEmpty() + } + + @Test + fun `a device reading is rounded to about a kilometre and saved`() = runTest(dispatcher) { + device.granted = true + device.result = DeviceLocationResult.Available(Coordinates(47.60621, -122.33207)) + val vm = loadedViewModel(UserSettings()) + advanceUntilIdle() + + vm.useDeviceLocation() + advanceUntilIdle() + + assertThat(device.readCount).isEqualTo(1) + val sent = repo.updates.single() + assertThat(sent.location).isEqualTo(LocationUpdate.Set(Coordinates(47.61, -122.33))) + assertThat(sent.touchedFieldNames()).containsExactly("latitude", "longitude") + assertThat(vm.uiState.value.coordinates).isEqualTo(Coordinates(47.61, -122.33)) + assertThat(vm.uiState.value.isReadingDevice).isFalse() + } + + @Test + fun `a reading without the permission saves nothing and keeps manual entry available`() = + runTest(dispatcher) { + // Defence in depth: even if the UI called this without a grant, the source + // refuses and the section says so rather than capturing anything. + device.result = DeviceLocationResult.PermissionMissing + val vm = loadedViewModel(UserSettings()) + advanceUntilIdle() + + vm.useDeviceLocation() + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + assertThat(vm.uiState.value.coordinates).isNull() + assertThat(vm.uiState.value.notice?.text) + .isEqualTo(ProfileLocationViewModel.PERMISSION_DENIED_NOTE) + assertThat(vm.uiState.value.notice?.isError).isFalse() + + // ...and typing it in still works. + vm.saveLocation(47.6062, -122.3321) + advanceUntilIdle() + assertThat(repo.updates.single().location).isEqualTo(LocationUpdate.Set(seattle)) + assertThat(vm.uiState.value.coordinates).isEqualTo(seattle) + } + + @Test + fun `a device with no fix reports it and changes nothing`() = runTest(dispatcher) { + device.granted = true + device.result = DeviceLocationResult.Unavailable + val vm = loadedViewModel(UserSettings(latitude = 47.6062, longitude = -122.3321)) + advanceUntilIdle() + + vm.useDeviceLocation() + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + assertThat(vm.uiState.value.coordinates).isEqualTo(seattle) + assertThat(vm.uiState.value.notice?.text).isEqualTo(ProfileLocationViewModel.NO_FIX_NOTE) + } + + // --- Refusing the permission --------------------------------------------- + + @Test + fun `refusing the permission leaves everything else working`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings(latitude = 47.6062, longitude = -122.3321)) + advanceUntilIdle() + + vm.onPermissionDenied(permanently = false) + advanceUntilIdle() + + // Nothing was read, nothing was sent, nothing was lost. + assertThat(device.readCount).isEqualTo(0) + assertThat(repo.updates).isEmpty() + assertThat(vm.uiState.value.coordinates).isEqualTo(seattle) + assertThat(vm.uiState.value.isReadingDevice).isFalse() + val notice = vm.uiState.value.notice + assertThat(notice?.text).isEqualTo(ProfileLocationViewModel.PERMISSION_DENIED_NOTE) + // A refusal is a choice, not an error. + assertThat(notice?.isError).isFalse() + assertThat(notice?.text).contains("enter coordinates below") + + // Manual entry still works afterwards, including over a stored location. + vm.saveLocation(10.5, 20.5) + advanceUntilIdle() + assertThat(vm.uiState.value.coordinates).isEqualTo(Coordinates(10.5, 20.5)) + } + + @Test + fun `a permanent refusal says where the decision can be changed`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings()) + advanceUntilIdle() + + vm.onPermissionDenied(permanently = true) + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + val notice = vm.uiState.value.notice + assertThat(notice?.text).isEqualTo(ProfileLocationViewModel.PERMISSION_BLOCKED_NOTE) + assertThat(notice?.isError).isFalse() + assertThat(notice?.text).contains("Android Settings") + assertThat(notice?.text).contains("enter coordinates below") + + // The section is still fully usable. + vm.saveLocation(47.6062, -122.3321) + advanceUntilIdle() + assertThat(vm.uiState.value.coordinates).isEqualTo(seattle) + } + + @Test + fun `a notice can be dismissed`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings()) + advanceUntilIdle() + vm.onPermissionDenied(permanently = false) + + vm.dismissNotice() + + assertThat(vm.uiState.value.notice).isNull() + } + + // --- Removal: not offered, because the API cannot do it ------------------ + + @Test + fun `a stored location is replaced rather than removed`() = runTest(dispatcher) { + // The only supported way to change a saved location: PATCH a different pair. + // `PATCH /api/user/update` refuses every spelling of "no value" with + // 400 {"error":"latitude must be a number between -90 and 90"}, so nothing + // here can ask for a removal — see LocationUpdate.Clear. + val vm = loadedViewModel(UserSettings(latitude = 47.6062, longitude = -122.3321)) + advanceUntilIdle() + + vm.saveLocation(45.52, -122.68) + advanceUntilIdle() + + val sent = repo.updates.single() + assertThat(sent.location).isEqualTo(LocationUpdate.Set(Coordinates(45.52, -122.68))) + assertThat(sent.touchedFieldNames()).containsExactly("latitude", "longitude") + assertThat(vm.uiState.value.coordinates).isEqualTo(Coordinates(45.52, -122.68)) + } + + @Test + fun `no user action can send a clear`() = runTest(dispatcher) { + // The guarantee is structural — the view model builds LocationUpdate.Set and + // nothing else — and this pins it against every entry point at once, so a + // future edit cannot quietly reintroduce a request the server always rejects. + device.granted = true + device.result = DeviceLocationResult.Available(Coordinates(47.61, -122.33)) + val vm = loadedViewModel(UserSettings(latitude = 47.6062, longitude = -122.3321)) + advanceUntilIdle() + + vm.saveLocation(10.0, 20.0) + vm.useDeviceLocation() + vm.onPermissionDenied(permanently = true) + vm.dismissNotice() + advanceUntilIdle() + + assertThat(repo.updates).isNotEmpty() + assertThat(repo.updates.map { it.location }) + .doesNotContain(LocationUpdate.Clear) + } + + // --- Following the shared settings cache ---------------------------------- + + @Test + fun `the section shows the stored location after a refresh`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings()) + + vm.uiState.test { + advanceUntilIdle() + assertThat(vm.uiState.value.coordinates).isNull() + + // A refresh anywhere in the app (the Settings screen owns the loading). + repo.refreshResult = + ApiResult.Success(UserSettings(latitude = 47.6062, longitude = -122.3321)) + repo.refresh() + advanceUntilIdle() + + assertThat(expectMostRecentItem().coordinates).isEqualTo(seattle) + } + } + + @Test + fun `half a coordinate reads as no location at all`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings(latitude = 47.6062, longitude = null)) + advanceUntilIdle() + + assertThat(vm.uiState.value.coordinates).isNull() + } +}