From 07a088bf62233f058bbc5cda1d00cb43e736131c Mon Sep 17 00:00:00 2001 From: Thomas Date: Sat, 20 Jun 2026 16:43:29 +1000 Subject: [PATCH] fix(nearby): filter stop sheet routes to the tapped mode (shared stop_id) 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 sharing a stop_id (Richmond, stop_id 1162: the metro platforms and the V/Line platform are one stop) this mixed modes, so the Nearby sheet's Routes chips on the V/Line pin showed metro train lines (Alamein, Belgrave, ...). Departures are fetched per (stopId, routeType) and were already correct, which is why only the Routes chips were wrong. Filter servingRoutes to the requested route_type in StopResponseDto.toDomain so every getStopDetail consumer gets routes consistent with the mode it asked for. Both current consumers (Nearby sheet, stop-detail departure-grouping lookup) want the mode-consistent list. Co-Authored-By: ai-tiro Co-Authored-By: Claude Opus 4.8 (1M context) --- .../network/RetrofitStopDetailDataSource.kt | 2 +- .../core/network/model/StopResponseDto.kt | 19 +++- .../core/network/StopDetailDtoMapperTest.kt | 86 +++++++++++++++++-- 3 files changed, 99 insertions(+), 8 deletions(-) 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") + } }