From 40ba37ae5c1a603af5654b3e5c9ad99af6a40498 Mon Sep 17 00:00:00 2001 From: andreia Date: Tue, 18 Aug 2026 18:34:38 +0200 Subject: [PATCH 1/6] make existingLoiFeatures contain only features within the camera bounds --- .../android/ui/common/BaseMapViewModel.kt | 46 ++++++------ .../android/ui/common/BaseMapViewModelTest.kt | 72 ++++++++++++------- 2 files changed, 70 insertions(+), 48 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/common/BaseMapViewModel.kt b/app/src/main/java/org/groundplatform/android/ui/common/BaseMapViewModel.kt index 05c1b1661b..7e582bf5d3 100644 --- a/app/src/main/java/org/groundplatform/android/ui/common/BaseMapViewModel.kt +++ b/app/src/main/java/org/groundplatform/android/ui/common/BaseMapViewModel.kt @@ -39,8 +39,8 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.mapNotNull import kotlinx.coroutines.flow.merge -import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.shareIn import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update @@ -63,6 +63,7 @@ import org.groundplatform.android.ui.util.getDefaultColor import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.geometry.Coordinates import org.groundplatform.domain.model.imagery.TileSource +import org.groundplatform.domain.model.locationofinterest.LocationOfInterest import org.groundplatform.domain.model.map.CameraPosition import org.groundplatform.domain.model.map.MapType import org.groundplatform.domain.repository.LocationOfInterestRepositoryInterface @@ -135,32 +136,33 @@ constructor( .asLiveData() /** - * Read-only LOI features for the active survey. Lazily initialized to avoid unnecessary database - * queries when not rendered. + * Read-only LOI features for the active survey which fall within the visible viewport. Lazily + * initialized to avoid unnecessary database queries when not rendered. */ - val existingLoiFeatures: Flow> by lazy { - surveyRepository.activeSurveyFlow - .flatMapLatest { survey -> - if (survey == null) flowOf(emptySet()) - else locationOfInterestRepository.getValidLois(survey) + val existingLoiFeatures: StateFlow> by lazy { + combine( + surveyRepository.activeSurveyFlow, + getCurrentCameraPosition().mapNotNull { it.bounds }.distinctUntilChanged(), + ) { survey, bounds -> + survey to bounds } - .map { lois -> - lois - .map { - Feature( - id = it.id, - type = Feature.Type.LOCATION_OF_INTEREST, - geometry = it.geometry, - style = Feature.Style(it.job.getDefaultColor()), - clusterable = false, - ) - } - .toSet() + .flatMapLatest { (survey, bounds) -> + if (survey == null) flowOf(emptyList()) + else locationOfInterestRepository.getWithinBounds(survey, bounds) } - .onStart { emit(setOf()) } - .distinctUntilChanged() + .map { lois -> lois.map { it.toFeature() }.toSet() } + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), setOf()) } + private fun LocationOfInterest.toFeature() = + Feature( + id = id, + type = Feature.Type.LOCATION_OF_INTEREST, + geometry = geometry, + style = Feature.Style(job.getDefaultColor()), + clusterable = false, + ) + /** Returns whether the user has granted fine location permission. */ fun hasLocationPermission() = permissionsManager.isGranted(Manifest.permission.ACCESS_FINE_LOCATION) diff --git a/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt b/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt index 4ea4b05c92..318b4eca07 100644 --- a/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt @@ -40,6 +40,9 @@ import org.groundplatform.android.system.SettingsManager import org.groundplatform.android.ui.components.MapFloatingActionButtonType import org.groundplatform.android.ui.map.Feature import org.groundplatform.android.ui.util.getDefaultColor +import org.groundplatform.domain.model.geometry.Coordinates +import org.groundplatform.domain.model.map.Bounds +import org.groundplatform.domain.model.map.CameraPosition import org.groundplatform.domain.repository.LocationOfInterestRepositoryInterface import org.groundplatform.domain.repository.MapStateRepositoryInterface import org.groundplatform.domain.repository.OfflineAreaRepositoryInterface @@ -48,6 +51,8 @@ import org.junit.Assert.assertEquals import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock +import org.mockito.kotlin.any +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner @@ -67,6 +72,9 @@ class BaseMapViewModelTest : BaseHiltTest() { private lateinit var viewModel: BaseMapViewModel + private val viewport = Bounds(south = -10.0, west = -10.0, north = 10.0, east = 10.0) + private val otherViewport = Bounds(south = 10.0, west = 10.0, north = 20.0, east = 20.0) + @Test fun `Should display the correct location icon and hide the recenter button when the location is locked`() = runTest { @@ -170,35 +178,47 @@ class BaseMapViewModelTest : BaseHiltTest() { } @Test - fun `Should show existing features on the map`() = + fun `Should show existing features within the viewport on the map`() = runWithTestDispatcher { + setupMocks() + val areaOfInterest = AREA_OF_INTEREST.copy(id = "loi id 2") + whenever(surveyRepository.activeSurveyFlow).thenReturn(MutableStateFlow(SURVEY)) + whenever(locationOfInterestRepository.getWithinBounds(SURVEY, viewport)) + .thenReturn(flowOf(listOf(LOCATION_OF_INTEREST, areaOfInterest))) + + viewModel.onMapCameraMoved(CameraPosition(Coordinates(0.0, 0.0), bounds = viewport)) + + val features = viewModel.existingLoiFeatures.first { it.isNotEmpty() } + + assertThat(features) + .containsExactly( + Feature( + id = LOCATION_OF_INTEREST.id, + type = Feature.Type.LOCATION_OF_INTEREST, + geometry = LOCATION_OF_INTEREST.geometry, + style = Feature.Style(JOB.getDefaultColor()), + clusterable = false, + selected = false, + ), + Feature( + id = areaOfInterest.id, + type = Feature.Type.LOCATION_OF_INTEREST, + geometry = areaOfInterest.geometry, + style = Feature.Style(JOB.getDefaultColor()), + clusterable = false, + selected = false, + ), + ) + } + + @Test + fun `Should not query for existing features until the viewport is known`() = runWithTestDispatcher { setupMocks() - val areaOfInterest = AREA_OF_INTEREST.copy(id = "loi id 2") whenever(surveyRepository.activeSurveyFlow).thenReturn(MutableStateFlow(SURVEY)) - whenever(locationOfInterestRepository.getValidLois(SURVEY)) - .thenReturn(flowOf(setOf(LOCATION_OF_INTEREST, areaOfInterest))) - - val features = viewModel.existingLoiFeatures.first { it.isNotEmpty() } - - assertThat(features) - .containsExactly( - Feature( - id = LOCATION_OF_INTEREST.id, - type = Feature.Type.LOCATION_OF_INTEREST, - geometry = LOCATION_OF_INTEREST.geometry, - style = Feature.Style(JOB.getDefaultColor()), - clusterable = false, - selected = false, - ), - Feature( - id = areaOfInterest.id, - type = Feature.Type.LOCATION_OF_INTEREST, - geometry = areaOfInterest.geometry, - style = Feature.Style(JOB.getDefaultColor()), - clusterable = false, - selected = false, - ), - ) + + assertThat(viewModel.existingLoiFeatures.first()).isEmpty() + + verify(locationOfInterestRepository, never()).getWithinBounds(any(), any()) } private fun setupMocks( From 0a25c974fdf81df18c728a5cd279f8da03156fe7 Mon Sep 17 00:00:00 2001 From: andreia Date: Tue, 18 Aug 2026 18:00:06 +0200 Subject: [PATCH 2/6] remove launchWhenTaskVisible from data collection task fragments --- .../datacollection/DataCollectionViewModel.kt | 6 - .../tasks/AbstractTaskMapFragment.kt | 10 +- .../tasks/TaskFragmentExtensions.kt | 47 ------ .../CaptureLocationTaskMapFragment.kt | 5 +- .../tasks/point/DropPinTaskMapFragment.kt | 3 +- .../tasks/polygon/DrawAreaTaskMapFragment.kt | 30 ++-- .../tasks/TaskFragmentExtensionsTest.kt | 147 ------------------ 7 files changed, 21 insertions(+), 227 deletions(-) delete mode 100644 app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/TaskFragmentExtensions.kt delete mode 100644 app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/TaskFragmentExtensionsTest.kt diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt index c99116a8e3..ad613a46a1 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt @@ -27,7 +27,6 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.update @@ -500,11 +499,6 @@ internal constructor( } } - fun isCurrentActiveTaskFlow(taskId: String): Flow = - uiState - .map { (it as? DataCollectionUiState.Ready)?.currentTaskId == taskId } - .distinctUntilChanged() - companion object { private const val TASK_JOB_ID_KEY = "jobId" private const val TASK_LOI_ID_KEY = "locationOfInterestId" diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/AbstractTaskMapFragment.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/AbstractTaskMapFragment.kt index a78395d4b0..51a120c794 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/AbstractTaskMapFragment.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/AbstractTaskMapFragment.kt @@ -149,12 +149,12 @@ abstract class AbstractTaskMapFragment : @MustBeInvokedByOverriders override fun onMapReady(map: MapFragment) { - launchWhenTaskVisible(dataCollectionViewModel, taskId) { - launch { getMapViewModel().getCurrentCameraPosition().collect { onMapCameraMoved(it) } } - launch { renderFeatures().collect { map.setFeatures(it) } } - // Allow the fragment to restore map viewport to previously drawn feature. - setDefaultViewPort() + launchWhenStarted { + getMapViewModel().getCurrentCameraPosition().collect { onMapCameraMoved(it) } } + launchWhenStarted { renderFeatures().collect { map.setFeatures(it) } } + // Allow the fragment to restore map viewport to previously drawn feature. + launchWhenStarted { setDefaultViewPort() } } /** Must be overridden by subclasses. */ diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/TaskFragmentExtensions.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/TaskFragmentExtensions.kt deleted file mode 100644 index e0dbc2d855..0000000000 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/TaskFragmentExtensions.kt +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright 2026 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.groundplatform.android.ui.datacollection.tasks - -import androidx.fragment.app.Fragment -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.coroutineScope -import kotlinx.coroutines.flow.collectLatest -import kotlinx.coroutines.launch -import org.groundplatform.android.ui.datacollection.DataCollectionViewModel - -/** - * Launches a coroutine that runs the given [block] only when the task with [taskId] is visible to - * the user (active in ViewPager) and the fragment is in at least the STARTED state. The block is - * automatically canceled when the task becomes inactive. - */ -internal fun Fragment.launchWhenTaskVisible( - viewModel: DataCollectionViewModel, - taskId: String, - block: suspend CoroutineScope.() -> Unit, -) { - viewLifecycleOwner.lifecycleScope.launch { - repeatOnLifecycle(Lifecycle.State.STARTED) { - viewModel.isCurrentActiveTaskFlow(taskId).collectLatest { isActive -> - if (isActive) { - coroutineScope { block() } - } - } - } - } -} diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/location/CaptureLocationTaskMapFragment.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/location/CaptureLocationTaskMapFragment.kt index 5063d8852e..767bbeaa77 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/location/CaptureLocationTaskMapFragment.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/location/CaptureLocationTaskMapFragment.kt @@ -21,7 +21,6 @@ import dagger.hilt.android.AndroidEntryPoint import javax.inject.Inject import org.groundplatform.android.ui.common.MapConfig import org.groundplatform.android.ui.datacollection.tasks.AbstractTaskMapFragment -import org.groundplatform.android.ui.datacollection.tasks.launchWhenTaskVisible import org.groundplatform.android.ui.map.MapFragment @AndroidEntryPoint @@ -30,7 +29,7 @@ class CaptureLocationTaskMapFragment @Inject constructor() : override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - launchWhenTaskVisible(dataCollectionViewModel, taskId) { + launchWhenStarted { getMapViewModel().getLocationUpdates().collect { taskViewModel.updateLocation(it) } } } @@ -39,7 +38,7 @@ class CaptureLocationTaskMapFragment @Inject constructor() : override fun onMapReady(map: MapFragment) { super.onMapReady(map) - launchWhenTaskVisible(dataCollectionViewModel, taskId) { + launchWhenStarted { taskViewModel.initLocationUpdates(getMapViewModel()) } } diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/point/DropPinTaskMapFragment.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/point/DropPinTaskMapFragment.kt index 41af4cde3f..0ac31b717a 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/point/DropPinTaskMapFragment.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/point/DropPinTaskMapFragment.kt @@ -20,7 +20,6 @@ import javax.inject.Inject import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine import org.groundplatform.android.ui.datacollection.tasks.AbstractTaskMapFragment -import org.groundplatform.android.ui.datacollection.tasks.launchWhenTaskVisible import org.groundplatform.android.ui.map.Feature import org.groundplatform.android.ui.map.MapFragment import org.groundplatform.domain.model.map.CameraPosition @@ -33,7 +32,7 @@ class DropPinTaskMapFragment @Inject constructor() : super.onMapReady(map) // Disable pan/zoom gestures if a marker has been placed on the map. - launchWhenTaskVisible(dataCollectionViewModel, taskId) { + launchWhenStarted { taskViewModel.features.collect { features -> updateGestures(features) } } } diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskMapFragment.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskMapFragment.kt index 2c314eb3c0..c31e9008a9 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskMapFragment.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskMapFragment.kt @@ -22,9 +22,7 @@ import javax.inject.Inject import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.map -import kotlinx.coroutines.launch import org.groundplatform.android.ui.datacollection.tasks.AbstractTaskMapFragment -import org.groundplatform.android.ui.datacollection.tasks.launchWhenTaskVisible import org.groundplatform.android.ui.map.Feature import org.groundplatform.android.ui.map.gms.GmsExt.toBounds import org.groundplatform.domain.model.map.CameraPosition @@ -36,26 +34,24 @@ class DrawAreaTaskMapFragment @Inject constructor() : override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - launchWhenTaskVisible(dataCollectionViewModel, taskId) { - launch { - taskViewModel.sessionState - .map { state -> !state.isTooClose && !state.isMarkedComplete } - .collect { shouldShow -> setCenterMarkerVisibility(shouldShow) } - } + launchWhenStarted { + taskViewModel.sessionState + .map { state -> !state.isTooClose && !state.isMarkedComplete } + .collect { shouldShow -> setCenterMarkerVisibility(shouldShow) } + } - launch { - map.cameraDragEvents.collect { coord -> - if (!taskViewModel.isMarkedComplete()) { - taskViewModel.updateLastVertexAndMaybeCompletePolygon(coord) { c1, c2 -> - map.getDistanceInPixels(c1, c2) - } + launchWhenStarted { + map.cameraDragEvents.collect { coord -> + if (!taskViewModel.isMarkedComplete()) { + taskViewModel.updateLastVertexAndMaybeCompletePolygon(coord) { c1, c2 -> + map.getDistanceInPixels(c1, c2) } } } + } - launch { - taskViewModel.cameraMoveEvents.collect { coordinates -> moveToPosition(coordinates) } - } + launchWhenStarted { + taskViewModel.cameraMoveEvents.collect { coordinates -> moveToPosition(coordinates) } } } diff --git a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/TaskFragmentExtensionsTest.kt b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/TaskFragmentExtensionsTest.kt deleted file mode 100644 index 60e54c0e48..0000000000 --- a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/TaskFragmentExtensionsTest.kt +++ /dev/null @@ -1,147 +0,0 @@ -/* - * Copyright 2026 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.groundplatform.android.ui.datacollection.tasks - -import androidx.fragment.app.Fragment -import com.google.common.truth.Truth.assertThat -import dagger.hilt.android.testing.HiltAndroidTest -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.awaitCancellation -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.test.advanceUntilIdle -import org.groundplatform.android.BaseHiltTest -import org.groundplatform.android.testrules.FragmentScenarioRule -import org.groundplatform.android.ui.datacollection.DataCollectionViewModel -import org.junit.Rule -import org.junit.Test -import org.junit.runner.RunWith -import org.mockito.Mock -import org.mockito.kotlin.whenever -import org.robolectric.RobolectricTestRunner - -/** A minimal fragment used to provide a viewLifecycleOwner for testing. */ -class TestVisibilityFragment : Fragment(android.R.layout.simple_list_item_1) - -@OptIn(ExperimentalCoroutinesApi::class) -@HiltAndroidTest -@RunWith(RobolectricTestRunner::class) -class TaskFragmentExtensionsTest : BaseHiltTest() { - @get:Rule val fragmentScenario = FragmentScenarioRule() - - @Mock lateinit var dataCollectionViewModel: DataCollectionViewModel - - private val isCurrentTaskActiveFlow = MutableStateFlow(false) - private val taskId = "test_task" - - override fun setUp() { - super.setUp() - whenever(dataCollectionViewModel.isCurrentActiveTaskFlow(taskId)) - .thenReturn(isCurrentTaskActiveFlow) - } - - @Test - fun `block does not execute when task is inactive`() = runWithTestDispatcher { - var blockExecuted = false - - fragmentScenario.launchFragmentInHiltContainer { - launchWhenTaskVisible(dataCollectionViewModel, taskId) { blockExecuted = true } - } - - advanceUntilIdle() - assertThat(blockExecuted).isFalse() - } - - @Test - fun `block executes when task becomes active`() = runWithTestDispatcher { - var blockExecuted = false - - fragmentScenario.launchFragmentInHiltContainer { - launchWhenTaskVisible(dataCollectionViewModel, taskId) { blockExecuted = true } - } - - isCurrentTaskActiveFlow.value = true - advanceUntilIdle() - - assertThat(blockExecuted).isTrue() - } - - @Test - fun `block is cancelled when task becomes inactive`() = runWithTestDispatcher { - var isRunning = false - var executionCount = 0 - - fragmentScenario.launchFragmentInHiltContainer { - launchWhenTaskVisible(dataCollectionViewModel, taskId) { - executionCount++ - isRunning = true - try { - awaitCancellation() - } finally { - isRunning = false - } - } - } - - advanceUntilIdle() - assertThat(isRunning).isFalse() - - // Activate the task - block should start running - isCurrentTaskActiveFlow.value = true - advanceUntilIdle() - assertThat(isRunning).isTrue() - assertThat(executionCount).isEqualTo(1) - - // Deactivate the task - block should be canceled - isCurrentTaskActiveFlow.value = false - advanceUntilIdle() - assertThat(isRunning).isFalse() - } - - @Test - fun `block restarts when task becomes active again`() = runWithTestDispatcher { - var executionCount = 0 - var isRunning = false - - fragmentScenario.launchFragmentInHiltContainer { - launchWhenTaskVisible(dataCollectionViewModel, taskId) { - executionCount++ - isRunning = true - try { - awaitCancellation() - } finally { - isRunning = false - } - } - } - - // First activation - isCurrentTaskActiveFlow.value = true - advanceUntilIdle() - assertThat(executionCount).isEqualTo(1) - assertThat(isRunning).isTrue() - - // Deactivate - isCurrentTaskActiveFlow.value = false - advanceUntilIdle() - assertThat(isRunning).isFalse() - - // Reactivate - block should restart - isCurrentTaskActiveFlow.value = true - advanceUntilIdle() - assertThat(executionCount).isEqualTo(2) - assertThat(isRunning).isTrue() - } -} From df3a17b720782c01dc19ecc2c4499222f549d58f Mon Sep 17 00:00:00 2001 From: andreia Date: Wed, 19 Aug 2026 09:46:29 +0200 Subject: [PATCH 3/6] fix code quality checks --- .../android/ui/datacollection/DataCollectionViewModel.kt | 2 -- .../groundplatform/android/ui/common/BaseMapViewModelTest.kt | 1 - 2 files changed, 3 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt index ad613a46a1..439f4746a5 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/DataCollectionViewModel.kt @@ -23,11 +23,9 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch diff --git a/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt b/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt index 318b4eca07..b1b7b7c586 100644 --- a/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt @@ -73,7 +73,6 @@ class BaseMapViewModelTest : BaseHiltTest() { private lateinit var viewModel: BaseMapViewModel private val viewport = Bounds(south = -10.0, west = -10.0, north = 10.0, east = 10.0) - private val otherViewport = Bounds(south = 10.0, west = 10.0, north = 20.0, east = 20.0) @Test fun `Should display the correct location icon and hide the recenter button when the location is locked`() = From a26f1d44dc568bac160a4321abb144a6097111af Mon Sep 17 00:00:00 2001 From: andreia Date: Fri, 21 Aug 2026 11:37:15 +0200 Subject: [PATCH 4/6] stop querying all LOIs in existingFeatures on every camera move --- .../android/ui/common/BaseMapViewModel.kt | 17 +++---- .../android/ui/common/BaseMapViewModelTest.kt | 47 +++++++++++++++---- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/common/BaseMapViewModel.kt b/app/src/main/java/org/groundplatform/android/ui/common/BaseMapViewModel.kt index 7e582bf5d3..3d5467d0a3 100644 --- a/app/src/main/java/org/groundplatform/android/ui/common/BaseMapViewModel.kt +++ b/app/src/main/java/org/groundplatform/android/ui/common/BaseMapViewModel.kt @@ -57,6 +57,7 @@ import org.groundplatform.android.ui.map.Feature import org.groundplatform.android.ui.map.NewCameraPositionViaBounds import org.groundplatform.android.ui.map.NewCameraPositionViaCoordinates import org.groundplatform.android.ui.map.NewCameraPositionViaCoordinatesAndZoomLevel +import org.groundplatform.android.ui.map.gms.GmsExt.contains import org.groundplatform.android.ui.map.gms.GmsExt.toBounds import org.groundplatform.android.ui.map.gms.toCoordinates import org.groundplatform.android.ui.util.getDefaultColor @@ -140,15 +141,15 @@ constructor( * initialized to avoid unnecessary database queries when not rendered. */ val existingLoiFeatures: StateFlow> by lazy { - combine( - surveyRepository.activeSurveyFlow, - getCurrentCameraPosition().mapNotNull { it.bounds }.distinctUntilChanged(), - ) { survey, bounds -> - survey to bounds + surveyRepository.activeSurveyFlow + .flatMapLatest { survey -> + if (survey == null) flowOf(emptySet()) + else locationOfInterestRepository.getValidLois(survey) } - .flatMapLatest { (survey, bounds) -> - if (survey == null) flowOf(emptyList()) - else locationOfInterestRepository.getWithinBounds(survey, bounds) + .combine(getCurrentCameraPosition().mapNotNull { it.bounds }.distinctUntilChanged()) { + lois, + bounds -> + lois.filter { bounds.contains(it.geometry) } } .map { lois -> lois.map { it.toFeature() }.toSet() } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), setOf()) diff --git a/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt b/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt index b1b7b7c586..1be28902e4 100644 --- a/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt @@ -16,6 +16,7 @@ package org.groundplatform.android.ui.common import android.Manifest +import android.os.Looper import com.google.android.gms.common.api.ApiException import com.google.android.gms.common.api.CommonStatusCodes import com.google.android.gms.common.api.Status @@ -26,7 +27,9 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.groundplatform.android.BaseHiltTest import org.groundplatform.android.FakeData.AREA_OF_INTEREST @@ -51,11 +54,11 @@ import org.junit.Assert.assertEquals import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock -import org.mockito.kotlin.any -import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf @HiltAndroidTest @RunWith(RobolectricTestRunner::class) @@ -181,8 +184,8 @@ class BaseMapViewModelTest : BaseHiltTest() { setupMocks() val areaOfInterest = AREA_OF_INTEREST.copy(id = "loi id 2") whenever(surveyRepository.activeSurveyFlow).thenReturn(MutableStateFlow(SURVEY)) - whenever(locationOfInterestRepository.getWithinBounds(SURVEY, viewport)) - .thenReturn(flowOf(listOf(LOCATION_OF_INTEREST, areaOfInterest))) + whenever(locationOfInterestRepository.getValidLois(SURVEY)) + .thenReturn(flowOf(setOf(LOCATION_OF_INTEREST, areaOfInterest))) viewModel.onMapCameraMoved(CameraPosition(Coordinates(0.0, 0.0), bounds = viewport)) @@ -210,14 +213,40 @@ class BaseMapViewModelTest : BaseHiltTest() { } @Test - fun `Should not query for existing features until the viewport is known`() = + fun `Should not emit existing features until the viewport is known`() = runWithTestDispatcher { + setupMocks() + whenever(surveyRepository.activeSurveyFlow).thenReturn(MutableStateFlow(SURVEY)) + whenever(locationOfInterestRepository.getValidLois(SURVEY)) + .thenReturn(flowOf(setOf(LOCATION_OF_INTEREST))) + + assertThat(viewModel.existingLoiFeatures.first()).isEmpty() + } + + @Test + fun `Should filter existing features on camera move without re-querying`() = runWithTestDispatcher { setupMocks() whenever(surveyRepository.activeSurveyFlow).thenReturn(MutableStateFlow(SURVEY)) - - assertThat(viewModel.existingLoiFeatures.first()).isEmpty() - - verify(locationOfInterestRepository, never()).getWithinBounds(any(), any()) + whenever(locationOfInterestRepository.getValidLois(SURVEY)) + .thenReturn(flowOf(setOf(LOCATION_OF_INTEREST))) + + backgroundScope.launch { viewModel.existingLoiFeatures.collect {} } + runCurrent() + + viewModel.onMapCameraMoved(CameraPosition(Coordinates(0.0, 0.0), bounds = viewport)) + shadowOf(Looper.getMainLooper()).idle() + assertThat(viewModel.existingLoiFeatures.value).hasSize(1) + + // Panning away from the only LOI must change what is rendered + viewModel.onMapCameraMoved( + CameraPosition( + coordinates = Coordinates(50.0, 50.0), + bounds = Bounds(south = 40.0, west = 40.0, north = 60.0, east = 60.0), + ) + ) + shadowOf(Looper.getMainLooper()).idle() + assertThat(viewModel.existingLoiFeatures.value).isEmpty() + verify(locationOfInterestRepository, times(1)).getValidLois(SURVEY) } private fun setupMocks( From c72152c0e1f8f87e1b87872b6c86b5a0a0710713 Mon Sep 17 00:00:00 2001 From: andreia Date: Fri, 21 Aug 2026 11:44:46 +0200 Subject: [PATCH 5/6] fix code style --- .../tasks/location/CaptureLocationTaskMapFragment.kt | 4 +--- .../ui/datacollection/tasks/point/DropPinTaskMapFragment.kt | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/location/CaptureLocationTaskMapFragment.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/location/CaptureLocationTaskMapFragment.kt index 767bbeaa77..4d87229850 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/location/CaptureLocationTaskMapFragment.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/location/CaptureLocationTaskMapFragment.kt @@ -38,8 +38,6 @@ class CaptureLocationTaskMapFragment @Inject constructor() : override fun onMapReady(map: MapFragment) { super.onMapReady(map) - launchWhenStarted { - taskViewModel.initLocationUpdates(getMapViewModel()) - } + launchWhenStarted { taskViewModel.initLocationUpdates(getMapViewModel()) } } } diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/point/DropPinTaskMapFragment.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/point/DropPinTaskMapFragment.kt index 0ac31b717a..8b8652a8c8 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/point/DropPinTaskMapFragment.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/point/DropPinTaskMapFragment.kt @@ -32,9 +32,7 @@ class DropPinTaskMapFragment @Inject constructor() : super.onMapReady(map) // Disable pan/zoom gestures if a marker has been placed on the map. - launchWhenStarted { - taskViewModel.features.collect { features -> updateGestures(features) } - } + launchWhenStarted { taskViewModel.features.collect { features -> updateGestures(features) } } } private fun updateGestures(features: Set) { From ec97a1654f190992ac13005124d499ffd3f13c62 Mon Sep 17 00:00:00 2001 From: andreia Date: Fri, 21 Aug 2026 12:32:59 +0200 Subject: [PATCH 6/6] improve test coverage --- .../android/ui/common/BaseMapViewModelTest.kt | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt b/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt index 1be28902e4..1bfaef9c09 100644 --- a/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/common/BaseMapViewModelTest.kt @@ -43,7 +43,9 @@ import org.groundplatform.android.system.SettingsManager import org.groundplatform.android.ui.components.MapFloatingActionButtonType import org.groundplatform.android.ui.map.Feature import org.groundplatform.android.ui.util.getDefaultColor +import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.geometry.Coordinates +import org.groundplatform.domain.model.geometry.Point import org.groundplatform.domain.model.map.Bounds import org.groundplatform.domain.model.map.CameraPosition import org.groundplatform.domain.repository.LocationOfInterestRepositoryInterface @@ -249,6 +251,58 @@ class BaseMapViewModelTest : BaseHiltTest() { verify(locationOfInterestRepository, times(1)).getValidLois(SURVEY) } + @Test + fun `Should render existing features when moving camera`() = runWithTestDispatcher { + setupMocks() + val lois = MutableStateFlow(setOf(LOCATION_OF_INTEREST)) + whenever(surveyRepository.activeSurveyFlow).thenReturn(MutableStateFlow(SURVEY)) + whenever(locationOfInterestRepository.getValidLois(SURVEY)).thenReturn(lois) + + backgroundScope.launch { viewModel.existingLoiFeatures.collect {} } + runCurrent() + + viewModel.onMapCameraMoved(CameraPosition(Coordinates(0.0, 0.0), bounds = viewport)) + shadowOf(Looper.getMainLooper()).idle() + assertThat(viewModel.existingLoiFeatures.value).hasSize(1) + + val added = LOCATION_OF_INTEREST.copy(id = "inside", geometry = Point(Coordinates(1.0, 1.0))) + val offscreen = + LOCATION_OF_INTEREST.copy(id = "outside", geometry = Point(Coordinates(50.0, 50.0))) + lois.value = setOf(LOCATION_OF_INTEREST, added, offscreen) + shadowOf(Looper.getMainLooper()).idle() + + assertThat(viewModel.existingLoiFeatures.value.map { it.tag.id }) + .containsExactly(LOCATION_OF_INTEREST.id, added.id) + } + + @Test + fun `Should re-query and replace existing features when the active survey changes`() = + runWithTestDispatcher { + setupMocks() + val otherSurvey = SURVEY.copy(id = "survey id 2") + val otherLoi = LOCATION_OF_INTEREST.copy(id = "loi id 2") + val activeSurvey = MutableStateFlow(SURVEY) + whenever(surveyRepository.activeSurveyFlow).thenReturn(activeSurvey) + whenever(locationOfInterestRepository.getValidLois(SURVEY)) + .thenReturn(flowOf(setOf(LOCATION_OF_INTEREST))) + whenever(locationOfInterestRepository.getValidLois(otherSurvey)) + .thenReturn(flowOf(setOf(otherLoi))) + + backgroundScope.launch { viewModel.existingLoiFeatures.collect {} } + runCurrent() + + viewModel.onMapCameraMoved(CameraPosition(Coordinates(0.0, 0.0), bounds = viewport)) + shadowOf(Looper.getMainLooper()).idle() + assertThat(viewModel.existingLoiFeatures.value.map { it.tag.id }) + .containsExactly(LOCATION_OF_INTEREST.id) + + activeSurvey.value = otherSurvey + shadowOf(Looper.getMainLooper()).idle() + + verify(locationOfInterestRepository).getValidLois(otherSurvey) + assertThat(viewModel.existingLoiFeatures.value.map { it.tag.id }).containsExactly(otherLoi.id) + } + private fun setupMocks( isLocationLocked: Boolean = false, hasLocationPermissions: Boolean = true,