From 4257b862e2097ef17a4c69af3da03fe1b780b8d1 Mon Sep 17 00:00:00 2001 From: Ellen Poe Date: Sun, 5 Oct 2025 13:36:59 -0700 Subject: [PATCH 1/4] refactor: simplify & increase testability of the geocoding service test: add NearbyViewModelTest.kt --- .../cardinal/geocoding/GeocodingService.kt | 51 ++-- .../geocoding/MultiplexedGeocodingService.kt | 6 +- .../geocoding/OfflineGeocodingService.kt | 42 ++-- .../geocoding/PeliasGeocodingService.kt | 109 ++++---- .../ui/directions/DirectionsViewModel.kt | 7 +- .../maps/cardinal/ui/home/HomeViewModel.kt | 7 +- .../maps/cardinal/ui/home/NearbyViewModel.kt | 53 ++-- .../cardinal/ui/home/NearbyViewModelTest.kt | 238 ++++++++++++++++++ 8 files changed, 367 insertions(+), 146 deletions(-) create mode 100644 cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/GeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/GeocodingService.kt index 5763df4..b98f0a7 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/GeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/GeocodingService.kt @@ -21,10 +21,8 @@ package earth.maps.cardinal.geocoding import earth.maps.cardinal.data.GeocodeResult import earth.maps.cardinal.data.GeocodeResult.Companion.generatePlaceId import earth.maps.cardinal.data.LatLng -import earth.maps.cardinal.data.Place import earth.maps.cardinal.data.LocationRepository -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.map +import earth.maps.cardinal.data.Place abstract class GeocodingService(private val locationRepository: LocationRepository) { @@ -32,9 +30,9 @@ abstract class GeocodingService(private val locationRepository: LocationReposito * Geocode a query string to find matching locations, returning Place objects. * @param query The search query (e.g., address, place name) * @param focusPoint Optional focus point for viewport biasing - * @return Flow of Place objects + * @return Place objects */ - suspend fun geocode(query: String, focusPoint: LatLng? = null): Flow> { + suspend fun geocode(query: String, focusPoint: LatLng? = null): List { return convertResultsToPlaces(geocodeRaw(query, focusPoint)) } @@ -42,9 +40,9 @@ abstract class GeocodingService(private val locationRepository: LocationReposito * Reverse geocode coordinates to find address information, returning Place objects. * @param latitude The latitude coordinate * @param longitude The longitude coordinate - * @return Flow of Place objects + * @return Place objects */ - suspend fun reverseGeocode(latitude: Double, longitude: Double): Flow> { + suspend fun reverseGeocode(latitude: Double, longitude: Double): List { return convertResultsToPlaces(reverseGeocodeRaw(latitude, longitude)) } @@ -52,24 +50,21 @@ abstract class GeocodingService(private val locationRepository: LocationReposito * Find nearby places around a given point, returning Place objects. * @param latitude The latitude coordinate * @param longitude The longitude coordinate - * @return Flow of Place objects + * @return Place objects */ - suspend fun nearby(latitude: Double, longitude: Double): Flow> { + suspend fun nearby(latitude: Double, longitude: Double): List { return convertResultsToPlaces(nearbyRaw(latitude, longitude)) } /** - * Converts a Flow of GeocodeResult to a Flow of Place, including deduplication. - * @param resultsFlow The Flow of GeocodeResult to convert. - * @return Flow of Place objects. + * Converts a list of GeocodeResult to a list of Place, including deduplication. + * @param results The GeocodeResults to convert. + * @return Place objects. */ - protected open suspend fun convertResultsToPlaces(resultsFlow: Flow>): Flow> { - return resultsFlow.map { results -> - // Deduplicate based on GeocodeResult before converting to Place - val deduplicatedResults = deduplicateSearchResults(results) - deduplicatedResults.map { geocodeResult -> - locationRepository.createSearchResultPlace(geocodeResult) - } + protected open suspend fun convertResultsToPlaces(results: List): List { + val deduplicatedResults = deduplicateSearchResults(results) + return deduplicatedResults.map { geocodeResult -> + locationRepository.createSearchResultPlace(geocodeResult) } } @@ -77,25 +72,31 @@ abstract class GeocodingService(private val locationRepository: LocationReposito * Geocode a query string to find matching locations, returning raw GeocodeResult objects. * @param query The search query (e.g., address, place name) * @param focusPoint Optional focus point for viewport biasing - * @return Flow of raw geocoding results + * @return Raw geocoding results */ - abstract suspend fun geocodeRaw(query: String, focusPoint: LatLng? = null): Flow> + abstract suspend fun geocodeRaw( + query: String, + focusPoint: LatLng? = null + ): List /** * Reverse geocode coordinates to find address information, returning raw GeocodeResult objects. * @param latitude The latitude coordinate * @param longitude The longitude coordinate - * @return Flow of raw geocoding results + * @return Raw geocoding results */ - abstract suspend fun reverseGeocodeRaw(latitude: Double, longitude: Double): Flow> + abstract suspend fun reverseGeocodeRaw( + latitude: Double, + longitude: Double + ): List /** * Find nearby places around a given point, returning raw GeocodeResult objects. * @param latitude The latitude coordinate * @param longitude The longitude coordinate - * @return Flow of raw nearby places + * @return Raw nearby places */ - abstract suspend fun nearbyRaw(latitude: Double, longitude: Double): Flow> + abstract suspend fun nearbyRaw(latitude: Double, longitude: Double): List } fun deduplicateSearchResults(results: List): List { diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/MultiplexedGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/MultiplexedGeocodingService.kt index 72406ee..a26efc9 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/MultiplexedGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/MultiplexedGeocodingService.kt @@ -31,7 +31,7 @@ class MultiplexedGeocodingService( locationRepository: LocationRepository, ) : GeocodingService(locationRepository) { - override suspend fun geocodeRaw(query: String, focusPoint: LatLng?): Flow> { + override suspend fun geocodeRaw(query: String, focusPoint: LatLng?): List { return if (appPreferenceRepository.offlineMode.value) { offlineGeocodingService.geocodeRaw(query, focusPoint) } else { @@ -42,7 +42,7 @@ class MultiplexedGeocodingService( override suspend fun reverseGeocodeRaw( latitude: Double, longitude: Double - ): Flow> { + ): List { return if (appPreferenceRepository.offlineMode.value) { offlineGeocodingService.reverseGeocodeRaw(latitude, longitude) } else { @@ -50,7 +50,7 @@ class MultiplexedGeocodingService( } } - override suspend fun nearbyRaw(latitude: Double, longitude: Double): Flow> { + override suspend fun nearbyRaw(latitude: Double, longitude: Double): List { return if (appPreferenceRepository.offlineMode.value) { offlineGeocodingService.nearbyRaw(latitude, longitude) } else { diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt index 4139c18..c7ff407 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt @@ -25,8 +25,6 @@ import earth.maps.cardinal.data.Address import earth.maps.cardinal.data.GeocodeResult import earth.maps.cardinal.data.LatLng import earth.maps.cardinal.data.LocationRepository -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.flow import uniffi.cardinal_geocoder.newAirmailIndex import java.io.File @@ -37,36 +35,34 @@ class OfflineGeocodingService( private val geocoderDir = File(context.filesDir, "geocoder").apply { mkdirs() } private val airmailIndex = newAirmailIndex("en", geocoderDir.absolutePath) - override suspend fun geocodeRaw(query: String, focusPoint: LatLng?): Flow> = - flow { - try { - val results = airmailIndex.searchPhrase(query) - val geocodeResults = results.map { poi -> - val tagMap: HashMap = HashMap(poi.tags.size) - for (tag in poi.tags) { - tagMap[tag.key] = tag.value - } - buildResult(tagMap, poi.lat, poi.lng) + override suspend fun geocodeRaw(query: String, focusPoint: LatLng?): List { + try { + val results = airmailIndex.searchPhrase(query) + val geocodeResults = results.map { poi -> + val tagMap: HashMap = HashMap(poi.tags.size) + for (tag in poi.tags) { + tagMap[tag.key] = tag.value } - emit(geocodeResults) - } catch (e: Exception) { - Log.e(TAG, "Geocode failed with exception", e) - // If there's an error, return empty list - emit(emptyList()) + buildResult(tagMap, poi.lat, poi.lng) } + return geocodeResults + } catch (e: Exception) { + Log.e(TAG, "Geocode failed with exception", e) + // If there's an error, return empty list + return emptyList() } + } override suspend fun reverseGeocodeRaw( latitude: Double, longitude: Double - ): Flow> = flow { - emit(emptyList()) + ): List { + return emptyList() } - override suspend fun nearbyRaw(latitude: Double, longitude: Double): Flow> = - flow { - emit(emptyList()) - } + override suspend fun nearbyRaw(latitude: Double, longitude: Double): List { + return emptyList() + } override suspend fun beginTileProcessing() { Log.d(TAG, "Beginning tile processing") diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt index aa83106..af46e8f 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt @@ -32,8 +32,6 @@ import io.ktor.client.plugins.logging.Logging import io.ktor.client.request.get import io.ktor.client.request.parameter import io.ktor.serialization.kotlinx.json.json -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.flow import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonArray import kotlinx.serialization.json.JsonElement @@ -60,42 +58,41 @@ class PeliasGeocodingService( install(Logging) } - override suspend fun geocodeRaw(query: String, focusPoint: LatLng?): Flow> = - flow { - try { - Log.d(TAG, "Geocoding query: $query, focusPoint: $focusPoint") - val config = appPreferenceRepository.peliasApiConfig.value - val response = client.get("${config.baseUrl}/autocomplete") { - parameter("text", query) - parameter("size", "10") - config.apiKey?.let { parameter("api_key", it) } - focusPoint?.let { - parameter("focus.point.lat", it.latitude.toString()) - parameter("focus.point.lon", it.longitude.toString()) - } + override suspend fun geocodeRaw(query: String, focusPoint: LatLng?): List { + try { + Log.d(TAG, "Geocoding query: $query, focusPoint: $focusPoint") + val config = appPreferenceRepository.peliasApiConfig.value + val response = client.get("${config.baseUrl}/autocomplete") { + parameter("text", query) + parameter("size", "10") + config.apiKey?.let { parameter("api_key", it) } + focusPoint?.let { + parameter("focus.point.lat", it.latitude.toString()) + parameter("focus.point.lon", it.longitude.toString()) } + } - val result = response.body() - Log.d(TAG, "Response: $result") - val features = result["features"]?.jsonArray ?: JsonArray(emptyList()) - Log.d(TAG, "Number of features: ${features.size}") - - val geocodeResults = features.mapNotNull { element -> - parseGeocodeResult(element) - } - Log.d(TAG, "Parsed results: ${geocodeResults.size}") + val result = response.body() + Log.d(TAG, "Response: $result") + val features = result["features"]?.jsonArray ?: JsonArray(emptyList()) + Log.d(TAG, "Number of features: ${features.size}") - emit(geocodeResults) - } catch (e: Exception) { - Log.e(TAG, "Error during geocoding", e) - emit(emptyList()) + val geocodeResults = features.mapNotNull { element -> + parseGeocodeResult(element) } + Log.d(TAG, "Parsed results: ${geocodeResults.size}") + + return geocodeResults + } catch (e: Exception) { + Log.e(TAG, "Error during geocoding", e) + return emptyList() } + } override suspend fun reverseGeocodeRaw( latitude: Double, longitude: Double - ): Flow> = flow { + ): List { try { Log.d(TAG, "Reverse geocoding: $latitude, $longitude") val config = appPreferenceRepository.peliasApiConfig.value @@ -116,42 +113,40 @@ class PeliasGeocodingService( } Log.d(TAG, "Parsed reverse results: ${geocodeResults.size}") - emit(geocodeResults) + return geocodeResults } catch (e: Exception) { - Log.e(TAG, "Error during reverse geocoding", e) - emit(emptyList()) + return emptyList() } } - override suspend fun nearbyRaw(latitude: Double, longitude: Double): Flow> = - flow { - try { - Log.d(TAG, "Nearby: $latitude, $longitude") - val config = appPreferenceRepository.peliasApiConfig.value - val response = client.get("${config.baseUrl}/nearby") { - parameter("point.lat", latitude.toString()) - parameter("point.lon", longitude.toString()) - parameter("size", "50") - parameter("layers", "venue") - config.apiKey?.let { parameter("api_key", it) } - } - - val result = response.body() - Log.d(TAG, "Nearby response: $result") - val features = result["features"]?.jsonArray ?: JsonArray(emptyList()) - Log.d(TAG, "Number of nearby features: ${features.size}") + override suspend fun nearbyRaw(latitude: Double, longitude: Double): List { + try { + Log.d(TAG, "Nearby: $latitude, $longitude") + val config = appPreferenceRepository.peliasApiConfig.value + val response = client.get("${config.baseUrl}/nearby") { + parameter("point.lat", latitude.toString()) + parameter("point.lon", longitude.toString()) + parameter("size", "50") + parameter("layers", "venue") + config.apiKey?.let { parameter("api_key", it) } + } - val geocodeResults = features.mapNotNull { element -> - parseGeocodeResult(element) - } - Log.d(TAG, "Parsed nearby results: ${geocodeResults.size}") + val result = response.body() + Log.d(TAG, "Nearby response: $result") + val features = result["features"]?.jsonArray ?: JsonArray(emptyList()) + Log.d(TAG, "Number of nearby features: ${features.size}") - emit(geocodeResults) - } catch (e: Exception) { - Log.e(TAG, "Error during nearby", e) - emit(emptyList()) + val geocodeResults = features.mapNotNull { element -> + parseGeocodeResult(element) } + Log.d(TAG, "Parsed nearby results: ${geocodeResults.size}") + + return geocodeResults + } catch (e: Exception) { + Log.e(TAG, "Error during nearby", e) + return emptyList() } + } private fun parseGeocodeResult(element: JsonElement): GeocodeResult? { return try { diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt index 907c1ff..cc182ef 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt @@ -459,10 +459,9 @@ class DirectionsViewModel @Inject constructor( // Use fromPlace as focus point for viewport biasing if available, // otherwise fall back to current viewport center val focusPoint = fromPlace?.latLng ?: viewportRepository.viewportCenter.value - geocodingService.geocode(query, focusPoint).collect { results -> - geocodeResults.value = results - isSearching = false - } + + geocodeResults.value = geocodingService.geocode(query, focusPoint) + isSearching = false } catch (e: Exception) { // Handle error searchError = e.message ?: "An error occurred during search" diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/HomeViewModel.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/HomeViewModel.kt index 788480e..17f3593 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/HomeViewModel.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/HomeViewModel.kt @@ -25,7 +25,6 @@ import androidx.compose.ui.text.input.TextFieldValue import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel -import earth.maps.cardinal.data.GeocodeResult import earth.maps.cardinal.data.LocationRepository import earth.maps.cardinal.data.Place import earth.maps.cardinal.data.ViewportRepository @@ -106,10 +105,8 @@ class HomeViewModel @Inject constructor( try { // Use current viewport center as focus point for viewport biasing val focusPoint = viewportRepository.viewportCenter.value - geocodingService.geocode(query, focusPoint).collect { results -> - geocodeResults.value = results - isSearching = false - } + geocodeResults.value = geocodingService.geocode(query, focusPoint) + isSearching = false } catch (e: Exception) { // Handle error searchError = e.message ?: "An error occurred during search" diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt index 1c782ab..aa4f713 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt @@ -18,13 +18,11 @@ package earth.maps.cardinal.ui.home -import android.content.Context import android.location.Location import android.util.Log import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel -import dagger.hilt.android.qualifiers.ApplicationContext import earth.maps.cardinal.data.LocationRepository import earth.maps.cardinal.data.Place import earth.maps.cardinal.geocoding.GeocodingService @@ -39,7 +37,6 @@ import javax.inject.Inject @HiltViewModel class NearbyViewModel @Inject constructor( - @param:ApplicationContext private val context: Context, private val geocodingService: GeocodingService, private val locationRepository: LocationRepository ) : ViewModel() { @@ -61,47 +58,43 @@ class NearbyViewModel @Inject constructor( init { // Start observing location updates - observeLocationUpdates() + startLocationObservation() } /** - * Observes location updates from the LocationRepository and fetches nearby data - * when the location changes significantly. + * Starts observing location updates from the LocationRepository. + * This method is made open for testing purposes. */ @OptIn(FlowPreview::class) - private fun observeLocationUpdates() { + fun startLocationObservation() { viewModelScope.launch { - locationRepository.locationFlow.distinctUntilChanged { old, new -> - // Only update if location changed significantly (more than 500 meters) - if (new != null) { - (old?.distanceTo(new) ?: 1000f) < 500f - } else { - false + locationRepository.locationFlow.distinctUntilChanged { pos1, pos2 -> + // Change if either is null (*new* null values are filtered by the null check in collectLatest + if (pos1 == null || pos2 == null) { + return@distinctUntilChanged false } + // Only update if location changed significantly (more than 250 meters) + pos2.distanceTo(pos1) >= 250f }.collectLatest { location -> location?.let { lastLocation = it Log.d(TAG, "Location updated: $lastLocation") - fetchNearby(it.latitude, it.longitude) + fetchNearby(location.latitude, location.longitude) } } } } - fun fetchNearby(latitude: Double, longitude: Double) { - viewModelScope.launch { - _isLoading.value = true - _error.value = null - try { - geocodingService.nearby(latitude, longitude).collect { results -> - _nearbyResults.value = results - _isLoading.value = false - } - } catch (e: Exception) { - _error.value = e.message ?: "Error fetching nearby places" - _nearbyResults.value = emptyList() - _isLoading.value = false - } + suspend fun fetchNearby(latitude: Double, longitude: Double) { + _isLoading.value = true + _error.value = null + try { + _nearbyResults.value = geocodingService.nearby(latitude, longitude) + _isLoading.value = false + } catch (e: Exception) { + _error.value = e.message ?: "Error fetching nearby places" + _nearbyResults.value = emptyList() + _isLoading.value = false } } @@ -110,7 +103,9 @@ class NearbyViewModel @Inject constructor( */ fun refreshData() { lastLocation?.let { location -> - fetchNearby(location.latitude, location.longitude) + viewModelScope.launch { + fetchNearby(location.latitude, location.longitude) + } } } } diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt new file mode 100644 index 0000000..891ae5c --- /dev/null +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt @@ -0,0 +1,238 @@ +package earth.maps.cardinal.ui.home + +import earth.maps.cardinal.MainCoroutineRule +import earth.maps.cardinal.data.LocationRepository +import earth.maps.cardinal.data.Place +import earth.maps.cardinal.geocoding.GeocodingService +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +class NearbyViewModelTest { + + @ExperimentalCoroutinesApi + @get:Rule + var mainCoroutineRule = MainCoroutineRule() + + private lateinit var viewModel: NearbyViewModel + + private val mockGeocodingService = mockk(relaxed = true) + private val mockLocationRepository = mockk() + + @Before + fun setup() { + // Mock location flow + coEvery { mockLocationRepository.locationFlow } returns MutableStateFlow(null) + + viewModel = NearbyViewModel( + geocodingService = mockGeocodingService, + locationRepository = mockLocationRepository + ) + } + + @Test + fun `onLocationUpdated should call fetchNearby with correct coordinates`() = runTest { + val testLocation = android.location.Location("test").apply { + latitude = 37.7749 + longitude = -122.4194 + } + + // Mock the geocoding service + val testPlaces = listOf( + Place( + id = "1", + name = "Test Place", + latLng = earth.maps.cardinal.data.LatLng(0.0, 0.0), + address = null + ) + ) + coEvery { mockGeocodingService.nearby(any(), any()) } returns testPlaces + + // Call onLocationUpdated directly + viewModel.fetchNearby(testLocation.latitude, testLocation.longitude) + + // Allow coroutines to complete + advanceUntilIdle() + + // Verify that fetchNearby was called with the correct coordinates + coVerify { + mockGeocodingService.nearby(testLocation.latitude, testLocation.longitude) + } + assertEquals(testPlaces, viewModel.nearbyResults.value) + } + + @Test + fun `initial state should have empty nearby results, not loading, and no error`() = runTest { + assertTrue(viewModel.nearbyResults.value.isEmpty()) + assertFalse(viewModel.isLoading.value) + assertNull(viewModel.error.value) + } + + @Test + fun `fetchNearby should set loading state and update results when successful`() = runTest { + val testPlaces = listOf( + Place( + id = "1", + name = "Test Place 1", + latLng = earth.maps.cardinal.data.LatLng(0.0, 0.0), + address = null + ), + Place( + id = "2", + name = "Test Place 2", + latLng = earth.maps.cardinal.data.LatLng(1.0, 1.0), + address = null + ) + ) + + coEvery { mockGeocodingService.nearby(any(), any()) } returns testPlaces + + viewModel.fetchNearby(37.7749, -122.4194) + + // Allow coroutines to complete + advanceUntilIdle() + + // Verify results + assertEquals(testPlaces, viewModel.nearbyResults.value) + assertFalse(viewModel.isLoading.value) + assertNull(viewModel.error.value) + } + + @Test + fun `fetchNearby should handle error and set error state`() = runTest { + val errorMessage = "Failed to fetch nearby places" + coEvery { mockGeocodingService.nearby(any(), any()) } throws Exception(errorMessage) + + viewModel.fetchNearby(37.7749, -122.4194) + + // Allow coroutines to complete + advanceUntilIdle() + + // Verify error state + assertTrue(viewModel.nearbyResults.value.isEmpty()) + assertFalse(viewModel.isLoading.value) + assertEquals(errorMessage, viewModel.error.value) + } + + @Test + fun `refreshData should call fetchNearby with last location when available`() = runTest { + val testLocation = android.location.Location("test").apply { + latitude = 37.7749 + longitude = -122.4194 + } + val locationFlow = MutableStateFlow(testLocation) + + // Set up the location repository to return a location + coEvery { mockLocationRepository.locationFlow } returns locationFlow + + // Mock the geocoding service + val testPlaces = listOf( + Place( + id = "1", + name = "Test Place", + latLng = earth.maps.cardinal.data.LatLng(0.0, 0.0), + address = null + ) + ) + coEvery { mockGeocodingService.nearby(any(), any()) } returns testPlaces + + viewModel.startLocationObservation() + + advanceUntilIdle() + + // Now refresh the data + viewModel.refreshData() + + // Allow coroutines to complete + advanceUntilIdle() + + // Verify that fetchNearby was called with the correct coordinates + coVerify { + mockGeocodingService.nearby(testLocation.latitude, testLocation.longitude) + } + assertEquals(testPlaces, viewModel.nearbyResults.value) + } + + @Test + fun `refreshData should do nothing when no last location is available`() = runTest { + // Ensure no location is set + coEvery { mockLocationRepository.locationFlow } returns MutableStateFlow(null) + + // Call refreshData + viewModel.refreshData() + + // Allow coroutines to complete + advanceUntilIdle() + + // Verify that no geocoding service call was made + coVerify(exactly = 0) { + mockGeocodingService.nearby(any(), any()) + } + assertTrue(viewModel.nearbyResults.value.isEmpty()) + } + + @Test + fun `distinctUntilChanged should filter out location updates closer than 250 meters`() = + runTest { + // Create a MutableStateFlow to control location emissions + val locationFlow = MutableStateFlow(null) + + // Make the location repository return our controlled flow + coEvery { mockLocationRepository.locationFlow } returns locationFlow + + // Mock the geocoding service to prevent actual network calls and simplify assertions + coEvery { mockGeocodingService.nearby(any(), any()) } returns emptyList() + + // Start location observation, which will begin collecting from our flow + viewModel.startLocationObservation() + advanceUntilIdle() // Allow the initial collection to start + + // Define test locations + val loc1 = android.location.Location("gps").apply { + latitude = 37.7749 + longitude = -122.4194 + } + val loc2 = android.location.Location("gps").apply { + latitude = 37.7749 + longitude = -122.4193 // Approx 111 meters east of loc1 + } + val loc3 = android.location.Location("gps").apply { + latitude = 37.7754 // Approx 501 meters north of loc1 + longitude = -122.4194 + } + + // --- Test Scenario 1: Initial location update --- + locationFlow.value = loc1 + advanceUntilIdle() // Process loc1 + + // --- Test Scenario 2: Location update closer than 250m (should be filtered out) --- + locationFlow.value = loc2 // Should be filtered out by distinctUntilChanged (< 250m) + advanceUntilIdle() // Process loc2 (should be ignored) + + // --- Test Scenario 3: Location update 250m or more away (should pass through) --- + locationFlow.value = loc3 // This should pass through (>= 250m from loc1) + advanceUntilIdle() // Process loc3 + // Verify onLocationUpdated was called exactly once more, with loc3 + coVerify(exactly = 2) { + mockGeocodingService.nearby( + match { it == loc1.latitude || it == loc3.latitude }, + match { it == loc1.longitude || it == loc3.longitude }, + ) + } + } +} -- GitLab From 35e8cca88785266188aff22b912e9c3f3b80c035 Mon Sep 17 00:00:00 2001 From: Ellen Poe Date: Sun, 5 Oct 2025 14:06:31 -0700 Subject: [PATCH 2/4] feat: nearby filter chips --- .../cardinal/geocoding/GeocodingService.kt | 10 ++++-- .../geocoding/MultiplexedGeocodingService.kt | 11 ++++--- .../geocoding/OfflineGeocodingService.kt | 6 +++- .../geocoding/PeliasGeocodingService.kt | 9 +++++- .../maps/cardinal/ui/home/NearbyScreen.kt | 21 +++++++++++++ .../maps/cardinal/ui/home/NearbyViewModel.kt | 29 +++++++++++++++-- .../app/src/main/res/values/strings.xml | 10 ++++++ .../cardinal/ui/home/NearbyViewModelTest.kt | 31 ++++++++++++------- 8 files changed, 105 insertions(+), 22 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/GeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/GeocodingService.kt index b98f0a7..0f9d869 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/GeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/GeocodingService.kt @@ -52,8 +52,8 @@ abstract class GeocodingService(private val locationRepository: LocationReposito * @param longitude The longitude coordinate * @return Place objects */ - suspend fun nearby(latitude: Double, longitude: Double): List { - return convertResultsToPlaces(nearbyRaw(latitude, longitude)) + suspend fun nearby(latitude: Double, longitude: Double, selectedCategories: List): List { + return convertResultsToPlaces(nearbyRaw(latitude, longitude, selectedCategories)) } /** @@ -96,7 +96,11 @@ abstract class GeocodingService(private val locationRepository: LocationReposito * @param longitude The longitude coordinate * @return Raw nearby places */ - abstract suspend fun nearbyRaw(latitude: Double, longitude: Double): List + abstract suspend fun nearbyRaw( + latitude: Double, + longitude: Double, + selectedCategories: List + ): List } fun deduplicateSearchResults(results: List): List { diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/MultiplexedGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/MultiplexedGeocodingService.kt index a26efc9..84435eb 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/MultiplexedGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/MultiplexedGeocodingService.kt @@ -22,7 +22,6 @@ import earth.maps.cardinal.data.AppPreferenceRepository import earth.maps.cardinal.data.GeocodeResult import earth.maps.cardinal.data.LatLng import earth.maps.cardinal.data.LocationRepository -import kotlinx.coroutines.flow.Flow class MultiplexedGeocodingService( private val appPreferenceRepository: AppPreferenceRepository, @@ -50,11 +49,15 @@ class MultiplexedGeocodingService( } } - override suspend fun nearbyRaw(latitude: Double, longitude: Double): List { + override suspend fun nearbyRaw( + latitude: Double, + longitude: Double, + selectedCategories: List + ): List { return if (appPreferenceRepository.offlineMode.value) { - offlineGeocodingService.nearbyRaw(latitude, longitude) + offlineGeocodingService.nearbyRaw(latitude, longitude, selectedCategories) } else { - onlineGeocodingService.nearbyRaw(latitude, longitude) + onlineGeocodingService.nearbyRaw(latitude, longitude, selectedCategories) } } } diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt index c7ff407..4aa84a1 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt @@ -60,7 +60,11 @@ class OfflineGeocodingService( return emptyList() } - override suspend fun nearbyRaw(latitude: Double, longitude: Double): List { + override suspend fun nearbyRaw( + latitude: Double, + longitude: Double, + selectedCategories: List + ): List { return emptyList() } diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt index af46e8f..a90c634 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt @@ -119,7 +119,11 @@ class PeliasGeocodingService( } } - override suspend fun nearbyRaw(latitude: Double, longitude: Double): List { + override suspend fun nearbyRaw( + latitude: Double, + longitude: Double, + selectedCategories: List + ): List { try { Log.d(TAG, "Nearby: $latitude, $longitude") val config = appPreferenceRepository.peliasApiConfig.value @@ -128,6 +132,9 @@ class PeliasGeocodingService( parameter("point.lon", longitude.toString()) parameter("size", "50") parameter("layers", "venue") + if (selectedCategories.isNotEmpty()) { + parameter("categories", selectedCategories.joinToString(",")) + } config.apiKey?.let { parameter("api_key", it) } } diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyScreen.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyScreen.kt index dbea5b7..e6980ce 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyScreen.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyScreen.kt @@ -22,6 +22,7 @@ import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.FlowRow import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize @@ -36,6 +37,7 @@ import androidx.compose.material3.Button import androidx.compose.material3.Card import androidx.compose.material3.CardDefaults import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.FilterChip import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -44,7 +46,9 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.layout.onGloballyPositioned @@ -92,7 +96,24 @@ fun NearbyScreenContent(viewModel: NearbyViewModel, onPlaceSelected: (Place) -> contentDescription = stringResource(string.refresh_nearby_places) ) } + } + FlowRow { + for (chipSpec in viewModel.allCategories) { + var isSelected by remember { mutableStateOf(false) } + FilterChip( + modifier = Modifier.padding(end = 8.dp), + selected = isSelected, + onClick = { + isSelected = !isSelected + viewModel.setCategorySelection(chipSpec.category, isSelected) + }, + label = { + val label = stringResource(chipSpec.labelResource) + Text(text = label) + } + ) + } } when { diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt index aa4f713..ec77a24 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt @@ -23,6 +23,7 @@ import android.util.Log import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel +import earth.maps.cardinal.R.string import earth.maps.cardinal.data.LocationRepository import earth.maps.cardinal.data.Place import earth.maps.cardinal.geocoding.GeocodingService @@ -35,6 +36,8 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.launch import javax.inject.Inject +data class FilterChipSpec(val category: String, val labelResource: Int) + @HiltViewModel class NearbyViewModel @Inject constructor( private val geocodingService: GeocodingService, @@ -56,11 +59,33 @@ class NearbyViewModel @Inject constructor( private var lastLocation: Location? = null + private val selectedCategories: MutableSet = mutableSetOf() + init { // Start observing location updates startLocationObservation() } + val allCategories = listOf( + FilterChipSpec("food", string.category_food), + FilterChipSpec("food:coffee_shop", string.category_coffee_shop), + FilterChipSpec("recreation", string.category_recreation), + FilterChipSpec("health", string.category_health), + FilterChipSpec("transportation", string.category_transportation), + FilterChipSpec("entertainment", string.category_entertainment), + FilterChipSpec("nightlife", string.category_nightlife), + FilterChipSpec("accommodation", string.category_accommodation), + ) + + fun setCategorySelection(category: String, selection: Boolean) { + if (selection) { + selectedCategories.add(category) + } else { + selectedCategories.remove(category) + } + refreshData() + } + /** * Starts observing location updates from the LocationRepository. * This method is made open for testing purposes. @@ -74,7 +99,7 @@ class NearbyViewModel @Inject constructor( return@distinctUntilChanged false } // Only update if location changed significantly (more than 250 meters) - pos2.distanceTo(pos1) >= 250f + pos2.distanceTo(pos1) < 250f }.collectLatest { location -> location?.let { lastLocation = it @@ -89,7 +114,7 @@ class NearbyViewModel @Inject constructor( _isLoading.value = true _error.value = null try { - _nearbyResults.value = geocodingService.nearby(latitude, longitude) + _nearbyResults.value = geocodingService.nearby(latitude, longitude, selectedCategories.toList()) _isLoading.value = false } catch (e: Exception) { _error.value = e.message ?: "Error fetching nearby places" diff --git a/cardinal-android/app/src/main/res/values/strings.xml b/cardinal-android/app/src/main/res/values/strings.xml index fce9953..30cd012 100644 --- a/cardinal-android/app/src/main/res/values/strings.xml +++ b/cardinal-android/app/src/main/res/values/strings.xml @@ -254,4 +254,14 @@ Allow Cardinal Maps to access your location to show you where you are on the map and provide a better experience. Allow Not Now + + + Food + Coffee + Healthcare + Hotels + Recreation + Transportation + Entertainment + Nightlife diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt index 891ae5c..7a96fb0 100644 --- a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt @@ -61,7 +61,7 @@ class NearbyViewModelTest { address = null ) ) - coEvery { mockGeocodingService.nearby(any(), any()) } returns testPlaces + coEvery { mockGeocodingService.nearby(any(), any(), listOf()) } returns testPlaces // Call onLocationUpdated directly viewModel.fetchNearby(testLocation.latitude, testLocation.longitude) @@ -71,7 +71,11 @@ class NearbyViewModelTest { // Verify that fetchNearby was called with the correct coordinates coVerify { - mockGeocodingService.nearby(testLocation.latitude, testLocation.longitude) + mockGeocodingService.nearby( + testLocation.latitude, + testLocation.longitude, + listOf() + ) } assertEquals(testPlaces, viewModel.nearbyResults.value) } @@ -100,7 +104,7 @@ class NearbyViewModelTest { ) ) - coEvery { mockGeocodingService.nearby(any(), any()) } returns testPlaces + coEvery { mockGeocodingService.nearby(any(), any(), listOf()) } returns testPlaces viewModel.fetchNearby(37.7749, -122.4194) @@ -116,7 +120,7 @@ class NearbyViewModelTest { @Test fun `fetchNearby should handle error and set error state`() = runTest { val errorMessage = "Failed to fetch nearby places" - coEvery { mockGeocodingService.nearby(any(), any()) } throws Exception(errorMessage) + coEvery { mockGeocodingService.nearby(any(), any(), listOf()) } throws Exception(errorMessage) viewModel.fetchNearby(37.7749, -122.4194) @@ -149,7 +153,7 @@ class NearbyViewModelTest { address = null ) ) - coEvery { mockGeocodingService.nearby(any(), any()) } returns testPlaces + coEvery { mockGeocodingService.nearby(any(), any(), listOf()) } returns testPlaces viewModel.startLocationObservation() @@ -163,7 +167,11 @@ class NearbyViewModelTest { // Verify that fetchNearby was called with the correct coordinates coVerify { - mockGeocodingService.nearby(testLocation.latitude, testLocation.longitude) + mockGeocodingService.nearby( + testLocation.latitude, + testLocation.longitude, + listOf() + ) } assertEquals(testPlaces, viewModel.nearbyResults.value) } @@ -181,7 +189,7 @@ class NearbyViewModelTest { // Verify that no geocoding service call was made coVerify(exactly = 0) { - mockGeocodingService.nearby(any(), any()) + mockGeocodingService.nearby(any(), any(), listOf()) } assertTrue(viewModel.nearbyResults.value.isEmpty()) } @@ -196,7 +204,7 @@ class NearbyViewModelTest { coEvery { mockLocationRepository.locationFlow } returns locationFlow // Mock the geocoding service to prevent actual network calls and simplify assertions - coEvery { mockGeocodingService.nearby(any(), any()) } returns emptyList() + coEvery { mockGeocodingService.nearby(any(), any(), listOf()) } returns emptyList() // Start location observation, which will begin collecting from our flow viewModel.startLocationObservation() @@ -209,11 +217,11 @@ class NearbyViewModelTest { } val loc2 = android.location.Location("gps").apply { latitude = 37.7749 - longitude = -122.4193 // Approx 111 meters east of loc1 + longitude = -122.4193 // Very close to loc1 } val loc3 = android.location.Location("gps").apply { - latitude = 37.7754 // Approx 501 meters north of loc1 - longitude = -122.4194 + latitude = 37.77549231327316 // Approx 445 meters west of loc1 + longitude = -122.42456035029012 } // --- Test Scenario 1: Initial location update --- @@ -232,6 +240,7 @@ class NearbyViewModelTest { mockGeocodingService.nearby( match { it == loc1.latitude || it == loc3.latitude }, match { it == loc1.longitude || it == loc3.longitude }, + listOf(), ) } } -- GitLab From da71e7e99ff9b65a678b5e068d7fb4dcac189816 Mon Sep 17 00:00:00 2001 From: Ellen Poe Date: Sun, 5 Oct 2025 14:11:17 -0700 Subject: [PATCH 3/4] test: test nearby filter logic --- .../maps/cardinal/ui/home/NearbyViewModel.kt | 4 ++ .../cardinal/ui/home/NearbyViewModelTest.kt | 59 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt index ec77a24..fc83219 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt @@ -110,6 +110,10 @@ class NearbyViewModel @Inject constructor( } } + fun getSelectedCategoriesSet(): Set { + return selectedCategories.toSet() + } + suspend fun fetchNearby(latitude: Double, longitude: Double) { _isLoading.value = true _error.value = null diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt index 7a96fb0..f6b779a 100644 --- a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt @@ -244,4 +244,63 @@ class NearbyViewModelTest { ) } } + + @Test + fun `setCategorySelection should add and remove categories correctly`() = runTest { + val category1 = "food" + val category2 = "health" + + // Initially no categories should be selected + assertTrue(viewModel.getSelectedCategoriesSet().isEmpty()) + + // Add categories + viewModel.setCategorySelection(category1, true) + viewModel.setCategorySelection(category2, true) + + // Verify categories were added + assertEquals(setOf(category1, category2), viewModel.getSelectedCategoriesSet()) + + // Remove one category + viewModel.setCategorySelection(category1, false) + + // Verify remaining category + assertEquals(setOf(category2), viewModel.getSelectedCategoriesSet()) + } + + @Test + fun `setCategorySelection should refresh data with selected categories`() = runTest { + val testLocation = android.location.Location("test").apply { + latitude = 37.7749 + longitude = -122.4194 + } + val locationFlow = MutableStateFlow(testLocation) + + // Set up the location repository to return a location + coEvery { mockLocationRepository.locationFlow } returns locationFlow + + // Mock the geocoding service + coEvery { mockGeocodingService.nearby(any(), any(), any()) } returns emptyList() + + viewModel.startLocationObservation() + advanceUntilIdle() + + // Clear previous verification calls + io.mockk.clearMocks(mockGeocodingService) + + // Add categories + viewModel.setCategorySelection("food", true) + viewModel.setCategorySelection("health", true) + + // Allow coroutines to complete + advanceUntilIdle() + + // Verify that fetchNearby was called with both selected categories + coVerify { + mockGeocodingService.nearby( + testLocation.latitude, + testLocation.longitude, + listOf("food", "health") + ) + } + } } -- GitLab From b4ddeff746a3892161893184f45930cd031714fe Mon Sep 17 00:00:00 2001 From: Ellen Poe Date: Sun, 5 Oct 2025 14:26:21 -0700 Subject: [PATCH 4/4] fix: review comments --- .../maps/cardinal/ui/home/NearbyScreen.kt | 7 +++-- .../maps/cardinal/ui/home/NearbyViewModel.kt | 27 ++++++++++--------- .../cardinal/ui/home/NearbyViewModelTest.kt | 10 +++---- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyScreen.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyScreen.kt index e6980ce..125616b 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyScreen.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyScreen.kt @@ -99,14 +99,13 @@ fun NearbyScreenContent(viewModel: NearbyViewModel, onPlaceSelected: (Place) -> } FlowRow { + val selectedCategories by viewModel.selectedCategories.collectAsState() for (chipSpec in viewModel.allCategories) { - var isSelected by remember { mutableStateOf(false) } FilterChip( modifier = Modifier.padding(end = 8.dp), - selected = isSelected, + selected = selectedCategories.contains(chipSpec.category), onClick = { - isSelected = !isSelected - viewModel.setCategorySelection(chipSpec.category, isSelected) + viewModel.toggleCategorySelection(chipSpec.category) }, label = { val label = stringResource(chipSpec.labelResource) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt index fc83219..5930c9c 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/home/NearbyViewModel.kt @@ -20,6 +20,7 @@ package earth.maps.cardinal.ui.home import android.location.Location import android.util.Log +import androidx.annotation.VisibleForTesting import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel @@ -27,7 +28,6 @@ import earth.maps.cardinal.R.string import earth.maps.cardinal.data.LocationRepository import earth.maps.cardinal.data.Place import earth.maps.cardinal.geocoding.GeocodingService -import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -59,7 +59,8 @@ class NearbyViewModel @Inject constructor( private var lastLocation: Location? = null - private val selectedCategories: MutableSet = mutableSetOf() + private val _selectedCategories = MutableStateFlow>(emptySet()) + val selectedCategories = _selectedCategories.asStateFlow() init { // Start observing location updates @@ -77,11 +78,11 @@ class NearbyViewModel @Inject constructor( FilterChipSpec("accommodation", string.category_accommodation), ) - fun setCategorySelection(category: String, selection: Boolean) { - if (selection) { - selectedCategories.add(category) + fun toggleCategorySelection(category: String) { + if (_selectedCategories.value.contains(category)) { + _selectedCategories.value = _selectedCategories.value.minus(category) } else { - selectedCategories.remove(category) + _selectedCategories.value = _selectedCategories.value.plus(category) } refreshData() } @@ -90,17 +91,17 @@ class NearbyViewModel @Inject constructor( * Starts observing location updates from the LocationRepository. * This method is made open for testing purposes. */ - @OptIn(FlowPreview::class) + @VisibleForTesting fun startLocationObservation() { viewModelScope.launch { - locationRepository.locationFlow.distinctUntilChanged { pos1, pos2 -> + locationRepository.locationFlow.distinctUntilChanged(areEquivalent = { pos1, pos2 -> // Change if either is null (*new* null values are filtered by the null check in collectLatest if (pos1 == null || pos2 == null) { return@distinctUntilChanged false } // Only update if location changed significantly (more than 250 meters) - pos2.distanceTo(pos1) < 250f - }.collectLatest { location -> + pos2.distanceTo(pos1) < 250f // true means equivalency + }).collectLatest { location -> location?.let { lastLocation = it Log.d(TAG, "Location updated: $lastLocation") @@ -110,15 +111,17 @@ class NearbyViewModel @Inject constructor( } } + @VisibleForTesting fun getSelectedCategoriesSet(): Set { - return selectedCategories.toSet() + return selectedCategories.value.toSet() } suspend fun fetchNearby(latitude: Double, longitude: Double) { _isLoading.value = true _error.value = null try { - _nearbyResults.value = geocodingService.nearby(latitude, longitude, selectedCategories.toList()) + _nearbyResults.value = + geocodingService.nearby(latitude, longitude, selectedCategories.value.toList()) _isLoading.value = false } catch (e: Exception) { _error.value = e.message ?: "Error fetching nearby places" diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt index f6b779a..6fee7bb 100644 --- a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/home/NearbyViewModelTest.kt @@ -254,14 +254,14 @@ class NearbyViewModelTest { assertTrue(viewModel.getSelectedCategoriesSet().isEmpty()) // Add categories - viewModel.setCategorySelection(category1, true) - viewModel.setCategorySelection(category2, true) + viewModel.toggleCategorySelection(category1) + viewModel.toggleCategorySelection(category2) // Verify categories were added assertEquals(setOf(category1, category2), viewModel.getSelectedCategoriesSet()) // Remove one category - viewModel.setCategorySelection(category1, false) + viewModel.toggleCategorySelection(category1) // Verify remaining category assertEquals(setOf(category2), viewModel.getSelectedCategoriesSet()) @@ -288,8 +288,8 @@ class NearbyViewModelTest { io.mockk.clearMocks(mockGeocodingService) // Add categories - viewModel.setCategorySelection("food", true) - viewModel.setCategorySelection("health", true) + viewModel.toggleCategorySelection("food") + viewModel.toggleCategorySelection("health") // Allow coroutines to complete advanceUntilIdle() -- GitLab