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: Display error state when trip doesn't exist #527

Merged
merged 7 commits into from
Nov 12, 2024
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
19 changes: 14 additions & 5 deletions iosApp/iosApp/Pages/TripDetails/TripDetailsPage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import SwiftPhoenixClient
import SwiftUI

struct TripDetailsPage: View {

Check notice on line 14 in iosApp/iosApp/Pages/TripDetails/TripDetailsPage.swift

View check run for this annotation

Xcode Cloud / MBTA Transit Staging | PR Branch Workflow | iosAppRetries - iOS

iosApp/iosApp/Pages/TripDetails/TripDetailsPage.swift#L14

Type Body Length Violation: Type body should span 250 lines or less excluding comments and whitespace: currently spans 299 lines (type_body_length)
let tripId: String
let vehicleId: String
let routeId: String
Expand All @@ -24,6 +24,7 @@
@State var globalResponse: GlobalResponse?
@State var tripPredictionsRepository: ITripPredictionsRepository
@State var tripPredictions: PredictionsStreamDataResponse?
@State var tripPredictionsLoaded: Bool = false
boringcactus marked this conversation as resolved.
Show resolved Hide resolved
@State var tripRepository: ITripRepository
@State var trip: Trip?
@State var tripSchedulesResponse: TripSchedulesResponse?
Expand Down Expand Up @@ -71,9 +72,10 @@
var body: some View {
VStack(spacing: 16) {
header
if let trip, let globalResponse, let vehicle = vehicleResponse?.vehicle,
if tripPredictionsLoaded, let globalResponse, let vehicle = vehicleResponse?.vehicle,
let stops = TripDetailsStopList.companion.fromPieces(
boringcactus marked this conversation as resolved.
Show resolved Hide resolved
trip: trip,
tripId: tripId,
directionId: trip?.directionId ?? vehicle.directionId,
tripSchedules: tripSchedulesResponse,
tripPredictions: tripPredictions,
vehicle: vehicle,
Expand Down Expand Up @@ -120,6 +122,10 @@
.onAppear { joinRealtime() }
.onDisappear { leaveRealtime() }
.onChange(of: tripId) {
errorBannerVM.errorRepository.clearDataError(key: "TripDetailsPage.loadTripSchedules")
errorBannerVM.errorRepository.clearDataError(key: "TripDetailsPage.loadTrip")
loadTripSchedules()
loadTrip()
leavePredictions()
joinPredictions(tripId: $0)
}
Expand Down Expand Up @@ -221,12 +227,14 @@
case let .ok(result): tripPredictions = result.data
case .error: break
}
tripPredictionsLoaded = true
checkPredictionsStale()
}
}
}

private func leavePredictions() {
tripPredictionsLoaded = false
tripPredictionsRepository.disconnect()
}

Expand Down Expand Up @@ -276,7 +284,7 @@
} else {
nil
}
let route: Route? = if let routeId = trip?.routeId {
let route: Route? = if let routeId = trip?.routeId ?? vehicle?.routeId {
globalResponse?.routes[routeId]
} else {
nil
Expand All @@ -285,14 +293,15 @@
vehicle: vehicle,
route: route,
stop: vehicleStop,
trip: trip
tripId: tripId
)
}

@ViewBuilder
var header: some View {
let trip: Trip? = tripPredictions?.trips[tripId]
let route: Route? = if let routeId = trip?.routeId {
let vehicle: Vehicle? = vehicleResponse?.vehicle
let route: Route? = if let routeId = trip?.routeId ?? vehicle?.routeId {
globalResponse?.routes[routeId]
} else {
nil
Expand Down
14 changes: 7 additions & 7 deletions iosApp/iosApp/Pages/TripDetails/VehicleCardView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ struct VehicleCardView: View {
let vehicle: Vehicle?
let route: Route?
let stop: Stop?
let trip: Trip?
let tripId: String

var body: some View {
if let vehicle, let route, let stop, let trip {
if let vehicle, let route, let stop {
VehicleOnTripView(
vehicle: vehicle,
route: route,
stop: stop,
trip: trip
tripId: tripId
)
} else {
Text("Loading...")
EmptyView()
}
}
}
Expand All @@ -34,7 +34,7 @@ struct VehicleOnTripView: View {
let vehicle: Vehicle
let route: Route
let stop: Stop
let trip: Trip
let tripId: String

var backgroundColor: Color {
Color(hex: route.color)
Expand All @@ -58,7 +58,7 @@ struct VehicleOnTripView: View {

@ViewBuilder
private var description: some View {
if vehicle.tripId == trip.id {
if vehicle.tripId == tripId {
VStack(alignment: .leading, spacing: 2) {
vehicleStatusDescription(vehicle.currentStatus)
.font(Typography.caption)
Expand Down Expand Up @@ -162,7 +162,7 @@ struct VehicleCardView_Previews: PreviewProvider {
}

List {
VehicleCardView(vehicle: vehicle, route: red, stop: stop, trip: trip)
VehicleCardView(vehicle: vehicle, route: red, stop: stop, tripId: trip.id)
}
.previewDisplayName("VehicleCard")
}
Expand Down
37 changes: 20 additions & 17 deletions iosApp/iosAppTests/Pages/TripDetails/TripDetailsPageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -172,21 +172,21 @@ final class TripDetailsPageTests: XCTestCase {
let vehicleId = "999"
let nearbyVM = NearbyViewModel()
nearbyVM.alerts = .init(alerts: [:])
var sut = TripDetailsPage(
let sut = TripDetailsPage(
tripId: tripId,
vehicleId: vehicleId,
routeId: trip.routeId,
target: .init(stopId: stop1.id, stopSequence: 998),
errorBannerVM: .init(),
nearbyVM: nearbyVM,
mapVM: .init(),
globalRepository: MockGlobalRepository(response: .init(objects: objects, patternIdsByStop: [:])),
globalRepository: MockGlobalRepository(response: .init(objects: objects)),
tripPredictionsRepository: FakeTripPredictionsRepository(response: .init(objects: objects)),
tripRepository: tripRepository,
vehicleRepository: FakeVehicleRepository(response: .init(vehicle: vehicle))
)

let splitViewExp = sut.on(\.didLoadData) { view in
let splitViewExp = sut.inspection.inspect(onReceive: tripSchedulesLoaded, after: 0.5) { view in
XCTAssertNotNil(try view.find(TripDetailsStopListSplitView.self))
}

Expand Down Expand Up @@ -307,15 +307,13 @@ final class TripDetailsPageTests: XCTestCase {
wait(for: [routeExp], timeout: 5)
}

@MainActor func testTripRequestError() throws {
@MainActor func testMissingTrip() throws {
let objects = ObjectCollectionBuilder()

let trip = objects.trip { trip in
trip.routeId = "Red"
}
let tripId = "ADDED-trip"

let vehicle = objects.vehicle { vehicle in
vehicle.tripId = trip.id
vehicle.tripId = tripId
vehicle.currentStatus = .inTransitTo
}

Expand All @@ -336,15 +334,16 @@ final class TripDetailsPageTests: XCTestCase {
onConnect: { _ in tripPredictionsLoaded.send() }
)

let tripId = trip.id
let vehicleId = vehicle.id
let nearbyVM = NearbyViewModel(alertsRepository: MockAlertsRepository())
nearbyVM.alerts = .init(alerts: [:])
let sut = TripDetailsPage(
tripId: tripId,
vehicleId: vehicleId,
routeId: trip.routeId,
routeId: "Red",
target: nil,
errorBannerVM: .init(),
nearbyVM: .init(),
nearbyVM: nearbyVM,
mapVM: .init(),
globalRepository: MockGlobalRepository(response: globalData),
tripPredictionsRepository: tripPredictionsRepository,
Expand All @@ -355,9 +354,8 @@ final class TripDetailsPageTests: XCTestCase {
let everythingLoaded = tripSchedulesLoaded.zip(tripPredictionsLoaded)

let routeExp = sut.inspection.inspect(onReceive: everythingLoaded, after: 0.1) { view in
XCTAssertNotNil(try view.find(where: { view in
(try? view.modifier(LoadingPlaceholderModifier.self)) != nil
}))
XCTAssertNotNil(try view.find(VehicleCardView.self))
XCTAssertThrowsError(try view.find(TripDetailsStopView.self))
}

ViewHosting.host(view: sut)
Expand Down Expand Up @@ -647,11 +645,11 @@ final class TripDetailsPageTests: XCTestCase {
}

class FakeTripPredictionsRepository: ITripPredictionsRepository {
let response: PredictionsStreamDataResponse
let response: PredictionsStreamDataResponse?
let onConnect: ((_ tripId: String) -> Void)?
let onDisconnect: (() -> Void)?

init(response: PredictionsStreamDataResponse,
init(response: PredictionsStreamDataResponse?,
onConnect: ((_ tripId: String) -> Void)? = nil,
onDisconnect: (() -> Void)? = nil) {
self.response = response
Expand All @@ -664,7 +662,12 @@ final class TripDetailsPageTests: XCTestCase {
onReceive: @escaping (ApiResult<PredictionsStreamDataResponse>) -> Void
) {
onConnect?(tripId)
onReceive(ApiResultOk(data: response))
let result = if let response {
ApiResultOk(data: response)
} else {
ApiResultError<PredictionsStreamDataResponse>(code: 404, message: "Failed to load predictions")
}
onReceive(result)
}

var lastUpdated: Instant?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ final class VehicleCardViewTests: XCTestCase {
}
let stop = objects.stop { _ in }

let sut = VehicleCardView(vehicle: vehicle, route: route, stop: stop, trip: trip)
let sut = VehicleCardView(vehicle: vehicle, route: route, stop: stop, tripId: trip.id)
XCTAssertNotNil(try sut.inspect().find(text: "This vehicle is completing another trip"))
}

Expand All @@ -49,7 +49,7 @@ final class VehicleCardViewTests: XCTestCase {
stop.name = "Davis"
}

let sut = VehicleCardView(vehicle: vehicle, route: route, stop: stop, trip: trip)
let sut = VehicleCardView(vehicle: vehicle, route: route, stop: stop, tripId: trip.id)
XCTAssertNotNil(try sut.inspect().find(text: "Approaching"))
XCTAssertNotNil(try sut.inspect().find(text: "Davis"))
}
Expand All @@ -67,7 +67,7 @@ final class VehicleCardViewTests: XCTestCase {
let route = objects.route { _ in }
let stop = objects.stop { _ in }

let sut = VehicleCardView(vehicle: vehicle, route: route, stop: stop, trip: trip)
let sut = VehicleCardView(vehicle: vehicle, route: route, stop: stop, tripId: trip.id)
XCTAssertNotNil(try sut.inspect().find(text: "Next stop"))
}

Expand All @@ -84,7 +84,7 @@ final class VehicleCardViewTests: XCTestCase {
let route = objects.route { _ in }
let stop = objects.stop { _ in }

let sut = VehicleCardView(vehicle: vehicle, route: route, stop: stop, trip: trip)
let sut = VehicleCardView(vehicle: vehicle, route: route, stop: stop, tripId: trip.id)
XCTAssertNotNil(try sut.inspect().find(text: "Now at"))
}

Expand All @@ -95,7 +95,7 @@ final class VehicleCardViewTests: XCTestCase {
trip.headsign = "Alewife"
}

let sut = VehicleCardView(vehicle: nil, route: nil, stop: nil, trip: trip)
XCTAssertNotNil(try sut.inspect().find(text: "Loading..."))
let sut = VehicleCardView(vehicle: nil, route: nil, stop: nil, tripId: trip.id)
XCTAssertThrowsError(try sut.inspect().find(VehicleOnTripView.self))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,24 +126,28 @@ data class TripDetailsStopList(val stops: List<Entry>) {
}

fun fromPieces(
trip: Trip,
tripId: String,
directionId: Int,
tripSchedules: TripSchedulesResponse?,
tripPredictions: PredictionsStreamDataResponse?,
vehicle: Vehicle?,
alertsData: AlertsStreamDataResponse?,
globalData: GlobalResponse,
): TripDetailsStopList? {
if (tripPredictions == null || alertsData == null) return null
if (alertsData == null) return null
val entries = mutableMapOf<Int, WorkingEntry>()

val predictions =
deduplicatePredictionsByStopSequence(
tripPredictions.predictions.values,
tripSchedules,
globalData
)
var predictions = emptyList<Prediction>()
if (tripPredictions != null) {
predictions =
deduplicatePredictionsByStopSequence(
tripPredictions.predictions.values,
tripSchedules,
globalData
)

predictions.forEach { prediction -> entries.putPrediction(prediction, vehicle) }
predictions.forEach { prediction -> entries.putPrediction(prediction, vehicle) }
}

if (tripSchedules is TripSchedulesResponse.Schedules) {
tripSchedules.schedules.forEach { entries.putSchedule(it) }
Expand All @@ -159,15 +163,15 @@ data class TripDetailsStopList(val stops: List<Entry>) {
}

if (entries.isEmpty()) {
return null
return TripDetailsStopList(emptyList())
}
return TripDetailsStopList(
entries.entries
.sortedBy { it.key }
.dropWhile {
if (
vehicle == null ||
vehicle.tripId != trip.id ||
vehicle.tripId != tripId ||
vehicle.currentStopSequence == null
) {
false
Expand All @@ -179,7 +183,7 @@ data class TripDetailsStopList(val stops: List<Entry>) {
Entry(
globalData.stops.getValue(it.value.stopId),
it.value.stopSequence,
getAlert(it.value, alertsData, globalData, trip),
getAlert(it.value, alertsData, globalData, tripId, directionId),
it.value.schedule,
it.value.prediction,
it.value.vehicle,
Expand Down Expand Up @@ -316,8 +320,9 @@ data class TripDetailsStopList(val stops: List<Entry>) {
while (scheduleIndex in stopIds.indices && predictionIndex in predictions.indices) {
val stopId = stopIds[scheduleIndex]
val prediction = predictions[predictionIndex]
check(Stop.equalOrFamily(stopId, prediction.stopId, globalData.stops)) {
"predictions=$predictions stopIds=$stopIds predictionIndex=$predictionIndex scheduleIndex=$scheduleIndex"
if (!Stop.equalOrFamily(stopId, prediction.stopId, globalData.stops)) {
scheduleIndex--
continue
}
lastDelta = lastStopSequence?.minus(prediction.stopSequence)
lastStopSequence = prediction.stopSequence
Expand Down Expand Up @@ -359,7 +364,8 @@ data class TripDetailsStopList(val stops: List<Entry>) {
entry: WorkingEntry,
alertsData: AlertsStreamDataResponse?,
globalData: GlobalResponse,
trip: Trip
tripId: String,
directionId: Int
): Alert? {
val entryTime = entry.prediction?.predictionTime ?: entry.schedule?.scheduleTime
val entryRoute = entry.prediction?.routeId ?: entry.schedule?.routeId
Expand All @@ -371,11 +377,11 @@ data class TripDetailsStopList(val stops: List<Entry>) {
alert.isActive(entryTime) &&
alert.anyInformedEntity {
it.appliesTo(
directionId = trip.directionId,
directionId = directionId,
routeId = entryRoute,
routeType = entryRouteType,
stopId = entry.stopId,
tripId = trip.id
tripId = tripId
)
} &&
// there's no UI yet for secondary alerts in trip details
Expand Down
Loading