diff --git a/app/src/main/java/org/groundplatform/android/repository/LocationOfInterestRepository.kt b/app/src/main/java/org/groundplatform/android/repository/LocationOfInterestRepository.kt index 5cbbbb57d4..bc276f01ad 100644 --- a/app/src/main/java/org/groundplatform/android/repository/LocationOfInterestRepository.kt +++ b/app/src/main/java/org/groundplatform/android/repository/LocationOfInterestRepository.kt @@ -65,10 +65,14 @@ constructor( // Single-page buffering. Persist immediately to avoid OOM on geometry-heavy surveys. val syncedLoiIds = mutableSetOf() syncedLoiIds += savePages(remoteDataStore.loadPredefinedLois(survey)) - syncedLoiIds += savePages(remoteDataStore.loadUserLois(survey, ownerUserId)) - if (survey.dataVisibility == Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS) { - syncedLoiIds += savePages(remoteDataStore.loadSharedLois(survey)) - } + // Shared LOIs are visible to all survey participants, so a user's own LOIs are already + // included. + syncedLoiIds += + if (survey.dataVisibility == Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS) { + savePages(remoteDataStore.loadSharedLois(survey)) + } else { + savePages(remoteDataStore.loadUserLois(survey, ownerUserId)) + } val mutations = localLoiStore.getAllSurveyMutations(survey).firstOrNull().orEmpty() diff --git a/app/src/test/java/org/groundplatform/android/data/remote/FakeRemoteDataStore.kt b/app/src/test/java/org/groundplatform/android/data/remote/FakeRemoteDataStore.kt index 2f191796c5..7569879dc4 100644 --- a/app/src/test/java/org/groundplatform/android/data/remote/FakeRemoteDataStore.kt +++ b/app/src/test/java/org/groundplatform/android/data/remote/FakeRemoteDataStore.kt @@ -18,6 +18,7 @@ package org.groundplatform.android.data.remote import javax.inject.Inject import javax.inject.Singleton import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.SurveyListItem @@ -26,6 +27,7 @@ import org.groundplatform.domain.model.User import org.groundplatform.domain.model.locationofinterest.LocationOfInterest import org.groundplatform.domain.model.mutation.Mutation import org.groundplatform.domain.model.toListItem +import org.groundplatform.testing.FakeCall @Singleton class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore { @@ -51,6 +53,9 @@ class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore { var applyMutationError: Error? = null val subscribedSurveyIds = mutableSetOf() + val loadUserLoisCall = FakeCall> { userLois } + + val loadSharedLoisCall = FakeCall> { sharedLois } override fun getRestrictedSurveyList(user: User): Flow> = flowOf(surveys.map { it.toListItem(false) }) @@ -84,7 +89,11 @@ class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore { } override fun loadUserLois(survey: Survey, ownerUserId: String): Flow> = - flowOf(userLois) + flow { + emit(loadUserLoisCall(survey)) + } - override fun loadSharedLois(survey: Survey): Flow> = flowOf(sharedLois) + override fun loadSharedLois(survey: Survey): Flow> = flow { + emit(loadSharedLoisCall(survey)) + } } diff --git a/app/src/test/java/org/groundplatform/android/repository/LocationOfInterestRepositoryTest.kt b/app/src/test/java/org/groundplatform/android/repository/LocationOfInterestRepositoryTest.kt index 5a7943fe30..d8d29f6a64 100644 --- a/app/src/test/java/org/groundplatform/android/repository/LocationOfInterestRepositoryTest.kt +++ b/app/src/test/java/org/groundplatform/android/repository/LocationOfInterestRepositoryTest.kt @@ -86,6 +86,10 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { fakeRemoteDataStore.predefinedLois = TEST_LOCATIONS_OF_INTEREST activateSurvey(TEST_SURVEY.id) advanceUntilIdle() + + // Clear query records + fakeRemoteDataStore.loadUserLoisCall.reset() + fakeRemoteDataStore.loadSharedLoisCall.reset() } } @@ -307,7 +311,7 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { val sharedLoi = FakeData.LOCATION_OF_INTEREST.copy(id = "shared_id") fakeRemoteDataStore.predefinedLois = listOf(predefinedLoi) fakeRemoteDataStore.userLois = listOf(userLoi) - fakeRemoteDataStore.sharedLois = listOf(sharedLoi) + fakeRemoteDataStore.sharedLois = listOf(userLoi, sharedLoi) val expected = setOf(predefinedLoi, userLoi, sharedLoi) @@ -318,6 +322,22 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { assertThat(actual).isEqualTo(expected) } + @Test + fun `should not query user LOIs separately when shared LOIs already include them`() = + runWithTestDispatcher { + val survey = TEST_SURVEY.copy(dataVisibility = Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS) + fakeRemoteDataStore.surveys = listOf(survey) + + val userLoi = FakeData.LOCATION_OF_INTEREST.copy(id = "user_id") + fakeRemoteDataStore.userLois = listOf(userLoi) + fakeRemoteDataStore.sharedLois = listOf(userLoi) + + syncSurvey(survey.id) + + assertThat(fakeRemoteDataStore.loadUserLoisCall.callCount).isEqualTo(0) + assertThat(fakeRemoteDataStore.loadSharedLoisCall.callCount).isEqualTo(1) + } + @Test fun `should not load shared LOIs when visibility is not ALL_SURVEY_PARTICIPANTS`() = runWithTestDispatcher { @@ -337,8 +357,22 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { val actual = locationOfInterestRepository.getValidLois(survey).first() assertThat(actual).isEqualTo(expected) + assertThat(fakeRemoteDataStore.loadUserLoisCall.callCount).isEqualTo(1) + assertThat(fakeRemoteDataStore.loadSharedLoisCall.callCount).isEqualTo(0) } + @Test + fun `should only query user LOIs when visibility is UNSPECIFIED`() = runWithTestDispatcher { + val survey = TEST_SURVEY.copy(dataVisibility = Survey.DataVisibility.UNSPECIFIED) + fakeRemoteDataStore.surveys = listOf(survey) + fakeRemoteDataStore.userLois = listOf(FakeData.LOCATION_OF_INTEREST.copy(id = "user_id")) + + syncSurvey(survey.id) + + assertThat(fakeRemoteDataStore.loadUserLoisCall.callCount).isEqualTo(1) + assertThat(fakeRemoteDataStore.loadSharedLoisCall.callCount).isEqualTo(0) + } + companion object { private val COORDINATE_1 = Coordinates(-20.0, -20.0) private val COORDINATE_2 = Coordinates(0.0, 0.0) diff --git a/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeCall.kt b/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeCall.kt index 29b4ea0956..8f3719990e 100644 --- a/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeCall.kt +++ b/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeCall.kt @@ -38,6 +38,10 @@ class FakeCall(private var behavior: suspend (I) -> O) { return behavior(input) } + fun reset() { + _calls.clear() + } + /** Replaces the current behavior with [newBehavior]. */ fun overrideBehavior(newBehavior: suspend (I) -> O) { behavior = newBehavior