Update UI on remote survey changes (#2152)

ground/src/main/java/com/google/android/ground/domain/usecases/survey/ActivateSurveyUseCase.kt

@@ -38,8 +38,9 @@ constructor(
       return
     }
 
-    surveyRepository.activeSurvey =
-      surveyRepository.getOfflineSurvey(surveyId)
-        ?: makeSurveyAvailableOffline(surveyId) ?: error("Survey $surveyId not found in remote db")
+    surveyRepository.getOfflineSurvey(surveyId)
+      ?: makeSurveyAvailableOffline(surveyId) ?: error("Survey $surveyId not found in remote db")
+
+    surveyRepository.selectedSurveyId = surveyId
   }
 }

ground/src/main/java/com/google/android/ground/persistence/local/room/dao/SurveyDao.kt

@@ -29,4 +29,8 @@ interface SurveyDao : BaseDao<SurveyEntity> {
   @Transaction
   @Query("SELECT * FROM survey WHERE id = :id")
   suspend fun findSurveyById(id: String): SurveyEntityAndRelations?
+
+  @Transaction
+  @Query("SELECT * FROM survey WHERE id = :id")
+  fun survey(id: String): Flow<SurveyEntityAndRelations?>
 }

ground/src/main/java/com/google/android/ground/persistence/local/room/stores/RoomSurveyStore.kt

@@ -48,6 +48,8 @@ class RoomSurveyStore @Inject internal constructor() : LocalSurveyStore {
   override val surveys: Flow<List<Survey>>
     get() = surveyDao.getAll().map { surveyEntities -> surveyEntities.map { it.toModelObject() } }
 
+  override fun survey(id: String): Flow<Survey?> = surveyDao.survey(id).map { it?.toModelObject() }
+
   /**
    * Attempts to update persisted data associated with a [Survey] in the local database. If the
    * provided survey does not exist, inserts the given survey into the database.

ground/src/main/java/com/google/android/ground/persistence/local/stores/LocalSurveyStore.kt

@@ -23,6 +23,8 @@ interface LocalSurveyStore {
   /** Load surveys stored in local database. */
   val surveys: Flow<List<Survey>>
 
+  fun survey(id: String): Flow<Survey?>
+
   /** Load last active survey, if any. */
   suspend fun getSurveyById(id: String): Survey?
 

ground/src/main/java/com/google/android/ground/repository/SurveyRepository.kt

@@ -31,12 +31,18 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.SharedFlow
 import kotlinx.coroutines.flow.SharingStarted
+import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
+import kotlinx.coroutines.flow.distinctUntilChanged
+import kotlinx.coroutines.flow.filterNotNull
 import kotlinx.coroutines.flow.flatMapLatest
+import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.flow.shareIn
+import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.flow.stateIn
+import kotlinx.coroutines.flow.transformLatest
 import kotlinx.coroutines.withTimeoutOrNull
 import timber.log.Timber
 
@@ -47,6 +53,7 @@ private const val LOAD_REMOTE_SURVEY_TIMEOUT_MILLS: Long = 15 * 1000
  * data stores. For more details on this pattern and overall architecture, see
  * https://developer.android.com/jetpack/docs/guide.
  */
+@OptIn(ExperimentalCoroutinesApi::class)
 @Singleton
 class SurveyRepository
 @Inject
@@ -57,29 +64,36 @@ constructor(
   private val networkManager: NetworkManager,
   @ApplicationScope private val externalScope: CoroutineScope
 ) {
-  private val _activeSurvey = MutableStateFlow<Survey?>(null)
-
-  val activeSurveyFlow: SharedFlow<Survey?> =
-    _activeSurvey.shareIn(externalScope, replay = 1, started = SharingStarted.Eagerly)
-
-  /**
-   * The currently active survey, or `null` if no survey is active. Updating this property causes
-   * [lastActiveSurveyId] to be updated with the id of the specified survey, or `""` if the
-   * specified survey is `null`.
-   */
-  var activeSurvey: Survey?
-    get() = _activeSurvey.value
+  private val _selectedSurveyIdFlow = MutableStateFlow<String?>(null)
+  var selectedSurveyId: String?
+    get() = _selectedSurveyIdFlow.value
     set(value) {
-      _activeSurvey.value = value
-      lastActiveSurveyId = value?.id ?: ""
+      _selectedSurveyIdFlow.value = value
     }
 
+  val activeSurveyFlow: StateFlow<Survey?> =
+    _selectedSurveyIdFlow
+      .flatMapLatest { id -> offlineSurvey(id) }
+      .stateIn(externalScope, SharingStarted.Lazily, null)
+
+  val activeSurveyIdFlow: Flow<String?> =
+    activeSurveyFlow.transformLatest<Survey?, String> { it?.id }.distinctUntilChanged()
+
+  /** The currently active survey, or `null` if no survey is active. */
+  val activeSurvey: Survey?
+    get() = activeSurveyFlow.value
+
   val localSurveyListFlow: Flow<List<SurveyListItem>>
     get() = localSurveyStore.surveys.map { list -> list.map { it.toListItem(true) } }
 
+  /** The id of the last activated survey. */
   var lastActiveSurveyId: String by localValueStore::lastActiveSurveyId
     internal set
 
+  init {
+    activeSurveyFlow.filterNotNull().onEach { lastActiveSurveyId = it.id }.launchIn(externalScope)
+  }
+
   /** Listens for remote changes to the survey with the specified id. */
   suspend fun subscribeToSurveyUpdates(surveyId: String) =
     remoteDataStore.subscribeToSurveyUpdates(surveyId)
@@ -89,6 +103,9 @@ constructor(
    */
   suspend fun getOfflineSurvey(surveyId: String): Survey? = localSurveyStore.getSurveyById(surveyId)
 
+  private fun offlineSurvey(id: String?): Flow<Survey?> =
+    if (id == null) flowOf(null) else localSurveyStore.survey(id)
+
   /**
    * Loads the survey with the specified id from remote and writes to local db. If the survey isn't
    * found or operation times out, then we return null and don't fetch the survey from local db.
@@ -103,7 +120,7 @@ constructor(
       ?.apply { localSurveyStore.insertOrUpdateSurvey(this) }
 
   fun clearActiveSurvey() {
-    activeSurvey = null
+    selectedSurveyId = null
   }
 
   fun getSurveyList(user: User): Flow<List<SurveyListItem>> =

ground/src/main/java/com/google/android/ground/ui/common/BaseMapViewModel.kt

@@ -192,7 +192,10 @@ constructor(
 
   /** Emits a stream of camera update requests. */
   fun getCameraUpdateRequests(): SharedFlow<CameraUpdateRequest> =
-    merge(updateCameraPositionOnSurveyChange(), updateCameraPositionOnLocationChange())
+    merge(
+        getCameraUpdateRequestsForSurveyActivations(),
+        getCameraUpdateRequestsForDeviceLocationChanges()
+      )
       .shareIn(viewModelScope, SharingStarted.Eagerly, replay = 0)
 
   /** Emits a stream of current camera position. */
@@ -205,7 +208,7 @@ constructor(
    * appropriate zoom level and subsequent ones only pan the map.
    */
   @OptIn(ExperimentalCoroutinesApi::class)
-  private fun updateCameraPositionOnLocationChange(): Flow<CameraUpdateRequest> =
+  private fun getCameraUpdateRequestsForDeviceLocationChanges(): Flow<CameraUpdateRequest> =
     locationLock
       .flatMapLatest { enabled ->
         getLocationUpdates()
@@ -229,10 +232,12 @@ constructor(
       CameraUpdateRequest(CameraPosition(coordinates), true)
     }
 
-  /** Updates map camera when active survey changes. */
-  private fun updateCameraPositionOnSurveyChange(): Flow<CameraUpdateRequest> =
-    surveyRepository.activeSurveyFlow.filterNotNull().transform {
-      getLastSavedPositionOrDefaultBounds(it)?.apply { emit(this) }
+  /** Emits a new camera update request when active survey changes. */
+  private fun getCameraUpdateRequestsForSurveyActivations(): Flow<CameraUpdateRequest> =
+    surveyRepository.activeSurveyIdFlow.transform {
+      surveyRepository.activeSurvey?.let {
+        getLastSavedPositionOrDefaultBounds(it)?.apply { emit(this) }
+      }
     }
 
   private suspend fun getLastSavedPositionOrDefaultBounds(survey: Survey): CameraUpdateRequest? {

ground/src/main/java/com/google/android/ground/ui/home/mapcontainer/HomeScreenMapContainerViewModel.kt

@@ -46,7 +46,6 @@ import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
-import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.flow.map
@@ -107,7 +106,7 @@ internal constructor(
     // TODO: Since we depend on survey stream from repo anyway, this transformation can be moved
     //  into the repository.
 
-    val activeSurvey = surveyRepository.activeSurveyFlow.distinctUntilChanged()
+    val activeSurvey = surveyRepository.activeSurveyFlow
 
     mapLoiFeatures =
       activeSurvey.flatMapLatest {

ground/src/test/java/com/google/android/ground/domain/usecase/ActivateSurveyUseCaseTest.kt

@@ -18,24 +18,20 @@ package com.google.android.ground.domain.usecase
 
 import com.google.android.ground.BaseHiltTest
 import com.google.android.ground.domain.usecases.survey.ActivateSurveyUseCase
-import com.google.android.ground.domain.usecases.survey.MakeSurveyAvailableOfflineUseCase
 import com.google.android.ground.persistence.local.stores.LocalSurveyStore
 import com.google.android.ground.repository.SurveyRepository
 import com.google.common.truth.Truth.assertThat
 import com.sharedtest.FakeData.SURVEY
 import com.sharedtest.persistence.remote.FakeRemoteDataStore
-import dagger.hilt.android.testing.BindValue
 import dagger.hilt.android.testing.HiltAndroidTest
 import javax.inject.Inject
 import kotlin.test.assertFails
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.advanceUntilIdle
+import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.mockito.Mock
-import org.mockito.Mockito.`when`
-import org.mockito.kotlin.never
-import org.mockito.kotlin.verify
+import org.mockito.Mockito.times
 import org.robolectric.RobolectricTestRunner
 
 @OptIn(ExperimentalCoroutinesApi::class)
@@ -47,68 +43,66 @@ class ActivateSurveyUseCaseTest : BaseHiltTest() {
   @Inject lateinit var surveyRepository: SurveyRepository
   @Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore
 
-  @BindValue @Mock lateinit var makeSurveyAvailableOffline: MakeSurveyAvailableOfflineUseCase
+  @Before
+  override fun setUp() {
+    super.setUp()
+    fakeRemoteDataStore.surveys = listOf(SURVEY)
+  }
 
   @Test
-  fun `Activates survey when no survey is active`() = runWithTestDispatcher {
-    `when`(makeSurveyAvailableOffline(SURVEY.id)).thenReturn(SURVEY)
+  fun `Makes survey available offline`() = runWithTestDispatcher {
+    activateSurvey(SURVEY.id)
+    advanceUntilIdle()
 
+    assertThat(localSurveyStore.getSurveyById(SURVEY.id)).isEqualTo(SURVEY)
+  }
+
+  @Test
+  fun `Activates survey`() = runWithTestDispatcher {
     activateSurvey(SURVEY.id)
     advanceUntilIdle()
 
-    // Verify survey is made available for offline use.
-    verify(makeSurveyAvailableOffline).invoke(SURVEY.id)
-    // Verify survey is active.
     assertThat(surveyRepository.activeSurvey).isEqualTo(SURVEY)
   }
 
   @Test
   fun `Throws error when survey can't be made available offline`() = runWithTestDispatcher {
-    `when`(makeSurveyAvailableOffline(SURVEY.id)).thenThrow(Error::class.java)
+    fakeRemoteDataStore.onLoadSurvey = { error("Boom!") }
 
     assertFails { activateSurvey(SURVEY.id) }
     advanceUntilIdle()
 
-    // Verify no survey is active.
     assertThat(surveyRepository.activeSurvey).isNull()
   }
 
   @Test
   fun `Throws error when survey doesn't exist`() = runWithTestDispatcher {
-    `when`(makeSurveyAvailableOffline(SURVEY.id)).thenReturn(null)
+    fakeRemoteDataStore.onLoadSurvey = { null }
 
     assertFails { activateSurvey(SURVEY.id) }
     advanceUntilIdle()
 
-    // Verify no survey is active.
     assertThat(surveyRepository.activeSurvey).isNull()
   }
 
   @Test
   fun `Uses local instance if available`() = runWithTestDispatcher {
-    fakeRemoteDataStore.surveys = listOf(SURVEY)
+    fakeRemoteDataStore.onLoadSurvey = { error("This should not be called") }
     localSurveyStore.insertOrUpdateSurvey(SURVEY)
 
     activateSurvey(SURVEY.id)
     advanceUntilIdle()
 
-    // Verify that we don't try to make survey available offline again.
-    verify(makeSurveyAvailableOffline, never()).invoke(SURVEY.id)
-    // Verify survey is active.
     assertThat(surveyRepository.activeSurvey).isEqualTo(SURVEY)
   }
 
   @Test
   fun `Does nothing when survey already active`() = runWithTestDispatcher {
-    localSurveyStore.insertOrUpdateSurvey(SURVEY)
-    surveyRepository.activeSurvey = SURVEY
-
     activateSurvey(SURVEY.id)
     advanceUntilIdle()
 
-    // Verify that we don't try to make survey available offline again.
-    verify(makeSurveyAvailableOffline, never()).invoke(SURVEY.id)
-    // Verify survey is active.
-    assertThat(surveyRepository.activeSurvey).isEqualTo(SURVEY)
+    fakeRemoteDataStore.onLoadSurvey = { error("loadSurvey() should not be called here") }
+    activateSurvey(SURVEY.id)
+    advanceUntilIdle()
   }
 }

ground/src/test/java/com/google/android/ground/domain/usecase/ReactivateLastSurveyUseCaseTest.kt

@@ -30,6 +30,7 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
 import org.mockito.kotlin.never
+import org.mockito.kotlin.times
 import org.mockito.kotlin.verify
 import org.robolectric.RobolectricTestRunner
 
@@ -43,33 +44,31 @@ class ReactivateLastSurveyUseCaseTest : BaseHiltTest() {
   @BindValue @Mock lateinit var activateSurvey: ActivateSurveyUseCase
 
   @Test
-  fun reactivateLastSurvey_lastIdSet_noActiveSurvey() = runWithTestDispatcher {
+  fun `Reactivate last survey`() = runWithTestDispatcher {
     surveyRepository.lastActiveSurveyId = SURVEY.id
-
     reactivateLastSurvey()
     advanceUntilIdle()
 
-    // Verify survey is made available for offline use.
     verify(activateSurvey).invoke(SURVEY.id)
   }
 
   @Test
-  fun reactivateLastSurvey_lastIdNotSet() = runWithTestDispatcher {
+  fun `Does nothing when a survey is already active`() = runWithTestDispatcher {
+    activateSurvey(SURVEY.id)
     reactivateLastSurvey()
     advanceUntilIdle()
 
-    // Verify survey is made available for offline use.
-    verify(activateSurvey, never()).invoke(SURVEY.id)
+    // Tries to activate survey.
+    verify(activateSurvey, times(1)).invoke(SURVEY.id)
   }
 
   @Test
-  fun reactivateLastSurvey_lastIdSet_activeSurvey() = runWithTestDispatcher {
-    surveyRepository.activeSurvey = SURVEY
-
+  fun `Does nothing when survey never activated`() = runWithTestDispatcher {
+    surveyRepository.lastActiveSurveyId = ""
     reactivateLastSurvey()
     advanceUntilIdle()
 
-    // Verify survey is made available for offline use.
+    // Should never try to activate survey.
     verify(activateSurvey, never()).invoke(SURVEY.id)
   }
 }