diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index a604afa..cba5c2c 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() + } +}