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

fix(StopDetailsPage): Distinct errorKeys from API requests on NearbyTransitPage #641

Merged
merged 5 commits into from
Jan 14, 2025
Merged
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
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
Loading