Skip to content

Commit

Permalink
fix(StopDetailsPage): Distinct errorKeys from API requests on NearbyT…
Browse files Browse the repository at this point in the history
…ransitPage (#641)

* fix(android.StopDetailsPage): Separate errorKeys from NearbyTransitView fetchces

* fix(subscribeToPredictions): periodically check predictions stale

* cleanup: merge conflict linting

* fix(MockErrorBannerStateRepository): Default argument interop

* fix(MockGlobalRepository): Default argument interop enabled
  • Loading branch information
KaylaBrady authored Jan 14, 2025
1 parent 716b7d5 commit 0c9883c
Show file tree
Hide file tree
Showing 14 changed files with 160 additions and 40 deletions.
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package com.mbta.tid.mbta_app.android.state

import androidx.compose.ui.test.junit4.createComposeRule
import com.mbta.tid.mbta_app.model.ErrorBannerState
import com.mbta.tid.mbta_app.model.ObjectCollectionBuilder
import com.mbta.tid.mbta_app.model.response.ApiResult
import com.mbta.tid.mbta_app.model.response.GlobalResponse
import com.mbta.tid.mbta_app.repositories.IGlobalRepository
import com.mbta.tid.mbta_app.repositories.MockErrorBannerStateRepository
import com.mbta.tid.mbta_app.repositories.MockGlobalRepository
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
Expand Down Expand Up @@ -39,7 +42,7 @@ class GetGlobalDataTest {
}

var actualData: GlobalResponse? = globalData
composeTestRule.setContent { actualData = getGlobalData(globalRepo) }
composeTestRule.setContent { actualData = getGlobalData("errorKey", globalRepo) }

composeTestRule.awaitIdle()
assertNull(actualData)
Expand All @@ -49,4 +52,20 @@ class GetGlobalDataTest {
composeTestRule.waitUntil { globalData == actualData }
assertEquals(globalData, actualData)
}

@Test
fun testApiError() = runTest {
val globalRepo = MockGlobalRepository(ApiResult.Error(500, "oops"))

val errorRepo = MockErrorBannerStateRepository()

composeTestRule.setContent { getGlobalData("errorKey", globalRepo, errorRepo) }

composeTestRule.waitUntil {
when (val errorState = errorRepo.state.value) {
is ErrorBannerState.DataError -> errorState.messages == setOf("errorKey")
else -> false
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.ui.test.junit4.createComposeRule
import com.mbta.tid.mbta_app.model.ErrorBannerState
import com.mbta.tid.mbta_app.model.ObjectCollectionBuilder
import com.mbta.tid.mbta_app.model.response.ApiResult
import com.mbta.tid.mbta_app.model.response.ScheduleResponse
import com.mbta.tid.mbta_app.repositories.ISchedulesRepository
import com.mbta.tid.mbta_app.repositories.MockErrorBannerStateRepository
import com.mbta.tid.mbta_app.repositories.MockScheduleRepository
import kotlin.test.assertNotNull
import kotlinx.coroutines.test.runTest
Expand Down Expand Up @@ -53,7 +55,7 @@ class GetScheduleTest {
var stopIds by mutableStateOf(stops1)
var actualSchedules: ScheduleResponse? = expectedSchedules1
composeTestRule.setContent {
actualSchedules = getSchedule(stopIds = stopIds, schedulesRepo)
actualSchedules = getSchedule(stopIds = stopIds, "errorKey", schedulesRepo)
}

composeTestRule.awaitIdle()
Expand All @@ -73,10 +75,28 @@ class GetScheduleTest {
var actualSchedules: ScheduleResponse? = null

composeTestRule.setContent {
actualSchedules = getSchedule(stopIds = emptyList(), schedulesRepo)
actualSchedules = getSchedule(stopIds = emptyList(), "errorKey", schedulesRepo)
}

composeTestRule.waitUntil { actualSchedules != null }
assertNotNull(actualSchedules)
}

@Test
fun testErrorCase() {
val schedulesRepo = MockScheduleRepository(ApiResult.Error(500, "oops"))

val errorRepo = MockErrorBannerStateRepository()

composeTestRule.setContent {
getSchedule(stopIds = listOf("stop1"), "errorKey", schedulesRepo, errorRepo)
}

composeTestRule.waitUntil {
when (val errorState = errorRepo.state.value) {
is ErrorBannerState.DataError -> errorState.messages == setOf("errorKey")
else -> false
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import com.mbta.tid.mbta_app.repositories.MockPredictionsRepository
import com.mbta.tid.mbta_app.repositories.MockSettingsRepository
import kotlin.test.assertNotNull
import kotlin.test.assertTrue
import kotlin.time.Duration.Companion.seconds
import kotlinx.coroutines.test.runTest
import kotlinx.datetime.Clock
import org.junit.Assert.assertEquals
import org.junit.Rule
import org.junit.Test
Expand Down Expand Up @@ -165,4 +167,38 @@ class SubscribeToPredictionsTest {
assertNotNull(predictions)
assertTrue(connected)
}

@Test
fun testCheckPredictionsStaleCalled() = runTest {
val objects = ObjectCollectionBuilder()
objects.prediction()
val predictionsOnJoin = PredictionsByStopJoinResponse(objects)
val predictionsRepo = MockPredictionsRepository({}, {}, {}, null, predictionsOnJoin)

predictionsRepo.lastUpdated = Clock.System.now()

var stopIds = mutableStateOf(listOf("place-a"))

var checkPredictionsStaleCount = 0
val mockErrorRepo =
MockErrorBannerStateRepository(
onCheckPredictionsStale = { checkPredictionsStaleCount += 1 }
)

composeTestRule.setContent {
var stopIds by remember { stopIds }
subscribeToPredictions(
stopIds,
predictionsRepo,
ErrorBannerViewModel(
false,
mockErrorRepo,
MockSettingsRepository(),
),
1.seconds
)
}

composeTestRule.waitUntil(timeoutMillis = 3000) { checkPredictionsStaleCount >= 2 }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fun ContentView(
) {
val navController = rememberNavController()
val alertData: AlertsStreamDataResponse? = subscribeToAlerts()
val globalResponse = getGlobalData()
val globalResponse = getGlobalData("ContentView.getGlobalData")
val hideMaps by viewModel.hideMaps.collectAsState()
val pendingOnboarding = viewModel.pendingOnboarding.collectAsState().value
val locationDataManager = rememberLocationDataManager()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ fun NearbyTransitView(
}
val now = timer(updateInterval = 5.seconds)
val stopIds = remember(nearbyVM.nearby) { nearbyVM.nearby?.stopIds()?.toList() }
val schedules = getSchedule(stopIds)
val schedules = getSchedule(stopIds, "NearbyTransitView.getSchedule")
val predictionsVM = subscribeToPredictions(stopIds, errorBannerViewModel = errorBannerViewModel)
val predictions by predictionsVM.predictionsFlow.collectAsState(initial = null)

LaunchedEffect(targetLocation == null) {
if (targetLocation == null) {
predictionsVM.reset()
}
}

val (pinnedRoutes, togglePinnedRoute) = managePinnedRoutes()

val nearbyWithRealtimeInfo =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fun StopDetailsPage(
updateDepartures: (StopDetailsDepartures?) -> Unit,
errorBannerViewModel: ErrorBannerViewModel
) {
val globalResponse = getGlobalData()
val globalResponse = getGlobalData("StopDetailsPage.getGlobalData")

val predictionsVM =
subscribeToPredictions(
Expand All @@ -47,7 +47,7 @@ fun StopDetailsPage(

val now = timer(updateInterval = 5.seconds)

val schedulesResponse = getSchedule(stopIds = listOf(stop.id))
val schedulesResponse = getSchedule(stopIds = listOf(stop.id), "StopDetailsPage.getSchedule")

val (pinnedRoutes, togglePinnedRoute) = managePinnedRoutes()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ fun SearchBarOverlay(
}
var expanded by rememberSaveable { mutableStateOf(false) }
var searchInputState by rememberSaveable { mutableStateOf("") }
val globalResponse = getGlobalData()
val globalResponse = getGlobalData("SearchBar.getGlobalData")
val searchResultsVm = getSearchResultsVm(globalResponse = globalResponse)
val searchResults = searchResultsVm.searchResults.collectAsState(initial = null).value

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ class GlobalDataViewModel(
private val _globalResponse = MutableStateFlow<GlobalResponse?>(null)
var globalResponse: StateFlow<GlobalResponse?> = _globalResponse

fun getGlobalData() {
fun getGlobalData(errorKey: String) {
CoroutineScope(Dispatchers.IO).launch {
fetchApi(
errorBannerRepo = errorBannerRepository,
errorKey = "GlobalDataViewModel.getGlobalData",
errorKey = errorKey,
getData = { globalRepository.getGlobalData() },
onSuccess = { _globalResponse.emit(it) },
onRefreshAfterError = { getGlobalData() }
onRefreshAfterError = { getGlobalData(errorKey) }
)
}
}
Expand All @@ -48,12 +48,13 @@ class GlobalDataViewModel(

@Composable
fun getGlobalData(
errorKey: String,
globalRepository: IGlobalRepository = koinInject(),
errorBannerRepository: IErrorBannerStateRepository = koinInject()
): GlobalResponse? {
val viewModel: GlobalDataViewModel =
viewModel(factory = GlobalDataViewModel.Factory(globalRepository, errorBannerRepository))

LaunchedEffect(key1 = null) { viewModel.getGlobalData() }
LaunchedEffect(key1 = null) { viewModel.getGlobalData(errorKey) }
return viewModel.globalResponse.collectAsState(initial = null).value
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ class ScheduleViewModel(
private val _schedule = MutableStateFlow<ScheduleResponse?>(null)
val schedule: StateFlow<ScheduleResponse?> = _schedule

fun getSchedule(stopIds: List<String>) {
fun getSchedule(stopIds: List<String>, errorKey: String) {
if (stopIds.isNotEmpty()) {
CoroutineScope(Dispatchers.IO).launch {
fetchApi(
errorBannerRepo = errorBannerRepository,
errorKey = "ScheduleViewModel.getSchedule",
errorKey = errorKey,
getData = { schedulesRepository.getSchedule(stopIds, Clock.System.now()) },
onSuccess = { _schedule.value = it },
onRefreshAfterError = { getSchedule(stopIds) }
onRefreshAfterError = { getSchedule(stopIds, errorKey) }
)
}
} else {
Expand All @@ -54,13 +54,14 @@ class ScheduleViewModel(
@Composable
fun getSchedule(
stopIds: List<String>?,
errorKey: String,
schedulesRepository: ISchedulesRepository = koinInject(),
errorBannerRepository: IErrorBannerStateRepository = koinInject(),
): ScheduleResponse? {
var viewModel: ScheduleViewModel =
viewModel(factory = ScheduleViewModel.Factory(schedulesRepository, errorBannerRepository))

LaunchedEffect(key1 = stopIds) { viewModel.getSchedule(stopIds ?: emptyList()) }
LaunchedEffect(key1 = stopIds) { viewModel.getSchedule(stopIds ?: emptyList(), errorKey) }

return viewModel?.schedule?.collectAsState(initial = null)?.value
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@ package com.mbta.tid.mbta_app.android.state

import android.util.Log
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.lifecycle.ViewModel
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.compose.LifecycleResumeEffect
import androidx.lifecycle.viewmodel.compose.viewModel
import com.mbta.tid.mbta_app.android.component.ErrorBannerViewModel
import com.mbta.tid.mbta_app.android.util.timer
import com.mbta.tid.mbta_app.model.response.ApiResult
import com.mbta.tid.mbta_app.model.response.PredictionsByStopJoinResponse
import com.mbta.tid.mbta_app.model.response.PredictionsByStopMessageResponse
import com.mbta.tid.mbta_app.repositories.IPredictionsRepository
import kotlin.time.Duration
import kotlin.time.Duration.Companion.seconds
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
Expand Down Expand Up @@ -94,15 +97,17 @@ class PredictionsViewModel(
}

fun checkPredictionsStale() {
predictionsRepository.lastUpdated?.let { lastPredictions ->
errorBannerViewModel.errorRepository.checkPredictionsStale(
predictionsLastUpdated = lastPredictions,
predictionQuantity = predictions.value?.predictionQuantity() ?: 0,
action = {
disconnect()
connect(currentStopIds)
}
)
CoroutineScope(Dispatchers.IO).launch {
predictionsRepository.lastUpdated?.let { lastPredictions ->
errorBannerViewModel.errorRepository.checkPredictionsStale(
predictionsLastUpdated = lastPredictions,
predictionQuantity = predictions.value?.predictionQuantity() ?: 0,
action = {
disconnect()
connect(currentStopIds)
}
)
}
}
}

Expand All @@ -120,13 +125,16 @@ class PredictionsViewModel(
fun subscribeToPredictions(
stopIds: List<String>?,
predictionsRepository: IPredictionsRepository = koinInject(),
errorBannerViewModel: ErrorBannerViewModel
errorBannerViewModel: ErrorBannerViewModel,
checkPredictionsStaleInterval: Duration = 5.seconds
): PredictionsViewModel {
val viewModel: PredictionsViewModel =
viewModel(
factory = PredictionsViewModel.Factory(predictionsRepository, errorBannerViewModel)
)

val timer = timer(checkPredictionsStaleInterval)

LifecycleResumeEffect(key1 = stopIds) {
CoroutineScope(Dispatchers.IO).launch {
viewModel.checkPredictionsStale()
Expand All @@ -135,5 +143,8 @@ fun subscribeToPredictions(

onPauseOrDispose { viewModel.disconnect() }
}

LaunchedEffect(key1 = timer) { viewModel.checkPredictionsStale() }

return viewModel
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ fun StopDetailsView(
updateStopFilter: (StopDetailsFilter?) -> Unit,
errorBannerViewModel: ErrorBannerViewModel
) {
val globalResponse = getGlobalData()
val globalResponse = getGlobalData("SopDetailsView.getGlobalData")

val now = timer(updateInterval = 5.seconds)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.mbta.tid.mbta_app.repositories

import co.touchlab.skie.configuration.annotations.DefaultArgumentInterop
import com.mbta.tid.mbta_app.model.ErrorBannerState
import com.mbta.tid.mbta_app.network.INetworkConnectivityMonitor
import kotlin.time.Duration.Companion.minutes
Expand Down Expand Up @@ -52,7 +53,7 @@ abstract class IErrorBannerStateRepository(initialState: ErrorBannerState? = nul
}
}

fun checkPredictionsStale(
open fun checkPredictionsStale(
predictionsLastUpdated: Instant,
predictionQuantity: Int,
action: () -> Unit
Expand Down Expand Up @@ -90,15 +91,28 @@ abstract class IErrorBannerStateRepository(initialState: ErrorBannerState? = nul

class ErrorBannerStateRepository : IErrorBannerStateRepository(), KoinComponent

class MockErrorBannerStateRepository(
class MockErrorBannerStateRepository
@DefaultArgumentInterop.Enabled
constructor(
state: ErrorBannerState? = null,
onSubscribeToNetworkChanges: (() -> Unit)? = null,
onCheckPredictionsStale: (() -> Unit)? = null
) : IErrorBannerStateRepository(state) {
private val onSubscribeToNetworkChanges = onSubscribeToNetworkChanges
private val onCheckPredictionsStale = onCheckPredictionsStale

val mutableFlow
get() = flow

override fun subscribeToNetworkStatusChanges() {
onSubscribeToNetworkChanges?.invoke()
}

override fun checkPredictionsStale(
predictionsLastUpdated: Instant,
predictionQuantity: Int,
action: () -> Unit
) {
onCheckPredictionsStale?.invoke()
}
}
Loading

0 comments on commit 0c9883c

Please sign in to comment.