Skip to content

Commit

Permalink
Cleanup and refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
gino-m committed Oct 30, 2023
1 parent 9647e4f commit f4f2c92
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@ data class SurveyListItem(
val description: String,
val availableOffline: Boolean
)

fun Survey.toListItem(availableOffline: Boolean): SurveyListItem =
SurveyListItem(id, title, description, availableOffline)
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import com.google.android.ground.model.mutation.LocationOfInterestMutation
import com.google.android.ground.model.mutation.Mutation
import com.google.android.ground.model.mutation.SubmissionMutation
import com.google.android.ground.model.submission.Submission
import com.google.android.ground.model.toListItem
import com.google.android.ground.persistence.remote.RemoteDataStore
import com.google.firebase.firestore.WriteBatch
import com.google.firebase.functions.FirebaseFunctions
Expand Down Expand Up @@ -73,7 +74,7 @@ internal constructor(
emitAll(
db().surveys().getReadable(user).map { list ->
// TODO(#2031): Return SurveyListItem from getReadable(), only fetch required fields.
list.map { SurveyListItem(it.id, it.title, it.description, false) }
list.map { it.toListItem(false) }
}
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import com.google.android.ground.coroutines.ApplicationScope
import com.google.android.ground.model.Survey
import com.google.android.ground.model.SurveyListItem
import com.google.android.ground.model.User
import com.google.android.ground.model.toListItem
import com.google.android.ground.persistence.local.LocalValueStore
import com.google.android.ground.persistence.local.stores.LocalSurveyStore
import com.google.android.ground.persistence.remote.RemoteDataStore
Expand Down Expand Up @@ -86,8 +87,8 @@ constructor(
val activeSurveyFlowable: @Cold Flowable<Optional<Survey>> =
activeSurveyFlow.map { if (it == null) Optional.empty() else Optional.of(it) }.asFlowable()

val localSurveysFlow: Flow<List<Survey>>
get() = localSurveyStore.surveys
val localSurveyListFlow: Flow<List<SurveyListItem>>
get() = localSurveyStore.surveys.map { list -> list.map { it.toListItem(true) } }

var lastActiveSurveyId: String by localValueStore::lastActiveSurveyId
internal set
Expand Down Expand Up @@ -125,22 +126,18 @@ constructor(
if (networkStatus == NetworkStatus.AVAILABLE) {
getRemoteSurveyList(user)
} else {
getLocalSurveyList()
localSurveyListFlow
}
}

private fun getRemoteSurveyList(user: User): Flow<List<SurveyListItem>> =
remoteDataStore.getSurveyList(user).combine(localSurveysFlow) { remoteSurveys, localSurveys ->
remoteDataStore.getSurveyList(user).combine(localSurveyListFlow) { remoteSurveys, localSurveys
->
remoteSurveys.map { remoteSurvey ->
remoteSurvey.copy(availableOffline = localSurveys.any { it.id == remoteSurvey.id })
}
}

private fun getLocalSurveyList(): Flow<List<SurveyListItem>> =
localSurveysFlow.map { localSurveys ->
localSurveys.map { SurveyListItem(it.id, it.title, it.description, true) }
}

/** 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.getSurveyByIdSuspend(surveyId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class SurveyRepositoryTest : BaseHiltTest() {
advanceUntilIdle()

// Verify survey is deleted
surveyRepository.localSurveysFlow.test { assertThat(expectMostRecentItem()).isEmpty() }
surveyRepository.localSurveyListFlow.test { assertThat(expectMostRecentItem()).isEmpty() }
// Verify survey deactivated
assertThat(surveyRepository.activeSurvey).isNull()
}
Expand All @@ -103,7 +103,7 @@ class SurveyRepositoryTest : BaseHiltTest() {
// Verify active survey isn't cleared
assertThat(surveyRepository.activeSurvey).isEqualTo(survey1)
// Verify survey is deleted
surveyRepository.localSurveysFlow.test {
surveyRepository.localSurveyListFlow.test {
assertThat(expectMostRecentItem()).isEqualTo(listOf(survey1))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import androidx.test.espresso.matcher.RootMatchers.isPlatformPopup
import androidx.test.espresso.matcher.ViewMatchers.*
import com.google.android.ground.*
import com.google.android.ground.domain.usecases.survey.ActivateSurveyUseCase
import com.google.android.ground.model.Survey
import com.google.android.ground.model.SurveyListItem
import com.google.android.ground.repository.SurveyRepository
import com.google.android.ground.repository.UserRepository
import com.google.android.ground.ui.common.Navigator
Expand Down Expand Up @@ -78,7 +78,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
@Test
fun created_surveysAvailable_whenNoSurveySynced() {
setSurveyList(listOf(TEST_SURVEY_1, TEST_SURVEY_2))
setOfflineSurveys(listOf())
setLocalSurveys(listOf())
setUpFragment()

// Assert that 2 surveys are displayed
Expand All @@ -100,7 +100,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
@Test
fun created_surveysAvailable_whenOneSurveySynced() {
setSurveyList(listOf(TEST_SURVEY_1, TEST_SURVEY_2))
setOfflineSurveys(listOf(TEST_SURVEY_2))
setLocalSurveys(listOf(TEST_SURVEY_2))
setUpFragment()

// Assert that 2 surveys are displayed
Expand All @@ -122,7 +122,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
@Test
fun click_activatesSurvey() = runWithTestDispatcher {
setSurveyList(listOf(TEST_SURVEY_1, TEST_SURVEY_2))
setOfflineSurveys(listOf())
setLocalSurveys(listOf())
setUpFragment()

// Click second item
Expand All @@ -139,7 +139,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
@Test
fun shouldExitAppOnBackPress_defaultFalse() {
setSurveyList(listOf())
setOfflineSurveys(listOf())
setLocalSurveys(listOf())
setUpFragment()

assertThat(fragment.onBack()).isFalse()
Expand All @@ -149,7 +149,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
@Test
fun shouldExitAppOnBackPress_whenArgIsPresent() {
setSurveyList(listOf())
setOfflineSurveys(listOf())
setLocalSurveys(listOf())
setUpFragment(bundleOf(Pair("shouldExitApp", true)))

assertThat(fragment.onBack()).isTrue()
Expand All @@ -159,7 +159,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
@Test
fun `hide sign out button when survey list is not empty`() {
setSurveyList(listOf(TEST_SURVEY_1, TEST_SURVEY_2))
setOfflineSurveys(listOf())
setLocalSurveys(listOf())
setUpFragment()

onView(withText("Sign out")).check(matches(not(isDisplayed())))
Expand All @@ -168,7 +168,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
@Test
fun `show sign out button when survey list is empty`() {
setSurveyList(listOf())
setOfflineSurveys(listOf())
setLocalSurveys(listOf())
setUpFragment()

onView(withText("Sign out")).check(matches(isDisplayed())).perform(click())
Expand All @@ -178,7 +178,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
@Test
fun `remove offline survey on menu item click`() = runWithTestDispatcher {
setSurveyList(listOf(TEST_SURVEY_1, TEST_SURVEY_2))
setOfflineSurveys(listOf(TEST_SURVEY_1, TEST_SURVEY_2))
setLocalSurveys(listOf(TEST_SURVEY_1, TEST_SURVEY_2))
setUpFragment()

// Click second item's overflow menu
Expand Down Expand Up @@ -210,13 +210,12 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
}
}

private fun setSurveyList(surveys: List<Pair<Survey, Boolean>>) = runWithTestDispatcher {
whenever(surveyRepository.getSurveyList(FakeData.USER))
.thenReturn(listOf(surveys).asFlow())
private fun setSurveyList(surveys: List<SurveyListItem>) = runWithTestDispatcher {
whenever(surveyRepository.getSurveyList(FakeData.USER)).thenReturn(listOf(surveys).asFlow())
}

private fun setOfflineSurveys(surveys: List<Survey>) {
whenever(surveyRepository.localSurveysFlow).thenReturn(listOf(surveys).asFlow())
private fun setLocalSurveys(surveys: List<SurveyListItem>) {
whenever(surveyRepository.localSurveyListFlow).thenReturn(listOf(surveys).asFlow())
}

private fun getViewHolder(index: Int): SurveyListAdapter.ViewHolder {
Expand All @@ -228,8 +227,8 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
companion object {
private val TEST_USER = FakeData.USER
private val TEST_SURVEY_1 =
FakeData.SURVEY.copy(id = "1", title = "survey 1", description = "description 1")
SurveyListItem(id = "1", title = "survey 1", description = "description 1", false)
private val TEST_SURVEY_2 =
FakeData.SURVEY.copy(id = "2", title = "survey 2", description = "description 2")
SurveyListItem(id = "2", title = "survey 2", description = "description 2", false)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,18 @@
package com.sharedtest.persistence.remote

import com.google.android.ground.model.Survey
import com.google.android.ground.model.SurveyListItem
import com.google.android.ground.model.TermsOfService
import com.google.android.ground.model.User
import com.google.android.ground.model.locationofinterest.LocationOfInterest
import com.google.android.ground.model.mutation.Mutation
import com.google.android.ground.model.submission.Submission
import com.google.android.ground.model.toListItem
import com.google.android.ground.persistence.remote.RemoteDataStore
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flowOf
import javax.inject.Inject
import javax.inject.Singleton
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flowOf

@Singleton
class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore {
Expand All @@ -40,7 +42,8 @@ class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore {

private val subscribedSurveyIds = mutableSetOf<String>()

override suspend fun getSurveyList(user: User): Flow<List<Survey>> = flowOf(surveys)
override fun getSurveyList(user: User): Flow<List<SurveyListItem>> =
flowOf(surveys.map { it.toListItem(false) })

override suspend fun loadSurvey(surveyId: String): Survey? = onLoadSurvey.invoke(surveyId)

Expand Down

0 comments on commit f4f2c92

Please sign in to comment.