Merge remote-tracking branch 'origin/gino-m/1971' into gino-m/1971

ground/src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt

@@ -75,7 +75,10 @@ internal constructor(
     Firebase.messaging.subscribeToTopic(surveyId).await()
   }
 
-  /** Calls Cloud Function to refresh the current user's profile info in the remote database. */
+  /**
+   * Calls Cloud Function to refresh the current user's profile info in the remote database if
+   * network is available.
+   */
   override suspend fun refreshUserProfile() {
     firebaseFunctions.getHttpsCallable(PROFILE_REFRESH_CLOUD_FUNCTION_NAME).call().await()
   }

ground/src/main/java/com/google/android/ground/persistence/remote/firebase/base/FluentCollectionReference.kt

@@ -16,7 +16,7 @@
 
 package com.google.android.ground.persistence.remote.firebase.base
 
-import com.google.android.ground.system.NetworkManager.requireActiveNetwork
+import com.google.android.ground.system.NetworkManager
 import com.google.firebase.firestore.CollectionReference
 import com.google.firebase.firestore.DocumentSnapshot
 import com.google.firebase.firestore.Query
@@ -44,7 +44,7 @@ protected constructor(
     query: Query,
     mappingFunction: Function<DocumentSnapshot, T>
   ): List<T> {
-    requireActiveNetwork(context)
+    NetworkManager(context).requireNetworkConnection()
     val querySnapshot = query.get().await()
     return querySnapshot.documents
       .filter { it.exists() }

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

@@ -19,6 +19,7 @@ package com.google.android.ground.repository
 import com.google.android.ground.model.TermsOfService
 import com.google.android.ground.persistence.local.LocalValueStore
 import com.google.android.ground.persistence.remote.RemoteDataStore
+import com.google.android.ground.system.NetworkManager
 import javax.inject.Inject
 import javax.inject.Singleton
 import kotlinx.coroutines.withTimeoutOrNull
@@ -30,16 +31,26 @@ private const val LOAD_REMOTE_SURVEY_TERMS_OF_SERVICE_TIMEOUT_MILLIS: Long = 30
 class TermsOfServiceRepository
 @Inject
 constructor(
+  private val networkManager: NetworkManager,
   private val remoteDataStore: RemoteDataStore,
   private val localValueStore: LocalValueStore
 ) {
 
   var isTermsOfServiceAccepted: Boolean by localValueStore::isTermsOfServiceAccepted
 
-  suspend fun getTermsOfService(): TermsOfService? =
+  /**
+   * @return [TermsOfService] from remote data store. Otherwise null if the request times out or
+   * network is unavailable.
+   */
+  suspend fun getTermsOfService(): TermsOfService? {
     // TODO(#1691): Maybe parse the exception and display to the user.
-    withTimeoutOrNull(LOAD_REMOTE_SURVEY_TERMS_OF_SERVICE_TIMEOUT_MILLIS) {
+    if (!networkManager.isNetworkConnected()) {
+      return null
+    }
+
+    return withTimeoutOrNull(LOAD_REMOTE_SURVEY_TERMS_OF_SERVICE_TIMEOUT_MILLIS) {
       Timber.d("Loading Terms of Service")
       remoteDataStore.loadTermsOfService()
     }
+  }
 }

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

@@ -20,9 +20,11 @@ import com.google.android.ground.model.User
 import com.google.android.ground.persistence.local.LocalValueStore
 import com.google.android.ground.persistence.local.stores.LocalUserStore
 import com.google.android.ground.persistence.remote.RemoteDataStore
+import com.google.android.ground.system.NetworkManager
 import com.google.android.ground.system.auth.AuthenticationManager
 import javax.inject.Inject
 import javax.inject.Singleton
+import timber.log.Timber
 
 /**
  * Coordinates persistence of [User] instance in local data store. For more details on this pattern
@@ -35,13 +37,23 @@ constructor(
   private val authenticationManager: AuthenticationManager,
   private val localValueStore: LocalValueStore,
   private val localUserStore: LocalUserStore,
+  private val networkManager: NetworkManager,
   private val surveyRepository: SurveyRepository,
   private val remoteDataStore: RemoteDataStore,
 ) : AuthenticationManager by authenticationManager {
 
   /** Stores the current user's profile details into the local and remote dbs. */
   suspend fun saveUserDetails() {
     localUserStore.insertOrUpdateUser(authenticationManager.currentUser)
+    updateRemoteUserInfo()
+  }
+
+  /** Attempts to refresh current user's profile in remote database if network is available. */
+  private suspend fun updateRemoteUserInfo() {
+    if (!networkManager.isNetworkConnected()) {
+      Timber.d("Skipped refreshing user profile as device is offline.")
+      return
+    }
     remoteDataStore.refreshUserProfile()
   }
 

ground/src/main/java/com/google/android/ground/system/NetworkManager.kt

@@ -18,22 +18,26 @@ package com.google.android.ground.system
 import android.content.Context
 import android.net.ConnectivityManager
 import androidx.annotation.RequiresPermission
+import dagger.hilt.android.qualifiers.ApplicationContext
 import java.net.ConnectException
+import javax.inject.Inject
+import javax.inject.Singleton
 
 /** Abstracts access to network state. */
-object NetworkManager {
+@Singleton
+class NetworkManager @Inject constructor(@ApplicationContext private val context: Context) {
 
   /** Returns true iff the device has internet connectivity, false otherwise. */
   @RequiresPermission("android.permission.ACCESS_NETWORK_STATE")
-  private fun isNetworkAvailable(context: Context): Boolean {
+  fun isNetworkConnected(): Boolean {
     val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
     val networkInfo = cm.activeNetworkInfo
     return networkInfo?.isConnected ?: false
   }
 
   /** Throws an error if network isn't available. */
-  fun requireActiveNetwork(context: Context) {
-    if (!isNetworkAvailable(context)) {
+  fun requireNetworkConnection() {
+    if (!isNetworkConnected()) {
       throw ConnectException()
     }
   }

ground/src/main/java/com/google/android/ground/system/auth/SignInState.kt

@@ -28,12 +28,12 @@ data class SignInState(val state: State, val result: Result<User?>) {
 
   companion object {
 
-    @JvmStatic fun signedOut() = SignInState(State.SIGNED_OUT, Result.success(null))
+    fun signedOut() = SignInState(State.SIGNED_OUT, Result.success(null))
 
-    @JvmStatic fun signingIn() = SignInState(State.SIGNING_IN, Result.success(null))
+    fun signingIn() = SignInState(State.SIGNING_IN, Result.success(null))
 
-    @JvmStatic fun signedIn(user: User) = SignInState(State.SIGNED_IN, Result.success(user))
+    fun signedIn(user: User) = SignInState(State.SIGNED_IN, Result.success(user))
 
-    @JvmStatic fun error(error: Throwable) = SignInState(State.ERROR, Result.failure(error))
+    fun error(error: Throwable) = SignInState(State.ERROR, Result.failure(error))
   }
 }

ground/src/main/java/com/google/android/ground/ui/map/gms/GoogleMapsFragment.kt

@@ -64,6 +64,7 @@ import timber.log.Timber
 
 const val TILE_OVERLAY_Z = 0f
 const val POLYGON_Z = 1f
+const val POLYLINE_Z = 1f
 const val CLUSTER_Z = 2f
 const val MARKER_Z = 3f
 

ground/src/main/java/com/google/android/ground/ui/map/gms/renderer/PolylineFeatureManager.kt

@@ -26,6 +26,7 @@ import com.google.android.ground.model.geometry.Coordinates
 import com.google.android.ground.model.geometry.LineString
 import com.google.android.ground.model.geometry.LinearRing
 import com.google.android.ground.ui.map.Feature
+import com.google.android.ground.ui.map.gms.POLYLINE_Z
 import com.google.android.ground.ui.map.gms.toLatLngList
 import com.google.android.ground.ui.util.BitmapUtil
 import dagger.hilt.android.qualifiers.ApplicationContext
@@ -75,6 +76,7 @@ constructor(@ApplicationContext context: Context, bitmapUtil: BitmapUtil) : Feat
       width = lineWidth * strokeScale
       color = style.color
       jointType = JointType.ROUND
+      zIndex = POLYLINE_Z
     }
 
     polylines.getOrPut(feature) { mutableListOf() }.add(polyline)

ground/src/test/java/com/google/android/ground/repository/TermsOfServiceRepositoryTest.kt

@@ -16,42 +16,77 @@
 package com.google.android.ground.repository
 
 import com.google.android.ground.BaseHiltTest
+import com.google.android.ground.system.NetworkManager
 import com.google.common.truth.Truth.assertThat
 import com.google.firebase.firestore.FirebaseFirestoreException
 import com.sharedtest.FakeData
 import com.sharedtest.persistence.remote.FakeRemoteDataStore
+import dagger.hilt.android.testing.BindValue
 import dagger.hilt.android.testing.HiltAndroidTest
 import javax.inject.Inject
 import kotlinx.coroutines.runBlocking
 import org.junit.Assert.assertThrows
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Mock
+import org.mockito.kotlin.whenever
 import org.robolectric.RobolectricTestRunner
 
 @HiltAndroidTest
 @RunWith(RobolectricTestRunner::class)
 class TermsOfServiceRepositoryTest : BaseHiltTest() {
   @Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore
-
   @Inject lateinit var termsOfServiceRepository: TermsOfServiceRepository
 
+  @BindValue @Mock lateinit var mockNetworkManager: NetworkManager
+
   @Test
   fun testGetTermsOfService() = runBlocking {
+    whenever(mockNetworkManager.isNetworkConnected()).thenReturn(true)
     fakeRemoteDataStore.termsOfService = Result.success(FakeData.TERMS_OF_SERVICE)
+
     assertThat(termsOfServiceRepository.getTermsOfService()).isEqualTo(FakeData.TERMS_OF_SERVICE)
   }
 
   @Test
   fun testGetTermsOfService_whenMissing_returnsNull() = runBlocking {
+    whenever(mockNetworkManager.isNetworkConnected()).thenReturn(true)
+
     assertThat(termsOfServiceRepository.getTermsOfService()).isNull()
   }
 
   @Test
   fun testGetTermsOfService_whenRequestFails_throwsError() {
+    whenever(mockNetworkManager.isNetworkConnected()).thenReturn(true)
+    fakeRemoteDataStore.termsOfService =
+      Result.failure(
+        FirebaseFirestoreException("user error", FirebaseFirestoreException.Code.ABORTED)
+      )
+
+    assertThrows(FirebaseFirestoreException::class.java) {
+      runBlocking { termsOfServiceRepository.getTermsOfService() }
+    }
+  }
+
+  @Test
+  fun testGetTermsOfService_whenOffline_returnsNull() = runBlocking {
+    whenever(mockNetworkManager.isNetworkConnected()).thenReturn(false)
     fakeRemoteDataStore.termsOfService =
       Result.failure(
         FirebaseFirestoreException("user error", FirebaseFirestoreException.Code.ABORTED)
       )
+
+    assertThat(termsOfServiceRepository.getTermsOfService()).isNull()
+  }
+
+  @Test
+  fun testGetTermsOfService_whenServiceUnavailable_throwsError() {
+    whenever(mockNetworkManager.isNetworkConnected()).thenReturn(true)
+    fakeRemoteDataStore.termsOfService =
+      Result.failure(
+        FirebaseFirestoreException("device offline", FirebaseFirestoreException.Code.UNAVAILABLE)
+      )
+
     assertThrows(FirebaseFirestoreException::class.java) {
       runBlocking { termsOfServiceRepository.getTermsOfService() }
     }

ground/src/test/java/com/google/android/ground/repository/UserRepositoryTest.kt

@@ -18,15 +18,19 @@ package com.google.android.ground.repository
 import com.google.android.ground.BaseHiltTest
 import com.google.android.ground.persistence.local.LocalValueStore
 import com.google.android.ground.persistence.local.stores.LocalUserStore
+import com.google.android.ground.system.NetworkManager
 import com.google.common.truth.Truth.assertThat
 import com.sharedtest.FakeData
 import com.sharedtest.persistence.remote.FakeRemoteDataStore
 import com.sharedtest.system.auth.FakeAuthenticationManager
+import dagger.hilt.android.testing.BindValue
 import dagger.hilt.android.testing.HiltAndroidTest
 import javax.inject.Inject
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Mock
+import org.mockito.kotlin.whenever
 import org.robolectric.RobolectricTestRunner
 
 @OptIn(ExperimentalCoroutinesApi::class)
@@ -39,6 +43,8 @@ class UserRepositoryTest : BaseHiltTest() {
   @Inject lateinit var userRepository: UserRepository
   @Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore
 
+  @BindValue @Mock lateinit var networkManager: NetworkManager
+
   @Test
   fun `currentUser returns current user`() {
     fakeAuthenticationManager.setUser(FakeData.USER)
@@ -49,6 +55,7 @@ class UserRepositoryTest : BaseHiltTest() {
   @Test
   fun `saveUserDetails() updates local user profile`() = runWithTestDispatcher {
     fakeAuthenticationManager.setUser(FakeData.USER)
+    whenever(networkManager.isNetworkConnected()).thenReturn(true)
 
     userRepository.saveUserDetails()
 
@@ -58,12 +65,24 @@ class UserRepositoryTest : BaseHiltTest() {
   @Test
   fun `saveUserDetails() updates remote user profile`() = runWithTestDispatcher {
     fakeAuthenticationManager.setUser(FakeData.USER)
+    whenever(networkManager.isNetworkConnected()).thenReturn(true)
 
     userRepository.saveUserDetails()
 
     assertThat(fakeRemoteDataStore.userProfileRefreshCount).isEqualTo(1)
   }
 
+  @Test
+  fun `saveUserDetails() doesn't update remote user profile when offline `() =
+    runWithTestDispatcher {
+      fakeAuthenticationManager.setUser(FakeData.USER)
+      whenever(networkManager.isNetworkConnected()).thenReturn(false)
+
+      userRepository.saveUserDetails()
+
+      assertThat(fakeRemoteDataStore.userProfileRefreshCount).isEqualTo(0)
+    }
+
   @Test
   fun `clearUserPreferences() clears lastActiveSurveyId`() {
     localValueStore.lastActiveSurveyId = "foo"