Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make activateSurvey call blocking #3021

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions app/src/main/java/com/google/android/ground/MainViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,18 @@ constructor(
private fun onUserSignedOut(): MainUiState {
// Scope of subscription is until view model is cleared. Dispose it manually otherwise, firebase
// attempts to maintain a connection even after user has logged out and throws an error.
surveyRepository.clearActiveSurvey()
userRepository.clearUserPreferences()

// TODO: Once multi-user login is supported, avoid clearing local db data. This is
// currently being done to prevent one user's data to be submitted as another user after
// re-login.
// Issue URL: https://github.com/google/ground-android/issues/1691
viewModelScope.launch { withContext(ioDispatcher) { localDatabase.clearAllTables() } }
viewModelScope.launch {
withContext(ioDispatcher) {
surveyRepository.clearActiveSurvey()
localDatabase.clearAllTables()
}
}
return MainUiState.OnUserSignedOut
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,22 @@ constructor(
private val makeSurveyAvailableOffline: MakeSurveyAvailableOfflineUseCase,
private val surveyRepository: SurveyRepository,
) {
suspend operator fun invoke(surveyId: String) {

/**
* @return `true` if the survey was successfully activated or was already active, otherwise false.
*/
suspend operator fun invoke(surveyId: String): Boolean {
if (surveyRepository.isSurveyActive(surveyId)) {
// Do nothing if survey is already active.
return
return true
}

localSurveyStore.getSurveyById(surveyId)
?: makeSurveyAvailableOffline(surveyId)
?: error("Survey $surveyId not found in remote db")

surveyRepository.activateSurvey(surveyId)

return surveyRepository.isSurveyActive(surveyId)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ constructor(
true
} else {
activateSurvey(getLastActiveSurveyId())
surveyRepository.hasActiveSurvey()
}

private fun getLastActiveSurveyId(): String = localValueStore.lastActiveSurveyId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,16 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.withTimeout

private const val ACTIVATE_SURVEY_TIMEOUT_MILLS: Long = 3 * 1000

/**
* Coordinates persistence and retrieval of [Survey] instances from remote, local, and in memory
Expand Down Expand Up @@ -77,26 +81,40 @@ constructor(
private fun getOfflineSurveyFlow(id: String?): Flow<Survey?> =
if (id.isNullOrBlank()) flowOf(null) else localSurveyStore.survey(id)

fun activateSurvey(surveyId: String) {
/**
* Activates the survey with the specified id. Waits for [ACTIVATE_SURVEY_TIMEOUT_MILLS] before
* throwing an error if the survey couldn't be activated.
*/
suspend fun activateSurvey(surveyId: String) {
_selectedSurveyId.update { surveyId }

// Wait for survey to be updated. Else throw an error after timeout.
withTimeout(ACTIVATE_SURVEY_TIMEOUT_MILLS) {
activeSurveyFlow.first { survey ->
if (surveyId.isBlank()) {
survey == null
} else {
survey?.id == surveyId
}
}
}

firebaseCrashLogger.setSelectedSurveyId(surveyId)
}

fun clearActiveSurvey() {
suspend fun clearActiveSurvey() {
activateSurvey("")
}

// TODO: Use activeSurvey instead of selectedSurveyId as it is possible to have no active survey.
// Issue URL: https://github.com/google/ground-android/issues/3020
fun hasActiveSurvey(): Boolean = _selectedSurveyId.value?.isNotBlank() ?: false
fun hasActiveSurvey(): Boolean = activeSurvey != null

fun isSurveyActive(surveyId: String): Boolean =
surveyId.isNotBlank() && activeSurvey?.id == surveyId

/** Attempts to remove the locally synced survey. Doesn't throw an error if it doesn't exist. */
suspend fun removeOfflineSurvey(surveyId: String) {
val survey = localSurveyStore.getSurveyById(surveyId)
survey?.let { localSurveyStore.deleteSurvey(survey) }
val survey = localSurveyStore.getSurveyById(surveyId) ?: return
localSurveyStore.deleteSurvey(survey)
if (isSurveyActive(surveyId)) {
clearActiveSurvey()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,30 @@
activateSurveyUseCase(surveyId)
}
.fold(
onSuccess = {
surveyActivationInProgress = false
_uiState.emit(UiState.SurveyActivated)
_uiState.emit(UiState.NavigateToHome)
},
onFailure = { e ->
Timber.e(e, "Failed to activate survey")
surveyActivationInProgress = false
_uiState.emit(UiState.Error)
onSuccess = { result ->
if (result) {
onSurveyActivated()
} else {
onSurveyActivationFailed()

Check warning on line 90 in app/src/main/java/com/google/android/ground/ui/surveyselector/SurveySelectorViewModel.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/com/google/android/ground/ui/surveyselector/SurveySelectorViewModel.kt#L90

Added line #L90 was not covered by tests
}
},
onFailure = { onSurveyActivationFailed(it) },
)
}
}

private suspend fun onSurveyActivated() {
surveyActivationInProgress = false
_uiState.emit(UiState.SurveyActivated)
_uiState.emit(UiState.NavigateToHome)
}

private suspend fun onSurveyActivationFailed(error: Throwable? = null) {

Check warning on line 104 in app/src/main/java/com/google/android/ground/ui/surveyselector/SurveySelectorViewModel.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/com/google/android/ground/ui/surveyselector/SurveySelectorViewModel.kt#L104

Added line #L104 was not covered by tests
Timber.e(error, "Failed to activate survey")
surveyActivationInProgress = false
_uiState.emit(UiState.Error)
}

fun deleteSurvey(surveyId: String) {
externalScope.launch(ioDispatcher) { surveyRepository.removeOfflineSurvey(surveyId) }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ class UserRepositoryTest : BaseHiltTest() {
val user = FakeData.USER
val survey = FakeData.SURVEY.copy(acl = mapOf(Pair(user.email, Role.OWNER.toString())))
fakeAuthenticationManager.setUser(user)
localSurveyStore.insertOrUpdateSurvey(survey)
surveyRepository.activateSurvey(survey.id)

assertThat(userRepository.canUserSubmitData()).isTrue()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,20 +142,8 @@ class HomeScreenFragmentTest : AbstractHomeScreenFragmentTest() {
*/
@get:Rule override val composeTestRule = createAndroidComposeRule<ComponentActivity>()

private val surveyWithoutBasemap: Survey =
Survey(
"SURVEY",
"Survey title",
"Test survey description",
mapOf(FakeData.JOB.id to FakeData.JOB),
mapOf(Pair(FakeData.USER.email, "data-collector")),
)

@Test
fun `all menu item is always enabled`() = runWithTestDispatcher {
surveyRepository.activateSurvey(surveyWithoutBasemap.id)
advanceUntilIdle()

openDrawer()
onView(withId(R.id.nav_offline_areas)).check(matches(isEnabled()))
onView(withId(R.id.sync_status)).check(matches(isEnabled()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
@Test
fun click_activatesSurvey() = runWithTestDispatcher {
setSurveyList(listOf(TEST_SURVEY_1, TEST_SURVEY_2))
whenever(activateSurvey(TEST_SURVEY_2.id)).thenReturn(true)

launchFragmentWithNavController<SurveySelectorFragment>(
fragmentArgs = bundleOf(Pair("shouldExitApp", false)),
Expand All @@ -156,8 +157,6 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
.perform(actionOnItemAtPosition<SurveyListAdapter.ViewHolder>(1, click()))
advanceUntilIdle()

// Assert survey is activated.
verify(activateSurvey).invoke(TEST_SURVEY_2.id)
// Assert that navigation to home screen was requested
assertThat(navController.currentDestination?.id).isEqualTo(R.id.home_screen_fragment)
// No error toast should be displayed
Expand Down
Loading