diff --git a/mobile/core/network/src/main/kotlin/ac/jfx/openptv/core/network/RetrofitStopDetailDataSource.kt b/mobile/core/network/src/main/kotlin/ac/jfx/openptv/core/network/RetrofitStopDetailDataSource.kt index 605254b6..e009998e 100644 --- a/mobile/core/network/src/main/kotlin/ac/jfx/openptv/core/network/RetrofitStopDetailDataSource.kt +++ b/mobile/core/network/src/main/kotlin/ac/jfx/openptv/core/network/RetrofitStopDetailDataSource.kt @@ -27,6 +27,6 @@ internal class RetrofitStopDetailDataSource ): StopDetail? { val typeCode = routeType.toPtvCode() val path = "stops/${stopId.value}/route_type/$typeCode?stop_location=true&stop_disruptions=true" - return api.getStop(urlResolver.resolve(path)).toDomain() + return api.getStop(urlResolver.resolve(path)).toDomain(requestedRouteType = routeType) } } diff --git a/mobile/core/network/src/main/kotlin/ac/jfx/openptv/core/network/model/StopResponseDto.kt b/mobile/core/network/src/main/kotlin/ac/jfx/openptv/core/network/model/StopResponseDto.kt index 5bf2c790..57fccbf7 100644 --- a/mobile/core/network/src/main/kotlin/ac/jfx/openptv/core/network/model/StopResponseDto.kt +++ b/mobile/core/network/src/main/kotlin/ac/jfx/openptv/core/network/model/StopResponseDto.kt @@ -71,8 +71,20 @@ internal data class RouteDto( * Map the PTV wire envelope into the domain [StopDetail]. Returns `null` when the response has * no stop block — the repository surfaces that as `Result.Error` so the UI can show "stop not * found" rather than rendering a half-empty header. + * + * [requestedRouteType] filters the serving-routes list to the mode the caller asked for. PTV's + * `/stops/{id}/route_type/{type}` endpoint IGNORES the `route_type` path param for the `routes` + * array and returns *every* route serving the physical `stop_id`. At co-located stops that share + * a `stop_id` (Richmond is the canonical case: the metro platforms and the V/Line platform are + * one `stop_id` 1162) this mixes modes, so a V/Line stop comes back carrying metro train routes + * (issue #175 — the Nearby sheet rendered Alamein/Belgrave/… chips on the V/Line pin). + * Departures are already fetched per `(stopId, routeType)` and are correct, so filtering the + * routes here keeps `servingRoutes` consistent with the mode every caller requested. + * [RouteType.Unknown] disables the filter (we never put Unknown on the wire — this only guards + * against an unexpected upstream value, where dropping every route would be worse than passing + * the raw list through). */ -internal fun StopResponseDto.toDomain(): StopDetail? { +internal fun StopResponseDto.toDomain(requestedRouteType: RouteType): StopDetail? { val s = stop ?: return null // Pick the nested `stop_location.gps` pair when the top-level fields are absent (= default // 0.0) — the single-stop endpoint only populates the nested shape. The nearby endpoint @@ -94,7 +106,10 @@ internal fun StopResponseDto.toDomain(): StopDetail? { latitude = lat, longitude = lng, ), - servingRoutes = s.routes.map { it.toDomain() }, + servingRoutes = + s.routes + .map { it.toDomain() } + .filter { requestedRouteType == RouteType.Unknown || it.routeType == requestedRouteType }, ) } diff --git a/mobile/core/network/src/test/kotlin/ac/jfx/openptv/core/network/StopDetailDtoMapperTest.kt b/mobile/core/network/src/test/kotlin/ac/jfx/openptv/core/network/StopDetailDtoMapperTest.kt index 836ecab4..f34f6210 100644 --- a/mobile/core/network/src/test/kotlin/ac/jfx/openptv/core/network/StopDetailDtoMapperTest.kt +++ b/mobile/core/network/src/test/kotlin/ac/jfx/openptv/core/network/StopDetailDtoMapperTest.kt @@ -36,7 +36,7 @@ class StopDetailDtoMapperTest { .build(), ) - val detail = response.toDomain() + val detail = response.toDomain(requestedRouteType = RouteType.Train) assertThat(detail).isNotNull() assertThat(detail!!.stop.name).isEqualTo("Flinders Street Railway Station") @@ -50,14 +50,14 @@ class StopDetailDtoMapperTest { @Test fun `response with no stop block returns null`() { val response = StopResponseDto(stop = null) - assertThat(response.toDomain()).isNull() + assertThat(response.toDomain(requestedRouteType = RouteType.Train)).isNull() } @Test fun `response with stop but no routes maps to empty servingRoutes`() { val response = StopResponseDto(stop = StopDetailsDtoMother.aStopDetailsDto().build()) - val detail = response.toDomain() + val detail = response.toDomain(requestedRouteType = RouteType.Train) assertThat(detail).isNotNull() assertThat(detail!!.servingRoutes).isEmpty() @@ -78,7 +78,7 @@ class StopDetailDtoMapperTest { .build(), ) - val detail = response.toDomain()!! + val detail = response.toDomain(requestedRouteType = RouteType.Tram)!! assertThat(detail.stop.routeType).isEqualTo(RouteType.Tram) assertThat(detail.servingRoutes[0].routeType).isEqualTo(RouteType.Tram) @@ -103,8 +103,84 @@ class StopDetailDtoMapperTest { .build(), ) - val detail = response.toDomain()!! + val detail = response.toDomain(requestedRouteType = RouteType.Tram)!! assertThat(detail.servingRoutes.map { it.number }).containsExactly("75", "70").inOrder() } + + @Test + fun `serving routes are filtered to the requested route type at a shared stop_id`() { + // Regression coverage for issue #175. Richmond's metro platforms and the V/Line platform + // share `stop_id` 1162, and PTV's `/stops/{id}/route_type/{type}` endpoint ignores the + // path `route_type` for the `routes` array — it returns every route serving the physical + // stop. Captured shape from `/api/v3/stops/1162/route_type/3`: metro (route_type 0) lines + // mixed in with the V/Line (route_type 3) services. When the caller asked for V/Line, the + // mapper must drop the metro routes so the Nearby sheet's Routes chips match the tapped + // mode (departures are already fetched per `(stopId, routeType)` and were correct). + val response = + StopResponseDto( + stop = + StopDetailsDtoMother.aStopDetailsDto() + .withStopName("Richmond Railway Station") + .withRouteType(3) + .withRoutes( + listOf( + RouteDto(routeId = 1, routeName = "Alamein", routeNumber = "", routeType = 0), + RouteDto(routeId = 2, routeName = "Belgrave", routeNumber = "", routeType = 0), + RouteDto( + routeId = 3, + routeName = "Bairnsdale - Melbourne via Sale & Traralgon", + routeNumber = "", + routeType = 3, + ), + RouteDto( + routeId = 4, + routeName = "Traralgon - Melbourne via Morwell & Moe & Pakenham", + routeNumber = "", + routeType = 3, + ), + ), + ) + .build(), + ) + + val detail = response.toDomain(requestedRouteType = RouteType.VLine)!! + + assertThat(detail.servingRoutes.map { it.routeType }) + .containsExactly(RouteType.VLine, RouteType.VLine) + assertThat(detail.servingRoutes.map { it.name }) + .containsExactly( + "Bairnsdale - Melbourne via Sale & Traralgon", + "Traralgon - Melbourne via Morwell & Moe & Pakenham", + ) + .inOrder() + } + + @Test + fun `requesting metro at a shared stop_id keeps only metro routes`() { + // Mirror of the #175 case from the other side: tapping the metro Richmond pin must show + // only the metro lines, not the co-located V/Line services. + val response = + StopResponseDto( + stop = + StopDetailsDtoMother.aStopDetailsDto() + .withRouteType(0) + .withRoutes( + listOf( + RouteDto(routeId = 1, routeName = "Alamein", routeNumber = "", routeType = 0), + RouteDto( + routeId = 3, + routeName = "Bairnsdale - Melbourne via Sale & Traralgon", + routeNumber = "", + routeType = 3, + ), + ), + ) + .build(), + ) + + val detail = response.toDomain(requestedRouteType = RouteType.Train)!! + + assertThat(detail.servingRoutes.map { it.name }).containsExactly("Alamein") + } }